Skip to content
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

Even Pods Spread - 4. Preemption Support #79062

Merged
merged 5 commits into from Jul 25, 2019

Conversation

@Huang-Wei
Copy link
Member

commented Jun 15, 2019

What type of PR is this?

/sig scheduling
/hold

/assign @bsalamat
/cc @krmayankk

What this PR does / why we need it:

This is the 4th PR of the "Even Pods Spread" KEP implementation. After this PR, users can run workloads using "hard topologySpreadConstraints", and high priority Pod can preempt low priority one. (PR to support Priority will be sent out later)

  • Supports preemption for EvenPodsSpread
  • Unit tests on PredicateMetadata
  • Unit tests on genericScheduler.Schedule()

Which issue(s) this PR fixes:

Part of #77284.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

(will document all the changes in one place)

NONE
@Huang-Wei

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2019

/kind feature
/priority important-soon

@Huang-Wei

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2019

/retest

Show resolved Hide resolved pkg/apis/core/validation/validation.go Outdated
Show resolved Hide resolved pkg/apis/core/validation/validation.go Outdated
Show resolved Hide resolved pkg/scheduler/algorithm/predicates/metadata.go Outdated
Show resolved Hide resolved pkg/scheduler/algorithm/predicates/metadata.go Outdated
for pair, podSet := range topologyPairsPodSpreadMap.topologyPairToPods {
// TODO(Huang-Wei): short circuit to skip non-visited <topologyKey: any value> pairs
// if we see 0 as min match of the topologyKey
if l := int32(len(podSet)); l < topologyPairsPodSpreadMap.minMatchMap[pair.key] {

This comment has been minimized.

Copy link
@tedyu

tedyu Jun 16, 2019

Contributor

Should we check that len(podSet) > 0 ?

This comment has been minimized.

Copy link
@Huang-Wei

Huang-Wei Jul 2, 2019

Author Member

Nope. If l == 0, it means there is a topology domain having 0 pod matching incoming pod's topologySpreadConstrains.

Show resolved Hide resolved pkg/scheduler/algorithm/predicates/metadata.go Outdated
Show resolved Hide resolved pkg/scheduler/algorithm/predicates/metadata.go Outdated
Show resolved Hide resolved pkg/scheduler/algorithm/predicates/metadata.go Outdated
Show resolved Hide resolved pkg/scheduler/algorithm/predicates/predicates.go Outdated

@Huang-Wei Huang-Wei force-pushed the Huang-Wei:eps-preemption branch from a4e775b to e3ce2bb Jul 3, 2019

@Huang-Wei Huang-Wei force-pushed the Huang-Wei:eps-preemption branch 2 times, most recently from 39de5fc to 926ca0f Jul 3, 2019

@Huang-Wei Huang-Wei force-pushed the Huang-Wei:eps-preemption branch from 926ca0f to 5424538 Jul 3, 2019

@Huang-Wei

This comment has been minimized.

Copy link
Member Author

commented Jul 4, 2019

/retest

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Huang-Wei

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Huang-Wei Huang-Wei force-pushed the Huang-Wei:eps-preemption branch from f2d2a88 to 487dbdf Jul 23, 2019

@Huang-Wei Huang-Wei force-pushed the Huang-Wei:eps-preemption branch 2 times, most recently from 6dd412e to 3a7b032 Jul 23, 2019

@Huang-Wei

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

/retest

@alculquicondor alculquicondor referenced this pull request Jul 24, 2019

Closed

REQUEST: New membership for alculquicondor #1042

6 of 6 tasks complete

Huang-Wei added some commits Apr 30, 2019

EvenPodsSpread: Supports Preemption (removePod)
- add removePod() for podSpreadMap
EvenPodsSpread: Supports Preemption (addPod)
- add addPod() for podSpreadMap
EvenPodsSpread: update addPod() logic to match individual constraint
- also add TODO items for potential perf optimization

@Huang-Wei Huang-Wei force-pushed the Huang-Wei:eps-preemption branch from 3a7b032 to 7948479 Jul 24, 2019

@k8s-ci-robot k8s-ci-robot added size/XL and removed size/XXL labels Jul 24, 2019

@Huang-Wei

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

/retest

1 similar comment
@Huang-Wei

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

/retest

@alculquicondor
Copy link
Member

left a comment

LGTM

existingPods []*v1.Pod
nodeIdx int // denotes which node 'addedPod' belongs to
nodes []*v1.Node
injectPodPointers map[topologyPair][]int // non-negative index refers to existingPods[i], negative index refers to addedPod

This comment has been minimized.

Copy link
@alculquicondor

alculquicondor Jul 25, 2019

Member

This is hard to understand. Maybe wantPodInTopology. Although, this might go away with the count based map, so we could leave it for now.

@ahg-g

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 25, 2019

@Huang-Wei

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

/hold cancel

@fejta-bot

This comment has been minimized.

Copy link

commented Jul 25, 2019

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@Huang-Wei

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 7fd8537 into kubernetes:master Jul 25, 2019

21 of 23 checks passed

pull-kubernetes-e2e-gce Job triggered.
Details
pull-kubernetes-kubemark-e2e-gce-big Job triggered.
Details
cla/linuxfoundation Huang-Wei authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

@Huang-Wei Huang-Wei deleted the Huang-Wei:eps-preemption branch Jul 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.