Skip to content

Conversation

@jsturtevant
Copy link
Contributor

@jsturtevant jsturtevant commented Sep 22, 2020

Initial support for configuring Windows nodes for CAPI. This is the first step for Windows support as described in Windows Proposal in CAPI: kubernetes-sigs/cluster-api#3616 and partly solves #30.

The current scripts work to create an image but I am still testing that the image works with a provider like CAPZ. I wanted to open the PR to get feedback on the setup. Update: now tested via CAPZ #382 (comment)

A few notes:

  • Kubeadm support for Windows is currently uses https://github.com/kubernetes-sigs/sig-windows-tools/blob/master/kubeadm/scripts/PrepareNode.ps1. I referred to that script for some of the set up.
  • Most windows users are use to scripts for configuration over ansible. I initially started this with scripts but the customization required in the scripts and use with Packer was quickly making it unmaintainable. The final ansible solutions feels much cleaner to me and closer to the Linux configuration which makes it easier to grok.
  • The Ansible and packer configuration has been split out since there is not enough in common. I think having them in separate folder will make the maintenance easier longer. Initially tried to keep the packer configuration re-use high but ended splitting it out into it's own folder under config/windows to keep it cleaner since there were so many different setting between Windows/linux.
  • The ansible files for windows are in an awkward spot, It would look cleaner if there was a clear split between Linux and Windows under the ansible folder. Didn't include that change here to minimize the diff.

Looking forward to feedback.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 22, 2020
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 22, 2020
@CecileRobertMichon
Copy link
Contributor

CI is failing with �[0;32m vhd-windows-2019: fatal: [default]: FAILED! => {"msg": "winrm or requests is not installed: No module named 'winrm'"}�[0m

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 24, 2020
@jsturtevant jsturtevant force-pushed the windows-vhd-azure branch 5 times, most recently from 1e54260 to 85d41c0 Compare September 24, 2020 23:37
@jsturtevant jsturtevant force-pushed the windows-vhd-azure branch 5 times, most recently from 5cff610 to 7b72c04 Compare October 1, 2020 02:39
@jsturtevant jsturtevant changed the title Windows vhd configuration scripts and Azure example Windows vhd configuration scripts and Azure builder Oct 1, 2020
@jsturtevant jsturtevant force-pushed the windows-vhd-azure branch 2 times, most recently from 47be782 to cff124c Compare October 12, 2020 15:21
@jsturtevant
Copy link
Contributor Author

This is ready for final review. I have tested it against a branch on CAPZ and have Windows machines and pods working:

kubectl get pods -A     
NAMESPACE     NAME                                                 READY   STATUS    RESTARTS   AGE
default   iis-1809-668c5b9c87-zxklh                            1/1     Running   0          105m

kubectl get nodes -o wide                          
NAME                         STATUS   ROLES    AGE   VERSION   INTERNAL-IP   EXTERNAL-IP   OS-IMAGE                         KERNEL-VERSION     CONTAINER-RUNTIME
test-5-control-plane-6crm6   Ready    master   39h   v1.19.1   10.0.0.6      <none>        Ubuntu 18.04.5 LTS               5.4.0-1025-azure   containerd://1.3.4
test-5-control-plane-fjw76   Ready    master   39h   v1.19.1   10.0.0.5      <none>        Ubuntu 18.04.5 LTS               5.4.0-1025-azure   containerd://1.3.4
test-5-control-plane-hn8rb   Ready    master   39h   v1.19.1   10.0.0.4      <none>        Ubuntu 18.04.5 LTS               5.4.0-1025-azure   containerd://1.3.4
test-5-md-0-cpj5q            Ready    <none>   39h   v1.19.1   10.1.0.7      <none>        Ubuntu 18.04.5 LTS               5.4.0-1025-azure   containerd://1.3.4
test-5-md-0-vsbmx            Ready    <none>   39h   v1.19.1   10.1.0.5      <none>        Ubuntu 18.04.5 LTS               5.4.0-1025-azure   containerd://1.3.4
test-5-w-757gp               Ready    <none>   39h   v1.19.1   10.1.0.4      <none>        Windows Server 2019 Datacenter   10.0.17763.1397    docker://19.3.12
test-5-w-p8vbj               Ready    <none>   39h   v1.19.1   10.1.0.6      <none>        Windows Server 2019 Datacenter   10.0.17763.1397    docker://19.3.12

This does not include support for running goss. When I tried it out I found native windows support didn't work and opened a PR that will enable it. Once that merges and a new release is cut I can follow up with support.

I also have an open question on the location of the Windows ansible scripts. It currently feels like an awkward spot mixed in with the Linux files. Any one have thoughts on if that is ok or files should be re-arranged?

@marosset
Copy link
Contributor

The changes LGTM

@jsturtevant
Copy link
Contributor Author

Yes this is ready. I just did a re-base to clean up the commits to a reasonable set and tested an image with kubernetes-sigs/cluster-api-provider-azure#1036

@jsturtevant
Copy link
Contributor Author

/test pull-azure-sigs

@perithompson
Copy link
Contributor

Changes LGTM as well @jsturtevant @CecileRobertMichon

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

will leave it open until tomorrow EOD unless there are any objections

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2020
@codenrhoden
Copy link
Contributor

No objections here.
/lgtm

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 21, 2020
@jsturtevant
Copy link
Contributor Author

/test pull-azure-sigs

@jsturtevant
Copy link
Contributor Author

/test pull-azure-sigs

@CecileRobertMichon
Copy link
Contributor

/lgtm
/approve

great work @jsturtevant 🎉

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 21, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, jsturtevant, ksubrmnn

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 21, 2020
@k8s-ci-robot k8s-ci-robot merged commit fa7baa8 into kubernetes-sigs:master Nov 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.