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

kubeadm: disable the kube-proxy DaemonSet on non-Linux nodes #76327

Merged
merged 1 commit into from May 3, 2019

Conversation

@neolit123
Copy link
Member

commented Apr 9, 2019

What this PR does / why we need it:

Windows worker nodes run kube-proxy as a Windows service.
In the future the kube-proxy DaemonSet might run on Windows nodes
too, but for now a temporary measure is needed to disable it.

Add a linux node selector in the kube-proxy manifest spec.
Similar was done already for CoreDNS.

Which issue(s) this PR fixes:

xref kubernetes/kubeadm#1393

Special notes for your reviewer:
NONE

Does this PR introduce a user-facing change?:

kubeadm: disable the kube-proxy DaemonSet on non-Linux nodes. This step is required to support Windows worker nodes.

/kind cleanup
/priority important-longterm
@kubernetes/sig-cluster-lifecycle-pr-reviews
/assign @timothysc @fabriziopandini
cc @PatrickLang

kubeadm: disable the kube-proxy DaemonSet on non-Linux nodes
Windows worker nodes run kube-proxy as a Windows service.
In the future the kube-proxy DaemonSet might run on Windows nodes
too, but for now a temporary measure is needed to disable it.

Add a linux node selector in the kube-proxy manifest spec.
@neolit123

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

/sig windows

@SataQiu

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

LGTM

@rosti

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

My question is how we are going to replace the DaemonSet? I know that the answer is a Windows service ATM. However, do we expect, that users do their own kube-proxy provisioning or should kubeadm do it instead?

It looks good to me, but I want to know if we should be providing some replacement.

@neolit123

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

My question is how we are going to replace the DaemonSet? I know that the answer is a Windows service ATM. However, do we expect, that users do their own kube-proxy provisioning or should kubeadm do it instead?

the plan is for kubeadm to not manage kube-proxy on Windows workers until the DaemonSet works.
we are going to have a wrapper around kubeadm that starts kube-proxy as a Windows service.

@rosti

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

we are going to have a wrapper around kubeadm that starts kube-proxy as a Windows service.

Now this sounds horrible, unfortunately the alternative is to have an optional add-on per-node that deploys kube-proxy as a service on Windows nodes and this is even worse.
So I guess, that leaving it alone until DaemonSet kube-proxy works on Windows is a good choice for now. It does not imply the introduction of some technical debt for us (other than making sure to remove the node selector later on).

/lgtm
/hold
For @timothysc & @fabriziopandini 's opinion.

@detiber

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

Can't say I'm a fan of the approach, but at the same time I don't have a better alternative, so a hesitant +1 from me.

@fabriziopandini

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

I'm ok with this PR, but I'm not sure about

we are going to have a wrapper around kubeadm that starts kube-proxy as a Windows service.

On Linux, systemd service creation is not a kubeadm task, but it is something delegated to OS specific packages (deb or rpm); kubeadm task usually is limited to service configuration/start.

My first reaction is that Iwe should follow the same approach also on windows/for kube-proxy.
An OS specific package should install kube-proxy and create the service, then Kubeadm should check the service exists in preflight, then pull the configuration and start the service during join

But TBH I didn't investigated this in detail, so might be I'm missing something 😅

@neolit123

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

My first reaction is that Iwe should follow the same approach also on windows/for kube-proxy.
An OS specific package should install kube-proxy and create the service, then Kubeadm should check the service exists in preflight, then pull the configuration and start the service during join

WIndows nodes are blocked on privileged containers at the moment.
so this is the reason kube-proxy is run as a Windows service.

one day kube-proxy will be able to run as a DaemonSet on Windows too, until then we are heavily leaning towards the wrapper approach so that we don't maintain such code in kubeadm.

also i wanted to explain the Windows / kubeadm discussions i had recently with SIG Windows, but i won't be able to attend this week's kubeadm office hours.

@@ -112,5 +112,7 @@ spec:
- key: CriticalAddonsOnly
operator: Exists
- operator: Exists
nodeSelector:

This comment has been minimized.

Copy link
@timothysc

timothysc Apr 9, 2019

Member

I'm not familiar with windows deployments, but how does service routing work on window then?

This comment has been minimized.

Copy link
@neolit123

neolit123 Apr 9, 2019

Author Member

it works somehow, this is more of a question for the SIG Window folk.

This comment has been minimized.

Copy link
@timothysc

This comment has been minimized.

Copy link
@benmoss

benmoss Apr 25, 2019

Member

kube-proxy still runs and handles service routing, it's just that it needs to run on the host directly and can't be run from inside a privileged container.

@timothysc
Copy link
Member

left a comment

? below

@neolit123

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

let's hold this PR regardless, so that we can get the KEP in place that answers the questions.
/approve cancel

@timothysc
Copy link
Member

left a comment

kubernetes/enhancements#994

/approve
/hold cancel

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented May 2, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: timothysc

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

@fejta-bot

This comment has been minimized.

Copy link

commented May 3, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@fejta-bot

This comment has been minimized.

Copy link

commented May 3, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 7defecb into kubernetes:master May 3, 2019

19 of 20 checks passed

pull-kubernetes-kubemark-e2e-gce-big Job triggered.
Details
cla/linuxfoundation neolit123 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 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-storage-slow Skipped.
pull-kubernetes-godeps Context retired. Status moved to "pull-kubernetes-dependencies".
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e 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
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.