Fix doNotDeploy not removing off-schedule BundleDeployments#5071
Open
dockerized-nl wants to merge 3 commits into
Open
Fix doNotDeploy not removing off-schedule BundleDeployments#5071dockerized-nl wants to merge 3 commits into
dockerized-nl wants to merge 3 commits into
Conversation
cleanupOrphanedBundleDeployments preserved any BD with Spec.OffSchedule=true, even when that BD was no longer targeted (e.g. because doNotDeploy was set in fleet.yaml after the app was already deployed). This left the app installed on the downstream cluster indefinitely until the cluster re-entered its maintenance window. The OffSchedule flag is intended to prevent updates/deployments during a maintenance-window gap, not to block removal of BDs that are entirely untargeted. BDs that are still actively targeted are already preserved by the uidsToKeep check; the additional `|| bd.Spec.OffSchedule` guard was therefore both redundant for targeted BDs and harmful for untargeted ones. Remove the OffSchedule guard from cleanupOrphanedBundleDeployments so that a BD is always deleted when its cluster is no longer targeted, regardless of schedule state. Add an integration test that explicitly sets OffSchedule=true on the BD before enabling doNotDeploy, confirming the BD is cleaned up. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
|
thanks @dockerized-nl, esp. for the extensive comments within the code 💚 |
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.
Summary
cleanupOrphanedBundleDeploymentshad a guard|| bd.Spec.OffSchedulethat preserved any BD withOffSchedule=true, even when that BD was no longer targeted (e.g. becausedoNotDeploy: truewas set infleet.yamlafter the app was already deployed).OffScheduleguard. TheuidsToKeepset already correctly identifies all actively-targeted BDs; the extra guard was redundant for those and harmful for untargeted ones.Root Cause
In
cleanupOrphanedBundleDeployments(bundle_controller.go):slices.DeleteFuncremoves elements where the predicate returnstrue, so any BD withOffSchedule=truewas never added to the delete list — even when its cluster was no longer targeted at all.Behaviour
doNotDeployset, cluster in active windowdoNotDeployset, cluster in maintenance gap (OffSchedule=true)doNotDeploynot set, cluster in maintenance gapTest plan
When("setting doNotDeploy after deployment when the existing BundleDeployment is off-schedule")that explicitly setsOffSchedule=trueon the BD before enablingdoNotDeploy, then asserts the BD is cleaned up.doNotDeployintegration tests continue to pass.)