Skip to content

Commit

Permalink
use describeUpdate instead of cluster status
Browse files Browse the repository at this point in the history
  • Loading branch information
Elad Ben-Israel committed Apr 23, 2020
1 parent aeeb384 commit 4f18dc3
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 104 deletions.
81 changes: 41 additions & 40 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { IsCompleteResponse, OnEventResponse } from '@aws-cdk/custom-resources/lib/provider-framework/types';
// eslint-disable-next-line import/no-extraneous-dependencies
import * as aws from 'aws-sdk';
import { EksClient, ResourceHandler } from './common';
import { EksClient, ResourceEvent, ResourceHandler } from './common';

const MAX_CLUSTER_NAME_LEN = 100;

Expand All @@ -19,7 +19,7 @@ export class ClusterResourceHandler extends ResourceHandler {
private readonly newProps: aws.EKS.CreateClusterRequest;
private readonly oldProps: Partial<aws.EKS.CreateClusterRequest>;

constructor(eks: EksClient, event: AWSLambda.CloudFormationCustomResourceEvent) {
constructor(eks: EksClient, event: ResourceEvent) {
super(eks, event);

this.newProps = parseProps(this.event.ResourceProperties);
Expand Down Expand Up @@ -127,15 +127,17 @@ export class ClusterResourceHandler extends ResourceHandler {
throw new Error(`Cannot remove cluster version configuration. Current version is ${this.oldProps.version}`);
}

await this.updateClusterVersion(this.newProps.version);
return await this.updateClusterVersion(this.newProps.version);
}

if (updates.updateLogging || updates.updateAccess) {
await this.eks.updateClusterConfig({
const updateResponse = await this.eks.updateClusterConfig({
name: this.clusterName,
logging: this.newProps.logging,
resourcesVpcConfig: this.newProps.resourcesVpcConfig,
});

return { EksUpdateId: updateResponse.update?.id };
}

// no updates
Expand All @@ -144,6 +146,12 @@ export class ClusterResourceHandler extends ResourceHandler {

protected async isUpdateComplete() {
console.log('isUpdateComplete');

// if this is an EKS update, we will monitor the update event itself
if (this.event.EksUpdateId) {
return this.isEksUpdateComplete(this.event.EksUpdateId);
}

return this.isActive();
}

Expand All @@ -158,37 +166,8 @@ export class ClusterResourceHandler extends ResourceHandler {
return;
}

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');
}
const updateResponse = await this.eks.updateClusterVersion({ name: this.clusterName, version: newVersion });
return { EksUpdateId: updateResponse.update?.id };
}

private async isActive(): Promise<IsCompleteResponse> {
Expand Down Expand Up @@ -217,6 +196,33 @@ export class ClusterResourceHandler extends ResourceHandler {
}
}

private async isEksUpdateComplete(eksUpdateId: string) {
this.log({ isEksUpdateComplete: eksUpdateId });

const describeUpdateResponse = await this.eks.describeUpdate({
name: this.clusterName,
updateId: eksUpdateId,
});

this.log({ describeUpdateResponse });

if (!describeUpdateResponse.update) {
throw new Error(`unable to describe update with id "${eksUpdateId}"`);
}

switch (describeUpdateResponse.update.status) {
case 'InProgress':
return { IsComplete: false };
case 'Successful':
return { IsComplete: true };
case 'Failed':
case 'Cancelled':
throw new Error(`cluster update id "${eksUpdateId}" failed with errors: ${JSON.stringify(describeUpdateResponse.update.errors)}`);
default:
throw new Error(`unknown status "${describeUpdateResponse.update.status}" for update id "${eksUpdateId}"`);
}
}

private generateClusterName() {
const suffix = this.requestId.replace(/-/g, ''); // 32 chars
const prefix = this.logicalResourceId.substr(0, MAX_CLUSTER_NAME_LEN - suffix.length - 1);
Expand Down Expand Up @@ -257,8 +263,3 @@ 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));
}
18 changes: 15 additions & 3 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,25 @@ import { IsCompleteResponse, OnEventResponse } from '@aws-cdk/custom-resources/l
// eslint-disable-next-line import/no-extraneous-dependencies
import * as aws from 'aws-sdk';

export interface EksUpdateId {
/**
* If this field is included in an event passed to "IsComplete", it means we
* initiated an EKS update that should be monitored using eks:DescribeUpdate
* instead of just looking at the cluster status.
*/
EksUpdateId?: string
}

export type ResourceEvent = AWSLambda.CloudFormationCustomResourceEvent & EksUpdateId;

export abstract class ResourceHandler {
protected readonly requestId: string;
protected readonly logicalResourceId: string;
protected readonly requestType: 'Create' | 'Update' | 'Delete';
protected readonly physicalResourceId?: string;
protected readonly event: AWSLambda.CloudFormationCustomResourceEvent;
protected readonly event: ResourceEvent;

constructor(protected readonly eks: EksClient, event: AWSLambda.CloudFormationCustomResourceEvent) {
constructor(protected readonly eks: EksClient, event: ResourceEvent) {
this.requestType = event.RequestType;
this.requestId = event.RequestId;
this.logicalResourceId = event.LogicalResourceId;
Expand Down Expand Up @@ -55,7 +66,7 @@ export abstract class ResourceHandler {

protected abstract async onCreate(): Promise<OnEventResponse>;
protected abstract async onDelete(): Promise<OnEventResponse | void>;
protected abstract async onUpdate(): Promise<OnEventResponse | void>;
protected abstract async onUpdate(): Promise<(OnEventResponse & EksUpdateId) | void>;
protected abstract async isCreateComplete(): Promise<IsCompleteResponse>;
protected abstract async isDeleteComplete(): Promise<IsCompleteResponse>;
protected abstract async isUpdateComplete(): Promise<IsCompleteResponse>;
Expand All @@ -68,6 +79,7 @@ export interface EksClient {
describeCluster(request: aws.EKS.DescribeClusterRequest): Promise<aws.EKS.DescribeClusterResponse>;
updateClusterConfig(request: aws.EKS.UpdateClusterConfigRequest): Promise<aws.EKS.UpdateClusterConfigResponse>;
updateClusterVersion(request: aws.EKS.UpdateClusterVersionRequest): Promise<aws.EKS.UpdateClusterVersionResponse>;
describeUpdate(req: aws.EKS.DescribeUpdateRequest): Promise<aws.EKS.DescribeUpdateResponse>;
createFargateProfile(request: aws.EKS.CreateFargateProfileRequest): Promise<aws.EKS.CreateFargateProfileResponse>;
describeFargateProfile(request: aws.EKS.DescribeFargateProfileRequest): Promise<aws.EKS.DescribeFargateProfileResponse>;
deleteFargateProfile(request: aws.EKS.DeleteFargateProfileRequest): Promise<aws.EKS.DeleteFargateProfileResponse>;
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const defaultEksClient: EksClient = {
createCluster: req => getEksClient().createCluster(req).promise(),
deleteCluster: req => getEksClient().deleteCluster(req).promise(),
describeCluster: req => getEksClient().describeCluster(req).promise(),
describeUpdate: req => getEksClient().describeUpdate(req).promise(),
updateClusterConfig: req => getEksClient().updateClusterConfig(req).promise(),
updateClusterVersion: req => getEksClient().updateClusterVersion(req).promise(),
createFargateProfile: req => getEksClient().createFargateProfile(req).promise(),
Expand Down
29 changes: 27 additions & 2 deletions packages/@aws-cdk/aws-eks/test/cluster-resource-handler-mocks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export let actualRequest: {
configureAssumeRoleRequest?: sdk.STS.AssumeRoleRequest;
createClusterRequest?: sdk.EKS.CreateClusterRequest;
describeClusterRequest?: sdk.EKS.DescribeClusterRequest;
describeUpdateRequest?: sdk.EKS.DescribeUpdateRequest;
deleteClusterRequest?: sdk.EKS.DeleteClusterRequest;
updateClusterConfigRequest?: sdk.EKS.UpdateClusterConfigRequest;
updateClusterVersionRequest?: sdk.EKS.UpdateClusterVersionRequest;
Expand All @@ -22,6 +23,8 @@ export let actualRequest: {
*/
export let simulateResponse: {
describeClusterResponseMockStatus?: string;
describeUpdateResponseMockStatus?: string;
describeUpdateResponseMockErrors?: sdk.EKS.ErrorDetails;
deleteClusterErrorCode?: string;
describeClusterExceptionCode?: string;
} = { };
Expand All @@ -31,6 +34,8 @@ export function reset() {
simulateResponse = { };
}

export const MOCK_UPDATE_STATUS_ID = 'MockEksUpdateStatusId';

export const client: EksClient = {

configureAssumeRole: req => {
Expand Down Expand Up @@ -87,14 +92,34 @@ export const client: EksClient = {
};
},

describeUpdate: async req => {
actualRequest.describeUpdateRequest = req;

return {
update: {
id: req.updateId,
errors: simulateResponse.describeUpdateResponseMockErrors,
status: simulateResponse.describeUpdateResponseMockStatus,
},
};
},

updateClusterConfig: async req => {
actualRequest.updateClusterConfigRequest = req;
return { };
return {
update: {
id: MOCK_UPDATE_STATUS_ID,
},
};
},

updateClusterVersion: async req => {
actualRequest.updateClusterVersionRequest = req;
return { };
return {
update: {
id: MOCK_UPDATE_STATUS_ID,
},
};
},

createFargateProfile: async req => {
Expand Down

0 comments on commit 4f18dc3

Please sign in to comment.