KEP-1981: WindowsHostProcessContainers beta major changes #3311

marosset · 2022-05-11T23:39:08Z

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

One-line PR description:

Issue link: Support for Windows privileged containers #1981

Other comments:

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

jsturtevant

great to see how we learned what worked and what could be improved to make feature better.
Looking forward to these improvements!

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

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

marosset · 2022-06-13T18:21:18Z

/hold
for reviews

batch committing suggested changes Co-authored-by: Jordan Liggitt <[email protected]>

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

marosset · 2022-06-14T22:44:49Z

@msau42 and/or @liggitt do either of you have any more thoughts/concerns here?

liggitt · 2022-06-16T15:28:12Z

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

+- On containerd v1.7 **bind** volume mount behavior will always be used.
+  - If users are running nodes with Windows Server 2019 with security patches older than July 2022 the volume mounts for HostProcessContainers will fail.
+
+Users who have workloads that depend on the **symlink** mount behavior (ex: are expecting to find mounted volumes under `$CONTAINER_SANDBOX_MOUNT_POINT`) will need to stay on containerd v1.6 releases until their workloads are updated to be compatible with **bind** mount behavior.


workloads that depend on the symlink mount behavior (ex: are expecting to find mounted volumes under $CONTAINER_SANDBOX_MOUNT_POINT) will need to stay on containerd v1.6 releases until their workloads are updated to be compatible with bind mount behavior

This positioning is questionable for a GA feature. Remaining in beta until bind mount is available, then saying GA only supports bind mount is more what I would have expected

We can keep the feature in beta for one more release and then go to stable after we containerd v1.7 releases and is deemed stable if that makes more sense.

Either way we'll need to document that a breaking change will occur when updating from containerd v1.6 to v1.7 w.r.t. volume mount behavior.

liggitt · 2022-06-16T15:28:56Z

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

+Graduation
+
+- Add e2e tests to validate running `hostProcess` containers as non SYSTEM/admin accounts
+- Update e2e tests for new volume mount behavior as desdribed in [Container Mounts](#container-mounts)


Suggested change

- Update e2e tests for new volume mount behavior as desdribed in [Container Mounts](#container-mounts)

- Update e2e tests for new volume mount behavior as described in [Container Mounts](#container-mounts)

since bind mount is not yet released in containerd 1.7, do we even have the ability to add tests for this yet?

Yes we do.

We have a github action that runs every night that builds containerd and hcsshim from main and publishes the package to https://github.com/kubernetes-sigs/sig-windows-tools/releases/tag/windows-containerd-nightly.
This package already has the changes to use the new bind mount behavior (if running on Windows Server 2022 for now, it will work on Windows Server 2019 in a few weeks).

We use this package in https://testgrid.k8s.io/sig-windows-signal#capz-windows-containerd-nightly-master (and a few others).

My plans were to update the e2e tests to check for the contaienrd version being used on the nodes and add skips to the e2e tests that require a different version of containerd.

liggitt · 2022-06-16T15:29:37Z

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

@@ -952,7 +1075,7 @@ _This section must be completed when targeting beta graduation to a release._
  - [ContainerD]
    - Usage description:
      - `HostProcess` containers support will not be added to dockershim.
-      - Containerd v1.5.6+ is required.
+      - Containerd v1.6+ is required.


if 1.7 is when bind mount functionality is targeted, I'd expect this to be 1.7

Yes, will update thanks!

marosset · 2022-06-16T22:06:07Z

I spoke with a few other people and I think most everyone is in agreement to move the target for graduating to stable to v1.26
We'll use the v1.25 release to have users migrate any workloads that need migration to use the new volume mount behaviors and add all the necessary e2e validation.

I'll make updates to reflect this.
I think it'd still be nice to get this PR merged after these updates so the new volume mount behaviors is more discoverable.

dchen1107 · 2022-06-17T00:35:52Z

Sync-ed with @marosset and agreed with his plan on moving the target for graduating to stable to v1.26 from SIG Node perspective. Also removed this from SIG Node 1.25 tracking sheet.

But we will continue move forward with this KEP. Thanks!

marosset · 2022-06-17T22:37:46Z

One concern shared by several folks (including myself) is the migration story for deployments that were authored using the existing volume mount behaviors.
I've added a note in the KEP that we'll continue to look for solutions here during v1.25

jsturtevant · 2022-06-22T16:14:31Z

/lgtm

jsturtevant · 2022-06-22T16:22:30Z

@msau42 @dchen1107 @liggitt any further concerns with the updates? We will be staying in beta for 1.25.
Can we release the hold?

liggitt · 2022-06-22T19:25:05Z

I won't have a chance to sweep it again, but I'm happy with tying stable to the bind mount capability and focusing on giving users guidance about how to bridge from the current symlink behavior to the bind behavior compatibly

marosset · 2022-09-22T17:32:28Z

/hold cancel

I forgot to merge this in v1.25 but we do have e2e up and running validating the bind mount behaviors against pre-release containerd.
I'll create another PR with updates for going a v1.26 stable release

marosset · 2022-09-22T17:32:41Z

/label tide/merge-method-squash

WindowsHostProcessContainers GA in 1.25

5100542

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/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 11, 2022

k8s-ci-robot requested a review from jayunit100 May 11, 2022 23:39

k8s-ci-robot added the kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory label May 11, 2022

k8s-ci-robot requested a review from jsturtevant May 11, 2022 23:39

k8s-ci-robot added 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 May 11, 2022

marosset marked this pull request as draft May 11, 2022 23:40

marosset mentioned this pull request May 11, 2022

Support for Windows privileged containers #1981

Closed

35 tasks

marosset changed the title ~~WIP:WindowsHostProcessContainers GA in 1.25~~ WIP: WindowsHostProcessContainers GA in 1.25 May 11, 2022

kikisdeliveryservice changed the title ~~WIP: WindowsHostProcessContainers GA in 1.25~~ WIP: KEP-1981: WindowsHostProcessContainers GA in 1.25 May 13, 2022

marosset added 2 commits May 16, 2022 12:08

Update implementation history for previous milestones

15a5fad

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

WindowsHostProcessContainers to stable KEP updates

4a91fce

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 16, 2022

jsturtevant reviewed May 17, 2022

View reviewed changes

address some pr feedback

832a60d

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

marosset marked this pull request as ready for review May 23, 2022 18:30

marosset changed the title ~~WIP: KEP-1981: WindowsHostProcessContainers GA in 1.25~~ KEP-1981: WindowsHostProcessContainers GA in 1.25 May 23, 2022

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 May 23, 2022

k8s-ci-robot requested review from claudiubelu and jsturtevant May 23, 2022 18:30

marosset added 5 commits May 23, 2022 16:35

updates

857ba88

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

Updating metrics section to reflect reality

8583950

typos

bd862b0

Updating kep.yaml with final actual metric names

a8a3282

Adding new Test Plan sections

82ed23d

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

marosset and others added 2 commits June 13, 2022 13:20

Apply suggestions from code review

71a0a95

batch committing suggested changes Co-authored-by: Jordan Liggitt <[email protected]>

Addressing PRR feedback / typos

16fecea

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

liggitt reviewed Jun 16, 2022

View reviewed changes

keep in beta for v1.25

9afbffd

marosset changed the title ~~KEP-1981: WindowsHostProcessContainers GA in 1.25~~ KEP-1981: WindowsHostProcessContainers beta major changes Jun 16, 2022

fixup

5d22c71

Add note about explore migration options for volume mounts in v1.25

603d6c2

marosset added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 17, 2022

jsturtevant mentioned this pull request Jun 21, 2022

Windows Hostprocess containers can't use the InClientConfig for api access kubernetes/kubernetes#104562

Closed

k8s-ci-robot assigned jsturtevant Jun 22, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 22, 2022

marosset mentioned this pull request Aug 1, 2022

Hostprocess container test updates kubernetes/kubernetes#111609

Merged

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 22, 2022

k8s-ci-robot merged commit 0e2bafd into kubernetes:master Sep 22, 2022

k8s-ci-robot added this to the v1.26 milestone Sep 22, 2022

marosset mentioned this pull request Oct 5, 2022

KEP 1981 Windows HostProcessContainers to stable #3540

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KEP-1981: WindowsHostProcessContainers beta major changes #3311

KEP-1981: WindowsHostProcessContainers beta major changes #3311

marosset commented May 11, 2022 •

edited

Loading

jsturtevant left a comment

marosset commented Jun 13, 2022

marosset commented Jun 14, 2022

liggitt Jun 16, 2022

marosset Jun 16, 2022

liggitt Jun 16, 2022

marosset Jun 16, 2022

liggitt Jun 16, 2022

marosset Jun 16, 2022

marosset commented Jun 16, 2022

dchen1107 commented Jun 17, 2022

marosset commented Jun 17, 2022

jsturtevant commented Jun 22, 2022

jsturtevant commented Jun 22, 2022

liggitt commented Jun 22, 2022

marosset commented Sep 22, 2022

marosset commented Sep 22, 2022

	- Update e2e tests for new volume mount behavior as desdribed in [Container Mounts](#container-mounts)
	- Update e2e tests for new volume mount behavior as described in [Container Mounts](#container-mounts)

KEP-1981: WindowsHostProcessContainers beta major changes #3311

KEP-1981: WindowsHostProcessContainers beta major changes #3311

Conversation

marosset commented May 11, 2022 • edited Loading

jsturtevant left a comment

Choose a reason for hiding this comment

marosset commented Jun 13, 2022

marosset commented Jun 14, 2022

liggitt Jun 16, 2022

Choose a reason for hiding this comment

marosset Jun 16, 2022

Choose a reason for hiding this comment

liggitt Jun 16, 2022

Choose a reason for hiding this comment

marosset Jun 16, 2022

Choose a reason for hiding this comment

liggitt Jun 16, 2022

Choose a reason for hiding this comment

marosset Jun 16, 2022

Choose a reason for hiding this comment

marosset commented Jun 16, 2022

dchen1107 commented Jun 17, 2022

marosset commented Jun 17, 2022

jsturtevant commented Jun 22, 2022

jsturtevant commented Jun 22, 2022

liggitt commented Jun 22, 2022

marosset commented Sep 22, 2022

marosset commented Sep 22, 2022

marosset commented May 11, 2022 •

edited

Loading