fix(nodejs): Do not fallback on cluster name when creating node group #492
Conversation
`createManagedNodeGroup` had a fallback on the cluster name for `nodeGroupName` which disabled the auto naming feature which led `deleteBeforeReplace` to be true. Fixes #491
|
PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. |
|
PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. |
| @@ -847,7 +847,6 @@ export function createManagedNodeGroup(name: string, args: ManagedNodeGroupOptio | |||
| const nodeGroup = new aws.eks.NodeGroup(name, { | |||
| ...nodeGroupArgs, | |||
| clusterName: args.clusterName || core.cluster.name, | |||
| nodeGroupName: args.nodeGroupName || name, | |||
There was a problem hiding this comment.
Ah, I thought in your original issue you posted, you were proposing something different, which was to not provide nodeGroupName if there's no arg passed in, but to keep it if there was. At least this way, this is less breaking for folks who are explicitly passing in the arg.
There was a problem hiding this comment.
The nodeGroupName will be taken if provided from the ...nodeGroupArgs spread. Passing the nodeGroupName argument will still work as before
There was a problem hiding this comment.
Ah, of course, sorry, yes. Thinking about this more, I guess one way to help folks who do rely on defaulting to the cluster name would be to indicate that they can still do this via a transformation. Let me ask one other person to take a look to make sure I'm not off in any of my thinking here.
Proposed changes
createManagedNodeGrouphad a fallback on the cluster name fornodeGroupNamewhich disabled the auto naming feature which leddeleteBeforeReplaceto be true.The fix is probably a breaking change as it will update the NodeGroup after an update.
Related issues
Fixes #491