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(eks): Helm chart timeout expects duration #8773

Merged
merged 5 commits into from
Jun 29, 2020

Conversation

eduardomourar
Copy link
Contributor

This resolves a problem with timeout parameter being passed down to Helm. The Helm version used (v3) expects a duration (in our case, in seconds such as 600s) instead of integer.

fixes #8718


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

@eduardomourar
Copy link
Contributor Author

Is there a way to have a unit test for the python EKS custom resources?

@pahud
Copy link
Contributor

pahud commented Jun 29, 2020

Not sure if it's related to this issue but I kept hitting lambda timeout when I deploy the integ.eks-cluster.ts.

@eduardomourar did you hit this error?

圖片

@pahud
Copy link
Contributor

pahud commented Jun 29, 2020

Hi @eladb

Looks like we have to first get this PR sorted before I can successfully build my integ test and continue my PRs.

eladb
eladb previously requested changes Jun 29, 2020
cluster.addChart('dashboard', { chart: 'kubernetes-dashboard', repository: 'https://kubernetes-charts.storage.googleapis.com' });
cluster.addChart('nginx-ingress', { chart: 'nginx-ingress', repository: 'https://helm.nginx.com/stable', namespace: 'kube-system' });
cluster.addChart('nginx-ingress', {
chart: 'nginx-ingress',
Copy link
Contributor

Choose a reason for hiding this comment

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

Expecting to see an integ test update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, i am update the integ test to check the logic for the both wait and timeout

@mergify mergify bot dismissed eladb’s stale review June 29, 2020 06:32

Pull request has been modified.

@eladb eladb self-assigned this Jun 29, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: aadc727
  • 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 Jun 29, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit d1c2ef2 into aws:master Jun 29, 2020
@eduardomourar
Copy link
Contributor Author

Not sure if it's related to this issue but I kept hitting lambda timeout when I deploy the integ.eks-cluster.ts.

@eduardomourar did you hit this error?

No, I did not. But this looks like a error coming from the fact that we are actually running all those helm charts with wait set to true (because there is a bug even if you set to false).

@eduardomourar eduardomourar deleted the feature/helm-timeout-duration branch June 29, 2020 16:17
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.

[aws-eks]: CDK uses integer for timeout; Helm expects duration
4 participants