test(vpa): guard camel-cased update mode through the reconcile path (closes #862) - #884
Open
abhyudayareddy wants to merge 1 commit into
Open
abhyudayareddy wants to merge 1 commit into
abhyudayareddy wants to merge 1 commit into
Conversation
Test_ReconcileNamespace_ChangeUpdateMode only exercised the legacy "auto" alias, which resolves to the single-word mode "Recreate". The regression in FairwindsOps#862 was a camel-cased mode ("InPlace") reaching the VPA spec mangled as "Inplace" and being rejected by the VPA admission webhook. Extend the test to switch a namespace to InPlace and assert the emitted VPA carries the exact casing the VPA API expects. Complements the parser-level coverage added in FairwindsOps#883.
abhyudayareddy
requested review from
Azahorscak,
sudermanjr and
transient1
as code owners
August 28, 2026 01:43
2 tasks
Contributor
Author
|
cc @sudermanjr — this one's been open a couple weeks with no review (test coverage for the camel-cased VPA update-mode bug, closes #862). Small, self-contained change. Happy to rebase or adjust if needed. Thanks! |
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
1 participant
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.
Checklist
Description
What's the goal of this PR?
Close out #862 (and its duplicate #873) by adding a regression guard at the layer where that bug actually surfaced.
Background
#862 reported that
goldilocks.fairwinds.com/vpa-update-mode=InPlaceproduced:Tracing the history:
vpaUpdateModeForResourcedidstrings.ToUpper(s[:1]) + strings.ToLower(s[1:]), soInPlace→Inplaceand any camel-cased mode was mangled.allowedUpdateModes, returning the typed constant — but the list did not yet includevpav1.UpdateModeInPlace, soInPlacefell through to theOffdefault.vpav1.UpdateModeInPlacetoallowedUpdateModes, which is what actually fixes the reported error on currentmain.Test_vpaUpdateModeForNamespace.So the bug is already resolved as of the current release line. The remaining gap is test coverage at the reconcile/emission layer:
Test_ReconcileNamespace_ChangeUpdateModeonly exercised the legacyautoalias, which resolves to the single-word modeRecreateand would not have caught the casing mangle.What changes did you make?
Extended
Test_ReconcileNamespace_ChangeUpdateModeto also switch the namespace toInPlace, reconcile, and assert the emitted VPA'sspec.updatePolicy.updateModeis exactlyInPlace. Verified the assertion fails (mode falls back toOff) ifvpav1.UpdateModeInPlaceis removed fromallowedUpdateModes.What alternative solution should we consider, if any?
None — test-only change.