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.

Sign up for GitHub

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

Jump to bottom

fix(eks): failures when creating or updating clusters #5540

Merged
merged 2 commits into from
Dec 30, 2019
Merged

fix(eks): failures when creating or updating clusters #5540

merged 2 commits into from
Dec 30, 2019
+14,382 −1,320

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Dec 23, 2019 •
edited
Loading

There were two causes of timeouts for EKS cluster creation: create time which is longer than the AWS Lambda timeout (15min) and lack of retry when applying kubectl after the cluster has been created.

The change fixes the first issue by leveraging the custom resource provider framework to implement the cluster resource as an async resource. The custom resource providers are now bundled as nested stacks so they don't take up too many resources from users, and are also reused by multiple clusters within the same stack. This required that the creation role will not be the same as the lambda role, so we define this role separately and assume it within the providers.

The second issue is fixed by adding 3 retries to "kubectl apply".

Backwards compatibility: as described in #5544, since the resource provider handler of Cluster and KubernetesResource has been changed, this change requires a replacement of existing clusters (deployment fails with "service token cannot be changed" error). Since this can be disruptive to users, this change includes an exact copy of the previous version under a new module called @aws-cdk/aws-eks-legacy, which can be used as a drop-in replacement until users decide to upgrade to the new version. Using the legacy cluster will emit a synthesis warning that this module will no longer be released as part of the CDK starting March 1st, 2020.

BREAKING CHANGE: (in experimental module) the providers behind the AWS EKS module have been rewritten to address multiple stability issues. Since this change requires cluster replacement, the old version of this module is available under @aws-cdk/aws-eks-legacy. Please read #5544 carefully for upgrade instructions.

TODO:

  • Decide what to do about backwards compatibility (since we replaced the resource providers)
  • Verify if this resolves other pending EKS bugs
  • Migrate the HelmChartResource construct to CR framework
  • Unit tests for KubernetesResource and HelmChartResource

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

@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Dec 23, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@eladb eladb mentioned this pull request Dec 24, 2019
Closed
@eladb eladb marked this pull request as ready for review December 24, 2019 12:16
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

RomainMuller
RomainMuller reviewed Dec 24, 2019
View reviewed changes
packages/@aws-cdk/aws-eks-legacy/README.md Show resolved Hide resolved
packages/@aws-cdk/aws-eks-legacy/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks-legacy/lib/aws-auth-mapping.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/handler.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/handler.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/cluster-resource.ts Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/helm-chart.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/helm-chart/index.py Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/k8s-resource-handler/index.py Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/k8s-resource.ts Outdated Show resolved Hide resolved
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@eladb eladb added the pr/do-not-merge This PR should not be merged at this time. label Dec 30, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

fix(eks): failures when creating or updating clusters
6a3946d 
There were two causes of timeouts for EKS cluster creation: create time which is longer than the AWS Lambda timeout (15min) and lack of retry when applying kubectl after the cluster has been created.

The change fixes the first issue by leveraging the custom resource provider framework to implement the cluster resource as an async resource. The custom resource providers are now bundled as nested stacks so they don't take up too many resources from users, and are also reused by multiple clusters within the same stack. This required that the creation role will not be the same as the lambda role, so we define this role separately and assume it within the providers.

The second issue is fixed by adding 3 retries to "kubectl apply".

**Backwards compatibility**: as described in #5544, since the resource provider handler of `Cluster` and `KubernetesResource` has been changed, this change requires a replacement of existing clusters (deployment fails with "service token cannot be changed" error). Since this can be disruptive to users, this change includes an exact copy of the previous version under a new module called `@aws-cdk/aws-eks-legacy`, which can be used as a drop-in replacement until users decide to upgrade to the new version. Using the legacy cluster will emit a synthesis warning that this module will no longer be released as part of the CDK starting March 1st, 2020.

- Fixes #4087
- Fixes #4695
- Fixes #5259
- Fixes #5501

---

BREAKING CHANGE: (in experimental module) the providers behind the AWS EKS module have been rewritten to address multiple stability issues. Since this change requires cluster replacement, the old version of this module is available under `@aws-cdk/aws-eks-legacy`. Please read #5544 carefully for upgrade instructions.
@eladb eladb force-pushed the benisrae/fix-eks-timeouts branch from 71faeed to 6a3946d Compare December 30, 2019 14:59
@eladb eladb removed the pr/do-not-merge This PR should not be merged at this time. label Dec 30, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Dec 30, 2019

Thank you for contributing! Your pull request is now being automatically merged.

@mergify
Copy link
Contributor

mergify bot commented Dec 30, 2019

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit a13cfe6 into master Dec 30, 2019
@mergify mergify bot deleted the benisrae/fix-eks-timeouts branch December 30, 2019 15:53
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@stefanolczak stefanolczak mentioned this pull request Feb 14, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RomainMuller RomainMuller RomainMuller approved these changes

@nija-at nija-at Awaiting requested review from nija-at

@rix0rrr rix0rrr Awaiting requested review from rix0rrr

@skinny85 skinny85 Awaiting requested review from skinny85

@SoManyHs SoManyHs Awaiting requested review from SoManyHs

Assignees
No one assigned
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Milestone
No milestone
3 participants
@eladb @aws-cdk-automation @RomainMuller