Update kubeadm Windows KEP to reflect new DaemonSet-based approach

[benmoss]

Based on discussions we've had with SIG Windows and SIG Cluster Lifecycle we've decided to revise this KEP to reflect a new approach using DaemonSets to run CNI and kube-proxy. This is the first pass at updating it to reflect the new workflow.

@PatrickLang
@neolit123

/assign @michmike @fabriziopandini @timothysc

/sig windows

[neolit123]

thanks for the update,
some comments for the graduation (as the PR review is not letting me to comment inline there):

Alpha -> Beta Graduation

Kube-proxy and CNI plugins are run as Kubernetes pods.

👍

The feature is maintained by active contributors.

this is unfortunately not true, currently and i'm hoping that people will sign up to do that:
https://github.com/kubernetes-sigs/sig-windows-tools

The feature is tested by the community and feedback is adapted with changes.

people gave some feedback and there are an number of issues and PRs logged in the repository, but nobody is taking ownership.

Kubeadm join performs complete preflight checks on the host node

"complete" might be demanding a bit too much. i think we might want to move this to the GA graduation section.

E2e tests might not be complete but provide good signal.

this section needs and update:

E2e testing for kubeadm on Windows is still being planned. One available option is to run “kubeadm join” periodically on Azure nodes and federate the reports to test-infra/testgrid. The CI signal will be owned by SIG Windows.

we should definitely say that we are going to use CAPA for this.
i (and possibly CAPA folks) can help with the e2e / test-infra logistics.
WDYT?

Documentation is in a good state. Kubeadm documentation is edited to point to documentation provided by SIG Windows.

it will need updates for the beta docs, given the changes in the script.

Kubeadm upgrade is implemented.

PR is here:
kubernetes-sigs/sig-windows-tools#33
my proposal is to have a separate upgrade script, that does not upgrade CNI.

[fejta-bot]

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/check-cla

[benmoss]

this is unfortunately not true, currently and i'm hoping that people will sign up to do that:
https://github.com/kubernetes-sigs/sig-windows-tools

people gave some feedback and there are an number of issues and PRs logged in the repository, but nobody is taking ownership.

The plan is that me and @gab-satchi will take ownership for the scripts once they've been updated in sig-windows-tools. We just haven't been supporting the existing scripts since we've been investigating this alternate approach that we think should make it a lot more simple to maintain and extend.

we should definitely say that we are going to use CAPA for this

I've updated this section

it will need updates for the beta docs, given the changes in the script.

Yes, once the KEP is merged we will move new scripts into sig-windows-tools and update docs.

my proposal is to have a separate upgrade script, that does not upgrade CNI.

I think we might be able to just document kubeadm upgrade on this page, we don't need a wrapper script. I think it should be exactly the same: upgrade kubeadm, kubectl drain, kubeadm upgrade node, upgrade kubelet, restart kubelet, uncordon node.

[neolit123]

thanks for the updates!
/lgtm

/hold
for more review if needed.

/assign @timothysc
for approval.

[kartiklunkad26]

The user value or user story could be re-framed to be
"A user can join a Windows VM to an existing Kubernetes Cluster"

[kartiklunkad26]

Somehow Github didn't allow me to select the line that I actually wanted to review, so here it is!

A user will have a Windows machine that they want to join to an existing Kubernetes cluster.

[neolit123]

The user value or user story could be re-framed to be
"A user can join a Windows VM to an existing Kubernetes Cluster"

Windows Nodes in a k8s cluster are not necessarily VMs only, thus "machines" is a better generalization.

[neolit123]

/approve
hold for review until Monday. deadline is EOD Tuesday, Pacific.

[neolit123]

/hold

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benmoss, neolit123

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:

• keps/sig-cluster-lifecycle/OWNERS [neolit123]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[fabriziopandini]

The windows land is out of my area of knowledge, but my personal understanding is that the user story is much more simpler now. If Introducing the new dependency to wings is ok for the majority, I'm +1 on this changes

[PatrickLang]

/milestone v1.18

[neolit123]

/hold cancel
let's amend with followup PRs if needed.

[tallclair]

What are the advantages to running this way, as opposed to just using a native service (as with kubelet)?

[neolit123]

kube-proxy and flanneld, require a scoped kubeconfig to access certain resources related to these components. the kubeconfig has to be copied to the workers, using e.g. SSH.

running kube-proxy and flannel as DaemonSets

• solves the problem of having to copy the priv. kubeconfig as part of the setup steps.
• aligns the deployment of the components with kubeadm on Linux,. where these are run as DS.

[tallclair]

How will kube-proxy access it's service account credentials?

[neolit123]

this is currently done like so:
https://github.com/benmoss/kubeadm-windows/blob/master/kube-proxy/kube-proxy.yml#L10

the KubeProxyConfiguration has:
certificate-authority: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt

[tallclair]

I'm not wild about depending on PSP to enforce this behavior. Here are 2 alternative ideas:

1. Custom admission controller for dealing with windows pods. Only allow the wins pipe to be mounted (applies to the whole directory tree - does windows support hard linking?) if the requesting container is running as privileged.
2. Put an authenticating proxy in front of wins. Require a service account token to make privileged requests, and check against the podspec whether a request with that privilege level should be allowed. I'm not sure I understand how windows containers / wins work well enough to know whether this approach would be viable.
3. (A weaker version of 2) Use the authenticating proxy, but just check for the presence of a static secret. The secret would be added to a kubernetes secret, and mounted into containers that need to access wins. Perhaps rancher would consider an upstream patch to build this capability in?

[neolit123]

I'm not wild about depending on PSP to enforce this behavior.

i concur.

would defer to @benmoss and @PatrickLang to the Windows specific questions.

1 seems fine to me, but if understand this correctly it requires that the controller is included in core k8s. AFAIK, wins currently lacks an authorization / authentication model so maybe this is the way forward as long as the project is OK with that.

[gab-satchi]

Thanks @tallclair @neolit123

We arrived at using PSPs by following the pattern for privileged containers in Linux as privileged containers were kind of similar to what was being offered by wins. Can you elaborate on your reluctance to use PSPs?

• Are PSPs insufficient in protecting against malicious pods mounting that wins pipe?
• Is it the idea of relying on an operator to apply PSPs and not having things secured out of the box that's hard to accept?

[neolit123]

thanks for the comments @tallclair ,

@benmoss is the main driver for this change, but he is on PTO until mid-February.
@gab-satchi might have comments too, as he helped with some of the work on the KEP.