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

Add Bind extension point of the scheduling framework #78513

Merged
merged 1 commit into from Jun 22, 2019

Conversation

@chenchun
Copy link
Contributor

commented May 30, 2019

What type of PR is this?
/kind feature
/priority important-soon
/sig scheduling

What this PR does / why we need it:
Add Bind extension point of the scheduling framework

Which issue(s) this PR fixes:
Fixes #78006

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Add Bind extension point of the scheduling framework
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

Hi @chenchun. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@@ -42,6 +42,8 @@ const (
Unschedulable
// Wait is used when a permit plugin finds a pod scheduling should wait.
Wait
// Whether a bind plugin chooses to skip binding

This comment has been minimized.

Copy link
@draveness

draveness May 30, 2019

Member

Please start comments with Skip ...

Show resolved Hide resolved pkg/scheduler/scheduler.go

@chenchun chenchun force-pushed the chenchun:binder branch from 3d31541 to c81fd43 Jun 4, 2019

@wgliang

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

/ok-to-test

@draveness

This comment has been minimized.

Copy link
Member

commented Jun 18, 2019

/assign @bsalamat

@bsalamat
Copy link
Member

left a comment

Thanks, @chenchun! Generally LGTM. Only a couple of minor comments.

// RunBindPlugins runs the set of configured bind plugins. A bind plugin may choose
// whether or not to handle the given Pod. If a bind plugin chooses to skip the
// binding, it should return code=4("skip") status. Otherwise, it should return "Error"
// or "Success". If none of the plugins handled binding, RunBindPlugins returns

This comment has been minimized.

Copy link
@bsalamat

bsalamat Jun 19, 2019

Member

RunBindPlugins should one day return an error if all bind plugins 'Skip' the pod, but this is fine for now that we want to run the default bind function if none of the bind plugins process the pod. My hope is that the current bind function will one day be a (default) plugin.

This comment has been minimized.

Copy link
@chenchun

chenchun Jun 20, 2019

Author Contributor

Maybe we can do it in a following PR.

This comment has been minimized.

Copy link
@draveness

draveness Jun 20, 2019

Member

I opened an issue #79215 to track this and I could raise a PR after this get merged.

Show resolved Hide resolved pkg/scheduler/scheduler.go Outdated
},
}
// Set reserve and bind config for testing
pluginConfig := []schedulerconfig.PluginConfig{

This comment has been minimized.

Copy link
@bsalamat

bsalamat Jun 19, 2019

Member

Not related to this PR, but we should make providing a pluginConfig for plugins optional so that something like this is not necessary if one does not need to pass any args to plugins.

This comment has been minimized.

Copy link
@chenchun

chenchun Jun 20, 2019

Author Contributor

Agreed.

This comment has been minimized.

Copy link
@chenchun

chenchun Jul 25, 2019

Author Contributor

Actually plugin config can be nil. I opened #80567 to clean up the verbose code.

Show resolved Hide resolved test/integration/scheduler/framework_test.go Outdated

@chenchun chenchun force-pushed the chenchun:binder branch from c81fd43 to 621bf90 Jun 20, 2019

@chenchun chenchun force-pushed the chenchun:binder branch from 621bf90 to b339c0a Jun 20, 2019

@bsalamat
Copy link
Member

left a comment

/lgtm
/approve

Thanks, @chenchun!

@k8s-ci-robot k8s-ci-robot added the lgtm label Jun 21, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsalamat, chenchun

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

@chenchun

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 6efc5a8 into kubernetes:master Jun 22, 2019

21 of 23 checks passed

pull-kubernetes-bazel-test Job triggered.
Details
pull-kubernetes-integration Job triggered.
Details
cla/linuxfoundation chenchun authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce 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-kubemark-e2e-gce-big 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

@chenchun chenchun deleted the chenchun:binder branch Jun 22, 2019

@@ -599,6 +637,165 @@ func TestUnreservePlugin(t *testing.T) {
}
}

// TestBindPlugin tests invocation of bind plugins.

This comment has been minimized.

Copy link
@draveness
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.