fix(eks): version update completes prematurely

The `UpdateClusterVersion` operation takes a while to begin and until then, the cluster's status is still `ACTIVE` instead `UPDATING` as expected. This causes the `isComplete` handler, which is called immediately, to think that the operation is complete, when it hasn't even began.

Add logic to the cluster version update `onEvent` method to wait up to 5 minutes until the cluster status is no longer `ACTIVE`, so that the subsequent `isComplete` query will be based on the version update operation itself.

Extended the timeout of `onEvent` to 15m to ensure it does not interrupt the operation.

TESTING: Updated unit tests to verify this retry behavior and performed a manual upgrade tests while examining the logs.

Fixes #7457

Author: Elad Ben-Israel

packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts

@@ -159,6 +159,36 @@ export class ClusterResourceHandler extends ResourceHandler {
     }
 
     await this.eks.updateClusterVersion({ name: this.clusterName, version: newVersion });
+
+    // it take a while for the cluster to start the version update, and until
+    // then the cluster's status is still "ACTIVE", which causes version
+    // upgrades to complete prematurely. so, we wait here until the cluster
+    // status changes status from "ACTIVE" and only then yield execution to the
+    // async waiter. technically the status is expected to be "UPDATING", but it
+    // is more robust to just make sure it's not "ACTIVE" before we carry on
+
+    // wait a total of 5 minutes for this to happen.
+    let remainingSec = 5 * 60;
+
+    while (remainingSec > 0) {
+      console.log(`waiting for cluster to transition from ACTIVE status (remaining time: ${remainingSec}s)`);
+      const resp = await this.eks.describeCluster({ name: this.clusterName });
+      console.log('describeCluster result:', JSON.stringify(resp, undefined, 2));
+
+      const status = resp.cluster?.status;
+      if (status !== 'ACTIVE') {
+        console.log(`cluster is now in ${status} state`);
+        break;
+      }
+
+      // wait 2sec before trying again
+      await sleep(2000);
+      remainingSec -= 2;
+    }
+
+    if (remainingSec === 0) {
+      throw new Error('version update failure: cluster did not transition from ACTIVE status after 5 minutes elapsed');
+    }
   }
 
   private async isActive(): Promise<IsCompleteResponse> {
@@ -227,3 +257,8 @@ function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps
     updateLogging: JSON.stringify(newProps.logging) !== JSON.stringify(oldProps.logging),
   };
 }
+
+async function sleep(ms: number) {
+  console.log(`waiting ${ms} milliseconds`);
+  return new Promise(ok => setTimeout(ok, ms));
+}

packages/@aws-cdk/aws-eks/lib/cluster-resource-provider.ts

@@ -41,7 +41,7 @@ export class ClusterResourceProvider extends NestedStack {
       description: 'onEvent handler for EKS cluster resource provider',
       runtime: HANDLER_RUNTIME,
       handler: 'index.onEvent',
-      timeout: Duration.minutes(1),
+      timeout: Duration.minutes(15),
     });
 
     const isComplete = new lambda.Function(this, 'IsCompleteHandler', {

packages/@aws-cdk/aws-eks/lib/cluster.ts

@@ -1078,6 +1078,6 @@ export enum MachineImageType {
 
 const GPU_INSTANCETYPES = ['p2', 'p3', 'g4'];
 
-export function nodeTypeForInstanceType(instanceType: ec2.InstanceType) {
+function nodeTypeForInstanceType(instanceType: ec2.InstanceType) {
   return GPU_INSTANCETYPES.includes(instanceType.toString().substring(0, 2)) ? NodeType.GPU : NodeType.STANDARD;
 }

packages/@aws-cdk/aws-eks/test/test.cluster-resource-provider.ts

@@ -361,13 +361,21 @@ export = {
           }, {
             version: undefined,
           }));
+
+          let count = 3;
+          const restore = mocks.client.describeCluster;
+          mocks.client.describeCluster = async () => ({
+            cluster: { status: --count === 0 ? 'UPDATING' : 'ACTIVE' },
+          });
           const resp = await handler.onEvent();
+          mocks.client.describeCluster = restore;
           test.equal(resp, undefined);
           test.deepEqual(mocks.actualRequest.updateClusterVersionRequest!, {
             name: 'physical-resource-id',
             version: '12.34',
           });
           test.equal(mocks.actualRequest.createClusterRequest, undefined);
+          test.equal(count, 0); // make sure "describeCluster" was called until it returned 'UPDATING'
           test.done();
         },
 
@@ -377,13 +385,21 @@ export = {
           }, {
             version: '1.1',
           }));
+
+          let count = 3;
+          const restore = mocks.client.describeCluster;
+          mocks.client.describeCluster = async () => ({
+            cluster: { status: --count === 0 ? 'UPDATING' : 'ACTIVE' },
+          });
           const resp = await handler.onEvent();
+          mocks.client.describeCluster = restore;
           test.equal(resp, undefined);
           test.deepEqual(mocks.actualRequest.updateClusterVersionRequest!, {
             name: 'physical-resource-id',
             version: '2.0',
           });
           test.equal(mocks.actualRequest.createClusterRequest, undefined);
+          test.equal(count, 0); // make sure "describeCluster" was called until it returned 'UPDATING'
           test.done();
         },