-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 for Windows KEP #994
Conversation
/assign @timothysc |
@kubernetes/sig-cluster-lifecycle-pr-reviews |
/assign @michmike @fabriziopandini @rosti |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this proposal!
The KEP in its shape and form is looking like imposing a wrapper script and Flannel in the long run. These are both unacceptable as a long term solution, but I can see the point of their use in a few releases.
I think, that the long term goal of this KEP should be to have kubeadm join and reset be implemented on Windows with as similar and as simple UX as on Linux.
For that matter we'll have to employ privileged containers on Windows, get kube-proxy & CNI plugins to run in pods again, and ideally, get rid of the wrapper script.
Of course, this rosy dream of mine relies on getting privileged Windows containers.
@PatrickLang is there any possibility of getting privileged containers on Windows in the future?
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
I have a questionably acceptable idea to workaround this, but basically a RPC server running on the host that gets mounted into the container via a named pipe. It obviously is still less ideal than if privileged containers could be added to Windows, but assuming we can get a general-enough RPC API, we could then be in the position of distributing/maintaining the provisioning code via pods. |
we need to evaluate the roadmap of privilege containers on Windows. the RPC idea seems like something that can be proposed in a document, but workarounds may still block the Beta. for this KEP it seems that we might want to hold onto the Windows services idea. |
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I stopped reviewing part way through b/c imo this effort should span releases. I don't want to force this through given other higher priority efforts. I'd much rather us refactor properly.
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
|
||
To support Windows specific flags for the kubelet, there is a requirement to split kubeadm’s app/phases/kubelet/flags.go files into two: | ||
* app/phases/kubelet/flags_windows.go | ||
* app/phases/kubelet/flags_linux.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like work from the componentconfig folks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you elaborate on your ideas?
this could end up being something quite isolated to kubeadm -> kubelet.
if os == "windows" { use flagset A } else { use flag set B }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could build meta-data in apis ~= +omitempty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the current state of kubelet's component config (which is at v1beta1), there are a few command line flags that don't have a corresponding field in the config. Those are:
- dockershim related flags
container-runtime
&&container-runtime-endpoint
flagsregister-with-taints
flaghostname-override
Only the resolv-conf
flag has a representation in the component config. However, we would have to patch it, after fetching the config from the config map, for the local machine setting to take place.
|
||
Kubeadm makes a number of non-portable assumptions about paths. E.g. “/etc/kubernetes” is a hardcoded path in kubeadm. | ||
|
||
We need to evaluate the kubeadm codebase for such instances of non-portable paths - CRI sockets, Cert paths, etc. Such paths need to be defaulted properly in the kubeadm configuration API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this should span several releases and be done judiciously, I want us todo this wisely and not in a rush.
/hold |
This is a feasible idea, and I completed a POC for this a month ago. It works. I think we can propose both ideas in the doc, and I can advocate for both. This is a more immediate option than waiting on privileged containers, but I think the community should discuss. |
There's a lot of circling around "privileged containers on Windows". There are some Windows engineers looking into this, but I don't have a clear answer on whether some form of privileged containers could be made to work on Windows Server 2019, or if it would take a new OS version. I'm trying to get them to write up a public proposal but don't expect anything for at least a few weeks. |
Thanks for the clarification @PatrickLang ! |
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
|
||
To support Windows specific flags for the kubelet, there is a requirement to split kubeadm’s app/phases/kubelet/flags.go files into two: | ||
* app/phases/kubelet/flags_windows.go | ||
* app/phases/kubelet/flags_linux.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you elaborate on your ideas?
this could end up being something quite isolated to kubeadm -> kubelet.
if os == "windows" { use flagset A } else { use flag set B }
poke me on slack after it's been updated to include the content changes from yesterdays review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
/hold
Please address the comments and I'll let @neolit123 do the final lgtm.
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
|
||
To support Windows specific flags for the kubelet, there is a requirement to split kubeadm’s app/phases/kubelet/flags.go files into two: | ||
* app/phases/kubelet/flags_windows.go | ||
* app/phases/kubelet/flags_linux.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could build meta-data in apis ~= +omitempty
|
||
This proposal plans for FlannelD as the default option. Currently, FlannelD has to be started before the kube-proxy Windows service is started. FlannelD creates an HNS network on the Windows host, and kube-proxy will crash if it cannot find the network. This should be fixed in the scope of this project so that kube-proxy will wait until the network comes up. Therefore, kube proxy can be started at any time. | ||
|
||
However, if FlannelD is deployed in VXLAN (Overlay) mode, then we need to rewrite the KubeProxyConfiguration with the correct Overlay specific values, and kube-proxy will need to read this config again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're going to need to specify the default path settings that flow through the kubelet config as well here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few minors,
At first sight, this requires a considerable amount of work, so let's get this started!
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with how things are ATM. It's clear that this is still provisional and would require a lot of work done before things are set in stone.
However, let's be mindful, that we should update the KEP when the actual implementation deviates from initial designs here.
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
|
||
To support Windows specific flags for the kubelet, there is a requirement to split kubeadm’s app/phases/kubelet/flags.go files into two: | ||
* app/phases/kubelet/flags_windows.go | ||
* app/phases/kubelet/flags_linux.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the current state of kubelet's component config (which is at v1beta1), there are a few command line flags that don't have a corresponding field in the config. Those are:
- dockershim related flags
container-runtime
&&container-runtime-endpoint
flagsregister-with-taints
flaghostname-override
Only the resolv-conf
flag has a representation in the component config. However, we would have to patch it, after fetching the config from the config map, for the local machine setting to take place.
keps/sig-cluster-lifecycle/kubeadm/20190424-kubeadm-for-windows.md
Outdated
Show resolved
Hide resolved
* Installing the Container Runtime (e.g. Docker or containerd) | ||
* Implement kubeadm init for Windows | ||
* Implement kubeadm join --control-plane for Windows (at this time) | ||
* Supporting upgrades using kubeadm upgrade for Windows (to be revisited for Beta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going through the code, I think, that this would be quite easy to do for a worker node. The only kubeadm portion here that need revising is kubeadm upgrade node config
. It has a tiny bit of OS specific code, that is shared with join and, thus, it would have been taken care of with the kubeadm join
porting effort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates and squash @ksubrmnn !
Let's get this in.
/lgtm
/hold cancel
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ksubrmnn, rosti, 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 |
KEP PR for Issue #995