From 7e3fb33be85122d509756e197aac10c4cf9930b6 Mon Sep 17 00:00:00 2001
From: Matthew Heon <matthew.heon@pm.me>
Date: Fri, 8 Jan 2021 11:24:54 -0500
Subject: Ensure that `podman play kube` actually reports errors

In 2.2.x, we moved `play kube` to use the Start() API for pods,
which reported errors in a different way (all containers are
started in parallel, and then results reported as a block). The
migration attempted to preserve compatibility by returning only
one error, but that's not really a viable option as it can
obscure the real reason that a pod is failing. Further, the code
was not correctly handling the API's errors - Pod Start() will,
on any container error, return a map of container ID to error
populated for all container errors *and* return ErrPodPartialFail
for overall error - the existing code did not handle the partial
failure error and thus would never return container errors.

Refactor the `play kube` API to include a set of errors for
containers in each pod, so we can return all errors that occurred
to the frontend and print them for the user, and correct the
backend code so container errors are actually forwarded.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
---
 cmd/podman/play/kube.go      | 15 +++++++++++++++
 pkg/domain/entities/play.go  |  3 +++
 pkg/domain/infra/abi/play.go | 17 +++++------------
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/cmd/podman/play/kube.go b/cmd/podman/play/kube.go
index db7280b1d..1f54db203 100644
--- a/cmd/podman/play/kube.go
+++ b/cmd/podman/play/kube.go
@@ -12,6 +12,7 @@ import (
 	"github.com/containers/podman/v2/cmd/podman/utils"
 	"github.com/containers/podman/v2/pkg/domain/entities"
 	"github.com/containers/podman/v2/pkg/util"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -130,6 +131,8 @@ func kube(cmd *cobra.Command, args []string) error {
 		}
 	}
 
+	ctrsFailed := 0
+
 	for _, pod := range report.Pods {
 		fmt.Printf("Pod:\n")
 		fmt.Println(pod.ID)
@@ -145,9 +148,21 @@ func kube(cmd *cobra.Command, args []string) error {
 		for _, ctr := range pod.Containers {
 			fmt.Println(ctr)
 		}
+		ctrsFailed += len(pod.ContainerErrors)
+		// If We have errors, add a newline
+		if len(pod.ContainerErrors) > 0 {
+			fmt.Println()
+		}
+		for _, err := range pod.ContainerErrors {
+			fmt.Fprintf(os.Stderr, err+"\n")
+		}
 		// Empty line for space for next block
 		fmt.Println()
 	}
 
+	if ctrsFailed > 0 {
+		return errors.Errorf("failed to start %d containers", ctrsFailed)
+	}
+
 	return nil
 }
diff --git a/pkg/domain/entities/play.go b/pkg/domain/entities/play.go
index 0b42e1a3f..6883fe6c5 100644
--- a/pkg/domain/entities/play.go
+++ b/pkg/domain/entities/play.go
@@ -40,6 +40,9 @@ type PlayKubePod struct {
 	Containers []string
 	// Logs - non-fatal errors and log messages while processing.
 	Logs []string
+	// ContainerErrors - any errors that occurred while starting containers
+	// in the pod.
+	ContainerErrors []string
 }
 
 // PlayKubeReport contains the results of running play kube.
diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go
index cbc74a2f2..70c7104f1 100644
--- a/pkg/domain/infra/abi/play.go
+++ b/pkg/domain/infra/abi/play.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/containers/image/v5/types"
 	"github.com/containers/podman/v2/libpod"
+	"github.com/containers/podman/v2/libpod/define"
 	"github.com/containers/podman/v2/libpod/image"
 	"github.com/containers/podman/v2/pkg/domain/entities"
 	"github.com/containers/podman/v2/pkg/specgen/generate"
@@ -251,21 +252,13 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 	}
 
 	if options.Start != types.OptionalBoolFalse {
-		//start the containers
+		// Start the containers
 		podStartErrors, err := pod.Start(ctx)
-		if err != nil {
+		if err != nil && errors.Cause(err) != define.ErrPodPartialFail {
 			return nil, err
 		}
-
-		// Previous versions of playkube started containers individually and then
-		// looked for errors.  Because we now use the uber-Pod start call, we should
-		// iterate the map of possible errors and return one if there is a problem. This
-		// keeps the behavior the same
-
-		for _, e := range podStartErrors {
-			if e != nil {
-				return nil, e
-			}
+		for id, err := range podStartErrors {
+			playKubePod.ContainerErrors = append(playKubePod.ContainerErrors, errors.Wrapf(err, "error starting container %s", id).Error())
 		}
 	}
 
-- 
cgit v1.2.3-54-g00ecf