-
Notifications
You must be signed in to change notification settings - Fork 885
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
feat: set conflictResolution for dependent resources. #4418
Conversation
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 #4418 +/- ##
==========================================
- Coverage 41.38% 41.36% -0.02%
==========================================
Files 651 651
Lines 55259 55265 +6
==========================================
- Hits 22867 22863 -4
- Misses 30913 30921 +8
- Partials 1479 1481 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
/lgtm |
/assign |
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.
Thanks~
/lgtm
/cc @RainbowMango
/cc @RainbowMango PTAL. |
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.
/assign
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.
Hi @chaunceyjiang
I think it's time to move this forward:
- The [Summer OSPP 2024] Karmada supports smooth rollback of a single cluster #5380 is relying on this fix
- I will open another issue to track the task that disables sharing the dependencies.
Reduplicated PR: #5731
bf0db45
to
481cf24
Compare
187defe
to
2a44275
Compare
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
2a44275
to
38bb553
Compare
@@ -561,6 +561,11 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding | |||
bindingKey := client.ObjectKeyFromObject(attachedBinding) | |||
err := d.Client.Get(context.TODO(), bindingKey, existBinding) | |||
if err == nil { | |||
// If the spec.Placement is nil, this means that existBinding is generated by the dependency mechanism. | |||
// If the spec.Placement is not nil, then it must be generated by PropagationPolicy. | |||
if existBinding.Spec.Placement == nil { |
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.
Increase this IF
statement to determine the existBinding
, make sure it was created by the dependencies_distributor
,then we can modify ConflictResolution
.
/cc @chaosi-zju @RainbowMango PTAL. |
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.
/lgtm
/approve
[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 |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #4410
Special notes for your reviewer:
Does this PR introduce a user-facing change?: