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

[aws-eks]: CDK uses integer for timeout; Helm expects duration #8718

Closed
arjanschaaf opened this issue Jun 24, 2020 · 5 comments · Fixed by #8773
Closed

[aws-eks]: CDK uses integer for timeout; Helm expects duration #8718

arjanschaaf opened this issue Jun 24, 2020 · 5 comments · Fixed by #8773
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. p1

Comments

@arjanschaaf
Copy link
Contributor

Wanted to test the new timeout parameter in the EKS Helm construct as I had problems deploying a large Helm chart like the prometheus-operator chart. But I just ran it and hit a problem: CDK translates the timeout to an integer value in seconds but the Helm version used (v3) expects a duration specification. So 900s instead of 900.

Reproduction Steps

        cluster.addChart('prometheus-operator', {
            chart: 'prometheus-operator',
            namespace: 'monitoring',
            release: 'prometheus-operator',
            repository: 'https://kubernetes-charts.storage.googleapis.com',
            timeout: Duration.minutes(15),
            wait: true

Error Log

[ERROR] Exception: b'Error: invalid argument "900" for "--timeout" flag: time: missing unit in duration 900\n' Traceback (most recent call last):   File "/var/task/index.py", line 16, in handler     return helm_handler(event, context)   File "/var/task/helm/__init__.py", line 49, in helm_handler     helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout)   File "/var/task/helm/__init__.py", line 90, in helm     raise Exception(output)

Environment

  • CLI Version :
  • Framework Version:
  • Node.js Version:
  • OS :
  • Language (Version):

Other

Not sure if this could be related to an upgrade @pahud is working on in aws-lambda-layer-kubectl?


This is 🐛 Bug Report

@arjanschaaf arjanschaaf added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 24, 2020
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Jun 24, 2020
@pahud
Copy link
Contributor

pahud commented Jun 24, 2020

cdk 1.46.0 has a pinned version of aws-lambda-layer-kubectl 2.0.0-beta2, which should be immutable.

layers: [ KubectlLayer.getOrCreate(this, { version: '2.0.0-beta2' }) ],

I haven't tried the helm timeout feature yet. Was it working and now it just failed?

@arjanschaaf
Copy link
Contributor Author

No @pahud It was the first time I used it as well. Then it is not related to your last change. As for as I can find in the Helm documentation the --timeout switch from Integer to Duration with the launch of Helm V3, so I'm a bit surprised I'm running into this issue.

@pahud
Copy link
Contributor

pahud commented Jun 25, 2020

I believe we just supported Helm --timeout in #8338 by @eduardomourar

Hi @eduardomourar , any idea on this issue?

@eduardomourar
Copy link
Contributor

I believe it was my mistake, so I will gladly fix it.

@eladb
Copy link
Contributor

eladb commented Jun 28, 2020

@eduardomourar thanks! let me know if you need assistance.

@eladb eladb added the p1 label Jun 28, 2020
@mergify mergify bot closed this as completed in #8773 Jun 29, 2020
mergify bot pushed a commit that referenced this issue Jun 29, 2020
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*
@iliapolo iliapolo added this to the EKS Dev Preview milestone Aug 10, 2020
@iliapolo iliapolo removed the needs-triage This issue or PR still needs to be triaged. label Aug 16, 2020
@iliapolo iliapolo changed the title (EKS): CDK uses integer for timeout; Helm expects duration [aws-eks]: CDK uses integer for timeout; Helm expects duration Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants