-
Notifications
You must be signed in to change notification settings - Fork 890
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
Add placement for bindingSpec #2702
Conversation
Please explain it in more detail. Who is |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #2702 +/- ##
==========================================
+ Coverage 47.98% 48.35% +0.36%
==========================================
Files 200 201 +1
Lines 18082 18070 -12
==========================================
+ Hits 8677 8738 +61
+ Misses 8924 8847 -77
- Partials 481 485 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
karmada/pkg/controllers/cluster/taint_manager.go Lines 219 to 248 in 1d2028b
Are you trying to optimize this piece of code? |
Not quite. Mainly to make calls simpler and make the API clearer. |
6c2fe27
to
689f11a
Compare
689f11a
to
3262d95
Compare
dc142e0
to
4d8d154
Compare
5eddfc3
to
632b94b
Compare
/cc @RainbowMango |
Please add an agenda to next week's community meeting for public comment. |
Let me take a review. |
632b94b
to
8ab7075
Compare
Please fix the CI. |
8ab7075
to
dc5225c
Compare
dc5225c
to
66c876d
Compare
/assign |
66c876d
to
6706c96
Compare
This PR could help #2968 for determining whether a binding is divided scheduling easily. |
@@ -273,20 +167,19 @@ func (s *Scheduler) updateCluster(oldObj, newObj interface{}) { | |||
case !equality.Semantic.DeepEqual(oldCluster.Labels, newCluster.Labels): | |||
fallthrough | |||
case !equality.Semantic.DeepEqual(oldCluster.Spec, newCluster.Spec): | |||
s.enqueueAffectedPolicy(oldCluster, newCluster) | |||
s.enqueueAffectedClusterPolicy(oldCluster, newCluster) | |||
s.enqueueAffectedBindings(oldCluster, newCluster) |
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.
Only the RB is processed. The CRB is not processed.
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.
enqueueAffectedBindings
alse enqueues the affected CRBs.
Signed-off-by: Poor12 <shentiecheng@huawei.com>
6706c96
to
5eff625
Compare
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.
Good job~
/lgtm
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.
/approve
@@ -19,7 +19,7 @@ import ( | |||
|
|||
// ScheduleAlgorithm is the interface that should be implemented to schedule a resource to the target clusters. | |||
type ScheduleAlgorithm interface { | |||
Schedule(context.Context, *policyv1alpha1.Placement, *workv1alpha2.ResourceBindingSpec, *ScheduleAlgorithmOption) (scheduleResult ScheduleResult, err error) |
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.
I'm not sure if we need to add this parameter back when implementing #3126, but it makes sense for this PR to do it.
[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 |
Signed-off-by: Poor12 shentiecheng@huawei.com
What type of PR is this?
/kind api-change
What this PR does / why we need it:
Add placement for BindingSpec can reduce the dependency of policy and make API more clear. Now the component above will check the placement of bindingSpec first and switch to get placement by label if placement of bindSpec is empty.
It can be smoothly upgraded from a lower version to a higher version. If karmada-controller-manager upgrades, spec.placement will occur according to the placement of policy.
Upgrade path: upgrade crd --> upgrade karmada-controller-manager --> upgrade karmada-scheduler.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: