Clear programClusterRoutes when clusterRoutingMode is removed - #13888
Draft
tomastigera wants to merge 1 commit into
Draft
Clear programClusterRoutes when clusterRoutingMode is removed#13888tomastigera wants to merge 1 commit into
tomastigera wants to merge 1 commit into
Conversation
Setting Installation.spec.calicoNetwork.clusterRoutingMode and then removing it left the cluster running whatever that mode last wrote, while the Installation looked exactly like one that never set the field. Nothing was written, nothing logged, and status.computed was empty. The state it could strand is the deprecated BIRD-programs-IPIP configuration, so a user who set BIRD to opt out of the v3.33 default and then removed the field to get the default back stayed on the deprecated arm with no indication anywhere. The two setters return early when the mode is nil. That is right for a field that was never set -- writing there would pin the operator's idea of today's Calico defaults into the datastore, which clusterRoutingMode() documents as the reason for the gate -- but it cannot tell never-set from set-then-unset. Use status.computed as the record of what we last applied. It is persisted on the CR, so it survives restarts, and it still holds the previous reconcile's value at the point the setters run. A mode there plus no mode in the spec is the set-to-unset transition, so clear programClusterRoutes and let Calico's own defaults decide again. A value we never wrote belongs to the user and stays, which keeps direct FelixConfiguration configuration working while the Installation field is unset. CORE-13588
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.
Problem
Setting
Installation.spec.calicoNetwork.clusterRoutingModeand then removing it leaves the cluster running whatever that mode last wrote, while theInstallationlooks exactly like one that never touched the field. Nothing is written, nothing is logged, andstatus.computedis empty.The state it can strand is the deprecated BIRD-programs-IPIP configuration. A user who sets
BIRDto opt out of the v3.33 default and then removes the field expecting the default back stays on the deprecated arm, with no indication anywhere.Why the early return is there
setClusterRoutingOnFelixConfigurationandsetClusterRoutingOnBGPConfigurationreturn early when the mode is nil. That is correct for a field that was never set: writing there would pin the operator's idea of today's Calico defaults into the datastore, which is exactly whatclusterRoutingMode()documents as the reason for the gate. What it cannot do is tell never-set from set-then-unset.Change
Use
status.computedas the record of what the operator last applied. It is persisted on the CR so it survives restarts, and it still holds the previous reconcile's value at the point the setters run. A mode there plus no mode in the spec is the set-to-unset transition.On that transition, clear
programClusterRoutesand let Calico's own defaults decide again — which is what the existing comment asks for. The gate is preserved: an unset field still never writes a value.A value the operator never wrote belongs to the user and is left alone, so configuring
programClusterRoutesdirectly on FelixConfiguration keeps working while the Installation field is unset.Tests
Two specs in
core_controller_test.go, both driving real reconciles against the fake client:programClusterRoutesclearedprogramClusterRoutesdirectly → left aloneMutation-tested: reverting either setter to
return false, nilfails the first spec. The existing "should not patch FelixConfig and BGPConfig when ClusterRouteMode not set" spec is unchanged and still passes, which is the regression guard for the documented no-write-on-unset behaviour.Suite: 311 passed / 22 failed. The 22 are all
cel_validation_test.go[BeforeEach]envtest setup failures; a control run on unmodifiedupstream/masterin the same worktree fails the identical 22 (309 passed / 22 failed), so they are pre-existing and environmental.Found during the v3.33 test cycle, Zephyr OS-R232 / RT-3.
CORE-13588
AI assistance
This PR was written in part with the assistance of generative AI.
Release Note