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

[migration phase 1] PodFitsHostPorts as filter plugin #83659

Merged

Conversation

@wgliang
Copy link
Member

commented Oct 9, 2019

What type of PR is this?

/kind cleanup

/sig scheduling
/priority important-soon

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #83632

Special notes for your reviewer:
/assign @ahg-g

Does this PR introduce a user-facing change?:

[migration phase 1] PodFitsHostPorts as filter plugin

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

NONE
Copy link
Member

left a comment

ditto to copying unit tests.

Copy link
Member

left a comment

Thanks Leon, ditto to copying unit tests.

@wgliang wgliang force-pushed the wgliang:scheduler-v2/pod-fits-host-ports branch from 051d88e to 81cd3d3 Oct 10, 2019
@k8s-ci-robot k8s-ci-robot added size/L and removed size/M labels Oct 10, 2019
@wgliang wgliang force-pushed the wgliang:scheduler-v2/pod-fits-host-ports branch from 81cd3d3 to 5b1b88a Oct 10, 2019
@wgliang

This comment has been minimized.

Copy link
Member Author

commented Oct 10, 2019

@ahg-g Ready for review. Thanks.

@wgliang wgliang force-pushed the wgliang:scheduler-v2/pod-fits-host-ports branch from 5b1b88a to 1dd6c5d Oct 10, 2019
Copy link
Member

left a comment

it seems redundant that we prefix the filter name with "podfits", everything we do is for the pod, lets name this plugin NodePorts

/cc @alculquicondor @Huang-Wei

@wgliang

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

@ahg-g
Ok, these are not synced here.
Are there other comments? I can solve it together, thanks.

@ahg-g

This comment has been minimized.

Copy link
Member

commented Oct 11, 2019

@ahg-g
Ok, these are not synced here.
Are there other comments? I can solve it together, thanks.

Nothing for now, it wasn't my intention to follow up with more comments, just happened that I noticed more things after the first round :)

@wgliang wgliang force-pushed the wgliang:scheduler-v2/pod-fits-host-ports branch 2 times, most recently from af618a6 to 9008f2e Oct 11, 2019
@wgliang

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

/test pull-kubernetes-e2e-gce-100-performance

@ahg-g

This comment has been minimized.

Copy link
Member

commented Oct 11, 2019

Can you please rebase

@wgliang

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

Can you please rebase

Sure. But let us come one by one, please review and merge in the order of #83662/#83660/#83650/#83659, so that it will avoid invalid rebase. Thanks.

@wgliang wgliang force-pushed the wgliang:scheduler-v2/pod-fits-host-ports branch from 9008f2e to 78be6a6 Oct 13, 2019
@wgliang

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2019

Cool, Ready for review. Please take a look, thanks. @ahg-g

@ahg-g

This comment has been minimized.

Copy link
Member

commented Oct 13, 2019

/retest

@ahg-g

This comment has been minimized.

Copy link
Member

commented Oct 13, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Oct 13, 2019
@ahg-g

This comment has been minimized.

Copy link
Member

commented Oct 13, 2019

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, wgliang

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

@ahg-g

This comment has been minimized.

Copy link
Member

commented Oct 13, 2019

/test pull-kubernetes-e2e-gce

@k8s-ci-robot k8s-ci-robot merged commit 5a427e8 into kubernetes:master Oct 13, 2019
15 checks passed
15 checks passed
cla/linuxfoundation wgliang authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
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-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kind Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
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
tide In merge pool.
Details
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Oct 13, 2019
ohsewon added a commit to ohsewon/kubernetes that referenced this pull request Oct 16, 2019
…s-host-ports

[migration phase 1] PodFitsHostPorts as filter plugin
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.