Surface failure information from pod evaluator (#3105)

Author: Mengqi Yu

porch/func/internal/podevaluator.go

@@ -134,6 +134,10 @@ func (pe *podEvaluator) EvaluateFunction(ctx context.Context, req *evaluator.Eva
 	if err != nil {
 		return nil, fmt.Errorf("unable to evaluate %v with pod evaluator: %w", req.Image, err)
 	}
+	// Log stderr when the function succeeded. If the function fails, stderr will be surfaced to the users.
+	if len(resp.Log) > 0 {
+		klog.Warningf("evaluating %v succeeded, but stderr is: %v", req.Image, string(resp.Log))
+	}
 	return resp, nil
 }
 
@@ -581,7 +585,7 @@ func (pm *podManager) podIpIfRunningAndReady(ctx context.Context, podKey client.
 		}
 		return false, nil
 	}); e != nil {
-		return "", fmt.Errorf("error when waiting the pod to be ready: %w", e)
+		return "", fmt.Errorf("error occured when waiting the pod to be ready. If the error is caused by timeout, you may want to examine the pods in namespace %q. Error: %w", pm.namespace, e)
 	}
 	return pod.Status.PodIP, nil
 }

porch/func/wrapper-server/main.go

@@ -23,6 +23,7 @@ import (
 	"os"
 	"os/exec"
 
+	"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
 	pb "github.com/GoogleContainerTools/kpt/porch/func/evaluator"
 	"github.com/GoogleContainerTools/kpt/porch/func/healthchecker"
 	"github.com/spf13/cobra"
@@ -98,15 +99,26 @@ func (e *singleFunctionEvaluator) EvaluateFunction(ctx context.Context, req *pb.
 
 	err := cmd.Run()
 	var exitErr *exec.ExitError
-	if err != nil && !errors.As(err, &exitErr) {
-		return nil, status.Errorf(codes.Internal, "Failed to execute function %q: %s (%s)", req.Image, err, stderr.String())
+	outbytes := stdout.Bytes()
+	stderrStr := stderr.String()
+	if err != nil {
+		if errors.As(err, &exitErr) {
+			// If the exit code is non-zero, we will try to embed the structured results and content from stderr into the error message.
+			rl, pe := fn.ParseResourceList(outbytes)
+			if pe != nil {
+				// If we can't parse the output resource list, we only surface the content in stderr.
+				return nil, status.Errorf(codes.Internal, "failed to evaluate function %q with stderr %v", req.Image, stderrStr)
+			}
+			return nil, status.Errorf(codes.Internal, "failed to evaluate function %q with structured results: %v and stderr: %v", req.Image, rl.Results.Error(), stderrStr)
+		} else {
+			return nil, status.Errorf(codes.Internal, "Failed to execute function %q: %s (%s)", req.Image, err, stderrStr)
+		}
 	}
 
-	outbytes := stdout.Bytes()
-	klog.Infof("Evaluated %q: stdout length: %d\nstderr:\n%v", req.Image, len(outbytes), stderr.String())
+	klog.Infof("Evaluated %q: stdout length: %d\nstderr:\n%v", req.Image, len(outbytes), stderrStr)
 
 	return &pb.EvaluateFunctionResponse{
 		ResourceList: outbytes,
-		Log:          stderr.Bytes(),
+		Log:          []byte(stderrStr),
 	}, nil
 }

porch/test/e2e/e2e_test.go

@@ -1040,6 +1040,11 @@ func (t *PorchSuite) TestPodEvaluator(ctx context.Context) {
 		t.Skipf("Skipping due to not having pod evalutor in local mode")
 	}
 
+	const (
+		generateFolderImage = "gcr.io/kpt-fn/generate-folders:v0.1.1" // This function is a TS based function.
+		setAnnotationsImage = "gcr.io/kpt-fn/set-annotations:v0.1.3"  // set-annotations:v0.1.3 is an older version that porch maps it neither to built-in nor exec.
+	)
+
 	// Register the repository as 'git-fn'
 	t.registerMainGitRepositoryF(ctx, "git-fn-pod")
 
@@ -1070,14 +1075,14 @@ func (t *PorchSuite) TestPodEvaluator(ctx context.Context) {
 				{
 					Type: "eval",
 					Eval: &porchapi.FunctionEvalTaskSpec{
-						Image: "gcr.io/kpt-fn/generate-folders:v0.1.1", // This function is a TS based function.
+						Image: generateFolderImage,
 					},
 				},
 				// Testing pod evaluator with golang function
 				{
 					Type: "eval",
 					Eval: &porchapi.FunctionEvalTaskSpec{
-						Image: "gcr.io/kpt-fn/set-annotations:v0.1.3", // set-annotations:v0.1.3 is an older version that porch maps it neither to built-in nor exec.
+						Image: setAnnotationsImage,
 						ConfigMap: map[string]string{
 							"test-key": "test-val",
 						},
@@ -1116,7 +1121,10 @@ func (t *PorchSuite) TestPodEvaluator(ctx context.Context) {
 	podList := &coreapi.PodList{}
 	t.ListF(ctx, podList, client.InNamespace("porch-fn-system"))
 	for _, pod := range podList.Items {
-		t.DeleteF(ctx, &pod)
+		img := pod.Spec.Containers[0].Image
+		if img == generateFolderImage || img == setAnnotationsImage {
+			t.DeleteF(ctx, &pod)
+		}
 	}
 
 	// Create another Package Revision
@@ -1146,14 +1154,14 @@ func (t *PorchSuite) TestPodEvaluator(ctx context.Context) {
 				{
 					Type: "eval",
 					Eval: &porchapi.FunctionEvalTaskSpec{
-						Image: "gcr.io/kpt-fn/generate-folders:v0.1.1", // This function is a TS based function.
+						Image: generateFolderImage,
 					},
 				},
 				// Testing pod evaluator with golang function
 				{
 					Type: "eval",
 					Eval: &porchapi.FunctionEvalTaskSpec{
-						Image: "gcr.io/kpt-fn/set-annotations:v0.1.3", // set-annotations:v0.1.3 is an older version that porch maps it neither to built-in nor exec.
+						Image: setAnnotationsImage,
 						ConfigMap: map[string]string{
 							"new-test-key": "new-test-val",
 						},
@@ -1188,6 +1196,53 @@ func (t *PorchSuite) TestPodEvaluator(ctx context.Context) {
 	}
 }
 
+func (t *PorchSuite) TestPodEvaluatorWithFailure(ctx context.Context) {
+	if t.local {
+		t.Skipf("Skipping due to not having pod evalutor in local mode")
+	}
+
+	t.registerMainGitRepositoryF(ctx, "git-fn-pod-failure")
+
+	// Create Package Revision
+	pr := &porchapi.PackageRevision{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: t.namespace,
+		},
+		Spec: porchapi.PackageRevisionSpec{
+			PackageName:    "test-fn-pod-bucket",
+			Revision:       "v1",
+			RepositoryName: "git-fn-pod-failure",
+			Tasks: []porchapi.Task{
+				{
+					Type: "clone",
+					Clone: &porchapi.PackageCloneTaskSpec{
+						Upstream: porchapi.UpstreamPackage{
+							Type: "git",
+							Git: &porchapi.GitPackage{
+								Repo:      "https://github.com/GoogleCloudPlatform/blueprints.git",
+								Ref:       "bucket-blueprint-v0.4.3",
+								Directory: "catalog/bucket",
+							},
+						},
+					},
+				},
+				{
+					Type: "eval",
+					Eval: &porchapi.FunctionEvalTaskSpec{
+						// This function is expect to fail due to not knowing schema for some CRDs.
+						Image: "gcr.io/kpt-fn/kubeval:v0.2.0",
+					},
+				},
+			},
+		},
+	}
+	err := t.client.Create(ctx, pr)
+	expectedErrMsg := "Validating arbitrary CRDs is not supported"
+	if err == nil || !strings.Contains(err.Error(), expectedErrMsg) {
+		t.Fatalf("expected the error to contain %q, but got %v", expectedErrMsg, err)
+	}
+}
+
 func (t *PorchSuite) TestRepositoryError(ctx context.Context) {
 	const (
 		repositoryName = "repo-with-error"