Skip to content

Commit

Permalink
chore(eks): remove propertyOverride for nodegroup launchTemplate (#14499
Browse files Browse the repository at this point in the history
)

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*
  • Loading branch information
kirintwn authored May 3, 2021
1 parent 12b6da0 commit 9570747
Showing 1 changed file with 7 additions and 19 deletions.
26 changes: 7 additions & 19 deletions packages/@aws-cdk/aws-eks/lib/managed-nodegroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down

0 comments on commit 9570747

Please sign in to comment.