Scheduler:remove direct dependency for k8s.io/kubernetes/pkg/util/node and remove // TODO: prefer stable labels for zone in v1.18 #90040
Conversation
|
/priority backlog |
|
/test pull-kubernetes-e2e-gce |
|
/test pull-kubernetes-e2e-gce |
pkg/scheduler/util/utils.go
Outdated
| return "" | ||
| } | ||
|
|
||
| // TODO: prefer stable labels for zone in v1.18 |
There was a problem hiding this comment.
can just look at the stable labels. a 1.19 scheduler can depend on a 1.18+ controller, which ensures the stable labels exist and are synced with the beta labels.
pkg/scheduler/util/utils.go
Outdated
| // TODO: prefer stable labels for zone in v1.18 | ||
| zone, ok := labels[v1.LabelZoneFailureDomain] | ||
| if !ok { | ||
| zone, _ = labels[v1.LabelZoneFailureDomainStable] | ||
| } | ||
|
|
||
| // TODO: prefer stable labels for region in v1.18 | ||
| region, ok := labels[v1.LabelZoneRegion] | ||
| if !ok { | ||
| region, _ = labels[v1.LabelZoneRegionStable] | ||
| } |
There was a problem hiding this comment.
| // TODO: prefer stable labels for zone in v1.18 | |
| zone, ok := labels[v1.LabelZoneFailureDomain] | |
| if !ok { | |
| zone, _ = labels[v1.LabelZoneFailureDomainStable] | |
| } | |
| // TODO: prefer stable labels for region in v1.18 | |
| region, ok := labels[v1.LabelZoneRegion] | |
| if !ok { | |
| region, _ = labels[v1.LabelZoneRegionStable] | |
| } | |
| zone := labels[v1.LabelZoneFailureDomainStable] | |
| region := labels[v1.LabelZoneRegionStable] |
|
/lgtm scheduling team has approval |
|
looks like unit tests need updating to stable labels |
|
/retest |
yes . i have updated it , could you review it again? |
|
lgtm |
|
/cc @ahg-g |
| @@ -109,3 +109,25 @@ func GetPodAntiAffinityTerms(podAntiAffinity *v1.PodAntiAffinity) (terms []v1.Po | |||
| } | |||
| return terms | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
defaultpodtopologyspread will be deprecated soon, so add a todo to move this function to node_tree.go once this happens.
|
On a second thought, there is no urgency to doing this, our plan is to deprecate the util pkg and so we shouldn't add anything to it. Also, what we should invest in is moving k8s.io/kubernetes/pkg/util/node pkg to staging instead of duplicating the code. |
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tanjunchen 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 |
…e and remove // TODO: prefer stable labels for zone in v1.18
|
@tanjunchen: PR needs rebase. 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. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Which issue(s) this PR fixes:
ref:#89930
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: