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.

Sign up for GitHub

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

Jump to bottom

Adding containerd as a runtime parameter option to windows bootstrapping #5083

Merged
merged 4 commits into from
Apr 7, 2022
Merged

Adding containerd as a runtime parameter option to windows bootstrapping #5083

merged 4 commits into from
Apr 7, 2022

Conversation

Skarlso
Copy link
Contributor

@Skarlso Skarlso commented Apr 7, 2022 •
edited
Loading

Description

Closes #5082

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@Skarlso Skarlso added the kind/feature New feature or request label Apr 7, 2022
@Skarlso Skarlso force-pushed the support-containerd-for-windows branch 2 times, most recently from 552414f to 176a0cc Compare April 7, 2022 07:10
@Skarlso Skarlso force-pushed the support-containerd-for-windows branch from 176a0cc to 5d27b55 Compare April 7, 2022 07:10
@Skarlso
Copy link
Contributor Author

Skarlso commented Apr 7, 2022 •
edited
Loading

Successfully created a cluster:

2022-04-07 09:35:36 [ℹ]  node "ip-192-168-8-28.us-west-2.compute.internal" is ready
2022-04-07 09:35:37 [ℹ]  kubectl command should work with "/Users/skarlso/.kube/config", try 'kubectl get nodes'
2022-04-07 09:35:37 [✔]  EKS cluster "gb-windows-container-runtime-1" in "us-west-2" region is ready
➜  windows-container-runtime

cluster:

➜  eksctl git:(support-containerd-for-windows) k get nodes
NAME                                           STATUS   ROLES    AGE     VERSION
ip-192-168-46-37.us-west-2.compute.internal    Ready    <none>   5m12s   v1.21.5-eks-9017834
ip-192-168-86-231.us-west-2.compute.internal   Ready    <none>   15s     v1.21.5-eks-bc4871b
➜  eksctl git:(support-containerd-for-windows) k get pods -A
NAMESPACE     NAME                       READY   STATUS    RESTARTS   AGE
kube-system   aws-node-nfrk9             1/1     Running   0          5m19s
kube-system   coredns-85d5b4454c-nh9vh   1/1     Running   0          14m
kube-system   coredns-85d5b4454c-srddv   1/1     Running   0          14m
kube-system   kube-proxy-6k2wx           1/1     Running   0          5m19s

Both, Windows and Linux nodes are online and all pods are ready.

Configuration used:

apiVersion: eksctl.io/v1alpha5
kind: ClusterConfig

metadata:
  name: gb-windows-container-runtime-1
  region: us-west-2

nodeGroups:
  - name: windows-ng
    amiFamily: WindowsServer2019FullContainer
    minSize: 1
    maxSize: 3
    desiredCapacity: 1
    containerRuntime: containerd

managedNodeGroups:
  - name: linux-ng
    instanceType: t2.large
    minSize: 1
    maxSize: 3
    desiredCapacity: 1

@Skarlso Skarlso force-pushed the support-containerd-for-windows branch from 5d27b55 to 81affdb Compare April 7, 2022 07:39
aclevername
aclevername approved these changes Apr 7, 2022
View reviewed changes
Copy link
Contributor

@aclevername aclevername left a comment

Choose a reason for hiding this comment

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

lgtm, worth adding an integration test to the windows suite?

@Skarlso
Copy link
Contributor Author

Skarlso commented Apr 7, 2022

lgtm, worth adding an integration test to the windows suite?

I don't wanna. :D

@Skarlso
Copy link
Contributor Author

Skarlso commented Apr 7, 2022

lgtm, worth adding an integration test to the windows suite?

I don't wanna. :D

Grumble grumble, I'll update it. :D

@Skarlso
Copy link
Contributor Author

Skarlso commented Apr 7, 2022

Updated.... local run result incoming...

@Skarlso
Copy link
Contributor Author

Skarlso commented Apr 7, 2022

Tests passed

2022-04-07 13:18:52 [▶]  started task: delete cluster control plane "ge-it-windows-wonderful-mushroom-1649327913" [async]
2022-04-07 13:18:52 [ℹ]  will delete stack "eksctl-ge-it-windows-wonderful-mushroom-1649327913-cluster"
2022-04-07 13:18:52 [▶]  completed task: delete cluster control plane "ge-it-windows-wonderful-mushroom-1649327913" [async]
2022-04-07 13:18:53 [✔]  all cluster resources were deleted

Ran 4 of 4 Specs in 3847.486 seconds
SUCCESS! -- 4 Passed | 0 Failed | 0 Pending | 0 Skipped

nikimanoledaki
nikimanoledaki approved these changes Apr 7, 2022
View reviewed changes
Copy link
Contributor

@nikimanoledaki nikimanoledaki left a comment

Choose a reason for hiding this comment

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

Very neat PR! LGTM :)

@Skarlso Skarlso merged commit 4f1f1b0 into eksctl-io:main Apr 7, 2022
SlevinWasAlreadyTaken pushed a commit to SlevinWasAlreadyTaken/eksctl that referenced this pull request Apr 11, 2022
@Skarlso @SlevinWasAlreadyTaken
Adding containerd as a runtime parameter option to windows bootstrapp…
9af742d 
…ing (eksctl-io#5083)

* Adding containerd as a runtime parameter option to windows bootstrapping

* Changed default in case of Windows ami family to `docker`

* Update the windows test
Skarlso added a commit that referenced this pull request Apr 13, 2022
@Skarlso
Revert "Adding containerd as a runtime parameter option to windows bo…
2621453 
…otstrapping (#5083)"

This reverts commit 4f1f1b0.
@Skarlso Skarlso mentioned this pull request Apr 13, 2022
Merged
Skarlso added a commit that referenced this pull request Apr 13, 2022
@Skarlso
Revert "Adding containerd as a runtime parameter option to windows bo…
e237668 
…otstrapping (#5083)" (#5118)

This reverts commit 4f1f1b0.
Skarlso added a commit that referenced this pull request Apr 13, 2022
@Skarlso
Revert "Adding containerd as a runtime parameter option to windows bo…
e18d6d4 
…otstrapping (#5083)"

This reverts commit 4f1f1b0.
SlevinWasAlreadyTaken pushed a commit to SlevinWasAlreadyTaken/eksctl that referenced this pull request Apr 13, 2022
@Skarlso @SlevinWasAlreadyTaken
Revert "Adding containerd as a runtime parameter option to windows bo…
e856604 
…otstrapping (eksctl-io#5083)" (eksctl-io#5118)

This reverts commit 4f1f1b0.
Skarlso added a commit that referenced this pull request Apr 13, 2022
@Skarlso
Revert "Adding containerd as a runtime parameter option to windows bo…
56ea2b4 
…otstrapping (#5083)" (#5120)

This reverts commit 4f1f1b0.
Skarlso added a commit that referenced this pull request Apr 20, 2022
@Skarlso
Adding containerd as a runtime parameter option to windows bootstrapp…
41e8980 
…ing (#5083) (#5127)

* Adding containerd as a runtime parameter option to windows bootstrapping

* Changed default in case of Windows ami family to `docker`

* Update the windows test
SlevinWasAlreadyTaken pushed a commit to SlevinWasAlreadyTaken/eksctl that referenced this pull request Apr 21, 2022
@Skarlso @SlevinWasAlreadyTaken
Adding containerd as a runtime parameter option to windows bootstrapp…
ebe4f81 
…ing (eksctl-io#5083) (eksctl-io#5127)

* Adding containerd as a runtime parameter option to windows bootstrapping

* Changed default in case of Windows ami family to `docker`

* Update the windows test
@hspencer77 hspencer77 mentioned this pull request Jul 8, 2022
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aclevername aclevername aclevername approved these changes

@nikimanoledaki nikimanoledaki nikimanoledaki approved these changes

@cPu1 cPu1 Awaiting requested review from cPu1

@Himangini Himangini Awaiting requested review from Himangini

Assignees
No one assigned
Labels
kind/feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature] Support setting container runtime for EKS Windows nodes
3 participants
@Skarlso @nikimanoledaki @aclevername