Fix flaky CR-cache E2E teardown: drain orphaned PackageRev CRs - #1167
Open
efiacor wants to merge 1 commit into
Open
Fix flaky CR-cache E2E teardown: drain orphaned PackageRev CRs#1167efiacor wants to merge 1 commit into
efiacor wants to merge 1 commit into
Conversation
Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech>
efiacor
requested review from
kispaljr,
liamfallon and
mozesl-nokia
as code owners
September 1, 2026 14:52
✅ Deploy Preview for kpt-porch ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes flaky CR-cache E2E teardown by ensuring orphaned internal PackageRev CRs are actively drained after their parent repository is deleted, preventing timeouts that previously caused the CR-cache nightly E2E job to fail.
Changes:
- Update
WaitUntilAllPackageRevsDeletedto attempt cleanup on every poll iteration by force-deleting any remaining internalPackageRevCRs. - Replace the finalizer-only helper with
forceDeletePkgRev, which issues aDeletefirst (to setdeletionTimestamp) and then clears finalizers with conflict retries.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
liamfallon
approved these changes
Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What changed:
WaitUntilAllPackageRevsDeletednow force-drains leftover internalPackageRevCRs on every poll iteration. ReplacedremovePkgRevFinalizerswithforceDeletePkgRev, which issues a delete then clears finalizers (with conflict-retry).Why it’s needed: The nightly "E2E Tests (CR Cache)" job failed every run with
FATAL: PackageRevs from repo <x> still remain. After a repository is deleted, orphaned internal PackageRev CRs are left behind with no controller to drain them. The old helper only cleared finalizers when present and never issued a delete, so an orphan with no deletionTimestamp was never removed. The poll then spun until the 60s timeout and FATAL'd, failing whichever test owned that repo.How it works: Deleting first sets a deletionTimestamp even when no finalizers exist; clearing finalizers then lets the deletion complete. Running every iteration ensures the poll retries until the CRs actually drain to zero.
Related Issue(s)
Type of Change
Checklist
Testing Instructions (Optional)
make test-e2eagainst a CR-cache deployment).TestBasicLifecycle/TestPackageRevisionMetadatateardown no longer FATALs with "PackageRevs from repo ... still remain".Additional Notes (Optional)
AI Disclosure
If so, please describe how: