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

feat: Support Windows for Warm Pools #290

Merged
merged 4 commits into from
May 4, 2021
Merged

Conversation

backjo
Copy link
Collaborator

@backjo backjo commented May 4, 2021

Adds to #277

@backjo backjo requested review from a team as code owners May 4, 2021 03:15
@backjo backjo added the WIP label May 4, 2021
@codecov
Copy link

codecov bot commented May 4, 2021

Codecov Report

Merging #290 (6cf774b) into master (dcabfb3) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #290      +/-   ##
==========================================
+ Coverage   70.02%   70.09%   +0.06%     
==========================================
  Files          19       19              
  Lines        3096     3103       +7     
==========================================
+ Hits         2168     2175       +7     
  Misses        788      788              
  Partials      140      140              
Impacted Files Coverage Δ
controllers/provisioners/eks/helpers.go 91.46% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dcabfb3...6cf774b. Read the comment docs.

docs/EKS.md Outdated Show resolved Hide resolved
Signed-off-by: Jonah Back <jonah@jonahback.com>
backjo added 2 commits May 3, 2021 21:02
Signed-off-by: Jonah Back <jonah@jonahback.com>
Signed-off-by: Jonah Back <jonah@jonahback.com>
@backjo backjo removed the WIP label May 4, 2021
@backjo
Copy link
Collaborator Author

backjo commented May 4, 2021

Got around to testing this on the EKS recommended Windows AMI - it works!

Boot time for warm instances is around 2-3 minutes, compared to 7 for fresh instances. Might even be shorted without the custom PreBootstrap scripts that we have.

@eytan-avisror
Copy link
Collaborator

Got around to testing this on the EKS recommended Windows AMI - it works!

Boot time for warm instances is around 2-3 minutes, compared to 7 for fresh instances. Might even be shorted without the custom PreBootstrap scripts that we have.

Wow thats awesome, AL2 is kinda light weight so we only saw 1m instead of 2m approx, but still a 40-50% improvement.
Nice 💪

@backjo
Copy link
Collaborator Author

backjo commented May 4, 2021

Yeah - I imagine BottleRocket will be even less of an improvement than AL2, since it's already starting up in 90 seconds or so.

@eytan-avisror
Copy link
Collaborator

eytan-avisror commented May 4, 2021

Yeah - I imagine BottleRocket will be even less of an improvement than AL2, since it's already starting up in 90 seconds or so.

Well, AWS provisioning the instance takes around 45-60 seconds (which you can potentially save by warming), booting up bottlerocket machine in 30 seconds, and making it ready in another 30 will be pretty damn impressive.

@backjo
Copy link
Collaborator Author

backjo commented May 4, 2021

Yeah - I imagine BottleRocket will be even less of an improvement than AL2, since it's already starting up in 90 seconds or so.

Well, AWS provisioning the instance takes around 45-60 seconds (which you can potentially save by warming), booting up bottlerocket machine in 30 seconds, and making it ready in another 30 will be pretty damn impressive.

For real - I just clocked a BottleRocket node (without warm pool) from

  1. Manually updating the desired capacity on the ASG
  2. The Kubelet reporting to the API server
  3. The Node being ready on the API server

1-2 took 41 seconds
2-3 took 30 seconds

For a total of 71 seconds from ASG capacity change to being ready in K8S 😱 .

Given the design of BottleRocket with it's read-only root file system and the already bleeding-fast startup times, it probably doesn't make sense to even attempt to support warm pools for it.

@eytan-avisror
Copy link
Collaborator

eytan-avisror commented May 4, 2021

Yeah - I imagine BottleRocket will be even less of an improvement than AL2, since it's already starting up in 90 seconds or so.

Well, AWS provisioning the instance takes around 45-60 seconds (which you can potentially save by warming), booting up bottlerocket machine in 30 seconds, and making it ready in another 30 will be pretty damn impressive.

For real - I just clocked a BottleRocket node (without warm pool) from

1. Manually updating the desired capacity on the ASG

2. The Kubelet reporting to the API server

3. The Node being ready on the API server

1-2 took 41 seconds
2-3 took 30 seconds

For a total of 71 seconds from ASG capacity change to being ready in K8S 😱 .

Given the design of BottleRocket with it's read-only root file system and the already bleeding-fast startup times, it probably doesn't make sense to even attempt to support warm pools for it.

Might reduce that by a good 20-30 seconds with warm pools :) don't forget you are completely removing the time it takes Amazon to simply create and propagate the instance object itself.
Still worth exploring given the cost of running this is almost free.

BTW, did you push the change for <persist>?

Signed-off-by: Jonah Back <jonah@jonahback.com>
@backjo
Copy link
Collaborator Author

backjo commented May 4, 2021

Just pushed the persist change

Copy link
Collaborator

@eytan-avisror eytan-avisror left a comment

Choose a reason for hiding this comment

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

🏆

@backjo backjo merged commit 3064052 into master May 4, 2021
@backjo backjo deleted the feature/WindowsWarmPool branch May 4, 2021 21:19
@eytan-avisror eytan-avisror mentioned this pull request May 8, 2021
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.

2 participants