feat(stateful failover support) Introduce PurgeMode to GracefulEvictionTask in ResourceBinding #5816
Conversation
karmada-bot
added
the
kind/api-change
karmada-bot
added
the
size/M
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #5816 +/- ##
==========================================
- Coverage 43.18% 43.16% -0.03%
==========================================
Files 656 658 +2
Lines 55921 56006 +85
==========================================
+ Hits 24151 24176 +25
- Misses 30213 30260 +47
- Partials 1557 1570 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| // Defaults to "Graciously". | ||
| // +kubebuilder:validation:Enum=Immediately;Graciously;Never | ||
| // +kubebuilder:default=Graciously |
There was a problem hiding this comment.
For application failover, the PurgeMode should be inherited from PropagationPolicy which manages the current workload, but for cluster failover, as I mentioned at #5788, we can set Graciously as the default.
So, from the API perspective, it might not have the default preference.
| // Defaults to "Graciously". | |
| // +kubebuilder:validation:Enum=Immediately;Graciously;Never | |
| // +kubebuilder:default=Graciously | |
| // +kubebuilder:validation:Enum=Immediately;Graciously;Never |
There was a problem hiding this comment.
Does it need to be a required field?
There was a problem hiding this comment.
Since we inherit PurgeMode from the PropagationPolicy (which has a default) and will have a default value for cluster failover, I don't believe this field should ever be empty. I can change this to required if we'd like.
Edit: Seems that using required fails some of the e2e failover tests (because when generating the EvictionTask is doesn't append PurgeMode). I can make this field required in the subsequent PR when I add PurgeMode as part of GracefulEvictCluster().
There was a problem hiding this comment.
It's not necessarily necessary, it's just a question of mine.
mszacillo
force-pushed
the
purgemode-api
branch
from
3f775d3
to
49fb25a
Compare
Signed-off-by: mszacillo <[email protected]>
mszacillo
force-pushed
the
purgemode-api
branch
from
49fb25a
to
9b92cc4
Compare
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
karmada-bot
added
the
approved
What type of PR is this?
/kind api-change
What this PR does / why we need it:
Part of #5788. This small PR adds PurgeMode to the GracefulEvictionTask as a prerequisite for ensuring scheduler skips clusters which triggered the failover.
Does this PR introduce a user-facing change?: