Skip to content

feat: move the rollout controller to event driven model#1029

Merged
michaelawyu merged 12 commits intoAzure:mainfrom
ryanzhang-oss:fix-rollout-watch
Feb 18, 2025
Merged

feat: move the rollout controller to event driven model#1029
michaelawyu merged 12 commits intoAzure:mainfrom
ryanzhang-oss:fix-rollout-watch

Conversation

@ryanzhang-oss
Copy link
Contributor

@ryanzhang-oss ryanzhang-oss commented Jan 31, 2025

Description of your changes

  1. move the rollout controller to event driven model
  2. minor fixes on comments and logs
  3. fix the bug that rollout got stuck when the master snapshot is deleted but the binding is not synced yet.

Fixes #

I have:

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

How has this code been tested

Special notes for your reviewer

@ryanzhang-oss ryanzhang-oss marked this pull request as draft February 1, 2025 00:50
Copy link
Contributor

@michaelawyu michaelawyu left a comment

Choose a reason for hiding this comment

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

Hi Ryan! I've added some comments, PTAL 🙏

@ryanzhang-oss ryanzhang-oss marked this pull request as ready for review February 7, 2025 23:50
@ryanzhang-oss
Copy link
Contributor Author

The bug was the workGenerator controller will mark the binding as overridden failed but rollout controller doesn't consider that binding has "FAILED". This leads to the roll out stuck because the binding can be ready thus it prevent the maxNumber to go over.

There needs to be a follow up on the workGenerator/applier to differentiate (best effort) user error/not retriable error from retriable.

michaelawyu
michaelawyu previously approved these changes Feb 14, 2025
Copy link
Contributor

@michaelawyu michaelawyu left a comment

Choose a reason for hiding this comment

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

LGTM ;) Has some minor comments; will add them later -> merging the PR now upon test completion to unblock progress

Copy link
Contributor

@michaelawyu michaelawyu left a comment

Choose a reason for hiding this comment

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

LGTM.

@michaelawyu michaelawyu merged commit 822fc47 into Azure:main Feb 18, 2025
12 checks passed
@ryanzhang-oss ryanzhang-oss deleted the fix-rollout-watch branch April 22, 2025 18:34
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