feat: ability to suspend Work
#4838
Conversation
karmada-bot
added
the
kind/feature
karmada-bot
requested review from
calvin0327,
chaunceyjiang,
mrlihanbo,
Poor12,
Vacant2333 and
XiShanYongYe-Chang
karmada-bot
added
the
size/L
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #4838 +/- ##
==========================================
- Coverage 53.19% 52.86% -0.33%
==========================================
Files 252 253 +1
Lines 20509 20665 +156
==========================================
+ Hits 10909 10924 +15
- Misses 8880 9018 +138
- Partials 720 723 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -173,6 +173,13 @@ type PropagationSpec struct { | |||
| // +kubebuilder:validation:Enum=Lazy | |||
| // +optional | |||
| ActivationPreference ActivationPreference `json:"activationPreference,omitempty"` | |||
|
|
|||
| // Suspend tells the controller to suspend subsequent executions. | |||
There was a problem hiding this comment.
Whether the suspension operation is transparently transferred to the work object or the policy execution is suspended?
There was a problem hiding this comment.
The suspension operation is transparently transferred to the work object.
Is this better?
| // Suspend tells the controller to suspend subsequent executions. | |
| // Suspend will instruct all work objects to pause propagation to member clusters. |
There was a problem hiding this comment.
Changed to the suggestion above, let me know what you think
There was a problem hiding this comment.
I wonder if what really needs to be paused is the propagation of work. Pauses on pp and rb are to facilitate the transfer of user intent to work.
There was a problem hiding this comment.
Would you please elaborate?
This PR facilitates the transfer of suspend to work and it pauses propagation of work.
There was a problem hiding this comment.
Please ignore my question above. I see that's how you've implemented it now.
| // | ||
| // +kubebuilder:default=false | ||
| // +optional | ||
| Suspend bool `json:"suspend,omitempty"` |
There was a problem hiding this comment.
Do we need to consider setting pause fields for different clusters so that different pause policies can be set for different work of the same resource?
|
@XiShanYongYe-Chang: GitHub didn't allow me to request PR reviews from the following users: CharlesQQ. Note that only karmada-io members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@XiShanYongYe-Chang What's your feeling on this PR? Should I rebase and keep it up to date or do you not see this as a feature? |
|
I agree with this feature. I'm currently working on this area to see if karmada can provide users with faster capabilities based on this. I'll sync when I make progress. Thanks. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
a7i
force-pushed
the
work-suspend
branch
2 times, most recently
from
6f1798d
to
d854b00
Compare
Signed-off-by: Amir Alavi <amir.alavi@zendesk.com>
|
Hi @a7i, we discussed how to design the API to provide users with the resources deletion policy in #4788 (comment). This is similar to Work suspension. They both describe the synchronization behavior of resource templates in member clusters, and can be resolved in three ways. Therefore, I'd like to know your opinions on how to design the API. Thanks a lot. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Ability to suspend work to ensure that changes are not being reconciled.
Which issue(s) this PR fixes:
Fixes #4688
Special notes for your reviewer:
Does this PR introduce a user-facing change?: