Merge pull request #145 from Nordix/render-pipeline-failure-blocks

Render pipeline failure should block package push to remote

pkg/engine/engine.go

@@ -504,7 +504,9 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj
 	}
 
 	renderStatus, err := cad.taskHandler.DoPRResourceMutations(ctx, pr2Update, draft, oldRes, newRes)
-
+	if err != nil {
+		return nil, renderStatus, err
+	}
 	// No lifecycle change when updating package resources; updates are done.
 	repoPkgRev, err := draft.Close(ctx, "")
 	if err != nil {

pkg/task/generictaskhandler.go

@@ -225,19 +225,24 @@ func (th *genericTaskHandler) DoPRResourceMutations(ctx context.Context, pr2Upda
 
 	var renderStatus *api.RenderStatus
 	if len(appliedResources.Contents) > 0 {
-		// render the package
-		// Render failure will not fail the overall API operation.
-		// The render error and result is captured as part of renderStatus above
-		// and is returned in packageresourceresources API's status field. We continue with
-		// saving the non-rendered resources to avoid losing user's changes.
-		// and supress this err.
-		_, renderStatus, _ = applyResourceMutations(ctx,
+		// Render the package
+		// Render failure will fail the overall API operation.
+		// The render error and result are captured as part of renderStatus above
+		// and are returned in the PackageRevisionResources API's status field.
+		// We do not push the package further to remote:
+		// the user's changes are captured on their local package,
+		// and can be amended using the error returned as a reference point to ensure
+		// the package renders properly, before retrying the push.
+		_, renderStatus, err = applyResourceMutations(ctx,
 			draft,
 			appliedResources,
 			[]mutation{&renderPackageMutation{
 				runnerOptions: runnerOptions,
 				runtime:       th.runtime,
 			}})
+		if err != nil {
+			return renderStatus, err
+		}
 	} else {
 		renderStatus = nil
 	}
@@ -455,6 +460,8 @@ func applyResourceMutations(ctx context.Context, draft repository.PackageRevisio
 			renderStatus = taskResult.RenderStatus
 		}
 		if err != nil {
+			klog.Error(err)
+			err = fmt.Errorf("%w\n\n%s\n%s\n%s", err, "Error occurred rendering package in kpt function pipeline.", "Package has NOT been pushed to remote.", "Please fix package locally (modify until 'kpt fn render' succeeds) and retry.")
 			return updatedResources, renderStatus, err
 		}
 

test/e2e/cli/config.go

@@ -38,6 +38,8 @@ type Command struct {
 	Yaml bool `yaml:"yaml,omitempty"`
 	// IgnoreWhitespace indicates that whitespace differences should be ignored in the output
 	IgnoreWhitespace bool `yaml:"ignoreWhitespace,omitempty"`
+	// StdErrTabToWhitespace replaces "\t" (tab) character with whitespace "  "
+	StdErrTabToWhitespace bool `yaml:"stdErrTabToWhitespace,omitempty"`
 }
 
 type TestCaseConfig struct {

test/e2e/cli/suite.go

@@ -148,6 +148,10 @@ func (s *CliTestSuite) RunTestCase(t *testing.T, tc TestCaseConfig) {
 			command.Stdout = strings.ReplaceAll(command.Stdout, search, replace)
 			command.Stderr = strings.ReplaceAll(command.Stderr, search, replace)
 		}
+
+		if command.StdErrTabToWhitespace {
+			stderrStr = strings.ReplaceAll(stderrStr, "\t", "  ") // Replace tabs with spaces
+		}
 
 		if command.IgnoreWhitespace {
 			command.Stdout = normalizeWhitespace(command.Stdout)

test/e2e/cli/testdata/rpkg-push/config.yaml

@@ -188,3 +188,82 @@ commands:
           name: kptfile.kpt.dev
       kind: ResourceList
     yaml: true
+  - args:
+      - porchctl
+      - rpkg
+      - push
+      - --namespace=rpkg-push
+      - git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f
+    stdin: |
+      apiVersion: config.kubernetes.io/v1
+      items:
+      - apiVersion: ""
+        kind: KptRevisionMetadata
+        metadata:
+          annotations:
+            config.kubernetes.io/index: "0"
+            config.kubernetes.io/path: .KptRevisionMetadata
+            internal.config.kubernetes.io/index: "0"
+            internal.config.kubernetes.io/path: .KptRevisionMetadata
+          name: git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f
+          namespace: rpkg-push
+          uid: 8d61685e-584c-5b89-bd30-a8d8dccb13f9
+          resourceVersion: "1"
+      - apiVersion: kpt.dev/v1
+        info:
+          description: Updated Test Package Description
+        kind: Kptfile
+        metadata:
+          annotations:
+            config.kubernetes.io/index: "0"
+            config.kubernetes.io/local-config: "true"
+            config.kubernetes.io/path: Kptfile
+            internal.config.kubernetes.io/index: "0"
+            internal.config.kubernetes.io/path: Kptfile
+          name: test-package
+      - apiVersion: v1
+        data:
+          name: example
+        kind: ConfigMap
+        metadata:
+          annotations:
+            config.kubernetes.io/index: "0"
+            config.kubernetes.io/local-config: "true"
+            config.kubernetes.io/path: package-context.yaml
+            internal.config.kubernetes.io/index: "0"
+            internal.config.kubernetes.io/path: package-context.yaml
+          name: kptfile.kpt.dev
+      kind: ResourceList
+    stderr: |
+      Error: Internal error occurred: Operation cannot be fulfilled on packagerevisionresources.porch.kpt.dev "git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f": the object has been modified; please apply your changes to the latest version and try again 
+    exitCode: 1
+    yaml: true
+  - args:
+      - porchctl
+      - rpkg
+      - pull
+      - --namespace=rpkg-push
+      - git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f
+      - /tmp/porch-e2e/testing-invalid-render
+  - args:
+    - sh
+    - -c
+    - |
+      echo "pipeline:\n  mutators:\n  - image: gcr.io/kpt-fn/set-namespace:v0.4.0\n    configMap:\n      namespace: example-ns\n  - image: gcr.io/kpt-fn/set-annotations:v0.1.4\n" >> /tmp/porch-e2e/testing-invalid-render/Kptfile
+  - args:
+      - porchctl
+      - rpkg
+      - push
+      - --namespace=rpkg-push
+      - git-efe3d01c68dfdcdd69114c9a7c65cce0d662a46f
+      - /tmp/porch-e2e/testing-invalid-render
+    stderr: |
+      Error: Internal error occurred: fn.render: pkg /:
+        pkg.render:
+        pipeline.run: func eval "gcr.io/kpt-fn/set-annotations:v0.1.4" failed: rpc error: code = Internal desc = Failed to execute function "gcr.io/kpt-fn/set-annotations:v0.1.4": exit status 1 ([error] : failed to configure function: `functionConfig` must be a `ConfigMap` or `SetAnnotations`)
+
+      Error occurred rendering package in kpt function pipeline.
+      Package has NOT been pushed to remote.
+      Please fix package locally (modify until 'kpt fn render' succeeds) and retry. 
+    stdErrTabToWhitespace: true
+    exitCode: 1