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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with machineImages when multiple autoscalinggroups in the same scope #3076

Closed
1 task done
AlexCheema opened this issue Jun 26, 2019 · 6 comments 路 Fixed by #3183 or MechanicalRock/tech-radar#14 路 May be fixed by MechanicalRock/cdk-constructs#5, MechanicalRock/cdk-constructs#6 or MechanicalRock/cdk-constructs#7
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug.

Comments

@AlexCheema
Copy link
Contributor

  • I'm submitting a ...
    • 馃 bug report

I create 2 AutoScalingGroups in the same scope but there seems to be a conflict in the creation of the machine image id.

<path>\node_modules\@aws-cdk\core\lib\construct.js:420
            throw new Error(`There is already a Construct with name '${childName}' in ${typeName}${name.length > 0 ? ' [' + name + ']' : ''}`);
            ^

Error: There is already a Construct with name 'SsmParameterValue:/aws/service/ecs/optimized-ami/amazon-linux-2/recommended/image_id:C96584B6-F00A-464E-AD19-53AFF4B05118.Parameter' in MyStack [MyStack]

Environment:

  • CDK CLI Version: 0.36.0
  • Module Version: 0.36.0
  • OS: Win10
  • Language: Typescript
@eladb
Copy link
Contributor

eladb commented Jun 26, 2019

Thanks for reporting. I think the solution should be to reuse the same parameter, so it becomes a singleton essentially. Feels like we have many of those singletones... maybe we can find a way to make it easier to author them.

@AlexCheema
Copy link
Contributor Author

Thanks for reporting. I think the solution should be to reuse the same parameter, so it becomes a singleton essentially. Feels like we have many of those singletones... maybe we can find a way to make it easier to author them.

No problem - great to see cdk moving forward so quickly and happy to help in the little ways I can.
The issue is not actually the IMachineImage being created twice but it is this line: const imageConfig = props.machineImage.getImage(this);. Even if using a singleton for the IMachineImage instance, this line is executed each time an AutoScalingGroup is created so it does not help. This blocks you from using more than AutoScalingGroup in the same scope, unless you use different machine images (with different imageId).

@mhuebner
Copy link

mhuebner commented Jun 26, 2019

Same here:

Error: There is already a Construct with name 'SsmParameterValue:/aws/service/ecs/optimized-ami/amazon-linux-2/recommended/image_id:C96584B6-F00A-464E-AD19-53AFF4B05118.Parameter' in EcsStack [EcsStack]

This issue completely breaks my Stack so sadly I have to go back to 0.35.0. Any chance to workaround or get the bug fixed in the next release @eladb?

I can confirm the suggestion of @AlexCheema - static definition of the IMachineImage does not solve this issue.

BTW Great work. I love the aws CDK!

@NGL321 NGL321 added @aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. labels Jun 27, 2019
@mhuebner
Copy link

mhuebner commented Jul 1, 2019

Can someone point me to the root cause of this issue? I cannot find any problematic code changes when looking at #3009.

@mhuebner
Copy link

mhuebner commented Jul 3, 2019

@NGL321 any chance to get this fixed? Unfortunately it breaks our Stacks (huge ones) and currently there is no workaround to get this working with > 0.36

We create ECS-Clusters in a loop (each app and environment).

rix0rrr added a commit that referenced this issue Jul 3, 2019
Parameter names with '/' in them would not be correctly deduplicated,
since the '/' is also used to delimit identifiers in construct paths.

Add protection in core to not allow construct identifiers to be created
with '/' in them, as tryFindChild() would not be able to find it
anymore.

Fixes #3076.
eladb pushed a commit that referenced this issue Jul 3, 2019
Parameter names with '/' in them would not be correctly deduplicated,
since the '/' is also used to delimit identifiers in construct paths.

Change findChild() so it no longer traverses into the tree; if we assume it
will only ever find one child deep, we can do the same kind of escaping
there as we apply to construct IDs.

Fixes #3076.

BREAKING CHANGE: `construct.findChild()` now only looks up direct children
@mhuebner
Copy link

mhuebner commented Jul 3, 2019

One word: Awesome!
Thank you very very much, guys. This helps me a lot!

Kaixiang-AWS pushed a commit to Kaixiang-AWS/aws-cdk that referenced this issue Jul 3, 2019
Parameter names with '/' in them would not be correctly deduplicated,
since the '/' is also used to delimit identifiers in construct paths.

Change findChild() so it no longer traverses into the tree; if we assume it
will only ever find one child deep, we can do the same kind of escaping
there as we apply to construct IDs.

Fixes aws#3076.

BREAKING CHANGE: `construct.findChild()` now only looks up direct children
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment