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: Allow users to specify the Windows Server version; 2019 (default) or 2022 #1078

Merged
merged 8 commits into from
Oct 25, 2022

Conversation

IgorEulalio
Copy link
Contributor

What does this PR do?

This PR allows clients to override the version of windows server and set the default version to 2022 for clusters in 1.22 or above.

Motivation

Windows Server 2022 is the new and recommend operation system to run Windows containers on Kubernetes. It accommodates all the new features previously only available on Windows Server SAC versions. The AWS Windows container SME team is already helping and spreading the message for customers to start migrating from 2019 to 2022.
With this new addition to EKS Blueprints, customers can leverage the new Windows Server 2022 on EKS clusters version 1.22+ and maintain their Windows Server 2019 on older EKS cluster versions.

  • Resolves #

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I have added a new example under examples to support my PR
    I used the example that already exists for windows cluster.
  • Yes, I have created another PR for add-ons under add-ons repo (if applicable)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR
    Note: Not all the PRs require a new example and/or doc page. In general:
  • Use an existing example when possible to demonstrate a new addons usage
  • A new docs page under docs/add-ons/* is required for new a new addon

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

This is a pontual change to make the default windows server used today more flexible and also push users to use the new version of windows server.

@IgorEulalio IgorEulalio changed the title [Windows Self Managed Node Groups] Change the default version for windows server to 2022 for clients that are using cluster version 1.22 or above. feat: [Windows Self Managed Node Groups] Change the default version for windows server to 2022 for clients that are using cluster version 1.22 or above. Oct 24, 2022
@marciogmorales
Copy link
Contributor

Marcio Morales - AWS Windows Container SME, reviewed and worked together with Igor in the proposed solution.

@IgorEulalio IgorEulalio changed the title feat: [Windows Self Managed Node Groups] Change the default version for windows server to 2022 for clients that are using cluster version 1.22 or above. feat: Change the default version for windows server to 2022 for clients that are using cluster version 1.22 or above. Oct 24, 2022
@@ -49,16 +51,20 @@ locals {
var.self_managed_ng
)

# WINDOWS CONFIGURATION
compatible_with_windows_server_2022 = var.context.cluster_version >= "1.22" ? true : false
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Can users create clusters with 2019 AMI on 1.22/1.23?
  2. Can users create clusters with 2022 AMI on 1.19-1.22?

Also, this is not a valid comparison var.context.cluster_version >= "1.22", this is not a numerical comparison

I think what we want to do is add a variable for the windows_server_version and have it default to 2019 but users have the ability to pass in 2022 and then we can get rid of the other changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are looking to change the default to 2022 instead of 2019, but do not exist AMI for Windows Server 2022 that are compatible with cluster below 1.22, so the default must be 2022 for all clusters that are in 1.22 or above and 2019 for cluster that are below.

Copy link
Contributor

Choose a reason for hiding this comment

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

If users have created clusters with 2019 AMI on 1.22/1.23 clusters, this would be a disruptive change for them and therefore it falls under a breaking change. "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to describe that on the new version disclaimer and make this new version default? This behavior is intentional.

@bryantbiggs bryantbiggs temporarily deployed to EKS Blueprints Test October 25, 2022 12:29 Inactive
@bryantbiggs bryantbiggs changed the title feat: Change the default version for windows server to 2022 for clients that are using cluster version 1.22 or above. feat: Allow users to specify the Windows Server version; 2019 (default) or 2022 Oct 25, 2022
@bryantbiggs bryantbiggs temporarily deployed to EKS Blueprints Test October 25, 2022 12:33 Inactive
@bryantbiggs bryantbiggs merged commit bc1c58e into aws-ia:main Oct 25, 2022
vara-bonthu pushed a commit that referenced this pull request Nov 11, 2022
…ult) or `2022` (#1078)

Co-authored-by: igor lopes <igoreul@amazon.com>
Co-authored-by: Bryant Biggs <bryantbiggs@gmail.com>
allamand pushed a commit to allamand/terraform-aws-eks-blueprints that referenced this pull request Dec 15, 2022
…ult) or `2022` (aws-ia#1078)

Co-authored-by: igor lopes <igoreul@amazon.com>
Co-authored-by: Bryant Biggs <bryantbiggs@gmail.com>
allamand pushed a commit to allamand/terraform-aws-eks-blueprints that referenced this pull request Jan 10, 2023
…ult) or `2022` (aws-ia#1078)

Co-authored-by: igor lopes <igoreul@amazon.com>
Co-authored-by: Bryant Biggs <bryantbiggs@gmail.com>
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.

None yet

3 participants