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 #19919

Closed
wants to merge 1 commit into from

Conversation

robertd
Copy link
Contributor

@robertd robertd commented Apr 14, 2022

Addendum to #19756.

@otaviomacedo @rix0rrr Sorry for missing this piece in my original PR.

More info: #19756 (comment)
Workaround: #19843 (comment) (Thanks @dtitenko-dev)

This introduces a breaking change if users are creating EKS clusters <1.21.

Should we capture this in readme, or add a note in jsdocs for versions <1.21, or both? I think it would be a bit too much if we label <1.21 versions as deprecated just because of this.

@otaviomacedo @rix0rrr Thoughts on which direction we should take?

Fix #19843
Link #15736


All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use 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

@gitpod-io
Copy link

gitpod-io bot commented Apr 14, 2022

@github-actions github-actions bot added bug This issue is a bug. p2 labels Apr 14, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team April 14, 2022 18:55
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@robertd
Copy link
Contributor Author

robertd commented Apr 15, 2022

I wonder if we should have multiple versions of kubectl handler and use the one that matches the version of the Kubernetes cluster. That would at least keep the backwards compatibility with <1.20 clusters.

@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 20, 2022

This introduces a breaking change if users are creating EKS clusters <1.21.

I was afraid of that.

I wonder if we should have multiple versions of kubectl handler and use the one that matches the version of the Kubernetes cluster. That would at least keep the backwards compatibility with <1.20 clusters.

Yes, unfortunately we are currently struggling with package size issues. kubectl is large, and our package is larger, and adding multiple kubectls to it will make it exceed size limits of package managers. So we can't do this right now, until we have invested significant engineering effort into lifting those limitations.

@rix0rrr rix0rrr added pr/blocked This PR cannot be merged or reviewed, because it is blocked for some reason. blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. labels Apr 20, 2022
@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 20, 2022

@robertd can you submit a new PR adding warning banners in all the right places indicating that the 1.22 version of Kubernetes cannot currently be used?

@mergify mergify bot closed this in #20000 Apr 21, 2022
mergify bot pushed a commit that referenced this pull request Apr 21, 2022
Revert "feat(eks): add k8s v1.22 (#19756)"

This commit reverts addition of the latest version of EKS (1.22) to CDK as there are some incompatibilities between the current lambda-layer-kubectl and v1.22 EKS cluster. Preferably we would have multiple versions fo kubectl lambda layer, but we are currently struggling with package size issues. kubectl is large, and our package is larger, and adding multiple kubectls to it will make it exceed size limits of package managers. So we can't do this right now, until we have invested significant engineering effort into lifting those limitations. In order to not break the backward compatibility with <=1.20 versions, it was decided to revert this addition PR until all the underlying issues around package size issues are resolved.

cc @rix0rrr

Close #19919 

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `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*
StevePotter pushed a commit to StevePotter/aws-cdk that referenced this pull request Apr 27, 2022
Revert "feat(eks): add k8s v1.22 (aws#19756)"

This commit reverts addition of the latest version of EKS (1.22) to CDK as there are some incompatibilities between the current lambda-layer-kubectl and v1.22 EKS cluster. Preferably we would have multiple versions fo kubectl lambda layer, but we are currently struggling with package size issues. kubectl is large, and our package is larger, and adding multiple kubectls to it will make it exceed size limits of package managers. So we can't do this right now, until we have invested significant engineering effort into lifting those limitations. In order to not break the backward compatibility with <=1.20 versions, it was decided to revert this addition PR until all the underlying issues around package size issues are resolved.

cc @rix0rrr

Close aws#19919 

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `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*
@pallabganai
Copy link

Is there any plan to support back v1.22?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. bug This issue is a bug. p2 pr/blocked This PR cannot be merged or reviewed, because it is blocked for some reason.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(aws-eks): kubectl layer is not compatible with k8s v1.22.0
4 participants