fix maxUnavailable for persistentNamingPolicy#362
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #362 +/- ##
==========================================
+ Coverage 47.35% 48.12% +0.77%
==========================================
Files 122 122
Lines 7491 7545 +54
==========================================
+ Hits 3547 3631 +84
+ Misses 3624 3583 -41
- Partials 320 331 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes PodTransitionRule maxUnavailable enforcement when CollaSet uses PersistentSequence naming and performs a rebuild-style upgrade, where the target pod list can temporarily shrink (old pod deleted before the new pod with the same name exists). The fix adjusts the available-rule quota calculation by accounting for “uncreated” replicas derived from the owning CollaSet’s desired replicas, and adds unit + e2e coverage.
Changes:
- Adjust
AvailableRulermax-unavailable quota by subtracting owner “uncreated replicas” (desired replicas minus currently targeted pods). - Add unit tests covering the owner-replica deficit logic and key filter scenarios.
- Add an e2e conformance test reproducing the PersistentSequence rebuild gap case; expand controller RBAC to allow reading CollaSets.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
pkg/controllers/podtransitionrule/processor/rules/available.go |
Implements owner-replica deficit offset to prevent over-granting maxUnavailable when targets temporarily shrink. |
pkg/controllers/podtransitionrule/processor/rules/available_test.go |
Adds unit tests validating deficit calculation and filter behavior with/without the shrink gap. |
test/e2e/apps/collaset.go |
Adds an e2e conformance test ensuring availableReplicas doesn’t drop below 1 during a PersistentSequence rebuild upgrade under maxUnavailable=1. |
pkg/controllers/podtransitionrule/podtransitionrule_controller.go |
Updates RBAC to permit reading CollaSets required by the new quota logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Check ownerRef.APIVersion in addition to Kind so only CollaSet owners from apps.kusionstack.io are considered, avoiding spurious Gets for same-Kind owners from other API groups. - Cache a zero-desired stat when the owner CollaSet is NotFound or its UID mismatches, so sibling pods sharing the ownerRef no longer repeat Client.Get within a single Filter call. Quota offset stays unchanged. - Replace fixed-length Consistently(120s) in the PersistentSequence rebuild e2e with an Eventually loop that asserts availableReplicas>=1 on each poll and exits as soon as the upgrade completes.
The PodTransitionRule carries a protected finalizer targeting the CollaSet pods, which blocks CollaSet deletion in the framework AfterEach when the test leaves it behind. Delete the rule at the end of the spec and wait for it to disappear so the shared cleanup can proceed.
- Check ownerRef.APIVersion in addition to Kind so only CollaSet owners from apps.kusionstack.io are considered, avoiding spurious Gets for same-Kind owners from other API groups. - Cache a zero-desired stat when the owner CollaSet is NotFound or its UID mismatches, so sibling pods sharing the ownerRef no longer repeat Client.Get within a single Filter call. Quota offset stays unchanged. - Replace fixed-length Consistently(120s) in the PersistentSequence rebuild e2e with an Eventually loop that asserts availableReplicas>=1 on each poll and exits as soon as the upgrade completes.
24aa33e to
55d50bb
Compare
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
6. Release note
Please refer to Release Notes Language Style Guide to write a quality release note.