Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows support hostprocess #7260

Closed

Conversation

fabi200123
Copy link

@fabi200123 fabi200123 commented Jan 31, 2023

Description

This PR helps moving the Windows Calico hostprocess from kubernetes-sigs/sig-windows-tools to this repository. This PR is based on the old PRs created in projectcalico/node and projectcalico/cni-plugin.

THIS PR IS STILL WORK IN PROGRESS!

Related issues/PRs

##Used for installing/building:

@fabi200123 fabi200123 requested a review from a team as a code owner January 31, 2023 17:35
@marvin-tigera marvin-tigera added this to the Calico v3.26.0 milestone Jan 31, 2023
@CLAassistant
Copy link

CLAassistant commented Jan 31, 2023

CLA assistant check
All committers have signed the CLA.

@marvin-tigera marvin-tigera added docs-pr-required Change is not yet documented release-note-required Change has user-facing impact (no matter how small) labels Jan 31, 2023
@fabi200123 fabi200123 force-pushed the windows-support-hostprocess branch 2 times, most recently from 59d1e90 to fb9b12c Compare February 13, 2023 14:52
Based on an old PR this commit adds the  Windows docker image for calico node
This commit is based on the changes done in an old PR that adds Windows installer for Hostprocess
@fabi200123 fabi200123 force-pushed the windows-support-hostprocess branch 2 times, most recently from 70356e3 to c92f769 Compare April 12, 2023 08:19
@coutinhop
Copy link
Contributor

Hi @fabi200123!

Apologies for the long delay. Thanks so much for your contribution, but I don't believe we should follow through with this though, seeing as projectcalico/node#1201 and projectcalico/cni-plugin#1148 were both closed due to kubernetes/enhancements#2865 (comment).

We already have hostprocess support (currently in tech-preview), you can find the powershell scripts and Dockerfile in https://github.com/projectcalico/calico/tree/master/node/windows-packaging, and the docs at https://docs.tigera.io/calico/latest/getting-started/kubernetes/windows-calico/quickstart#install-calico-for-windows-using-hostprocess-containers

Granted, I'm still trying to wrap my head around the original PRs you linked, but it doesn't seem like these changes ended up being necessary to support windows hostprocess containers? Please do comment back if there's any issue...

@jsturtevant
Copy link
Contributor

closed due to kubernetes/enhancements#2865 (comment).

This is no longer an issue with containerd 1.7. In cluster configuration with Hostprocess containers is now supported with no changes.

Granted, I'm still trying to wrap my head around the original PRs you linked, but it doesn't seem like these changes ended up being necessary to support windows hostprocess containers? Please do comment back if there's any issue...

Last time I looked it still required some custom set up (maybe this has changed). In general, I am in favor of getting rid of Powershell scripts installed directly on the host and moving towards go binaries for installation and running the calico components.

The current preview installation requires installing scripts on the host in a well-known spot and then referencing them from there. It would be cleaner to not have scripts installed directly on the node. It makes installation and cleanup easier. This also follows the installation process that Linux has (deploy a few templates and get calico!) instead of having to do custom configuration on the node.

@coutinhop
Copy link
Contributor

Thanks @jsturtevant! I see, and I agree with you that doing away with powershell scripts on the host would be best. Having containerd v1.7.0+ as a requirement might make this more complex (if we do want to support earlier versions we'd need to still have the scripts for them, @mgleung @caseydavenport @fasaxc any thoughts?)

@jsturtevant
Copy link
Contributor

Having containerd v1.7.0+ as a requirement might make this more complex

Agreed but I also thought since the current HPC work is preview and by the time something like this stabilizes 1.7 will be pretty far along the adoption curve.

Would need to maintain the "install on host and run as services" until 1.6.x went out of support, which is a definite maintaince concern, since you would have two ways of doing it. I think this can be mitigated to a degree though by tweaking and re-using some of the powershell scripts to support both formats.

@caseydavenport
Copy link
Member

Throwing in my 2 cents as a drive-by - we should probably just support containerd v1.7.0+ and go all-in on the hostprocess method of install.

@davhdavh
Copy link
Contributor

davhdavh commented Jun 1, 2023

A bit of feedback after trying to actually set this up.

  1. the docs at https://docs.tigera.io/calico/latest/getting-started/kubernetes/windows-calico/quickstart#install-calico-for-windows-using-hostprocess-containers is missing the step where you setup the calico-windows-vxlan.yaml and windows-kube-proxy.yaml because they are not default included anywhere.
  2. The scripts to setup things are all over the place. A couple of examples:
  • cni folder is in c:\etc\cni, or c:\k\cni or \cni depending on which script you use, and the defaults are different all over the place
  • hns.ps1 file is in c:\k\ or in C:\CalicoWindows\libs\hns
  • nssm is installed twice, both as part of containerd and in C:\CalicoWindows\nssm
  • Kubernetes gets installed in c:\k and C:\k\kubernetes
  1. The various calico help files is all over the place. Half the places you have to install the api server manually, half the places you have to install calicoctl manually.
  2. The helm chart for calico does not wait for all crd to be installed, and there is no method to setup the IPAM config etc necessary for a windows setup.
  3. the default calico-windows-vxlan.yaml doesnt actually work at all. It assumes the default working path is somewhere 2 levels deep, but the actual default working path is c:. The correct path to the various scripts has to be updated to "$env:CONTAINER_SANDBOX_MOUNT_POINT\host-process-install.ps1" and the working path updated to "c:\CalicoWindows".
  4. The requirements to the vSwitch is mentioned nowhere, and error messages is absolutely horrible in this case. E.g. if you dont have a free hardware card to make the vSwitch on, it will just spin forever waiting for the vSwitch to be created and the windows events will just say "bad interface"

@jsturtevant
Copy link
Contributor

2. The scripts to setup things are all over the place. A couple of examples:

I think that is the advantage of using containers with the scripts embedded (the approach proposed here). There will be a few things to be install on the host but otherwise when the container goes away we don't leave tons of stuff on the host.

coutinhop added a commit to coutinhop/calico that referenced this pull request Jul 11, 2023
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).
coutinhop added a commit to coutinhop/calico that referenced this pull request Jul 12, 2023
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).
@coutinhop
Copy link
Contributor

@fabi200123 @jsturtevant I'd like to thank you for your PR and I hope you don't take issue with the fact that I built upon it to push #7857 and tigera/operator#2732, still also works in progress, but which will add GA support for Windows HPC to Calico. I'd like to close this issue and ask you to review the above PRs if you'd like. Please let me know if that's OK with you!

@davhdavh thanks for your feedback. Yes, our current (non-HPC) Calico for Windows install has quite a bit of issues, but as @jsturtevant said, moving to hostprocess containers will clean this up quite a bit. I'd like to invite you to check out the PRs above as well.

coutinhop added a commit to coutinhop/calico that referenced this pull request Jul 27, 2023
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).
@jsturtevant
Copy link
Contributor

I think this can be closed. Thanks @coutinhop and @fabi200123 and everyone else that helped get this support merged into calico 🥳

@fabi200123 fabi200123 closed this Oct 9, 2023
@marvin-tigera marvin-tigera removed release-note-required Change has user-facing impact (no matter how small) docs-pr-required Change is not yet documented labels Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants