From 9570747aed9bf13252ff6f5ee00ab11ef19fe9dc Mon Sep 17 00:00:00 2001 From: kirintw Date: Tue, 4 May 2021 00:49:04 +0800 Subject: [PATCH] chore(eks): remove propertyOverride for nodegroup launchTemplate (#14499) Previously we use `addPropertyOverride()` when specifying launch template for a nodegroup: https://github.com/aws/aws-cdk/blob/e11d5378c33bea609ed09c998b305fdfd28999a9/packages/%40aws-cdk/aws-eks/lib/managed-nodegroup.ts#L359-L363 This is now available on new cfn spec that cdk uses. So we can safely migrate to cfn props instead of calling `addPropertyOverride()`. The unit test should still covered the changes: https://github.com/aws/aws-cdk/blob/e11d5378c33bea609ed09c998b305fdfd28999a9/packages/%40aws-cdk/aws-eks/test/test.nodegroup.ts#L565-L643 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-eks/lib/managed-nodegroup.ts | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/packages/@aws-cdk/aws-eks/lib/managed-nodegroup.ts b/packages/@aws-cdk/aws-eks/lib/managed-nodegroup.ts index 7d46fb00c6e92..859a94d9aee67 100644 --- a/packages/@aws-cdk/aws-eks/lib/managed-nodegroup.ts +++ b/packages/@aws-cdk/aws-eks/lib/managed-nodegroup.ts @@ -280,6 +280,12 @@ export class Nodegroup extends Resource implements INodegroup { throw new Error(`Minimum capacity ${this.minSize} can't be greater than desired size ${this.desiredSize}`); } + if (props.launchTemplateSpec && props.diskSize) { + // see - https://docs.aws.amazon.com/eks/latest/userguide/launch-templates.html + // and https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-eks-nodegroup.html#cfn-eks-nodegroup-disksize + throw new Error('diskSize must be specified within the launch template'); + } + if (props.instanceType && props.instanceTypes) { throw new Error('"instanceType is deprecated, please use "instanceTypes" only.'); } @@ -331,6 +337,7 @@ export class Nodegroup extends Resource implements INodegroup { // because this doesn't have a default value, meaning the user had to explicitly configure this. instanceTypes: instanceTypes?.map(t => t.toString()), labels: props.labels, + launchTemplate: props.launchTemplateSpec, releaseVersion: props.releaseVersion, remoteAccess: props.remoteAccess ? { ec2SshKey: props.remoteAccess.sshKeyName, @@ -345,25 +352,6 @@ export class Nodegroup extends Resource implements INodegroup { tags: props.tags, }); - if (props.launchTemplateSpec) { - if (props.diskSize) { - // see - https://docs.aws.amazon.com/eks/latest/userguide/launch-templates.html - // and https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-eks-nodegroup.html#cfn-eks-nodegroup-disksize - throw new Error('diskSize must be specified within the launch template'); - } - /** - * Instance types can be specified either in `instanceType` or launch template but not both. AS we can not check the content of - * the provided launch template and the `instanceType` property is preferrable. We allow users to define `instanceType` property here. - * see - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-eks-nodegroup.html#cfn-eks-nodegroup-instancetypes - */ - // TODO: update this when the L1 resource spec is updated. - resource.addPropertyOverride('LaunchTemplate', { - Id: props.launchTemplateSpec.id, - Version: props.launchTemplateSpec.version, - }); - } - - // managed nodegroups update the `aws-auth` on creation, but we still need to track // its state for consistency. if (this.cluster instanceof Cluster) {