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

fix(lambda-layer-kubectl): update kubectl layer to use v1.22.0 #24064

Closed
wants to merge 3 commits into from

Conversation

daisuke-yoshimoto
Copy link
Contributor

@daisuke-yoshimoto daisuke-yoshimoto commented Feb 7, 2023

Updated kubectl version for CDK v1 to 1.22. Because the Kubernetes version supported by EKS is now 1.22 and above, it no longer makes sense to use an older version of kubectl.

Closes #19843 #22604 for CDK v1


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

@gitpod-io
Copy link

gitpod-io bot commented Feb 7, 2023

@github-actions github-actions bot added the p2 label Feb 7, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team February 7, 2023 23:59
@daisuke-yoshimoto daisuke-yoshimoto changed the title Update kubectl and helm cli for EKS 1.22 fix(lambda-layer-kubectl): update kubectl layer to use v1.22.0 Feb 7, 2023
@mrgrain
Copy link
Contributor

mrgrain commented Feb 8, 2023

Hey @daisuke-yoshimoto

I think this is the solution you are looking for:
https://www.npmjs.com/package/@aws-cdk/lambda-layer-kubectl-v22

But I realize our documentation might not be clear about this. 🤔

@daisuke-yoshimoto
Copy link
Contributor Author

@mrgrain

Thanks. But, that library cannot be used with cdk v1.
Also, EKS can only use Kubernetes 1.22 or higher, so increasing the version of kubectl shouldn't be a problem.

Hey @daisuke-yoshimoto

I think this is the solution you are looking for: https://www.npmjs.com/package/@aws-cdk/lambda-layer-kubectl-v22

But I realize our documentation might not be clear about this. 🤔

@github-actions github-actions bot added effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1 and removed p2 labels Feb 8, 2023
@daisuke-yoshimoto daisuke-yoshimoto marked this pull request as ready for review February 8, 2023 16:11
@daisuke-yoshimoto
Copy link
Contributor Author

@mrgrain

Now that the PR is out of draft status, please review it.

Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

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

Apologies for the delayed response @daisuke-yoshimoto

I don't think we will be able to accept this change:

AWS CDK v1 is no longer receiving higher-level “L2” construct updates for new or existing services. It will continue receiving updates for new and updated resource level “L1” constructs, critical bug fixes, and security updates only. We recommend you upgrade your applications to the new major version of CDK – AWS CDK version 2 (v2), which continues receiving new features and bug fixes.

https://aws.amazon.com/blogs/developer/version-1-of-the-aws-cloud-development-kit-aws-cdk-is-now-in-maintenance-mode/

I also think this might be a breaking change with a risk of data loss. Checking with the EKS experts on our team.

@daisuke-yoshimoto
Copy link
Contributor Author

daisuke-yoshimoto commented Feb 8, 2023

@mrgrain

I don't understand why it's a risk.

Next week EKS will no longer have Kubernetes 1.20 and 1.21 clusters.
https://docs.aws.amazon.com/ja_jp/eks/latest/userguide/kubernetes-versions.html#kubernetes-release-calendar
There's no point in keeping kubectl for 1.20 and 1.21, right?

I understand that you won't be making any major changes to CDK v1 anymore, but this is a pressing issue that needs to be addressed, so please include it.

I also think this might be a breaking change with a risk of data loss. Checking with the EKS experts on our team.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: b4c5432
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mrgrain
Copy link
Contributor

mrgrain commented Feb 8, 2023

I don't understand why it's a risk.

Users on EKS 1.19 won't be able to use this version of kubectl.

@daisuke-yoshimoto
Copy link
Contributor Author

@mrgrain

Check Amazon EKS Kubernetes release calendar carefully. 1.19 has been out of support for over a year now, so 1.19 clusters don't exist on EKS.

Kubernetes version Upstream release Amazon EKS release Amazon EKS end of support
1.25 August 23, 2022 March 2023 May 2024
1.24 May 3, 2022 November 15, 2022 January 2024
1.23 December 7, 2021 August 11, 2022 October 2023
1.22 August 4, 2021 April 4, 2022 May 2023
1.21 April 8, 2021 July 19, 2021 February 15, 2023
1.20 December 8, 2020 May 18, 2021 November 1, 2022
1.19 August 26, 2020 February 16, 2021 August 1, 2022

@TheRealAmazonKendra
Copy link
Contributor

The fix here is to move to v2. Is there anything major blocking you from being able to do so? As you noted, we put v1 into maintenance mode on July 1st of 2022 so we no longer backport updates unless they are security updates.

@mrgrain mrgrain closed this Feb 9, 2023
@daisuke-yoshimoto
Copy link
Contributor Author

@TheRealAmazonKendra

I will also send a PR to v2, so could you please merge this fix first? If you don't include this fix, users running EKS with cdk v1 will be in a lot of trouble. Users will not be able to update the manifest via cdk. I think this is a very serious bug.

Also, the documentation also says that critical bugs will be fixed until June 1, 2023. It will continue receiving updates for new and updated resource level “L1” constructs, critical bug fixes, and security updates only. https://aws.amazon.com/jp/blogs/developer/version-1-of-the-aws-cloud-development-kit-aws-cdk-is-now-in-maintenance-mode/

Please, can you accept the modification? I think that users who are operating with cdk v1 are very troubled.

@mrgrain
Copy link
Contributor

mrgrain commented Feb 9, 2023

There is no need to create a PR for v2.

We have created a more flexible system for this, see the link I've posted earlier:

https://www.npmjs.com/package/@aws-cdk/lambda-layer-kubectl-v22

@daisuke-yoshimoto
Copy link
Contributor Author

@mrgrain

However, cdk v2 also has kubectl 1.20 set by default.
https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/lambda-layer-kubectl/package.json#L93

"@aws-cdk/asset-kubectl-v20": "^2.1.1",

I think it is necessary to modify the default setting to @aws-cdk/lambda-layer-kubectl-v22.

@mrgrain
Copy link
Contributor

mrgrain commented Feb 10, 2023

Unfortunately changing the default version for the layer is a breaking change. We could introduce this behind a feature flag, but this would require us to depend on multiple layer packages, which isn't ideal either. I absolutely agree that the user experience isn't great here, but neither is breaking people. In hindsight, the kubectlLayer prop should have never been optional, given its tight coupling with the EKS version.

Not sure I currently see a way out of this. Maybe we could do some stuff with the KubernetesVersion class. But not convinced this is a good API either.

new Cluster(scope, id, {
  version: ClusterVersion.for(KubernetesVersion.V1_24, new KubectlV24Layer(this, 'kubectl'))
};

class ClusterVersion implements IKubernetesVersion {
  static public for(version: KubernetesVersion, kubectlLayer: ILayerVersion {
  }
}

And props.kubectlLayer would be deprecated and get new default check: const kubectlLayer = props.version.kubectlLayer ?? props.kubectlLayer ?? currentDefault

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants