Add implementation of 'porch.kpt.dev/push-on-render-failure' for clone and upgrade

[liamfallon]

Add implementation of porch.kpt.dev/push-on-render-failure for subpackage clone and upgrade

---

Description

• What changed: The subpackage clone and upgrade operations now respect the porch.kpt.dev/push-on-render-failure annotation on a PR
• Why it’s needed: If the mutation pipeline of a parent PR is failing, subpackage clone and upgrade operations will always fail on the PR. Subpackage operations should work on draft PRs who's pipelines are failing because the PR is in preparation and the subpackage clones and upgrades may actually fix the pipeline.
• How it works: Porch is updated to push resources on render failures on clone and upgrade operations.

Note that there was no need to support the porch.kpt.dev/push-on-render-failure annotation on clones and upgrades until now because

• on a full clone of an upstream package to a full PR, the upstream package must be published so it's render pipeline should always be passing
• on a full upgrade of a PR to an upstream package, the upstream package must be published so it's render pipeline should always be passing

The implementation here will now also respect the porch.kpt.dev/push-on-render-failure if, for example, a non subpackage clone or upgrade fails due to connectivity issues etc.

---

Related Issue(s)

• Closes/Fixes # Faulty Render porch annotation is ignored on subpackage clone path #1113

---

Type of Change

• Bug fix
• New feature
• Enhancement
• Refactor
• Documentation
• Tests
• Other: ________

---

Checklist

• Code follows project style guidelines
• Self-reviewed changes
• Tests added/updated
• Documentation added/updated
• All tests and gating checks pass

---

AI Disclosure

• I have used AI in the creation of this PR.

I used Amazon Q to generate the unit tests and E2E tests for this PR. I inspected and ran the tests afterwards to ensure they work and are correct.

[netlify]

✅ Deploy Preview for kpt-porch ready!

Name	Link
🔨 Latest commit	af77188
🔍 Latest deploy log	https://app.netlify.com/projects/kpt-porch/deploys/6a62221dbdaf6e0008365ff6
😎 Deploy Preview	https://deploy-preview-1126--kpt-porch.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Implements support for the porch.kpt.dev/push-on-render-failure annotation during subpackage clone/upgrade by propagating render status through the mutation path and adding e2e coverage for the persisted-on-render-failure behavior.

Changes:

• Extend DoPRMutations / UpdatePackageRevision APIs to return *RenderStatus alongside errors.
• Refactor generic task handling to centralize render+persist logic and return render status.
• Add e2e tests covering subpackage clone/upgrade behavior with/without push-on-render-failure under render failures.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/e2e/api/rpkg_subpkg_test.go	Adds e2e coverage for subpackage clone/upgrade when render fails, with and without the annotation.
pkg/task/taskhandler.go	Updates task handler interface to return render status from PR mutations.
pkg/task/generictaskhandler.go	Refactors/render-path changes to return render status and reuse a shared renderResources helper.
pkg/task/generictaskhandler_test.go	Adjusts unit tests to the new DoPRMutations signature.
pkg/engine/engine.go	Plumbs render status return through UpdatePackageRevision and uses mutation error handling logic.
pkg/engine/engine_test.go	Updates mock task handler for new return type.
pkg/registry/porch/packagecommon.go	Attempts to attach render status to resources after update.
pkg/registry/porch/*_test.go	Updates mocks for the new UpdatePackageRevision return signature.

Files not reviewed (1)

• test/mockery/mocks/porch/pkg/engine/mock_CaDEngine.go: Generated file

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.

Files not reviewed (1)

• test/mockery/mocks/porch/pkg/engine/mock_CaDEngine.go: Generated file

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.

Files not reviewed (1)

• test/mockery/mocks/porch/pkg/engine/mock_CaDEngine.go: Generated file

Comments suppressed due to low confidence (3)

pkg/engine/engine.go:407

• UpdatePackageRevision computes renderStatus from DoPRMutations, but the success return drops it (return repoPkgRev, nil, nil). This makes the new (repo, *RenderStatus, error) signature largely useless and prevents callers from surfacing/persisting render details (especially important for push-on-render-failure).

	sent := cad.watcherManager.NotifyPackageRevisionChange(watch.Modified, repoPkgRev)
	klog.Infof("engine: sent %d for updated PackageRevision %s/%s", sent, repoPkgRev.KubeObjectNamespace(), repoPkgRev.KubeObjectName())

	return repoPkgRev, nil, nil

pkg/registry/porch/packagecommon.go:409

• This tries to persist resources.Status.RenderStatus, but then calls UpdatePackageResourcesWithoutRender which only writes Spec.Resources (it discards Status entirely). As a result, the render status update is not persisted and won’t be observable later.

		resources.Status.RenderStatus = *renderStatus

		rev, err = r.cad.UpdatePackageResourcesWithoutRender(ctx, &repositoryObj, oldRepoPkgRev, resources, resources)
		if err != nil {

test/e2e/api/e2e_test.go:21

• os was removed from imports when the E2E gating was commented out. If the gating is restored, os needs to be imported to keep the file compiling.

import (
	"testing"

	suiteutils "github.com/kptdev/porch/test/e2e/suiteutils"
	"github.com/stretchr/testify/suite"

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

• test/mockery/mocks/porch/pkg/engine/mock_CaDEngine.go: Generated file

Comments suppressed due to low confidence (4)

pkg/engine/engine.go:407

• UpdatePackageRevision computes renderStatus (from DoPRMutations) but the success return always drops it (return repoPkgRev, nil, nil). This makes the new (PackageRevision, *RenderStatus, error) signature misleading and prevents callers (e.g. packagecommon.updatePackageRevision) from observing render results on successful updates.

	sent := cad.watcherManager.NotifyPackageRevisionChange(watch.Modified, repoPkgRev)
	klog.Infof("engine: sent %d for updated PackageRevision %s/%s", sent, repoPkgRev.KubeObjectNamespace(), repoPkgRev.KubeObjectName())

	return repoPkgRev, nil, nil

pkg/registry/porch/packagecommon.go:408

• This attempts to persist resources.Status.RenderStatus, but UpdatePackageResourcesWithoutRender only writes newRes.Spec.Resources and constructs a new object without copying Status. As a result, the RenderStatus assignment here is effectively discarded, and may also fail if the PR lifecycle is no longer Draft (the method enforces Draft). A dedicated status-persist path (or extending UpdatePackageResourcesWithoutRender to persist status) is needed if RenderStatus must be retained across reads.

		resources.Status.RenderStatus = *renderStatus

		rev, err = r.cad.UpdatePackageResourcesWithoutRender(ctx, &repositoryObj, oldRepoPkgRev, resources, resources)

test/e2e/api/rpkg_subpkg_test.go:700

• Spelling: "rander" → "render" in the test description comment.

// TestSubpackageCloneRenderFailureWithPush verifies that when a subpackage clone
// to a parent PR where the parent PR has rander failures and HAS the
// push-on-render-failure annotation, the update does not return an error and the
// subpackage resources are persisted. The render pipeline on the PR should always fail.

test/e2e/api/rpkg_subpkg_test.go:786

• Spelling: "rander" → "render" in the test description comment.

// TestSubpackageUpgradeRenderFailureWithPush verifies that when a subpackage upgrade
// to a parent PR where the parent PR has rander failures and HAS the
// push-on-render-failure annotation, the update does not return an error and the
// subpackage resources are persisted. The render pipeline on the PR should always fail.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud