-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(appset): dont requeue appsets on status change #21364
fix(appset): dont requeue appsets on status change #21364
Conversation
Signed-off-by: rumstead <[email protected]>
Signed-off-by: rumstead <[email protected]>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
Co-authored-by: Michael Crenshaw <[email protected]> Signed-off-by: rumstead <[email protected]>
…appsets-on-status-change
…appsets-on-status-change # Conflicts: # applicationset/controllers/applicationset_controller.go # applicationset/controllers/applicationset_controller_test.go
Signed-off-by: rumstead <[email protected]>
…m:rumstead/argo-cd into fix/dont-requeue-appsets-on-status-change
Signed-off-by: rumstead <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #21364 +/- ##
==========================================
- Coverage 55.31% 52.96% -2.35%
==========================================
Files 337 337
Lines 56824 56863 +39
==========================================
- Hits 31430 30116 -1314
- Misses 22710 24094 +1384
+ Partials 2684 2653 -31 ☔ View full report in Codecov by Sentry. |
Signed-off-by: rumstead <[email protected]>
Signed-off-by: rumstead <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall code looks good!! Just a suggestion for rephrasing the comments.
Co-authored-by: Ishita Sequeira <[email protected]> Signed-off-by: rumstead <[email protected]>
Signed-off-by: rumstead <[email protected]>
Signed-off-by: rumstead <[email protected]>
* e2e Signed-off-by: rumstead <[email protected]> * fix(appset): don't requeue on status changes Signed-off-by: rumstead <[email protected]> * fix spelling Co-authored-by: Michael Crenshaw <[email protected]> Signed-off-by: rumstead <[email protected]> * merge in annotation changes Signed-off-by: rumstead <[email protected]> * merge in annotation changes Signed-off-by: rumstead <[email protected]> * add more tests Signed-off-by: rumstead <[email protected]> * lint fix Signed-off-by: rumstead <[email protected]> * Update applicationset/controllers/applicationset_controller.go Co-authored-by: Ishita Sequeira <[email protected]> Signed-off-by: rumstead <[email protected]> * fix linting Signed-off-by: rumstead <[email protected]> * fix linting Signed-off-by: rumstead <[email protected]> --------- Signed-off-by: rumstead <[email protected]> Co-authored-by: Michael Crenshaw <[email protected]> Co-authored-by: Ishita Sequeira <[email protected]> Signed-off-by: Brett C. Dudo <[email protected]>
* e2e Signed-off-by: rumstead <[email protected]> * fix(appset): don't requeue on status changes Signed-off-by: rumstead <[email protected]> * fix spelling Co-authored-by: Michael Crenshaw <[email protected]> Signed-off-by: rumstead <[email protected]> * merge in annotation changes Signed-off-by: rumstead <[email protected]> * merge in annotation changes Signed-off-by: rumstead <[email protected]> * add more tests Signed-off-by: rumstead <[email protected]> * lint fix Signed-off-by: rumstead <[email protected]> * Update applicationset/controllers/applicationset_controller.go Co-authored-by: Ishita Sequeira <[email protected]> Signed-off-by: rumstead <[email protected]> * fix linting Signed-off-by: rumstead <[email protected]> * fix linting Signed-off-by: rumstead <[email protected]> --------- Signed-off-by: rumstead <[email protected]> Co-authored-by: Michael Crenshaw <[email protected]> Co-authored-by: Ishita Sequeira <[email protected]>
* e2e Signed-off-by: rumstead <[email protected]> * fix(appset): don't requeue on status changes Signed-off-by: rumstead <[email protected]> * fix spelling Co-authored-by: Michael Crenshaw <[email protected]> Signed-off-by: rumstead <[email protected]> * merge in annotation changes Signed-off-by: rumstead <[email protected]> * merge in annotation changes Signed-off-by: rumstead <[email protected]> * add more tests Signed-off-by: rumstead <[email protected]> * lint fix Signed-off-by: rumstead <[email protected]> * Update applicationset/controllers/applicationset_controller.go Co-authored-by: Ishita Sequeira <[email protected]> Signed-off-by: rumstead <[email protected]> * fix linting Signed-off-by: rumstead <[email protected]> * fix linting Signed-off-by: rumstead <[email protected]> --------- Signed-off-by: rumstead <[email protected]> Co-authored-by: Michael Crenshaw <[email protected]> Co-authored-by: Ishita Sequeira <[email protected]> Signed-off-by: flbla <[email protected]>
When an applicationset is requeued and there are no changes to the child application's spec but there is a change to their sync status, eg progressing, healthy, outofsync, the applicationset will be requeued twice because the applicationset's status is patched with the sync status of the underlying applications.
Before changes
updating an applicationset when an underlying application has changed status
time="2025-01-02T16:55:06-05:00" level=debug msg="received update event" applicationset=argocd/cluster requeue=true
time="2025-01-02T16:55:06-05:00" level=debug msg="received update event" applicationset=argocd/cluster requeue=true
After changes
updating an applicationset when an underlying application has changed status
time="2025-01-03T14:46:00-05:00" level=debug msg="received update event" applicationset=argocd/cluster requeue=true
time="2025-01-03T14:46:00-05:00" level=debug msg="received update event" applicationset=argocd/cluster requeue=false
It would be great if this could be cherry-picked into 2.14
Checklist: