setting Windows Node KEP to implementable #743

michmike · 2019-01-28T22:48:18Z

this PR should be the final one to set the Windows Node KEP to implementable for the 1.14 release of K8s. We want to promote the functionality to stable

michmike · 2019-01-28T22:48:43Z

/hold
putting a hold until we get the 4 approvers to lgtm

bgrant0607 · 2019-01-28T23:09:32Z

Thanks. I'm looking over the graduation criteria carefully to make sure they are sufficiently clear, to ensure we all agree what we're agreeing to.

It's worth mentioning release binary distribution somewhere. I had to go look up whether that issue was already resolved.
https://kubernetes.io/docs/getting-started-guides/windows/#get-windows-binaries

Example:
https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.12.md#node-binaries
https://dl.k8s.io/v1.12.5/kubernetes-node-windows-amd64.tar.gz

bgrant0607 · 2019-01-28T23:18:54Z

keps/sig-windows/20190103-windows-node-support.md

+- QoS (guaranteed, burstable, best effort) (https://github.com/kubernetes/kubernetes/issues/73418)
+- Is terminationGracePeriodSeconds supported (https://github.com/moby/moby/issues/25982)
+- Pod DNS configuration like hostname, subdomain, hostAliases, dnsConfig, dnsPolicy (https://github.com/kubernetes/kubernetes/issues/73414)
+
 ### Windows Node Roadmap (post-GA work)
 - Group Managed Service Accounts, a way to assign an Active Directory identity to a Windows container, is forthcoming with KEP `Windows Group Managed Service Accounts for Container Identity`
 - `kubectl port-forward` hasn't been implemented due to lack of an `nsenter` equivalent to run a process inside a network namespace.


Do exec-based mechanisms work, such as liveness and readiness probes, postStart and preStop lifecycle hooks, and kubectl exec?

kubectl exec works, readiness and liveness work

kubectl exec works for sure. i am not sure about the other probes. we can find out

Is it that these features of Kubernetes are just not tested, or that the existing tests are all Linux-specific?

added a test issue to cover the rest and documented them in the KEP as well

bgrant0607 · 2019-01-28T23:31:48Z

Regarding graduation criteria, please remove "Advanced" from bullets 8-12, as that's unclear what it means. I imagine some of those topics may be considered optional for GA. I suggest just removing them from this KEP. For instance, hyper-v support isn't part of GA, so it seems like that should be removed. GMSA will be alpha at best in 1.14, and its documentation should be covered by its own KEP, rather than listing it here. OTOH, I think heterogeneous Linux/Windows clusters will be the norm (e.g., does kube-dns run on Windows nodes?), so I'd definitely include that. How to build from source should be in the contributor development guide.

bgrant0607 · 2019-01-28T23:35:46Z

Regarding metrics and Horizontal Pod Autoscaling: from a user point of view, what's different about them for Windows containers that requires Windows-specific documentation?

bgrant0607 · 2019-01-28T23:37:49Z

keps/sig-windows/20190103-windows-node-support.md

    - Resource limits
    - Pod & container metrics
    - Horizontal Pod Autoscaling
+    - Volumes can be shared between containers in a Pod


It would be helpful to group pod-related features together in this list, for readability.

Nit: This is still an issue, but not a blocker for merge.

I'd like to see something like:

Pod

single and multiple containers

resource limits

liveness and readiness probes

ConfigMap and Secret environment variables and volumes (except volume subpath does not work)

volumes can be shared between containers within a Pod

etc.

Workload controllers

ReplicaSet / ReplicationController

Deployment

Job

etc.

Service types ...

HorizontalPodAutoscaler, except ...

etc.

michmike · 2019-01-29T00:23:21Z

Regarding graduation criteria, please remove "Advanced" from bullets 8-12, as that's unclear what it means. I imagine some of those topics may be considered optional for GA. I suggest just removing them from this KEP. For instance, hyper-v support isn't part of GA, so it seems like that should be removed. GMSA will be alpha at best in 1.14, and its documentation should be covered by its own KEP, rather than listing it here. OTOH, I think heterogeneous Linux/Windows clusters will be the norm (e.g., does kube-dns run on Windows nodes?), so I'd definitely include that. How to build from source should be in the contributor development guide.

Brian, the key thing with that documentation was not that it will all reside in a single doc, but merely outlining all the sections or individual pages we need to fill out. i will clarify that for sure

michmike · 2019-01-29T00:24:49Z

cs and Horizontal Pod Autoscaling: from a user point of view, what's different about them for Windows containers that requires Windows-specific documentation?

there are some metrics that may not be available on windows. we don't know if any will have an impact to autoscaling, but if such an issue exists we will document it. an example is kubernetes/kubernetes#72788

bgrant0607 · 2019-01-29T03:51:30Z

keps/sig-windows/20190103-windows-node-support.md

@@ -111,20 +123,29 @@ As of 29-11-2018 much of the work for enabling Windows nodes has already been co

 ### What will never work (Note that some features are plain unsupported while some will not work without underlying OS changes)
 - Certain Pod functionality
-    - Privileged containers and other Pod security context privilege and access control settings
+    - Privileged containers
+    - Pod security context privilege and access control settings. Any Linux capabilities or any POSIX capabilities are not supported (this includes SELinux, AppArmor, Seccomp, etc)
    - Reservations are not enforced by the OS, but overprovisioning could be blocked with `--enforce-node-allocatable=pods` (pending: tests needed)
    - Certain volume mappings
      - Single file & subpath volume mounting


Will single-file mapping become possible? This comment makes it sound like it will:
kubernetes/kubernetes#70189 (comment)

My understanding is that this will be supported with containerd (WIP), but not possible right now with the docker integration (which is used in GA).

you are correct @yujuhong

bgrant0607 · 2019-01-29T03:56:34Z

keps/sig-windows/20190103-windows-node-support.md

+- Headless services (https://github.com/kubernetes/kubernetes/issues/73416)
+- OOM reporting (https://github.com/kubernetes/kubernetes/issues/73417)
+- QoS (guaranteed, burstable, best effort) (https://github.com/kubernetes/kubernetes/issues/73418)
+- Is terminationGracePeriodSeconds supported (https://github.com/moby/moby/issues/25982)


terminationGracePeriodSeconds is now mentioned below

bgrant0607 · 2019-01-29T03:57:07Z

keps/sig-windows/20190103-windows-node-support.md

@@ -111,20 +123,29 @@ As of 29-11-2018 much of the work for enabling Windows nodes has already been co

 ### What will never work (Note that some features are plain unsupported while some will not work without underlying OS changes)
 - Certain Pod functionality
-    - Privileged containers and other Pod security context privilege and access control settings
+    - Privileged containers
+    - Pod security context privilege and access control settings. Any Linux capabilities or any POSIX capabilities are not supported (this includes SELinux, AppArmor, Seccomp, etc)


FYI, capabilities is itself an actual field

bgrant0607 · 2019-01-29T03:58:05Z

The more up-to-date PR for skipping tests should be kubernetes/kubernetes#73204

bgrant0607 · 2019-01-29T04:04:55Z

Why is a windows-specific test needed to verify imagePullPolicy behaviors?

bgrant0607 · 2019-01-29T04:07:40Z

Is the reference to kubernetes/kubernetes#56875 still relevant? The KEP asserts that FlexVolume now works.

bgrant0607 · 2019-01-29T04:08:34Z

The other issues in the References section are to closed issues, also

bgrant0607 · 2019-01-29T04:10:44Z

Do service environment variables work, or only DNS?

https://kubernetes.io/docs/concepts/services-networking/connect-applications-service/#accessing-the-service

PatrickLang · 2019-01-29T20:50:17Z

Do service environment variables work, or only DNS?

https://kubernetes.io/docs/concepts/services-networking/connect-applications-service/#accessing-the-service

One of the apps & charts I used in Kubecon used service environment variables: https://github.com/PatrickLang/fabrikamfiber/tree/helm-2019-mssql-linux

https://github.com/PatrickLang/fabrikamfiber/blob/6a149e7269d7fcde87efd93ff791e37e45e54945/FabrikamFiber.Web/Web.config#L19

DB_SERVICE_HOST and DB_SERVICE_PORT are service variables, DB_SA_PASSWORD is a secret

dims · 2019-01-29T21:58:25Z

keps/sig-windows/20190103-windows-node-support.md

+9. How to use Group Managed Service Accounts
+10. How to use Taints and Tolerations for a heterogeneous compute cluster (Windows + Linux)
+11. How to use Hyper-V isolation (not a stable feature yet)
+12. How to build Kubernetes for Windows from source


This URL right? https://github.com/PatrickLang/kubernetes-windows-dev#setting-up-a-devbuild-environment

we will revamp it, but yes that's a good start

I think that's only if you need to build k8s on a Windows dev box. On Linux or OSX make cross KUBE_BUILD_PLATFORMS="windows/amd64" is all you need.

dims · 2019-01-29T22:08:18Z

@michmike i'd like the Summary section to explicitly state what is being talked about.

Allow Windows based nodes to be added to Kubernetes Clusters and enable Windows Server containers based workloads to be scheduled to these nodes. Right?

michmike · 2019-01-29T22:17:28Z

@michmike i'd like the Summary section to explicitly state what is being talked about.

Allow Windows based nodes to be added to Kubernetes Clusters and enable Windows Server containers based workloads to be scheduled to these nodes. Right?

added

bgrant0607 · 2019-01-29T22:45:22Z

keps/sig-windows/20190103-windows-node-support.md

+- `V1.Pod.shareProcessNamespace` - this is an beta feature, and depends on Linux namespaces which are not implemented on Windows. Windows cannot share process namespaces or the container's root filesystem. Only the network can be shared.
+- `V1.Pod.terminationGracePeriodSeconds` - this is not fully implemented in Docker on Windows, see: [reference](https://github.com/moby/moby/issues/25982). The behavior today is that the ENTRYPOINT process is sent `CTRL_SHUTDOWN_EVENT`, then Windows waits 5 seconds by hardcoded default, and finally shuts down all processes using the normal Windows shutdown behavior. The 5 second default is actually in the Windows registry [inside the container](https://github.com/moby/moby/issues/25982#issuecomment-426441183), so it can be overridden when the container is built. Runtime configuration will be feasible in CRI-ContainerD but not for v1.14. Issue [#73434](https://github.com/kubernetes/kubernetes/issues/73434) is tracking this for a later release.
+- `V1.Pod.volumeDevices` - this is an beta feature, and is not implemented on Windows. Windows cannot attach raw block devices to pods.
+- `V1.Pod.Volumes` - EmptyDir, Secret, ConfigMap, HostPath - all work and have tests in TestGrid


Nit: This is the Go field name whereas the above are json field names. But this can be cleaned up later. Definitely it should be consistent in user documentation. Ditto for the field below.

bgrant0607 · 2019-01-29T23:58:43Z

/lgtm
/approve

Thanks for all the work on this. I have a much more complete picture of the knowns and unknowns now.

I'm sure we'll discover more things as work progresses, so let's keep this up to date.

bgrant0607 · 2019-01-30T00:00:41Z

Please squash. Remaining issues can be addressed by follow-up PRs

yujuhong

Thanks for adding the compatibility version!

yujuhong · 2019-01-30T00:09:57Z

keps/sig-windows/20190103-windows-node-support.md

+As noted above, there are compatibility issues enforced by Microsoft where the host OS version must match the container base image OS. Changes to this compatibility policy must come from Microsoft. For GA, since we will only support Windows Server 2019 (aka 1809), both `container host OS` and `container OS` must be running the same version of Windows, 1809. 
+
+Having said that, a customer can deploy Kubernetes v1.14 with Windows 1809.
+- We will support Windows 1809 with at least 2 additional Kuberneres minor releases (v1.15 and v.1.16)


s/v.1.16/v1.16

yujuhong · 2019-01-30T00:14:15Z

/lgtm

bgrant0607 · 2019-01-30T01:42:25Z

/lgtm

k8s-ci-robot · 2019-01-30T01:42:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benmoss, bgrant0607, michmike, yujuhong

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-windows/OWNERS~~ [bgrant0607,michmike]

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

bgrant0607 · 2019-01-30T01:47:19Z

@spiffxp said he'd follow up later on test infrastructure issues, such as provider-specific dependencies

So:
/hold cancel

bgrant0607 · 2019-02-04T15:49:29Z

keps/sig-windows/20190103-windows-node-support.md

    - Workload controllers ReplicaSet, ReplicationController, Deployments, StatefulSets, DaemonSet, Job, CronJob
-    - ConfigMap, Secrets: as environment variables or volumes
+    - ConfigMap, Secrets: as environment variables or volumes (Volume subpath does not work)


When I was reviewing kubernetes/kubernetes#73204, another difference occurred to me: On Linux, Secret volumes are written to tmpfs, but I assume that they are written unencrypted to persistent storage on Windows, since it doesn't support ram disks?

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 28, 2019

k8s-ci-robot requested review from mattfarina and PatrickLang January 28, 2019 22:48

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 28, 2019

bgrant0607 reviewed Jan 28, 2019

View reviewed changes

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 28, 2019

bgrant0607 reviewed Jan 28, 2019

View reviewed changes

michmike mentioned this pull request Jan 29, 2019

Adding API section to Windows KEP #732

Merged

bgrant0607 reviewed Jan 29, 2019

View reviewed changes

k8s-ci-robot assigned benmoss Jan 29, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 29, 2019

dims reviewed Jan 29, 2019

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 29, 2019

bgrant0607 reviewed Jan 29, 2019

View reviewed changes

k8s-ci-robot assigned bgrant0607 Jan 29, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 29, 2019

yujuhong reviewed Jan 30, 2019

View reviewed changes

k8s-ci-robot assigned yujuhong Jan 30, 2019

Updating the Windows Node KEP to Implementable

78d0a66

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2019

k8s-ci-robot merged commit 94325b7 into kubernetes:master Jan 30, 2019

bgrant0607 reviewed Feb 4, 2019

View reviewed changes

justaugustus removed the sig/pm label Apr 19, 2020

setting Windows Node KEP to implementable #743

setting Windows Node KEP to implementable #743

Conversation

michmike commented Jan 28, 2019

michmike commented Jan 28, 2019

bgrant0607 commented Jan 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgrant0607 commented Jan 28, 2019

bgrant0607 commented Jan 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michmike commented Jan 29, 2019

michmike commented Jan 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgrant0607 commented Jan 29, 2019

bgrant0607 commented Jan 29, 2019

bgrant0607 commented Jan 29, 2019

bgrant0607 commented Jan 29, 2019

bgrant0607 commented Jan 29, 2019

PatrickLang commented Jan 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dims commented Jan 29, 2019

michmike commented Jan 29, 2019

Choose a reason for hiding this comment

bgrant0607 commented Jan 29, 2019

bgrant0607 commented Jan 30, 2019

yujuhong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yujuhong commented Jan 30, 2019

bgrant0607 commented Jan 30, 2019

k8s-ci-robot commented Jan 30, 2019

bgrant0607 commented Jan 30, 2019

Choose a reason for hiding this comment