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

Unexpected cycle error on destroy (no cycle present) #2359

Closed
payneio opened this Issue Jun 15, 2015 · 36 comments

Comments

Projects
None yet
@payneio
Copy link

payneio commented Jun 15, 2015

I have what appears to be a fairly simple infrastructure plan. terraform graph -draw-cycles results in:
image

As can be seen... there don't appear to be any cycles.

Yet, running terraform destroy results in:

* Cycle: template_file.follower-cloud-config (destroy), template_file.follower-cloud-config, aws_security_group.follower_services (destroy), aws_security_group.follower_services, aws_launch_configuration.follower, aws_autoscaling_group.follower, aws_launch_configuration.follower (destroy), template_file.control-cloud-config (destroy), template_file.control-cloud-config, aws_security_group.control_services (destroy), aws_security_group.control_services, aws_launch_configuration.control, aws_autoscaling_group.control, aws_launch_configuration.control (destroy), aws_security_group.cluster_services (destroy), aws_security_group.cluster_services

@payneio payneio changed the title Unexpected Unexpected cycle on destroy (no cycle present) Jun 16, 2015

@payneio payneio changed the title Unexpected cycle on destroy (no cycle present) Unexpected cycle error on destroy (no cycle present) Jun 16, 2015

@phinze

This comment has been minimized.

Copy link
Member

phinze commented Jun 16, 2015

The cycle includes "destroy" nodes - you should see it drawn when you add the -verbose flag to terraform graph.

The most common cause of cycles is when a resource that sets create_before_destroy = true depends on a resource that does not set it. Any dependencies of CBD resources must also be CBD.

Note also that you can get just the destroy plan by running terraform plan -destroy - this can help when debugging so you no you won't make any changes when you don't expect it.

Let me know what you find with these pointers and we'll get this figured out.

@phinze phinze added bug core labels Jun 16, 2015

@payneio

This comment has been minimized.

Copy link

payneio commented Jun 18, 2015

Aha... there's the cycle.
screen shot 2015-06-18 at 1 54 42 pm

I don't know why some of those dependencies are there, though. That seems to be a dependency created by terraform that doesn't need to be there. For example, the graph says that aws.security_group.follower_services depends on template_file.control-cloud-config(destroy), but the .tf doesn't seem to imply that dependency:

resource "aws_security_group" "follower_services" {
  name = "${var.stack_name}-follower_services"
  description = "STACK(${var.stack_name}): Opens ports for follower services"

  #mesos-slave
  ingress {
    from_port = 5051
    to_port = 5051
    protocol = "tcp"
    cidr_blocks = [
      "${var.aws_vpc_cidr}"
    ]
  }
}

AND:

resource "template_file" "control-cloud-config" {
  filename = "build/control-cloud-config.yaml"
  vars = {
    aws_instance_type = "${var.aws_instance_type}"
    etcd_discovery_url = "${var.etcd_discovery_url}"
    stack_name = "${var.stack_name}"
    aws_region = "${var.aws_region}"
    instance_aws_access_key = "${var.instance_aws_access_key}"
    instance_aws_secret_key = "${var.instance_aws_secret_key}"
  }
}
@d4v3y0rk

This comment has been minimized.

Copy link
Contributor

d4v3y0rk commented Jun 22, 2015

so how do you destroy resources that have the create_before_destroy = true?

@payneio

This comment has been minimized.

Copy link

payneio commented Jun 22, 2015

We don't have any with create_before_destroy.

@mitchellh

This comment has been minimized.

Copy link
Member

mitchellh commented Jun 25, 2015

Can you gist the TF_LOG=1 output when the error occurs? Please grep for secrets, it shouldn't have any but it is always safer to verify.

@mitchellh

This comment has been minimized.

Copy link
Member

mitchellh commented Jun 25, 2015

Also, if you could show us your config, that would also help immensely.

@banjiewen

This comment has been minimized.

Copy link

banjiewen commented Jun 25, 2015

@banjiewen

This comment has been minimized.

Copy link

banjiewen commented Jul 1, 2015

Any updates available here? Bumping for @phinze/@mitchellh.

@phinze phinze removed the waiting-response label Jul 1, 2015

@phinze

This comment has been minimized.

Copy link
Member

phinze commented Jul 1, 2015

Hi @banjiewen, thanks for the ping.

Looks like your config does have a create_before_destroy resource that depends on non-CBD resources.

Both of your aws_launch_configuration resources set it, and depend on template_file resources that do not. Can you try adding create_before_destroy = true to your template resources and see if that takes care of your cycle?

@banjiewen

This comment has been minimized.

Copy link

banjiewen commented Jul 2, 2015

Thanks, @phinze - applying create_before_destroy to our templates and security groups did the trick.

@rgabo

This comment has been minimized.

Copy link

rgabo commented Jul 4, 2015

I ran into the same issue and had to add create_before_destroy to most resources. Given that the only purpose would've been to work around the aws_launch_configuration vs aws_autoscaling_group limitation, I decided to remove the single create_before_destroy from the aws_launch_configuration.

This comment is to confirm that removing create_before_destroy from the aws_launch_configuration solves the issue. It's easy enough to taint or adjust the launch configuration manually that I'd rather avoid superfluous create_before_destroys.

@BrunoBonacci

This comment has been minimized.

Copy link

BrunoBonacci commented Sep 10, 2015

Hi,

After following the @phinze suggestion mentioned https://groups.google.com/d/msg/terraform-tool/7Gdhv1OAc80/JiFtQk-BMQAJ I do get the same cycle error on destroy.

Basically the only change done is to add the create_before_destroy in all the aws_launch_configuration and now I can't destroy the stack.

@phinze

This comment has been minimized.

Copy link
Member

phinze commented Sep 10, 2015

@BrunoBonacci any resource the launch config depends on (SGs, subnets, etc) must also have create_before_destroy set or there is a legitimate cycle. If you add those to your config, the cycle should go away.

@BrunoBonacci

This comment has been minimized.

Copy link

BrunoBonacci commented Sep 10, 2015

if I add a create_before_destroy on a resource will terraform try to create a new one before deleting the old one for every change?
does this apply to load balancers as well???,

@phinze

This comment has been minimized.

Copy link
Member

phinze commented Sep 10, 2015

Yep - for any change that requires replacement of the resource, create_before_destroy will cause creation to happen before destruction. The behavior for updates is not affected.

Note, to avoid the cycle, you only have to go up the dependency chain, not down. So LC -> SG -> Subnet all need create_before_destroy set to avoid a destroy cycle, but ASG -> ELB do not.

@BrunoBonacci

This comment has been minimized.

Copy link

BrunoBonacci commented Sep 10, 2015

ok, but what about subnet, how can a modification to a subnet work with the create_before_destroy???
If I change an attribute of the subnet it will try to create a new subnet with the same CIDR which won't work, isn't?

@phinze

This comment has been minimized.

Copy link
Member

phinze commented Sep 10, 2015

That's true - subnets do have that problem, though its surface area is pretty low.

The only two attributes besides cidr_block that require replacement on aws_subnet are vpc_id and availability_zone. If you were to change vpc_id, there's no CIDR collision since you're moving VPCs. If you needed to change availability_zone and keep the same cidr_block, then yes, you'd have to temporarily remove create_before_destroy to make it work.

The reason that CBD needs to be applied to all dependencies is because of the true cycles that are created in the resource graph if it is not. Soon we'll get some code in to recognize this scenario and output an explanation using the examples from the local config, which will hopefully help make the situation clearer.

@jasonkuhrt

This comment has been minimized.

Copy link

jasonkuhrt commented Sep 28, 2015

I ran into this issue today. So the TL;DR is that if create_before_destroy is used it "breaks" the destroy feature?

Is this a solvable bug or inherit problem that requires communication to the user?

@jimmycuadra

This comment has been minimized.

Copy link
Contributor

jimmycuadra commented Oct 2, 2015

I'm also experiencing this problem because of launch configurations and autoscaling groups with create before destroy set, as suggested by the documentation. Improved error messages could go a long way here, I think. The cycle that was reported was very confusing because 1) it happened only on destroy 2) it reported a giant list of resources that were part of the cycle that was just plain difficult to read.

After reading through this issue I still don't completely understand why create before destroy would cause a cycle when you're destroying resources (in my cases I was destroying all resources) since there is no creation/replacement involved at all.

@BrunoBonacci

This comment has been minimized.

Copy link

BrunoBonacci commented Oct 2, 2015

I agree on this one as well. All I wanted it was to be able to update the
AMI in a Launch Configuration
and I end up having to put create_before_destroy
everywhere. Once you put it on your subnets it is basically on your entire
infrastructure.

Once on your entire infrastructure it can be disastrous, everybody can make
a mistake, and this could literally tear your infrastructure down.
So I end up removing create_before_destroy from everywhere and go back to
update the LC manually for the time being.

I would like to encourage the terraform team to come up with a better
solution to the problem of updating ASG and LC.

Bruno

On Fri, Oct 2, 2015 at 2:25 PM, Jimmy Cuadra notifications@github.com
wrote:

I'm also experiencing this problem because of launch configurations and
autoscaling groups with create before destroy set, as suggested by the
documentation. Improved error messages could go a long way here, I think.
The cycle that was reported was very confusing because 1) it happened only
on destroy 2) it reported a giant list of resources that were part of the
cycle that was just plain difficult to read.

After reading through this issue I still don't completely understand why
create before destroy would cause a cycle when you're destroying
resources (in my cases I was destroying all resources) since there is
no creation/replacement involved at all.


Reply to this email directly or view it on GitHub
#2359 (comment)
.

@timbunce

This comment has been minimized.

Copy link

timbunce commented Oct 5, 2015

Copying this comment here from #2948 as that's closed.

for any resource with create_before_destroy that also depends on another resource, that [depended-on resource] must also have create_before_destroy

But why? Sure seems like a miss-feature to me, though I'm probably being dumb since I'm new to all this. I'd appreciate it if someone could explain why this is so.

I can see that it makes sense in cases where the depended-on resource can only be associated with a single other resource, in which case it would naturally need to be dis-associated (or destroyed/recreated) before being used by the new resource. But that's not the case for aws_security_group, for example.

I'm also unclear about what create_before_destroy has do do with terraform destroy.

I've recently update the Configuring Resources documentation to explain this

Perhaps I'm missing it but the linked-to docs don't explain why this is so.

Looking at some other issues that relate to this problem, e.g. #3238, I see it referred to as a "known issue that we plan to address in the near future". Is there an open issue for the core problem that the other issues could refer to?

@gposton

This comment has been minimized.

Copy link
Contributor

gposton commented Oct 6, 2015

For us, adding the CBD rule to all resources is not a viable option (same reasons as @BrunoBonacci)

I explain further in #3294

When the lifecycle rule is not on the ASG, I can change the AMI in the launch config w/o the ASG being destroyed (thus cycling my instances).
When the lifecycle rule is added to the ASG, both the ASG and the launch config is destroyed and re-created. This cycles my instances which happens too quickly for all of our services to initialize and pass health checks.

@rbowlby

This comment has been minimized.

Copy link

rbowlby commented Oct 20, 2015

The CBD dependency chain bit me as well.

In the short term better documentation regarding appropriate use of CBD (add to all upper deps) so as to avoid wasted time with destroy issues. Longer term, the inherit limitations of this design are starting to surface and a redesign would seem advantageous.

hectcastro added a commit to azavea/terraform-aws-vpc that referenced this issue Nov 14, 2015

Add create_before_destroy to subnet resources
The story behind this change is long and convoluted.

In order to complete the trick outlined below that cycles in instances
created by a new ASG before terminating the old:

https://groups.google.com/d/msg/terraform-tool/7Gdhv1OAc80/iNQ93riiLwAJ

The create_before_destroy lifecycle attribute has to be set to true on
all resources up the dependency chain of the ASG and its associated
launch configuration.

For an ASG resource, the vpc_zone_identifier hold references to VPC
subnet IDs, which means the associated subnet resources also need the
create_before_destroy attribute enabled.

For more detail, see: hashicorp/terraform#2359

TaylorMonacelli pushed a commit to TaylorMonacelli/terraform-aws-vpc that referenced this issue Nov 29, 2015

Add create_before_destroy to subnet resources
The story behind this change is long and convoluted.

In order to complete the trick outlined below that cycles in instances
created by a new ASG before terminating the old:

https://groups.google.com/d/msg/terraform-tool/7Gdhv1OAc80/iNQ93riiLwAJ

The create_before_destroy lifecycle attribute has to be set to true on
all resources up the dependency chain of the ASG and its associated
launch configuration.

For an ASG resource, the vpc_zone_identifier hold references to VPC
subnet IDs, which means the associated subnet resources also need the
create_before_destroy attribute enabled.

For more detail, see: hashicorp/terraform#2359

TaylorMonacelli pushed a commit to TaylorMonacelli/terraform-aws-vpc that referenced this issue Nov 29, 2015

@Seraf

This comment has been minimized.

Copy link

Seraf commented Dec 3, 2015

Hello,

I'm stuck with the same problem, it makes no sense to destroy the subnet which is common to other resources just for an autoscaling group. I subscribe to this issue to follow how it evolves.

@willejs

This comment has been minimized.

Copy link

willejs commented Dec 14, 2015

+1 this needs to be resolved in a reasonable way. Adding create before destroy to dependencies of the ASG is not really desirable, or feasible if your using community modules.
What is the plan for this issue?

@phinze

This comment has been minimized.

Copy link
Member

phinze commented Dec 16, 2015

Hey folks, this is still on my radar. This is definitely something we'd like to address with better messaging and there is also possibly some graph massaging we can do to reduce the chances of the scenario happening.

Reviewing the cycle workaround toolkit as it stands:

  • Add CBD to dependencies
  • If the cycle only occurs when a resource replacement is triggered (which is often the case, since replacements mean that "destroy nodes" are not pruned out of the graph):
    • tempoaraily add CBD to all dependencies to work around and remove afterwards
    • Attempt to use resource targeting to apply changes to a subset of resources
@t-pascal

This comment has been minimized.

Copy link

t-pascal commented Jan 27, 2016

I have also been hit with this bug. I believe this is not a case of documentation. It is a bug. Adding "create_before_destroy" on all dependent resources only makes sense if those resources also use or require "create_before_destroy", and only when we are not running a destroy. If I'm running terrraform destroy, how does "create" come into the picture at all?

CBD should be ignored during the destroy phase. It makes no sense, as @timbunce correctly described in October. Perhaps destroy triggers a "delete" plan and "delete" requires "create" if CBD is enabled. However, there should be an exception where destroy (as opposed to "delete") does not imply create. Because it doesn't.

@bigkraig

This comment has been minimized.

Copy link
Contributor

bigkraig commented Feb 11, 2016

It does seem odd to need to litter your templates with create_before_destroy. If create_before_destroy on aws_launch_configuration and aws_autoscaling_group is always required, why expose it to the user at all, terraform should insert it itself and it should insert it on any dependencies that need it. There are a lot of issues on github that come up about this and the cycle errors when you try to destroy.

@anosulchik

This comment has been minimized.

Copy link

anosulchik commented Feb 25, 2016

+1

@phinze

This comment has been minimized.

Copy link
Member

phinze commented Feb 25, 2016

Hey folks, #5096 made it out in Terraform v0.6.12, which should take care of this issue! 🎉

Please do follow up if you're seeing issues with cycles on terraform destroy on v0.6.12. 👍

@phinze phinze closed this Feb 25, 2016

@t-pascal

This comment has been minimized.

Copy link

t-pascal commented Feb 25, 2016

Great success! Tested it on two use cases with 0.6.12 and it works.

@ljsommer

This comment has been minimized.

Copy link

ljsommer commented Mar 2, 2016

This was a HUGE quality of life improvement- thank you so much! Tested on 0.6.12 via Homebrew.

thattommyhall added a commit to MastodonC/terraboot that referenced this issue Mar 14, 2016

@deepthawtz

This comment has been minimized.

Copy link

deepthawtz commented Mar 21, 2016

woohoo, thanks.. fixed things on our end too

@ghost

This comment has been minimized.

Copy link

ghost commented May 17, 2016

@phinze I have an aws_iam_server_certificate that is used in a module by an aws_elb. If I pass in a list of subnets and use it directly, then this works fine. If I pass in a list of subnets that I count over and create an elb per subnet - then I get a cycle and have destroy issues. The issue occurs (and everything else works fine) with or without cbd on the elbs, with or without cbd on the cert.

hectcastro added a commit to azavea/terraform-aws-vpc that referenced this issue Jul 18, 2016

hectcastro added a commit to azavea/terraform-aws-vpc that referenced this issue Jul 20, 2016

Remove create_before_destroy lifecycle resource
According to these issue threads, adding a create_before_destroy
lifecycle resource to all parent resources that touch a child resource
is no longer required.

See: hashicorp/terraform#2359
@number5

This comment has been minimized.

Copy link

number5 commented Sep 29, 2016

Still have this issue with terraform 0.7.4, terraform plan -destroy -out=killplan succeeded, but terraform apply killplan complaint about the cycle, we do have create_before_destroy in few places

Updates It works when I not using the plan but destroy it directly using terraform destroy

@ste-bah

This comment has been minimized.

Copy link

ste-bah commented Oct 27, 2016

I have to agree with some statements - why does changing an ASG require a subnet to be "destroyed" that dependency really shouldn't be there when using create before destroy as I can mimic the change I want by hacking together scripts using AWS-HA from the aws-missing tools repo.
Subnets should not need to be destroyed when you say make a change to an ASG which is typical if you specify the launch config name as part of the ASG name and have create before destroy specified. On infrastructure which has other systems deployed in said subnets it means it could be disastrous if everything has to be created before destroyed. I see this as a bug and incorrect behaviour. The new ASG +LC should be created with the existing ASG + LC in place after which the previous ASG + LC are destroyed. The subnet(s) should have no part in this cycle and certainly should not be destroyed & recreated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment