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

core: non-CBD depending on CBD won't depend on destroy #11753

Merged
merged 3 commits into from
Feb 7, 2017
Merged

Conversation

mitchellh
Copy link
Contributor

Fixes #11349

There are actually two bugs fixed here. Both aren't obvious so I'll explain both in detail. Both bugs manifested themselves in #11349 although it was also possible to hit them in isolation as well.

1. CBD with Counts

When CBD is used with count > 1, the edge transform wasn't working correctly and would not create the proper dependencies. The CBDEdgeTransformer creates a non-CBD graph to determine what the normal dependencies would be, then uses that to invert the edges for CBD.

This non-CBD graph doesn't expand counts, so if you had type.A[0] in your normal graph, it'd be type.A in the graph used to determine the edges. For now, the solution is simply to ignore indexes when comparing the graphs.

In the future, this won't be sufficient as we want to make our edges exact (type.A[0] should only depend directly on type.B[0], not type.B[1]). For now, we don't do this for a number of reasons much larger than CBD.

2. Non-CBD Depending on CBD Creates a Cycle

When a non-CBD resource depended on a CBD resource and the non-CBD resource wasn't being destroyed, a cycle would be created. This was caused by non-CBD resources advertising their dependence on any "destroy" portion of their dependencies as well.

Let's use a concrete example of an ASG depending on an LC to explain this bug.

Case 1: No CBD on the ASG and LC

The change should be: ASG(D) => LC(D) => LC => ASG.

This works correctly already.

Case 2: CBD on BOTH ASG and LC

The change should be: LC => ASG => ASG(D) => LC(D)

This also works correctly already since CBDEdgeTransformer modifies the edges of any CBD resource.

Case 3: CBD on ONLY ASG

This is identical to case 2, since we force all dependencies to also inherit CBD status.

Case 4: CBD on ONLY LC

The change should be: LC => ASG (update) => LC(D)

This is the case where we did the wrong thing. The ASG still advertised it depended on LC(D) because it looked like case 1 above. Instead, due to LC being CBD, we already handled the edge manipulations.

To fix this, we now have destroy nodes that are due to CBD advertise themselves as different from non-CBD destroy nodes. Regular nodes still depend on non-CBD destroy nodes for their dependencies. This makes everything connect together properly.

@2rs2ts
Copy link
Contributor

2rs2ts commented Feb 13, 2017

Is this going to go out in 0.9.0 or can a 0.8.7 be released so our team does not have to jump two major versions? :)

@mitchellh
Copy link
Contributor Author

This will be in 0.8.7 I'm pretty sure.

@setevoy2
Copy link

@mitchellh Could you please say - when 0.8.7 release planned?
We use hashicorp/terraform:light image atm with 0.8.6 and faced with a "Cannot delete launch configuration because it is attached to AutoScalingGroup" error from here.
And we can't roll back to 0.8.5 (because of state-files). And we really in trouble now, as our CI is not functional now...

@mitchellh
Copy link
Contributor Author

mitchellh commented Feb 14, 2017 via email

@setevoy2
Copy link

setevoy2 commented Feb 16, 2017

Hi, @mitchellh.
Thanks for the 0.8.7 release.
Unfortunately - we still have a "Cannot delete launch configuration" error:

  • aws_launch_configuration.lc (deposed #0): ResourceInUse: Cannot delete launch configuration terraform-00b8b118b0d5aba362e2090a63 because it is attached to AutoScalingGroup staging-api-green-asg

Both LC and ASG have create_before_destroy = true:

$ grep -r "before_destroy" .
./launch_configuration/launch_configuration.tf:  lifecycle { create_before_destroy = true }
./autoscaling_group_blue/autoscaling_group_blue.tf:  lifecycle { create_before_destroy = true }
./autoscaling_group_green/autoscaling_group_green.tf:  lifecycle { create_before_destroy = true }

I also removed name from the LC tf-file:

resource "aws_launch_configuration" "lc" {
  lifecycle { create_before_destroy = true }
  image_id        = "${var.image_id}"
  instance_type   = "${var.instance_type}"
  security_groups = ["${split(",", var.security_groups)}"]
  key_name        = "${var.key_name}"
  user_data       = "${var.user_data}"
}

But this doesn't help.
Our release currently blocked. Maybe there are any other fixes?

Thanks.

@2rs2ts
Copy link
Contributor

2rs2ts commented Feb 16, 2017

I am not having the same problem as @setevoy2. Not sure what the difference is for us.

@ghost
Copy link

ghost commented Apr 16, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"ResourceInUse: Cannot delete launch configuration " occured. When changing launch configuration
4 participants