fix(eks): Helm chart timeout expects duration (#8773)

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*
aws · Jun 29, 2020 · d1c2ef2 · d1c2ef2
1 parent 75e6d25
commit d1c2ef2
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 7 deletions.
diff --git a/packages/@aws-cdk/aws-eks/lib/helm-chart.ts b/packages/@aws-cdk/aws-eks/lib/helm-chart.ts
@@ -100,7 +100,7 @@ export class HelmChart extends Construct {
         Chart: props.chart,
         Version: props.version,
         Wait: props.wait ?? false,
-        Timeout: timeout,
+        Timeout: timeout ? `${timeout.toString()}s` : undefined, // Helm v3 expects duration instead of integer
         Values: (props.values ? stack.toJsonString(props.values) : undefined),
         Namespace: props.namespace ?? 'default',
         Repository: props.repository,

diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.expected.json b/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.expected.json
@@ -2481,7 +2481,8 @@
         },
         "Release": "awscdkeksclustertestclusterchartnginxingressa7f70129",
         "Chart": "nginx-ingress",
-        "Wait": false,
+        "Wait": true,
+        "Timeout": "900s",
         "Namespace": "kube-system",
         "Repository": "https://helm.nginx.com/stable"
       },

diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.ts b/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.ts
@@ -1,6 +1,6 @@
 import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
-import { App, CfnOutput } from '@aws-cdk/core';
+import { App, CfnOutput, Duration } from '@aws-cdk/core';
 import * as eks from '../lib';
 import * as hello from './hello-k8s';
 import { TestStack } from './util';
@@ -69,12 +69,18 @@ class EksClusterStack extends TestStack {
       nodeRole: cluster.defaultCapacity ? cluster.defaultCapacity.role : undefined,
     });
 
-    // // apply a kubernetes manifest
+    // apply a kubernetes manifest
     cluster.addResource('HelloApp', ...hello.resources);
 
-    // // add two Helm charts to the cluster. This will be the Kubernetes dashboard and the Nginx Ingress Controller
+    // add two Helm charts to the cluster. This will be the Kubernetes dashboard and the Nginx Ingress Controller
     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',
+      repository: 'https://helm.nginx.com/stable',
+      namespace: 'kube-system',
+      wait: true,
+      timeout: Duration.minutes(15),
+    });
 
     // add a service account connected to a IAM role
     cluster.addServiceAccount('MyServiceAccount');

diff --git a/packages/@aws-cdk/aws-eks/test/test.helm-chart.ts b/packages/@aws-cdk/aws-eks/test/test.helm-chart.ts
@@ -82,7 +82,7 @@ export = {
       new eks.HelmChart(stack, 'MyChart', { cluster, chart: 'chart', timeout: Duration.minutes(10) });
 
       // THEN
-      expect(stack).to(haveResource(eks.HelmChart.RESOURCE_TYPE, { Timeout: 600 }));
+      expect(stack).to(haveResource(eks.HelmChart.RESOURCE_TYPE, { Timeout: '600s' }));
       test.done();
     },
   },