feat: add force promote actions for Numaplane rollouts #22141
Conversation
Signed-off-by: Dillen Padhiar <[email protected]>
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
Signed-off-by: Dillen Padhiar <[email protected]>
Signed-off-by: Dillen Padhiar <[email protected]>
Signed-off-by: Dillen Padhiar <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #22141 +/- ##
==========================================
+ Coverage 55.74% 55.82% +0.08%
==========================================
Files 342 342
Lines 57213 57213
==========================================
+ Hits 31893 31939 +46
+ Misses 22678 22638 -40
+ Partials 2642 2636 -6 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Dillen Padhiar <[email protected]>
Signed-off-by: Dillen Padhiar <[email protected]>
Signed-off-by: Dillen Padhiar <[email protected]>
Signed-off-by: Dillen Padhiar <[email protected]>
Signed-off-by: Dillen Padhiar <[email protected]>
|
LGTM |
Signed-off-by: Dillen Padhiar <[email protected]>
| @@ -0,0 +1,7 @@ | |||
| actionTests: | |||
| - action: enable-full-promote | |||
There was a problem hiding this comment.
Is this analogous to Argo Rollouts' "promote-full" feature? If so, would it be a good idea to standardize on that naming?
There was a problem hiding this comment.
Hey @crenshaw-dev - I see Argo Rollouts has one action called "promote-full"?
One thing I wanted to make clear with how we're handling this is that it's a label that can be applied and removed. That it's not just a one time thing you're doing. If you "enable" it, then you're setting it that way going forward. (and generally, the user should be expected to remove it themselves)
There was a problem hiding this comment.
Note we made the decision that the Controller would not remove the label after performing the action because that would mean that the Controller was changing something other than Finalizer and Status, which is apparently a best practice.
There was a problem hiding this comment.
I actually have to admit I like "Enable Force Promote" and "Disable Force Promote" over what we have now, but "full" is more similar to the language of Argo Rollouts. I guess probably one difference between us and Argo Rollouts is that they have a promote which is "full" and one that is "partial" I believe. That's why the "Full" language makes sense for them. We don't have that.
There was a problem hiding this comment.
If there are solid reasons to use different language (and it seems like there are), I'd change the language. "Enable Force Promote" and "Disable Force Promote" make a lot of sense to me.
There was a problem hiding this comment.
Aside: if it's truly a declarative feature, maybe it belongs in the spec instead of on a label?
| actions["enable-full-promote"] = {["disabled"] = true} | ||
| actions["disable-full-promote"] = {["disabled"] = true} |
There was a problem hiding this comment.
Consider setting a custom display name and icon for the actions: #22202
There was a problem hiding this comment.
Is there an icon that Argo Rollouts is using that we should consider using as well?
There was a problem hiding this comment.
If not, we may want to do the icon as an enhancement later instead of right now.
Signed-off-by: Dillen Padhiar <[email protected]>
…go-cd into full-promote-actions
dpadhiar
changed the title
Adding custom action to enable and disable full promote feature for Numaplane rollouts during a Progressive upgrade.
Checklist: