[docs][windows]: Pod OS field update #30436

ravisantoshgudimetla · 2021-11-10T20:01:14Z

xref: kubernetes/enhancements#2802

cc @marosset @liggitt @jsturtevant @aravindhp @jayunit100

netlify · 2021-11-10T20:01:20Z

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

🔨 Explore the source changes: 89e7446

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/61a69d3a76a678000748cece

marosset · 2021-11-10T20:25:38Z

/sig windows

aravindhp · 2021-11-10T21:05:43Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

@@ -516,26 +517,34 @@ work between Windows and Linux:
  node. They should be applied to all containers as a best practice if the operator
  wants to avoid overprovisioning entirely.
 * `securityContext.allowPrivilegeEscalation` -
-   not possible on Windows; none of the capabilities are hooked up
+   not possible on Windows; none of the capabilities are hooked up. Pod API


There are other places in the doc that talks about these fields under securityContext. Shouldn't we also mention it there? Or is that not done for features behind a feature gate?

jsturtevant

Is there general guidance that we should include to configure this properly like you must also set the node selector == windows for the time being?

jsturtevant · 2021-11-10T21:53:04Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

   Windows does not have a root user. The closest equivalent is `ContainerAdministrator`
-   which is an identity that doesn't exist on the node.
+   which is an identity that doesn't exist on the node. 


Surprised this isn't updated against this base: https://github.com/kubernetes/website/pull/30256/files

jsturtevant · 2021-11-10T21:54:01Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

 * `securityContext.runAsUser` -
   use [`runAsUsername`](/docs/tasks/configure-pod-container/configure-runasusername)
-   instead
+   instead. Pod API
+   validation would fail when `IdentifyPodOS` featuregate is enabled.


can we re-order these two sentences. When I read it I thought we said runAsUsername would fail

jsturtevant · 2021-11-10T21:56:27Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

 * `hostNetwork` - There is no Windows OS support to share the host network
 * `dnsPolicy` - setting the Pod `dnsPolicy` to `ClusterFirstWithHostNet` is
   not supported on Windows because host networking is not provided. Pods always
   run with a container network.
 * `podSecurityContext` (see below)
 * `shareProcessNamespace` - this is a 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.
+  the container's root filesystem. Only the network can be shared. Pod API


side note, with HPC we do share the root filesystem not sure if that is worth noting since it is out of context of shared namespaces

jlbutler · 2021-11-11T15:33:11Z

/assign @mehabhalodiya

marosset · 2021-11-11T17:26:01Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

@@ -153,6 +153,7 @@ section refers to several key workload enablers and how they map to Windows.
  * `emptyDir` volumes
  * Named pipe host mounts
  * Resource limits
+  * OS field: This value should be set to `windows` to indicate that the current pod is a Windows pod.


We should either add a note around here saying this functionality is in alpha or link to another page/section that does so.

marosset · 2021-11-11T17:32:15Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

@@ -516,26 +517,34 @@ work between Windows and Linux:
  node. They should be applied to all containers as a best practice if the operator
  wants to avoid overprovisioning entirely.
 * `securityContext.allowPrivilegeEscalation` -
-   not possible on Windows; none of the capabilities are hooked up
+   not possible on Windows; none of the capabilities are hooked up. Pod API
+   validation would fail when `IdentifyPodOS` featuregate is enabled.


I think for now it might be less confusing if we left this section as is and added a new section just for describing the new behavior if IdenfityPodOS is enabled and set to windows.
Also these checks only happen if both the feature gate is enabled and the pod specifies OS=windows which i think should be called out as well. The current updates could cause some confusion if only the feature gate was enabled but the OS isn't specified.

mehabhalodiya · 2021-11-13T16:51:24Z

@ravisantoshgudimetla Can you please do some updates as per the suggested changes above?

RinkiyaKeDad · 2021-11-16T09:49:49Z

content/en/docs/reference/command-line-tools-reference/feature-gates.md

@@ -772,6 +773,8 @@ Each feature gate is designed for enabling/disabling a specific feature:
 - `HyperVContainer`: Enable
  [Hyper-V isolation](https://docs.microsoft.com/en-us/virtualization/windowscontainers/manage-containers/hyperv-container)
  for Windows containers.
+- `IdentifyPodOS`: Allows the Pod OS field to be specified. This helps in identifying the OS of the pod
+  authoritatively during API server admission time. The allowed values for OS field currently are `windows` and `linux`


nit:

Suggested change

authoritatively during API server admission time. The allowed values for OS field currently are `windows` and `linux`

authoritatively during the API server admission time. The allowed values for OS field currently are `windows` and `linux`.

Even better:

Suggested change

authoritatively during API server admission time. The allowed values for OS field currently are `windows` and `linux`

authoritatively during API admission. In Kubernetes {{< skew currentVersion >}}, the

allowed values for the field are `linux` and `windows`.

(we list 3rd party products in alphabetical order)

sftim

We should also update this page: https://kubernetes.io/docs/concepts/workloads/pods/

sftim · 2021-11-16T17:53:38Z

content/en/docs/reference/command-line-tools-reference/feature-gates.md

@@ -121,6 +121,7 @@ different Kubernetes components.
 | `GracefulNodeShutdown` | `true` | Beta | 1.21 | |
 | `HPAContainerMetrics` | `false` | Alpha | 1.20 | |
 | `HPAScaleToZero` | `false` | Alpha | 1.16 | |
+| `IdentifyPodOS` | `false` | Alpha | 1.23 | 1.23 |


content/en/docs/reference/command-line-tools-reference/feature-gates.md

sftim · 2021-11-23T17:17:52Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

+  * OS field: This `Name` field within OS field should be set to `windows` to indicate that the current pod is a Windows pod. 
+              Please note that there is no effect of having value for this field since it is behind a featuegate `IdentifyPodOS` and it is currently alpha.
+              If the `Name` field in `OS` field is set to `windows`, following fields must be unset on the Windows pods when the `IdentifyPodOS` featuregate is enabled:


What's the name of the field that should be set to windows? (It's unlikely to be OS.Name in the API; the API usually uses camelCase aka lowerCamelCase).

As you noted here, it is spec.os.name

sftim · 2021-11-23T17:18:17Z

content/en/docs/reference/command-line-tools-reference/feature-gates.md

@@ -779,6 +780,8 @@ Each feature gate is designed for enabling/disabling a specific feature:
 - `HyperVContainer`: Enable
  [Hyper-V isolation](https://docs.microsoft.com/en-us/virtualization/windowscontainers/manage-containers/hyperv-container)
  for Windows containers.
+- `IdentifyPodOS`: Allows the Pod OS field to be specified. This helps in identifying the OS of the pod
+  authoritatively during the API server admission time. In Kubernetes {{< skew currentVersion >}}, the allowed values for the `Name` field in `OS` field of the pod spec are `windows` and `linux`.


I presume this is .spec.os.name?

Yes, would it be helpful if mention directly .spec.os.name?

Definitely - readers don't need to know what those field names look like in the source code.

content/en/docs/setup/production-environment/windows/user-guide-windows-containers.md

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

jsturtevant · 2021-11-29T17:33:11Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

+  * OS field: `pod.spec.os.name` should be set to `windows` to indicate that the current pod is a Windows pod. 
+              Please note that there is no effect of having value for this field since it is behind a featuegate `IdentifyPodOS` and it is currently alpha.
+              {{< note >}}
+              If the `pod.spec.os.name` field is set to `windows`, following fields must be unset on the Windows pods when the `IdentifyPodOS` featuregate is enabled:


Are there fields that need to be unset when the Podfield is set to linux? Such as the securityContext.WindowsOptions? Are we documenting that anywhere?

This should be available in the API docs for the field. Since we have a separate documentation for the Windows pods, I am mentioning it explicitly.

Thanks, I remember those docs now. Makes sense to mention it here but I do wonder about maintaining this list in to places.

I am not sure of such a place. If you can point me to a doc, I can update it.

This looks like the same list here: https://github.com/kubernetes/kubernetes/blob/c1153d3353bd4f4b68d85245d53d2745586be474/pkg/apis/core/types.go#L2968-L2990 which will be published as API docs here: https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/ once the does are published.

My main concern here is these two will get out of sync.

So, you don't want this list to be mentioned here in our Windows docs?

I think it should be mentioned but there should be one place where all the fields are listed otherwise it will get out sync and be confusing.

content/en/docs/setup/production-environment/windows/user-guide-windows-containers.md

jsturtevant · 2021-11-29T18:30:19Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

@@ -153,6 +153,34 @@ section refers to several key workload enablers and how they map to Windows.
  * `emptyDir` volumes
  * Named pipe host mounts
  * Resource limits
+  * OS field: `pod.spec.os.name` should be set to `windows` to indicate that the current pod is a Windows pod. 
+              Please note that there is no effect when setting a value for this field since it is behind a feature gate `IdentifyPodOS` and it is currently alpha.


if the feature gate is enabled does it have an affect? It seems from below it does. Is this field available even when the feature gate is not set? If that is the case, it would be clearer to say something along those lines.

Usually all the fields will be available irrespective of the featuregates. We try to drop fields when persisting to storage so that it won't have any effect.

More info: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#adding-unstable-features-to-stable-versions. Look at the create and update strategy code.

What does no effect mean here? This says there is no effect but then goes on to say "the following fields must be unset on the Windows pods when the IdentifyPodOS featuregate is enabled" which implies there is a difference in behavior: if you set this field then your pod will be rejected if those fields are set.

What does no effect mean here?

OS field has no effect when the IdentifyPodOS featuregate is disabled. It can be windows or linux but it doesn't matter however when the IdentifyPodOS featuregate is enabled and if linux specific fields are set in the security context of the pod when the name is set to windows, pod validation would fail

It needs to be clearer that there is no affect (besides the older kubectl version that @sftim mentioned below) when disabled but when enabled there is an affect (validation could fail). Right now this reads as there is not affect for this field.

left a possible suggestion

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

sftim

How about these suggestions?

content/en/docs/reference/command-line-tools-reference/feature-gates.md

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

sftim · 2021-11-30T00:17:27Z

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

+               * spec.hostPID
+               * spec.hostIPC
+               * spec.securityContext.seLinuxOptions
+               * spec.securityContext.seccompProfile
+               * spec.securityContext.fsGroup
+               * spec.securityContext.fsGroupChangePolicy
+               * spec.securityContext.sysctls
+               * spec.shareProcessNamespace
+               * spec.securityContext.runAsUser
+               * spec.securityContext.runAsGroup
+               * spec.securityContext.supplementalGroups
+               * spec.containers[*].securityContext.seLinuxOptions
+               * spec.containers[*].securityContext.seccompProfile
+               * spec.containers[*].securityContext.capabilities
+               * spec.containers[*].securityContext.readOnlyRootFilesystem
+               * spec.containers[*].securityContext.privileged
+               * spec.containers[*].securityContext.allowPrivilegeEscalation
+               * spec.containers[*].securityContext.procMount
+               * spec.containers[*].securityContext.runAsUser
+               * spec.containers[*].securityContext.runAsGroup


Please put all these field paths in backticks (individually)

content/en/docs/setup/production-environment/windows/user-guide-windows-containers.md

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md

jsturtevant · 2021-11-30T21:31:51Z

/lgtm

k8s-ci-robot · 2021-11-30T21:32:15Z

LGTM label has been added.

Git tree hash: 90cfee8e83d65a379b0d5e1222dac15a4a0c7f4b

marosset · 2021-11-30T21:36:51Z

/lgtm
/approve

Co-authored-by: James Sturtevant <jsturtevant@gmail.com> Co-authored-by: Tim Bannister <tim@scalefactory.com>

jlbutler · 2021-11-30T21:56:35Z

Thanks for working out that final nit!
/lgtm
/approve

k8s-ci-robot · 2021-11-30T21:56:54Z

LGTM label has been added.

Git tree hash: 3fd18f9fcaf626de8b02e9050172c0f4c21cc899

k8s-ci-robot · 2021-11-30T21:58:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlbutler, marosset

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:

~~content/en/OWNERS~~ [jlbutler]

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

k8s-ci-robot added this to the 1.23 milestone Nov 10, 2021

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 10, 2021

ravisantoshgudimetla mentioned this pull request Nov 10, 2021

Identify Windows pods at API admission level authoritatively kubernetes/enhancements#2802

Closed

26 tasks

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 10, 2021

k8s-ci-robot requested review from bradtopol and jsturtevant November 10, 2021 20:02

k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Nov 10, 2021

k8s-ci-robot added the sig/windows Categorizes an issue or PR as relevant to SIG Windows. label Nov 10, 2021

aravindhp reviewed Nov 10, 2021

View reviewed changes

jsturtevant reviewed Nov 10, 2021

View reviewed changes

k8s-ci-robot assigned mehabhalodiya Nov 11, 2021

marosset reviewed Nov 11, 2021

View reviewed changes

RinkiyaKeDad reviewed Nov 16, 2021

View reviewed changes

sftim reviewed Nov 16, 2021

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2021

ravisantoshgudimetla force-pushed the windows-pods branch from 7d37245 to fdf5e7c Compare November 23, 2021 16:01

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 23, 2021

ravisantoshgudimetla force-pushed the windows-pods branch 2 times, most recently from 7efb7e0 to 931f875 Compare November 23, 2021 16:05

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 23, 2021

sftim reviewed Nov 23, 2021

View reviewed changes

ravisantoshgudimetla force-pushed the windows-pods branch from 931f875 to 56d5c2d Compare November 29, 2021 13:55

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 29, 2021

ravisantoshgudimetla force-pushed the windows-pods branch from 56d5c2d to d0448d9 Compare November 29, 2021 13:56

ravisantoshgudimetla commented Nov 29, 2021

View reviewed changes

content/en/docs/setup/production-environment/windows/user-guide-windows-containers.md Show resolved Hide resolved

jsturtevant reviewed Nov 29, 2021

View reviewed changes

content/en/docs/setup/production-environment/windows/user-guide-windows-containers.md Outdated Show resolved Hide resolved

jsturtevant reviewed Nov 29, 2021

View reviewed changes

marosset reviewed Nov 29, 2021

View reviewed changes

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md Outdated Show resolved Hide resolved

ravisantoshgudimetla force-pushed the windows-pods branch from 81fdcc5 to b7660bb Compare November 29, 2021 20:37

sftim reviewed Nov 30, 2021

View reviewed changes

ravisantoshgudimetla force-pushed the windows-pods branch from c3aae4c to 732050c Compare November 30, 2021 04:31

jsturtevant reviewed Nov 30, 2021

View reviewed changes

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md Outdated Show resolved Hide resolved

content/en/docs/setup/production-environment/windows/intro-windows-in-kubernetes.md Outdated Show resolved Hide resolved

k8s-ci-robot assigned jsturtevant Nov 30, 2021

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

k8s-ci-robot assigned marosset Nov 30, 2021

[docs][windows]: Pod OS field update

89e7446

Co-authored-by: James Sturtevant <jsturtevant@gmail.com> Co-authored-by: Tim Bannister <tim@scalefactory.com>

ravisantoshgudimetla force-pushed the windows-pods branch from ade8f8e to 89e7446 Compare November 30, 2021 21:52

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 30, 2021

k8s-ci-robot assigned jlbutler Nov 30, 2021

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

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 30, 2021

k8s-ci-robot merged commit 0660f9a into kubernetes:dev-1.23 Nov 30, 2021

	authoritatively during API server admission time. The allowed values for OS field currently are `windows` and `linux`
	authoritatively during the API server admission time. The allowed values for OS field currently are `windows` and `linux`.

	authoritatively during API server admission time. The allowed values for OS field currently are `windows` and `linux`
	authoritatively during API admission. In Kubernetes {{< skew currentVersion >}}, the
	allowed values for the field are `linux` and `windows`.

	\| `IdentifyPodOS` \| `false` \| Alpha \| 1.23 \| 1.23 \|
	\| `IdentifyPodOS` \| `false` \| Alpha \| 1.23 \| \|

[docs][windows]: Pod OS field update #30436

[docs][windows]: Pod OS field update #30436

Conversation

ravisantoshgudimetla commented Nov 10, 2021

netlify bot commented Nov 10, 2021 • edited Loading

marosset commented Nov 10, 2021

Choose a reason for hiding this comment

jsturtevant left a 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

jlbutler commented Nov 11, 2021

Choose a reason for hiding this comment

marosset Nov 11, 2021 • edited Loading

Choose a reason for hiding this comment

mehabhalodiya commented Nov 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sftim left a 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

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

Choose a reason for hiding this comment

ravisantoshgudimetla Nov 29, 2021 • edited Loading

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

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

sftim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsturtevant commented Nov 30, 2021

k8s-ci-robot commented Nov 30, 2021

marosset commented Nov 30, 2021

jlbutler commented Nov 30, 2021

k8s-ci-robot commented Nov 30, 2021

k8s-ci-robot commented Nov 30, 2021

netlify bot commented Nov 10, 2021 •

edited

Loading

marosset Nov 11, 2021 •

edited

Loading

ravisantoshgudimetla Nov 29, 2021 •

edited

Loading