Skip to content

fix: address an issue in the rollout controller where bindings might not receive status refresh after binding spec changes#1049

Merged
ryanzhang-oss merged 7 commits intoAzure:mainfrom
michaelawyu:feat/rollout-controller-fix
Feb 25, 2025
Merged

fix: address an issue in the rollout controller where bindings might not receive status refresh after binding spec changes#1049
ryanzhang-oss merged 7 commits intoAzure:mainfrom
michaelawyu:feat/rollout-controller-fix

Conversation

@michaelawyu
Copy link
Contributor

Description of your changes

This PR fixes an issue in the rollout controller where bindings might not receive status refresh after binding spec changes.

Specifically, when

a) a Bound binding with up to date resource/override snapshot; or
b) an Unscheduled binding that has not been deleted yet

has its spec changed (e.g., apply strategy update), rollout controller will stop processing them and such bindings will no longer have fresh RolloutStarted condition added to their status. As a result, work generator will refuse to process these bindings and the system will be stuck.

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

  • Unit tests
  • Integration tests
  • [] E2E tests will be added in a separate PR (enable the new work applier)

Special notes for your reviewer

N/A

// cluster, upgrading to a newer resource/override snapshot, or removing) but are blocked by
// the rollout strategy.
//
// Update the status first, so that if the rolling out (updateBindings func) fails in the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this comment mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Ryan! This (Update the status first...) was the original comment I believe.

@michaelawyu
Copy link
Contributor Author

Per offline discussions, will temporarily drop the change that populates the RolloutStarted condition for unscheduled bindings (as the work generator currently handles unscheduled bindings as an exception) and evaluate later how we could further improve the flow.

@ryanzhang-oss ryanzhang-oss merged commit 317f9e8 into Azure:main Feb 25, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants