fix: Add sync waves to Redis resources and config maps (#19798) #20946
Conversation
Fixes argoproj#19798 Some Redis stuff needs to be updated before others, same for config maps. That's relevant when managing ArgoCD with ArgoCD. Needs to be cherry-picked to v2.11-v2.13 Signed-off-by: Andrii Korotkov <[email protected]>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
|
We should do this in a way that's tool-agnostic and favors eventual consistency over explicit ordering. |
|
But it's blocking or messing up with people who try to upgrade to v2.12/2.13 from earlier. Maybe we messed up how we reconfigured Redis, but config maps would be tricky too, since we'd probably need hot reloading if cms and deployments are in the same sync wave. |
|
Yep, understood. So we need a solution that works regardless of whether you deploy Argo CD using Argo CD or a different tool. |
|
I don't know an easy way. I can think of direct kubectl apply, which seems to handle this ordering well and ArgoCD, which needs sync waves. What other tools do you have in mind? How do they order? If you wanna support all possible tools, we probably can't get around of hot reloading or using reloader for pods when cms are updated etc. Can we merge this PR as a hotfix and work on a better solution separately? I'm just not sure how much it'd take. |
|
That's one tool we can try to use https://github.com/stakater/Reloader. |
|
Have you confirmed that kubectl apply escapes this issue? I can't think of a way it could avoid the race condition. Flux, Helm, and ConfigSync would be examples of tools which would not be helped by Argo CD sync waves. We shouldn't merge this. Temporary fixes have a way of lasting forever. Reloader isn't really an option. We shouldn't require an external tool to get Argo CD running. I'm confused by the original bug. Why is argocd-redis not synced when the controller is synced? Is OP using a partial sync? |
|
I've been upgrading with kubectl and I think it was okay. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #20946 +/- ##
==========================================
- Coverage 55.04% 55.02% -0.02%
==========================================
Files 324 324
Lines 55421 55465 +44
==========================================
+ Hits 30504 30520 +16
- Misses 22303 22326 +23
- Partials 2614 2619 +5 ☔ View full report in Codecov by Sentry. |
Fixes #19798
Some Redis stuff needs to be updated before others, same for config maps. That's relevant when managing ArgoCD with ArgoCD.
Needs to be cherry-picked to v2.11-v2.13
Checklist: