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

Fix Kubeproxy to work for Windows Kernel mode #51064

Merged
merged 4 commits into from Sep 20, 2017

Conversation

@madhanrm
Copy link
Contributor

commented Aug 22, 2017

What this PR does / why we need it:
Kubeproxy doenst work for with windows kernel mode. This PR adds a Kernel Proxy for windows to use the underlying platform features.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #
#49666
Special notes for your reviewer:

Release note:

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2017

Hi @madhanrm. 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.

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. I understand the commands that are listed here.

@freehan

This comment has been minimized.

Copy link
Member

commented Aug 22, 2017

/ok-to-test

@dineshgovindasamy

This comment has been minimized.

Copy link

commented Aug 23, 2017

Tagging @jdumars for FYI

@jdumars

This comment has been minimized.

Copy link
Member

commented Aug 23, 2017

/sig node
/sig windows

@jdumars

This comment has been minimized.

Copy link
Member

commented Aug 23, 2017

@k8s-ci-robot k8s-ci-robot requested a review from mikedanese Aug 23, 2017

@JMesser81

This comment has been minimized.

Copy link

commented Aug 24, 2017

@freehan - let's discuss at next sig-network meeting. We believe this is a low risk change which does not affect Linux at all or current Windows implementations using OVN or Apprenda solution.

@madhanrm madhanrm force-pushed the madhanrm:winkernelproxy branch 2 times, most recently from 8a50167 to e65d663 Aug 24, 2017

@aserdean

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2017

(this is not a technical review)

Please break: 6e6d132 into
smaller commits, as described in https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#6-squashing-and-commit-titles.
It makes it easier for other people to review it and understand your logic
behind the changes.

Please add relevant commit messages to each individual commit, it helps
other people understand how you are trying to achieve your goal and of
course keep track of the evolution of the code.

@madhanrm madhanrm force-pushed the madhanrm:winkernelproxy branch from e65d663 to 40512e6 Aug 25, 2017

@madhanrm madhanrm force-pushed the madhanrm:winkernelproxy branch from 40512e6 to 9ee854f Aug 25, 2017

@jdumars

This comment has been minimized.

Copy link
Member

commented Aug 28, 2017

/cc @thockin

@k8s-ci-robot k8s-ci-robot requested a review from thockin Aug 28, 2017

@mikedanese mikedanese assigned bowei and thockin and unassigned lavalamp Aug 28, 2017

@madhanrm

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2017

/test pull-kubernetes-cross
/test pull-kubernetes-kubemark-e2e-gce
/test pull-kubernetes-e2e-gce-gpu
/test pull-kubernetes-bazel
/test pull-kubernetes-verify

@madhanrm madhanrm force-pushed the madhanrm:winkernelproxy branch from cb8c0e2 to b5ca797 Sep 15, 2017

Add exception to golint check
(*) Fix cleanup of NodePort resources. (*) Fix the logic to select existing policies

Fix review comment

Fix Bazel

Update GoDep License

Fix NodePort forwarding to target port

Fix Darwin Build break. +1

Implement IsCompatible to validate kernel support for kernel mode

@dineshgovindasamy dineshgovindasamy force-pushed the madhanrm:winkernelproxy branch from b5ca797 to a8d797a Sep 18, 2017

@dims

This comment has been minimized.

Copy link
Member

commented Sep 18, 2017

/retest

1 similar comment
@madhanrm

This comment has been minimized.

Copy link
Contributor Author

commented Sep 18, 2017

/retest

@dims

This comment has been minimized.

Copy link
Member

commented Sep 19, 2017

/test all

@dims

This comment has been minimized.

Copy link
Member

commented Sep 19, 2017

@madhanrm Can you please check the test failures and get the tests go green? i am afraid this will miss the 1.8 boat otherwise

@dineshgovindasamy

This comment has been minimized.

Copy link

commented Sep 19, 2017

/retest

1 similar comment
@dineshgovindasamy

This comment has been minimized.

Copy link

commented Sep 19, 2017

/retest

@thockin

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

Is this ready?

@dims

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

the kubeadm-gce is stuck in "job triggered", trying to get that to run

/test all

@dineshgovindasamy

This comment has been minimized.

Copy link

commented Sep 20, 2017

@dims @thockin i have tried multiple retest and Kubeadm-gce is not at all getting triggered. I see that job is failing across multiple PR's. How do we proceed?

@dims

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

@fejta @spiffxp any tips here?

@krzyzacy

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

kubernetes/test-infra#4649

but do you care about the kubeadm job on your PR? that's a non-blocking job.

@krzyzacy

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

If it's ready, ask for a lgtm label and this can be picked up by sq/get merged regardless of kubeadm presubmit.

@dims

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

Looks like @thockin applied lgtm at least 2 times... and it was rebased away. let me try doing that

/assign

@dims

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Sep 20, 2017

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2017

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, madhanrm, thockin

Associated issue: 49666

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2017

/test all [submit-queue is verifying that this PR is safe to merge]

@BenTheElder

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

@dims

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

/test pull-kubernetes-unit

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2017

Automatic merge from submit-queue (batch tested with PRs 51064, 52132). If you want to cherry-pick this change to another branch, please follow the instructions here..

@k8s-github-robot k8s-github-robot merged commit a16a126 into kubernetes:master Sep 20, 2017

12 of 15 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-unit
Details
pull-kubernetes-e2e-kubeadm-gce Parent Job Status Changed: Job triggered.
pull-kubernetes-unit Jenkins job triggered.
Details
cla/linuxfoundation madhanrm authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Jenkins job succeeded.
Details
pull-kubernetes-e2e-gce-bazel Job succeeded.
Details
pull-kubernetes-e2e-gce-etcd3 Jenkins job succeeded.
Details
pull-kubernetes-e2e-gce-gpu Jenkins job succeeded.
Details
pull-kubernetes-e2e-kops-aws Jenkins job succeeded.
Details
pull-kubernetes-federation-e2e-gce Jenkins job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Jenkins job succeeded.
Details
pull-kubernetes-node-e2e Jenkins job succeeded.
Details
pull-kubernetes-verify Jenkins job succeeded.
Details
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.