Respect metadata env refs in PUB resize checks#2501
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR updates PodUnavailableBudget’s in-place resize safety check to treat label/annotation changes as potentially disruptive when those metadata keys are referenced by containers via downward-API env vars, aligning PUB’s “harmless resize” decision with metadata-driven env behavior. It also adds unit tests covering referenced vs unrelated metadata changes during a resize.
Changes:
- Add metadata-change detection in
CanResizeInplacefor downward-API env references to labels/annotations. - Reuse
pkg/util/containermetahelpers to detect env references tometadata.labels['key']/metadata.annotations['key']. - Extend
TestCanResizeInplacewith cases for referenced label/annotation changes and an unrelated-label change.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/control/pubcontrol/pub_control.go | Adds a metadata/env reference check to PUB in-place resize gating logic. |
| pkg/control/pubcontrol/pub_control_test.go | Adds unit test cases validating the new metadata/env reference behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if isChangedMetadataReferencedByEnv(oldPod, newPod) { | ||
| klog.V(3).InfoS("Pod metadata referenced by env changed, and maybe cause unavailability", "pod", klog.KObj(newPod)) | ||
| return false | ||
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2501 +/- ##
==========================================
+ Coverage 49.91% 49.93% +0.02%
==========================================
Files 325 325
Lines 28437 28462 +25
==========================================
+ Hits 14194 14213 +19
- Misses 12588 12591 +3
- Partials 1655 1658 +3
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:
|
Signed-off-by: Jayant <212013719+Jayant-kernel@users.noreply.github.com>
940e53a to
c0f1f22
Compare
Ⅰ. Describe what this PR does
Teach
PodUnavailableBudgetin-place resize checks to reject a resize as harmless when Pod labels or annotations changed and a container reads the changed key through downward API env.This fills the existing TODO in
CanResizeInplaceand keeps PUB from treating this kind of update as safe when the metadata value can affect the running container environment.Ⅱ. Does this pull request fix one issue?
Related to #1192
Ⅲ. Describe how to verify it
Added
TestCanResizeInplacecases for:Not run locally because Go tooling is not installed in this environment.
Ⅳ. Special notes for reviews
NONE