Add GA support for Windows hostprocess containers #7857

coutinhop · 2023-07-11T22:56:41Z

Build on the changes from #7260:

Add a Windows cni-plugin hostprocess image, which will install the cni binaries and config on the host.
Add a Windows node hostprocess image, which no longer needs to match the hosts' Windows version

Manually tested with an operator image from tigera/operator#2732 and a VXLAN setup.

Many thanks to @jsturtevant and @fabi200123

Description

Related issues/PRs

Todos

Tests
Documentation
Release note

Release Note

Add support for Windows host process containers (HPC), with Calico for Windows daemonsets managed by kubernetes.

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

docs-pr-required: This change requires a change to the documentation that has not been completed yet.
docs-completed: This change has all necessary documentation completed.
docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

release-note-required: This PR has user-facing changes. Most PRs should have this label.
release-note-not-required: This PR has no user-facing changes.

Other optional labels:

cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

- Add windows calico-node daemonset with hostprocess containers. - Add windows kube-proxy daemonset using sig-windows hostprocess container image. - Add optional (required only for windows) fields KUBERNETES_SERVICE_CIDR and KUBERNETES_DNS_SERVERS to kubernetes-services-endpoint configmap. Manually tested with a clean installation and VXLAN setup and images from projectcalico/calico#7857. Still a work in progress. Pending: - Tests for windows calico-node rendering (pkg/render/windows_test.go) - Possibly make CNI BIN and CONF dirs configurable for Windows in the installation - Possibly roll out our own kube-proxy image instead of using the one from sig-windows - Possibly adding

- Add windows calico-node daemonset with hostprocess containers. - Add windows kube-proxy daemonset using sig-windows hostprocess container image. - Add optional (required only for windows) fields KUBERNETES_SERVICE_CIDR and KUBERNETES_DNS_SERVERS to kubernetes-services-endpoint configmap. Manually tested with a clean installation and VXLAN setup and images from projectcalico/calico#7857. Still a work in progress. Pending: - Tests for windows calico-node rendering (pkg/render/windows_test.go) - Possibly make CNI BIN and CONF dirs configurable for Windows in the installation - Possibly roll out our own kube-proxy image instead of using the one from sig-windows - Possibly adding an initContainer to clean up "old" style non-HPC Calico installations before proceeding

node/Dockerfile-windows

jsturtevant · 2023-07-17T17:33:36Z

cni-plugin/pkg/install/install.go

+// In the case of Windows HostProcess containers this prepends the CONTAINER_SANDBOX_MOUNT_POINT env variable
+// for other operating systems or if the sandbox env variable is not set it returns the standard mount points
+// see https://kubernetes.io/docs/tasks/configure-pod-container/create-hostprocess-pod/#volume-mounts
+func getHostPath(path string) string {


if you only support Containerd 1.7+ this shouldn't be needed and would clean up this section of code

I see, so then just referring to the paths as /host/foo would work on Windows without any need for change? Thanks, I hadn't tried that

yes, if they are volumemounts to the /host/ location that should work. This is why the In-Cluster config now works 🚀

@jsturtevant I got rid of this getHostPath() function on my latest commit, though I still needed to use the $CONTAINER_SANDBOX_MOUNT_POINT env var when referring to the binaries inside the container, which I thought wouldn't be needed. Will tag you on the relevant chunk of code...

Just a quick update: I ended up implementing support for containerd v1.6, so we're using $CONTAINER_SANDBOX_MOUNT_POINT just about everywhere that's necessary. Everything's working on my tests so far, and that way we can support a containerd version that's widely adopted right now...

coutinhop · 2023-07-19T01:01:24Z

cni-plugin/pkg/install/install.go

+		containerBinDir := "/opt/cni/bin/"
+		// The binaries dir in the container needs to be prepended by the CONTAINER_SANDBOX_MOUNT_POINT env var on Windows Host Process Containers
+		// see https://kubernetes.io/docs/tasks/configure-pod-container/create-hostprocess-pod/#containerd-v1-7-and-greater
+		if runtime.GOOS == "windows" {
+			containerBinDir = os.Getenv("CONTAINER_SANDBOX_MOUNT_POINT") + "/" + containerBinDir


@jsturtevant I still needed to use the $CONTAINER_SANDBOX_MOUNT_POINT env var when referring to the binaries inside the container here, which from what I read on https://kubernetes.io/docs/tasks/configure-pod-container/create-hostprocess-pod/#containerd-v1-7-and-greater shouldn't be needed, right? Could you please take a look and let me know if I missed something?

what does the yaml look like for your deployment? I don't see it in the PR here. Could it be that the Working directory is set to something?

I've mostly been working with the tigera operator with the changes from this PR: tigera/operator#2732, but the deployment also works with this manifest: https://gist.github.com/coutinhop/7528503be5d0aeb22001d69d82fa61ed

No working dir on the install-cni initContainer, but I did include $env:CONTAINER_SANDBOX_MOUNT_POINT in the path for command in it, could that cause issues?

No working dir on the install-cni initContainer, but I did include $env:CONTAINER_SANDBOX_MOUNT_POINT in the path for command in it, could that cause issues?

yes I think that would do it. could you try with something like command: ["opt/cni/bin/install.exe"] or /opt/cni/bin/install.exe depending on where those files live?

Thanks! I've tried it (removing mentions to $env:CONTAINER_SANDBOX_MOUNT_POINT), but I got

'/opt/cni/bin/install.exe' is not recognized as an internal or external command

I see that the yaml for kube-proxy also still uses $env:CONTAINER_SANDBOX_MOUNT_POINT
https://github.com/kubernetes-sigs/sig-windows-tools/blob/3ed28295f8694dffd03e144af4423137b2b8c30b/hostprocess/calico/kube-proxy/kube-proxy.yml#L25

Apparently the binaries (and scripts, I couldn't manage to make the uninstall-calico initContainer work either) are not in these absolute paths, I'm guessing some path in c:/hpc/... would work, but then why not just use $env:CONTAINER_SANDBOX_MOUNT_POINT, right?

This is by far not a deal breaker, stuff was working so I wouldn't block anything by it (unless you see $env:CONTAINER_SANDBOX_MOUNT_POINT being dropped at some point and these containers as they are becoming unsupported).

One other quick question: is the presence of $env:CONTAINER_SANDBOX_MOUNT_POINT a reliable indicator that we're inside a host process container? We've used it in a couple of places, I wonder if we should look into changing that...

Once again, thanks so much!

- Add windows calico-node daemonset with hostprocess containers. - Add windows kube-proxy daemonset using sig-windows hostprocess container image. - Add optional (required only for windows) fields KUBERNETES_SERVICE_CIDR and KUBERNETES_DNS_SERVERS to kubernetes-services-endpoint configmap. Manually tested with a clean installation and VXLAN setup and images from projectcalico/calico#7857. Still a work in progress. Pending: - Tests for windows calico-node rendering (pkg/render/windows_test.go) - Possibly make CNI BIN and CONF dirs configurable for Windows in the installation - Possibly roll out our own kube-proxy image instead of using the one from sig-windows - Possibly adding an initContainer to clean up "old" style non-HPC Calico installations before proceeding

Build on the changes from projectcalico#7260: - Add a Windows cni-plugin hostprocess image, which will install the cni binaries and config on the host. - Add a Windows node hostprocess image, which no longer needs to match the hosts' Windows version (e.g. only 1809 needs to be used and is supported on 2022).

base for windows HPC images. Add uninstall-calico powershell script to be run in initContainer, which will clean up non-HPC Calico installations from the host before starting Calico Windows HPC. Fix bug in confd-service.ps1

HPC when using containerd v1.7+

before substitution (this helps with operator testing).

…h containerd v1.6+ (instead of requiring v1.7+).

…d with '$env:CONTAINER_SANDBOX_MOUNT_POINT'). Fix logic regarding CNI paths in uninstall-calico-hpc.ps1.

Fix bug in GetHostPath()

- Add windows calico-node daemonset with hostprocess containers. - Add windows kube-proxy daemonset using sig-windows hostprocess container image. - Add optional (required only for windows) fields KUBERNETES_SERVICE_CIDR and KUBERNETES_DNS_SERVERS to kubernetes-services-endpoint configmap. Manually tested with a clean installation and VXLAN setup and images from projectcalico/calico#7857. Still a work in progress. Pending: - Tests for windows calico-node rendering (pkg/render/windows_test.go) - Possibly make CNI BIN and CONF dirs configurable for Windows in the installation - Possibly roll out our own kube-proxy image instead of using the one from sig-windows - Possibly adding an initContainer to clean up "old" style non-HPC Calico installations before proceeding

…ags() to winutils.go and use them instead of upstream's InClusterConfig(), BuildConfigFromFlags() in order to get Windows HPC to work with containerd v1.6. (tests intentionally left untouched in order to try to catch discrepancies in behavior from upstream if/when they happen). Fix PATH in Windows dockerfiles.

Use winutils.GetHostPath() on file paths in felix param_types.go. Add resolving of $env:CONTAINER_SANDBOX_MOUNT_POINT to winutils.GetHostPath().

cni-plugin/pkg/install/install.go

apiserver/cmd/apiserver/server/watch.go

- clarify HPC env var comments - add logging when skipping chmod on windows - fix kubeconfigPath prefix issue

mgleung

LGTM

* Remove calico windows upgrader code * Add support for Windows hostprocess containers - Add windows calico-node daemonset with hostprocess containers. Manually tested with a clean installation and VXLAN setup and images from projectcalico/calico#7857. * Add uninstall-calico initContainer to the calico windows daemonset * Make CNI conf filename configurable for Windows * Don't render windows daemonset if disabled in installation resource. * Fix windows HPC support checking and add tests for it * Make windows HPC requirements be containerd v1.6 (instead of v1.7) and fix file paths to make them compatible with v1.6 * Add nodes/status permissions to cni-plugin clusterrole. Add externalnetworks plugin to node clusterrole when on EE. Fix CNI paths and template in order for HPC to work with containerd v1.6. * Move windows daemonset rendering from core-controller into its own windows-controller. Remove ServiceCIDRs and DNSServers from k8s endpoint configmap. Move ServiceCIDRs to InstallationSpec and auto-detect dns servers from the cluster in windows-controller. Add getWindowsBackend() function to windows rendering, to deduplicate this calculation. Remove containerd v1.7+ requirement for windows daemonset (HPC) as it is no longer necessary. Fix mixups of calls to cniDirectories() in windows rendering. * Add old calico-windows-upgrade clean up and make default WindowsDataplane disabled on every provider except AKS. * Fix openshift and RKE2 dns service values on windows_controller * Remove finalizer logic from windows controller and rendering. * Further add windows rendering UTs and fix some edge cases. * Further add windows render UTs * Address review comments: Fill in windows default cni paths in installation resource Remove 'terminating' logic from windows objects Misc name changes and fixes * Further address review comments: - Only populate Installation.WindowsNodes when windows is enabled - Use same serviceaccount/clusterroles/bindings for windows and linux calico-node - Remove unused env vars from calico-node-windows containers - Test changes to accomodate code changes * Fix WindowsNodeSpec defaults * Fix path env vars * - Further address review comments - Move validation out of windows_controller - Remove unnecessary env vars from windows node containers - Test adjustments

The windows upgrade code was cleaned up in projectcalico#7857, but there were leftover calls to it in node-service.ps1, leading to errors: flag provided but not defined: -should-install-windows-upgrade In addition to that, the token refresher, when run in a non-HPC scenario, would fail (on top of being unnecessary in non-HPC since that already requires the kubeconfig to be copied into the windows host): 2024-07-16 20:38:31.133 [FATAL][5064] cni-config-monitor/token_watch.go 59: Failed to read service account namespace file error=open /var/run/secrets/kubernetes.io/serviceaccount/namespace: The system cannot find the path specified. These were masked by the fact that logs are complex to get in non-HPC windows (requires RDP access into the host machine, so e2e failures retrieve nothing), and kubectl get nodes would show the windows nodes as 'Ready' even though these errors were occurring.

coutinhop added release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Jul 11, 2023

coutinhop added this to the Calico v3.27.0 milestone Jul 11, 2023

coutinhop requested review from fasaxc, mgleung and caseydavenport July 11, 2023 22:56

coutinhop requested a review from a team as a code owner July 11, 2023 22:56

coutinhop self-assigned this Jul 11, 2023

coutinhop mentioned this pull request Jul 11, 2023

Add support for Windows hostprocess containers tigera/operator#2732

Merged

5 tasks

coutinhop force-pushed the pedro-win-hpc-1 branch from 693c0f2 to dbf4a29 Compare July 12, 2023 02:05

This was referenced Jul 12, 2023

Windows support hostprocess #7260

Closed

kubelet log spam when path "c:/etc/kubernetes/manifests" doesn't exist kubernetes-sigs/sig-windows-tools#334

Closed

jsturtevant reviewed Jul 17, 2023

View reviewed changes

node/Dockerfile-windows Show resolved Hide resolved

jsturtevant reviewed Jul 17, 2023

View reviewed changes

coutinhop commented Jul 19, 2023

View reviewed changes

coutinhop added 6 commits July 27, 2023 10:10

Remove getHostPath() function as it is no longer necessary for Windows

1a6e1be

HPC when using containerd v1.7+

Remove kube-proxy service from host in unistall-calico-hpc.ps1

d28c912

Add quotes to windows cni plugin template so that it's valid JSON even

58c1695

before substitution (this helps with operator testing).

Fix cni-plugin Makefile for target image-windows

e202970

coutinhop force-pushed the pedro-win-hpc-1 branch from 1ac6289 to e202970 Compare July 27, 2023 17:10

coutinhop added 2 commits August 1, 2023 18:06

image, build and file path changes to make windows HPC compatible wit…

d177bbd

…h containerd v1.6+ (instead of requiring v1.7+).

Make HPC work with containerd v1.6 (by referring to all paths prefixe…

c81279d

…d with '$env:CONTAINER_SANDBOX_MOUNT_POINT'). Fix logic regarding CNI paths in uninstall-calico-hpc.ps1.

coutinhop force-pushed the pedro-win-hpc-1 branch from cccded6 to c81279d Compare August 14, 2023 23:24

Add support for multiple kubernetes service CIDRs

bac65a5

Fix bug in GetHostPath()

coutinhop added 4 commits August 23, 2023 14:06

Run 'make fix' to fix 'make check-fmt' in cni-plugin and node

678854e

Remove winupgrade code

a7021e6

'make fix' on confd

f28243a

coutinhop mentioned this pull request Sep 5, 2023

Not possible to install on windows via proxy #7952

Closed

coutinhop changed the title ~~[WIP] Add GA support for Windows hostprocess~~ Add GA support for Windows hostprocess Sep 6, 2023

coutinhop mentioned this pull request Sep 6, 2023

[WIP] Fix windows deps in node/Makefile #7619

Closed

3 tasks

coutinhop changed the title ~~Add GA support for Windows hostprocess~~ Add GA support for Windows hostprocess containers Sep 7, 2023

coutinhop added 2 commits September 12, 2023 15:56

Fix KUBECONFIG env var bleeding from host to containers on windows.

1401e4a

Use winutils.GetHostPath() on file paths in felix param_types.go. Add resolving of $env:CONTAINER_SANDBOX_MOUNT_POINT to winutils.GetHostPath().

Use same serviceaccount for windows and linux cni-plugin

b67cd5a

mgleung reviewed Sep 14, 2023

View reviewed changes

cni-plugin/pkg/install/install.go Show resolved Hide resolved

cni-plugin/pkg/install/install.go Show resolved Hide resolved

apiserver/cmd/apiserver/server/watch.go Show resolved Hide resolved

Address review comments:

e90395c

- clarify HPC env var comments - add logging when skipping chmod on windows - fix kubeconfigPath prefix issue

coutinhop mentioned this pull request Sep 15, 2023

Calico Windows HostProcess manifests do not authorize with the projected service token, they use KUBECONFIG env var incorrectly #7445

Closed

mgleung approved these changes Sep 15, 2023

View reviewed changes

coutinhop merged commit 7b9f6ce into projectcalico:master Sep 15, 2023
2 checks passed

coutinhop deleted the pedro-win-hpc-1 branch September 15, 2023 22:16

coutinhop mentioned this pull request Sep 15, 2023

Calico Windows HostProcess manifests break with containerd 1.7.0-beta.3 (see related) #7337

Closed

coutinhop mentioned this pull request Sep 25, 2023

in some cases, windows calico node-agent doesnt mark networkUnavailable:false... but sometimes it does ... should agent be modifying this condition, at all? #7996

Open

jsturtevant mentioned this pull request Oct 31, 2023

calico: the hpc calico nodes won't configure the network until first non HPC pod is created in containerd 1.7+ kubernetes-sigs/sig-windows-tools#347

Closed

coutinhop mentioned this pull request Jul 16, 2024

Fix non-HPC Calico for Windows issues #9016

Merged

3 tasks

coutinhop mentioned this pull request Jul 18, 2024

[release-v3.28] Auto pick #9016: Fix non-HPC Calico for Windows issues #9028

Merged

3 tasks

coutinhop mentioned this pull request Jul 18, 2024

[release-v3.27] Auto pick #9016: Fix non-HPC Calico for Windows issues #9029

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GA support for Windows hostprocess containers #7857

Add GA support for Windows hostprocess containers #7857

coutinhop commented Jul 11, 2023 •

edited

Loading

jsturtevant Jul 17, 2023

coutinhop Jul 17, 2023

jsturtevant Jul 17, 2023 •

edited

Loading

coutinhop Jul 19, 2023 •

edited

Loading

coutinhop Sep 6, 2023

coutinhop Jul 19, 2023 •

edited

Loading

jsturtevant Jul 20, 2023

coutinhop Jul 20, 2023

jsturtevant Jul 24, 2023

coutinhop Jul 24, 2023

mgleung left a comment

Add GA support for Windows hostprocess containers #7857

Add GA support for Windows hostprocess containers #7857

Conversation

coutinhop commented Jul 11, 2023 • edited Loading

Description

Related issues/PRs

Todos

Release Note

Reminder for the reviewer

jsturtevant Jul 17, 2023

Choose a reason for hiding this comment

coutinhop Jul 17, 2023

Choose a reason for hiding this comment

jsturtevant Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

coutinhop Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

coutinhop Sep 6, 2023

Choose a reason for hiding this comment

coutinhop Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

jsturtevant Jul 20, 2023

Choose a reason for hiding this comment

coutinhop Jul 20, 2023

Choose a reason for hiding this comment

jsturtevant Jul 24, 2023

Choose a reason for hiding this comment

coutinhop Jul 24, 2023

Choose a reason for hiding this comment

mgleung left a comment

Choose a reason for hiding this comment

coutinhop commented Jul 11, 2023 •

edited

Loading

jsturtevant Jul 17, 2023 •

edited

Loading

coutinhop Jul 19, 2023 •

edited

Loading

coutinhop Jul 19, 2023 •

edited

Loading