feat: Add windows support #3698

topikachu · 2023-04-04T14:21:17Z

Description
This change try to integrate with the latest EKS window support.

Add two new Windows AMI Family, Windows2019 and Windows2022.
Only Core are supported OOTB.
Windows relevant code, "kubernetes.io/os" and "vpc.amazonaws.com/PrivateIPv4Address" is only active when Windows AMI Family defined in the AwsNodeTemplate

Users can use amiSelector to choose other AMIs.
Test on a Linux and Windows mixed system for provision and de-provision.

How was this change tested?

Test on a live system with mixed Windows and Linux workload.
Both provision and de-provision are tested

Does this change impact docs?

Yes, PR includes docs updates
Yes, issue opened: #
No

New document is required after this change is approved.
Release Note

Add Windows support. A new Windows AMI Family is introduced.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

netlify · 2023-04-04T14:21:28Z

✅ Deploy Preview for karpenter-docs-prod canceled.

Name	Link
🔨 Latest commit	`4435349`
🔍 Latest deploy log	https://app.netlify.com/sites/karpenter-docs-prod/deploys/6490a06a4c745b00081c1da7

ellistarn · 2023-04-04T15:33:14Z

Wow, thanks for writing this up. @jonathan-innis can you review?

topikachu · 2023-04-05T00:43:36Z

@jonathan-innis @ellistarn
Please tell me if more tests are required.
Not sure what kind of tests to add.

pkg/apis/v1alpha1/register.go

pkg/providers/amifamily/bootstrap/windows.go

pkg/providers/amifamily/windows.go

pkg/providers/instancetype/types.go

pkg/providers/amifamily/windows.go

pkg/providers/amifamily/resolver.go

topikachu · 2023-04-06T08:48:38Z

@jonathan-innis @rawahars
I need your suggestion about these:

Is it OK to give four new Windows AMI Family names? ~~If yes, shall we use the same long name as eksctl, like WindowsServer2019CoreContainer?~~ I add 4 new Windows AMI families in short name, like WindowsServer2019Core .
Shall we use a single value of the node requirement "kubernetes.io/os" or two values that have both Linux and Windows? If we choose two values, how to handle the default node labels created by schedule? This change may break the test should apply labels to the node Fixed.
Is there a clear rule about the ec2 instances supported for Windows workload?

Please review the new change.

Thanks

jonathan-innis · 2023-04-07T07:31:31Z

Is it OK to give four new Windows AMI Family names

Our general feeling is that an AMIFamily should be centered around the userData that is used to startup the node and attach it to the cluster. An AMIFamily should be a grouping of AMIs that have effectively the same configuration. With that in mind, I think it would be preferable to have Windows be a single AMIFamily and we can differentiate the different versions using a different mechanism.

Musing: I wonder if we can do this through the windows build number that we know will get populated to the node when the node registers with the cluster. We can support different AMI SSM aliases and have them be compatible with different sets of node.kubernetes.io/windows-build requirements

jonathan-innis · 2023-04-07T07:32:00Z

Is there a clear rule about the ec2 instances supported for Windows workload

@rawahars Can you comment on this one?

topikachu · 2023-04-07T08:24:17Z

Our general feeling is that an AMIFamily should be centered around the userData that is used to startup the node and attach it to the cluster. An AMIFamily should be a grouping of AMIs that have effectively the same configuration. With that in mind, I think it would be preferable to have Windows be a single AMIFamily and we can differentiate the different versions using a different mechanism.

It's reasonable but we need to first decide on the source of AMI. There are two options

The AMI can be resolved by the AWSNodeTemplate. In this case, all parameters must be defined in this file. This can be implemented by multiple AMIFamilies, annotations, or new fields in AWSNodeTemplate
The AMI can be resolved by the AWSNodeTemplate and Provisioner together. A user only defines a single AMI Family and declares the OS requirement in the provisioner. This needs to change both the code of provision and drift.

All are doable, and please shed some light on this.

jonathan-innis · 2023-04-07T16:21:04Z

declares the OS requirement in the provisioner. This needs to change both the code of provision and drift.

I'd argue either the Provisioner or the workload is the best place. At a high-level, the philosophy behind the different components is:

Provisioner defines a "world of possibilities" at the Cluster Admin level for a group of workloads
Workload-based scheduling defines the "world of possibilities" at the App Dev level for single workloads. I think in general, most users are going to be nodeSelecting on a version of windows if their workloads only support a specific version of windows.

You can imagine a scenario where a version of Windows isn't defined in the Provisioner at all and its just defined at the workload level, which will be passed into the launched machine requirements and be used to select the correct Windows AMI version for that workload

jonathan-innis · 2023-04-07T16:30:25Z

Referencing the Upstream Kubernetes Windows User Guide for more info on the windows build version and windows best-practices

topikachu · 2023-04-08T02:43:29Z

Referencing the Upstream Kubernetes Windows User Guide for more info on the windows build version and windows best-practices

I'm afraid I have no idea to implement the Windows build version without AWS EKS team support.
There's no enough information to resolve the matched AMI until AWS can provide more metadata on each AMI.

I notice that the new AMIFamily DefaultAMIs method returns multiple AMIs and each AMI output has its own requirements. https://github.com/aws/karpenter/blob/629859c790b8410b78d9b9c134b02fe02e32165f/pkg/providers/amifamily/resolver.go#L75
It's possible that a single Windows AMI Family returns all the Windows AMIs with 2019/2022, core/full as part of the requirements.
However, it still needs some code change around https://github.com/aws/karpenter/blob/629859c790b8410b78d9b9c134b02fe02e32165f/pkg/cloudprovider/cloudprovider.go#L224 and https://github.com/aws/karpenter/blob/629859c790b8410b78d9b9c134b02fe02e32165f/pkg/providers/amifamily/resolver.go#L122

@jonathan-innis Any idea?

jonathan-innis · 2023-04-10T16:56:34Z

It's possible that a single Windows AMI Family returns all the Windows AMIs with 2019/2022, core/full as part of the requirements.

@topikachu Yes, this is the idea, that the DefaultAMIs that are returned from the AMIFamily would contain the windows build numbers as requirements. Happy to help you along this path. Also, I'm not sure GH is the easiest place to have ad-hoc convos like this so feel free to reach out to me on the Kubernetes slack (I'm Jonathan Innis) on there as well or in our "karpenter-dev" channel on the Kubernetes slack.

topikachu · 2023-04-21T10:35:23Z

@jonathan-innis Please check this new change.
It uses two ami families with core as default.

jonathan-innis · 2023-04-25T17:42:26Z

@topikachu Can you rebase the change? It looks like you have a few conflicts in a few files in the PR

pkg/apis/v1alpha1/register.go

pkg/providers/amifamily/windows.go

pkg/providers/instancetype/types.go

pkg/apis/v1alpha1/register.go

jonathan-innis

Looking good! Just some high-level nits

pkg/providers/instancetype/types.go

pkg/providers/amifamily/bootstrap/windows.go

pkg/providers/instancetype/suite_test.go

pkg/providers/amifamily/windows.go

pkg/apis/v1alpha1/register.go

pkg/providers/instancetype/suite_test.go

pkg/providers/instancetype/types.go

jonathan-innis

/karpenter snapshot

github-actions · 2023-06-19T04:07:37Z

Snapshot successfully published to oci://public.ecr.aws/karpenter/karpenter:v0-1b4fec14fcf13723a6cf9ecbf386894b1e0d653c. Find the image tag and installation instructions at https://gallery.ecr.aws/karpenter/karpenter/

Fixes aws#1131 This change try to integrate with the latest EKS window support. Add two new Windows AMI Family, Windows2019 and Windows2022. Only Core are supported OOTB. Windows relevant code, "kubernetes.io/os" and "vpc.amazonaws.com/PrivateIPv4Address" is only active when Windows AMI Family defined in the AwsNodeTemplate Users can use amiSelector to choose other AMIs. Test on a Linux and Windows mixed system for provision and de-provision. Test on a live system with mixed Windows and Linux workload. Both provision and de-provision are tested

1. Remove variant in windows family since we only support "Core" now. 2. Follow the test case convention to test labels.

1. Remove unused consts 2. Add SupportsENILimitedPodDensity feature flag. 3. Make kubeletExtraArgs as a generic method of amifamily.Options 4. Add ContainerRuntime parameter in windows bootstrap 5. Change ENILimitedPods, maxPods, privateIPv4Address

1. Fix the issue that double ContainerRuntime is configured in Windows 2. Refactor getOSRequirement 3. Refine test and one new to test pods number on windows node

jonathan-innis

/karpenter snapshot

github-actions · 2023-06-19T18:42:56Z

Snapshot successfully published to oci://public.ecr.aws/karpenter/karpenter:v0-4435349baab8eb6cee4d04469a1524da6237bdd6. Find the image tag and installation instructions at https://gallery.ecr.aws/karpenter/karpenter/

jonathan-innis

/karpenter snapshot

github-actions · 2023-06-19T19:23:36Z

Snapshot successfully published to oci://public.ecr.aws/karpenter/karpenter:v0-4435349baab8eb6cee4d04469a1524da6237bdd6. Find the image tag and installation instructions at https://gallery.ecr.aws/karpenter/karpenter/

jonathan-innis

LGTM 🚀

bwagner5

lgtm

rawahars

LGTM 🚢

topikachu · 2023-06-21T05:30:49Z

Thank you for accepting my code change. I'm excited that the addition of Windows support to Karpeneter will benefit all users.

If there are any further modifications needed, please let me know. I'm eager to contribute more to the Karpeneter project and assist in any way I can.

Thanks again for the opportunity.

topikachu requested a review from a team as a code owner April 4, 2023 14:21

topikachu requested a review from jonathan-innis April 4, 2023 14:21

topikachu force-pushed the feat_windows branch from 48fea79 to 4b96ef3 Compare April 4, 2023 14:30

jonathan-innis self-assigned this Apr 4, 2023

topikachu force-pushed the feat_windows branch 2 times, most recently from 5c9cc40 to 99af8e5 Compare April 5, 2023 00:42

topikachu force-pushed the feat_windows branch from 99af8e5 to 3edca47 Compare April 5, 2023 01:10

rawahars suggested changes Apr 5, 2023

View reviewed changes

jonathan-innis reviewed Apr 5, 2023

View reviewed changes

pkg/providers/instancetype/types.go Outdated Show resolved Hide resolved

pkg/providers/instancetype/types.go Outdated Show resolved Hide resolved

pkg/providers/amifamily/windows.go Outdated Show resolved Hide resolved

pkg/providers/amifamily/resolver.go Outdated Show resolved Hide resolved

topikachu force-pushed the feat_windows branch 2 times, most recently from ff013a8 to a3b59a5 Compare April 21, 2023 10:10

jonathan-innis reviewed Apr 25, 2023

View reviewed changes

topikachu force-pushed the feat_windows branch from e95b91d to 22b12ee Compare May 3, 2023 06:51

rawahars reviewed May 3, 2023

View reviewed changes

pkg/apis/v1alpha1/register.go Outdated Show resolved Hide resolved

jonathan-innis reviewed May 4, 2023

View reviewed changes

jonathan-innis reviewed May 8, 2023

View reviewed changes

njtran mentioned this pull request May 8, 2023

feat: Private IPv4 address for AWS Windows. (#1131) #2388

Closed

3 tasks

jonathan-innis force-pushed the feat_windows branch 4 times, most recently from fa1d83e to 1b4fec1 Compare June 15, 2023 21:39

tzifudzi mentioned this pull request Jun 19, 2023

docs: Add Karpenter support for Windows docs #4074

Merged

1 task

jonathan-innis reviewed Jun 19, 2023

View reviewed changes

topikachu and others added 10 commits June 19, 2023 11:37

let windows family support both dockerd and containerd

18c5757

Refine the code

6fd9449

1. Remove variant in windows family since we only support "Core" now. 2. Follow the test case convention to test labels.

Refactor the code

47547c4

1. Remove unused consts 2. Add SupportsENILimitedPodDensity feature flag. 3. Make kubeletExtraArgs as a generic method of amifamily.Options 4. Add ContainerRuntime parameter in windows bootstrap 5. Change ENILimitedPods, maxPods, privateIPv4Address

Fix issues

fd37522

1. Fix the issue that double ContainerRuntime is configured in Windows 2. Refactor getOSRequirement 3. Refine test and one new to test pods number on windows node

Wrap kubeletExtraArgs for EKS bootstrap

6a61f19

Add E2E test for Windows AMI families

2bcbc83

Add build version requirement

10ee1fb

Add validation for userData and Windows AMIFamily

b4188a0

Fix flakes in E2E tests

4435349

jonathan-innis force-pushed the feat_windows branch from 02b1566 to 4435349 Compare June 19, 2023 18:37

jonathan-innis reviewed Jun 19, 2023

View reviewed changes

tzifudzi mentioned this pull request Jun 19, 2023

Add support for userdata in Windows nodes #4081

Closed

jonathan-innis approved these changes Jun 20, 2023

View reviewed changes

bwagner5 approved these changes Jun 20, 2023

View reviewed changes

rawahars approved these changes Jun 20, 2023

View reviewed changes

jonathan-innis enabled auto-merge (squash) June 20, 2023 17:11

jonathan-innis merged commit 1fef30f into aws:main Jun 20, 2023
17 checks passed

tzifudzi mentioned this pull request Jun 20, 2023

Automatically include ServiceCIDR as an argument to Windows nodes bootstrap #4088

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add windows support #3698

feat: Add windows support #3698

topikachu commented Apr 4, 2023 •

edited

netlify bot commented Apr 4, 2023 •

edited

ellistarn commented Apr 4, 2023

topikachu commented Apr 5, 2023

topikachu commented Apr 6, 2023 •

edited

jonathan-innis commented Apr 7, 2023

jonathan-innis commented Apr 7, 2023

topikachu commented Apr 7, 2023 •

edited

jonathan-innis commented Apr 7, 2023

jonathan-innis commented Apr 7, 2023

topikachu commented Apr 8, 2023

jonathan-innis commented Apr 10, 2023

topikachu commented Apr 21, 2023

jonathan-innis commented Apr 25, 2023

jonathan-innis left a comment

jonathan-innis left a comment

github-actions bot commented Jun 19, 2023

jonathan-innis left a comment

github-actions bot commented Jun 19, 2023

jonathan-innis left a comment

github-actions bot commented Jun 19, 2023

jonathan-innis left a comment

bwagner5 left a comment

rawahars left a comment

topikachu commented Jun 21, 2023

feat: Add windows support #3698

feat: Add windows support #3698

Conversation

topikachu commented Apr 4, 2023 • edited

netlify bot commented Apr 4, 2023 • edited

✅ Deploy Preview for karpenter-docs-prod canceled.

ellistarn commented Apr 4, 2023

topikachu commented Apr 5, 2023

topikachu commented Apr 6, 2023 • edited

jonathan-innis commented Apr 7, 2023

jonathan-innis commented Apr 7, 2023

topikachu commented Apr 7, 2023 • edited

jonathan-innis commented Apr 7, 2023

jonathan-innis commented Apr 7, 2023

topikachu commented Apr 8, 2023

jonathan-innis commented Apr 10, 2023

topikachu commented Apr 21, 2023

jonathan-innis commented Apr 25, 2023

jonathan-innis left a comment

Choose a reason for hiding this comment

jonathan-innis left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 19, 2023

jonathan-innis left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 19, 2023

jonathan-innis left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 19, 2023

jonathan-innis left a comment

Choose a reason for hiding this comment

bwagner5 left a comment

Choose a reason for hiding this comment

rawahars left a comment

Choose a reason for hiding this comment

topikachu commented Jun 21, 2023

topikachu commented Apr 4, 2023 •

edited

netlify bot commented Apr 4, 2023 •

edited

topikachu commented Apr 6, 2023 •

edited

topikachu commented Apr 7, 2023 •

edited