-
Notifications
You must be signed in to change notification settings - Fork 4k
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(eks): support for Kubernetes version 1.23 #22638
Conversation
The EKS Cluster construct has a property called `kubectlLayer`. When this property was not given, it would add a two default layers to the custom resource: - One with kubectl - One with the AWS CLI However, if the property was given, the one layer must contain both kubectl as well as the AWS CLI. This makes the `kubectl` layer unnecessarily large -- it must also contain the AWS CLI which the CDK already has and can bundle itself. Add a separate `awscliLayer` parameter to control the AWS CLI layer, if the user so wants. If not, the default AWS CLI layer will be added. If some user is already using a `kubectlLayer` which includes the AWS CLI, we now add both the default AWS CLI layer as well as the user's kubectl layer with the AWS CLI in it. There is no conflict: multiple layers can contain the same files. Last layer wins (which is the kubectl layer with the user's preferred AWS CLI).
Co-authored-by: Eli Polonsky <[email protected]>
…ws-cdk into huijbers/eks-separate-awscli-layer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
PR Linter has words for you. |
readme exempt; this is a feature for a new version and the method to include new kubectl versions is already included in the readme. |
✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.
@pahud We need MVP awards/recognition for each release. I nominate @madeline-k & @kaizencc for the upcoming v2.48.0 release 😃 🎉 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me and has my approval! I am not going to officially approve, as I am now a significant co-author, and I'd like someone else in the team to approve.
Annotations.of(this).addWarning(`You created a cluster with Kubernetes Version ${props.version} without specifying the kubectlLayer property. This may cause failures as the kubectl version provided with aws-cdk-lib is 1.20, which is only guaranteed to be compatible with Kubernetes versions 1.19-1.21. Please provide a kubectlLayer from @aws-cdk/lambda-layer-kubectl-v22.`); | ||
const kubectlVersion = Number(props.version.version) * 100; | ||
if (kubectlVersion >= 122 && !props.kubectlLayer) { | ||
Annotations.of(this).addWarning(`You created a cluster with Kubernetes Version ${props.version.version} without specifying the kubectlLayer property. This may cause failures as the kubectl version provided with aws-cdk-lib is 1.20, which is only guaranteed to be compatible with Kubernetes versions 1.19-1.21. Please provide a kubectlLayer from @aws-cdk/lambda-layer-kubectl-v${kubectlVersion % 100}.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it that it may cause errors or that it will? If we know it will, I think this should be an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not, no issues here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not for certain. In some cases you can use a version of kubectl that is not guaranteed to be compatible with the version of the Kubernetes engine you are using. It just depends on the APIs being used.
"data": "KubectlLayer600207B5" | ||
"data": "KubectlLayer600207B5", | ||
"trace": [ | ||
"!!DESTRUCTIVE_CHANGES: WILL_REPLACE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a concern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, a lambda layer is a stateless resource, and we are replacing it with an upgraded version of kubectl. So, the replacement is expected here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a small suggestion
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Add support for Kubernetes Version 1.23. In order to use this version, customers must pass in a
KubectlLayer
object from@aws-cdk/lambda-layer-kubectl-v23
to thekubectlLayer
construct prop ofCluster
.All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integ
to deploy the infrastructure and generate the snapshot (i.e.yarn integ
without--dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license