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): version update completes prematurely #7526

Merged
merged 10 commits into from
Apr 23, 2020
46 changes: 41 additions & 5 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts
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,7 +166,8 @@ export class ClusterResourceHandler extends ResourceHandler {
return;
}

await this.eks.updateClusterVersion({ name: this.clusterName, version: newVersion });
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 @@ -187,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
18 changes: 15 additions & 3 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts
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
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
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export class ClusterResource extends Construct {
actions: [
'eks:CreateCluster',
'eks:DescribeCluster',
'eks:DescribeUpdate',
'eks:DeleteCluster',
'eks:UpdateClusterVersion',
'eks:UpdateClusterConfig',
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
29 changes: 27 additions & 2 deletions packages/@aws-cdk/aws-eks/test/cluster-resource-handler-mocks.ts
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
39 changes: 20 additions & 19 deletions packages/@aws-cdk/aws-eks/test/integ.eks-cluster.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,7 @@
"Action": [
"eks:CreateCluster",
"eks:DescribeCluster",
"eks:DescribeUpdate",
"eks:DeleteCluster",
"eks:UpdateClusterVersion",
"eks:UpdateClusterConfig",
Expand Down Expand Up @@ -2231,7 +2232,7 @@
},
"/",
{
"Ref": "AssetParameters5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69S3Bucket835D19A2"
"Ref": "AssetParametersfa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103S3Bucket4281E0A4"
},
"/",
{
Expand All @@ -2241,7 +2242,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69S3VersionKeyBFF2DA61"
"Ref": "AssetParametersfa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103S3VersionKey3B54BD32"
}
]
}
Expand All @@ -2254,7 +2255,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69S3VersionKeyBFF2DA61"
"Ref": "AssetParametersfa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103S3VersionKey3B54BD32"
}
]
}
Expand All @@ -2264,11 +2265,11 @@
]
},
"Parameters": {
"referencetoawscdkeksclustertestAssetParameters54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8S3Bucket57C4C68FRef": {
"Ref": "AssetParameters54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8S3Bucket7CCCFC30"
"referencetoawscdkeksclustertestAssetParametersc0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602S3Bucket38D74D5ERef": {
"Ref": "AssetParametersc0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602S3BucketD006BE3B"
},
"referencetoawscdkeksclustertestAssetParameters54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8S3VersionKeyBB973CE7Ref": {
"Ref": "AssetParameters54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8S3VersionKeyA2A28538"
"referencetoawscdkeksclustertestAssetParametersc0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602S3VersionKey189EBCBARef": {
"Ref": "AssetParametersc0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602S3VersionKeyEC71339F"
},
"referencetoawscdkeksclustertestAssetParameters5e49cf64d8027f48872790f80cdb76c5b836ecf9a70b71be1eb937a5c25a47c1S3BucketC7CBF350Ref": {
"Ref": "AssetParameters5e49cf64d8027f48872790f80cdb76c5b836ecf9a70b71be1eb937a5c25a47c1S3Bucket663A709C"
Expand Down Expand Up @@ -2413,17 +2414,17 @@
}
},
"Parameters": {
"AssetParameters54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8S3Bucket7CCCFC30": {
"AssetParametersc0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602S3BucketD006BE3B": {
"Type": "String",
"Description": "S3 bucket for asset \"54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8\""
"Description": "S3 bucket for asset \"c0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602\""
},
"AssetParameters54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8S3VersionKeyA2A28538": {
"AssetParametersc0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602S3VersionKeyEC71339F": {
"Type": "String",
"Description": "S3 key for asset version \"54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8\""
"Description": "S3 key for asset version \"c0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602\""
},
"AssetParameters54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8ArtifactHashC0D1FA2A": {
"AssetParametersc0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602ArtifactHash35F5D0CC": {
"Type": "String",
"Description": "Artifact hash for asset \"54c9eae68c19c65a224969094e8447eed31d811384c7e32bdb72fffb4be15ac8\""
"Description": "Artifact hash for asset \"c0e453b77d5ccf090915fba7c771380f8370da5cbcc3c7ed757c98addd75b602\""
},
"AssetParameters5e49cf64d8027f48872790f80cdb76c5b836ecf9a70b71be1eb937a5c25a47c1S3Bucket663A709C": {
"Type": "String",
Expand All @@ -2449,17 +2450,17 @@
"Type": "String",
"Description": "Artifact hash for asset \"a6d508eaaa0d3cddbb47a84123fc878809c8431c5466f360912f70b5b9770afb\""
},
"AssetParameters5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69S3Bucket835D19A2": {
"AssetParametersfa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103S3Bucket4281E0A4": {
"Type": "String",
"Description": "S3 bucket for asset \"5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69\""
"Description": "S3 bucket for asset \"fa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103\""
},
"AssetParameters5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69S3VersionKeyBFF2DA61": {
"AssetParametersfa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103S3VersionKey3B54BD32": {
"Type": "String",
"Description": "S3 key for asset version \"5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69\""
"Description": "S3 key for asset version \"fa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103\""
},
"AssetParameters5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69ArtifactHash0E7708FD": {
"AssetParametersfa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103ArtifactHash733CC5DF": {
"Type": "String",
"Description": "Artifact hash for asset \"5c7de45abd07f88cf62deefa6399553786f3559084ff34bae66042cdd1987d69\""
"Description": "Artifact hash for asset \"fa73027e9f72f21daca2d67aa5a23e88f87d90536a7e3c36de9adbfb27fa9103\""
},
"AssetParameters36525a61abfaf5764fad460fd03c24215fd00da60805807d6138c51be4d03dbcS3Bucket2D824DEF": {
"Type": "String",
Expand Down