KEP-1981 Windows HostProcess containers KEP updates for beta #2865

marosset · 2021-08-16T21:15:56Z

Signed-off-by: Mark Rossetti [email protected]

One-line PR description: KEP updates for progressing WindowsHostProcessContainers feature to beta

Issue link: Support for Windows privileged containers #1981

Other comments:

Signed-off-by: Mark Rossetti <[email protected]>

k8s-ci-robot · 2021-08-16T21:15:58Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: Mark Rossetti <[email protected]>

brasmith-ms

Minor editorial edits

keps/sig-windows/1981-windows-privileged-container-support/README.md

…DME.md Co-authored-by: Brandon Smith <[email protected]>

keps/sig-windows/1981-windows-privileged-container-support/README.md

s/hostProcess/`HostProcess` Co-authored-by: Danny Canter <[email protected]> Co-authored-by: Brandon Smith <[email protected]>

msau42 · 2021-09-08T00:44:32Z

keps/sig-windows/1981-windows-privileged-container-support/README.md

+  - Note: We are prototyping a new approach to how the file system is created for `hostProcess` containers that would present the filesystem in a similar manner to non-hostProcess containers running on Windows (`c:\` (trailing \ included) would be the root instead of `c:\c\<container id>\`).
+  This would make it so files from volume mounts would be accessible via static paths. HostProcess containers would still have full access to the host file-system.
+  https://github.com/microsoft/hcsshim/pull/1107 is tracking this exploratory work.
+  This functionality will most-likely not be ready during Kubernetes v1.23 and any changes made to how volume mounts work would be done before this features becomes stable.


Can you outline how a workload could support both old and new behavior during a version upgrade?

Hopefully there shouldn't be anything needed. Right now the environment variable we set points to the root of the volume that contains a merged view of the image layers (so basically the root of the rootfs, or C:\ in a windows server container, / on linux). For the new logic, we'd still set the env var, but just have it point to C:\ instead, so if your workload made heavy use of this envvar nothing should change really.

Correct, if a workload is accessing files with the env var using the current behavior (where volume mounts show up under c:\c<contianer id>) the the workloads are expected to work without changes if/when we switch to the behavior Danny is prototyping.
After switching to the new behavior there use of the env var will be optional.
Let me update the KEP with this information.

I updated the doc, please take a look.

keps/sig-windows/1981-windows-privileged-container-support/README.md

msau42 · 2021-09-08T00:56:32Z

keps/sig-windows/1981-windows-privileged-container-support/README.md

+  - Note: it is not possible to feature-gate this behavior in client libraries and because of this the functionality should not be added to client libraries after `hostProcess` containers while this feature is in `alpha`.
+  - [kubernetes/kubernetes#104490](https://github.com/kubernetes/kubernetes/pull/104490) add support for `HostProcess` containers to the golang client library.
+- Named Pipe mounts will **not** be supported. Instead named pipes should be accessed via their path on the host (\\\\.\\pipe\\*).
+- Unix domain sockets mounts support will be added before `HostProcess` containers graduate to `stable`. For `alpha` and `beta` Unix domain sockets can be accessed via their paths on the host like named pipes.


It's surfaced today using hostpath but since hostprocess containers get access to the entire root filesystem, is it required?

keps/sig-windows/1981-windows-privileged-container-support/README.md

msau42 · 2021-09-08T15:30:49Z

/approve
from storage

k8s-ci-robot · 2021-09-08T15:31:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, marosset, msau42

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/prod-readiness/OWNERS~~ [deads2k]
~~keps/sig-windows/OWNERS~~ [marosset]

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

marosset · 2021-09-08T17:16:14Z

@liggitt @dchen1107 can you take a look? Thanks!

marosset · 2021-09-08T19:43:26Z

/label tide/merge-method-squash

SergeyKanzhelev · 2021-09-08T22:21:13Z

From sig node perspective, moving from annotations to native support is a good thing. Removing annotation string in CRI sounds OK to me.

/lgtm

liggitt · 2021-09-09T04:15:44Z

keps/sig-windows/1981-windows-privileged-container-support/README.md

+- An environment variable `$CONTAINER_SANDBOX_MOUNT_POINT` will be set to the absolute path where the container volume is mounted for `hostProcess` containers.
+  - Note: Syntax for referencing environment variables differs depending on what shell you are using. In cmd.exe env vars are surrounded by %'s (ex: `%CONTAINER_SANDBOX_MOUNT_POINT%`) and in powershell env vars are prefixed with $env: (ex: `$env:CONTAINER_SANDBOX_MOUNT_POINT`).
+  - This environment variable will be set to `c:\c\<containerid>\` (trailing \ included!) for each container.
+  - This environment variable can be used inside the Pod manifest / command line args for containers. See files in this [pull request](https://github.com/kubernetes-sigs/sig-windows-tools/pull/161/files#diff-b8195f7a2ad8f9ae9ebdd1bde8a0f3756c4508c1d9d9dd99f4a3bfa19fc3b828R135) for examples of using `$CONTAINER_SANDBOX_MOUNT_POINT` inside deployment manifests.


we do the expansion locally

asked in the linked pull request, but it wasn't clear where the expansion was done... are $env:... and %env% expansions done for non-hostProcess command/args/workingDir fields as well?

the $CONTAINER_SANDBOX_MOUNT_POINT env var is set by hcsshim for processes inside the Windows container when the container is started. Generally Windows does variable expansion for environment variables that are found in the path to an executable and for any env vars found in command line arguments when the executable is run.
This is the same behavior for non-hostProcess containers for command / args / workingDir.

For the CONTAINER_SANDBOX_MOUNT_POINT the variable expansion needs to happen inside of the container because the value won't be determined until after the hostProcesss containers are created.

keps/sig-windows/1981-windows-privileged-container-support/README.md

…mand / args / workingdir

liggitt · 2021-09-09T18:24:56Z

/lgtm

marosset · 2021-09-09T18:30:21Z

/hold cancel

Thank you so much for your reviews everyone - especially @liggitt @SergeyKanzhelev @msau42 @deads2k @dcantah @ddebroy

…tes#2865) * WIP: Windows HostProcess containers KEP updates for beta Signed-off-by: Mark Rossetti <[email protected]> * Fillling out PRR section Signed-off-by: Mark Rossetti <[email protected]> * Container Volume updates Signed-off-by: Mark Rossetti <[email protected]> * Container Image Build updates Signed-off-by: Mark Rossetti <[email protected]> * misc updates Signed-off-by: Mark Rossetti <[email protected]> * Update keps/sig-windows/1981-windows-privileged-container-support/README.md Co-authored-by: Brandon Smith <[email protected]> * Update keps/sig-windows/1981-windows-privileged-container-support/README.md Co-authored-by: Brandon Smith <[email protected]> * Update keps/sig-windows/1981-windows-privileged-container-support/README.md Co-authored-by: Brandon Smith <[email protected]> * Apply suggestions from code review s/hostProcess/`HostProcess` Co-authored-by: Danny Canter <[email protected]> Co-authored-by: Brandon Smith <[email protected]> * update keps/prod-readiness/sig-windows/1981.yaml * Addressing some TODOs * clarify CONTAINER_SANDBOX_MOUNT_POINT usage * Apply suggestions from code review Co-authored-by: Danny Canter <[email protected]> * cmd.exe and powershell env var behaviors * s/privileged/hostProcess * storage related feedback / updates * Add test criteria to validate different path syntax for container command / args / workingdir * do not update client libraries until file system investigations conclude Co-authored-by: Brandon Smith <[email protected]> Co-authored-by: Danny Canter <[email protected]>

WIP: Windows HostProcess containers KEP updates for beta

ee65b93

Signed-off-by: Mark Rossetti <[email protected]>

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 16, 2021

k8s-ci-robot requested review from ddebroy and jayunit100 August 16, 2021 21:16

k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/windows Categorizes an issue or PR as relevant to SIG Windows. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 16, 2021

marosset mentioned this pull request Aug 16, 2021

Support for Windows privileged containers #1981

Closed

35 tasks

marosset added 3 commits August 18, 2021 15:44

Fillling out PRR section

6400ce3

Signed-off-by: Mark Rossetti <[email protected]>

Container Volume updates

ab9a168

Signed-off-by: Mark Rossetti <[email protected]>

Container Image Build updates

c69229b

Signed-off-by: Mark Rossetti <[email protected]>

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 19, 2021

misc updates

b33bcd6

Signed-off-by: Mark Rossetti <[email protected]>

brasmith-ms suggested changes Aug 20, 2021

View reviewed changes

marosset changed the title ~~WIP: Windows HostProcess containers KEP updates for beta~~ WIP: KEP-1981 Windows HostProcess containers KEP updates for beta Aug 23, 2021

marosset and others added 3 commits August 23, 2021 13:57

Update keps/sig-windows/1981-windows-privileged-container-support/REA…

6e6c57d

…DME.md Co-authored-by: Brandon Smith <[email protected]>

Update keps/sig-windows/1981-windows-privileged-container-support/REA…

46102b0

…DME.md Co-authored-by: Brandon Smith <[email protected]>

Update keps/sig-windows/1981-windows-privileged-container-support/REA…

0996fc0

…DME.md Co-authored-by: Brandon Smith <[email protected]>

dcantah reviewed Aug 24, 2021

View reviewed changes

keps/sig-windows/1981-windows-privileged-container-support/README.md Outdated Show resolved Hide resolved

dcantah reviewed Aug 24, 2021

View reviewed changes

keps/sig-windows/1981-windows-privileged-container-support/README.md Outdated Show resolved Hide resolved

Apply suggestions from code review

74c7961

s/hostProcess/`HostProcess` Co-authored-by: Danny Canter <[email protected]> Co-authored-by: Brandon Smith <[email protected]>

marosset changed the title ~~WIP: KEP-1981 Windows HostProcess containers KEP updates for beta~~ KEP-1981 Windows HostProcess containers KEP updates for beta Aug 24, 2021

marosset marked this pull request as ready for review August 24, 2021 20:00

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 24, 2021

update keps/prod-readiness/sig-windows/1981.yaml

dd7a06c

msau42 reviewed Sep 8, 2021

View reviewed changes

storage related feedback / updates

4d7a053

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Sep 8, 2021

k8s-ci-robot assigned SergeyKanzhelev Sep 8, 2021

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

liggitt reviewed Sep 9, 2021

View reviewed changes

keps/sig-windows/1981-windows-privileged-container-support/README.md Outdated Show resolved Hide resolved

Add test criteria to validate different path syntax for container com…

d0aad04

…mand / args / workingdir

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

do not update client libraries until file system investigations conclude

5586e1f

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

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

k8s-ci-robot merged commit cc4052f into kubernetes:master Sep 9, 2021

k8s-ci-robot added this to the v1.23 milestone Sep 9, 2021

jsturtevant mentioned this pull request Sep 21, 2021

Handle for Windows HostProcess containers in go-client kubernetes/kubernetes#104490

Closed

dcantah mentioned this pull request Sep 27, 2021

REQUEST: New membership for dcantah kubernetes/org#3017

Closed

7 tasks

palnabarun mentioned this pull request Jun 12, 2022

canonical kep number json field palnabarun/enhancements#1

Closed

palnabarun mentioned this pull request Jul 26, 2022

KEP Manifest Generator palnabarun/enhancements#2

Closed

coutinhop mentioned this pull request Apr 25, 2023

Windows support hostprocess projectcalico/calico#7260

Closed

liggitt mentioned this pull request Oct 7, 2023

Does client go work for windows container? Cannot find service account token. kubernetes/client-go#1302

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KEP-1981 Windows HostProcess containers KEP updates for beta #2865

KEP-1981 Windows HostProcess containers KEP updates for beta #2865

marosset commented Aug 16, 2021 •

edited

Loading

k8s-ci-robot commented Aug 16, 2021

brasmith-ms left a comment

msau42 Sep 8, 2021

dcantah Sep 8, 2021

marosset Sep 8, 2021

marosset Sep 8, 2021

msau42 Sep 8, 2021

msau42 commented Sep 8, 2021

k8s-ci-robot commented Sep 8, 2021

marosset commented Sep 8, 2021

marosset commented Sep 8, 2021

SergeyKanzhelev commented Sep 8, 2021

liggitt Sep 9, 2021

marosset Sep 9, 2021 •

edited

Loading

liggitt commented Sep 9, 2021

marosset commented Sep 9, 2021

KEP-1981 Windows HostProcess containers KEP updates for beta #2865

KEP-1981 Windows HostProcess containers KEP updates for beta #2865

Conversation

marosset commented Aug 16, 2021 • edited Loading

k8s-ci-robot commented Aug 16, 2021

brasmith-ms left a comment

Choose a reason for hiding this comment

msau42 Sep 8, 2021

Choose a reason for hiding this comment

dcantah Sep 8, 2021

Choose a reason for hiding this comment

marosset Sep 8, 2021

Choose a reason for hiding this comment

marosset Sep 8, 2021

Choose a reason for hiding this comment

msau42 Sep 8, 2021

Choose a reason for hiding this comment

msau42 commented Sep 8, 2021

k8s-ci-robot commented Sep 8, 2021

marosset commented Sep 8, 2021

marosset commented Sep 8, 2021

SergeyKanzhelev commented Sep 8, 2021

liggitt Sep 9, 2021

Choose a reason for hiding this comment

marosset Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

liggitt commented Sep 9, 2021

marosset commented Sep 9, 2021

marosset commented Aug 16, 2021 •

edited

Loading

marosset Sep 9, 2021 •

edited

Loading