hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io
Other
42.76k stars 9.56k forks source link

What is the dependency cycle in this? #1475

Closed EvanKrall closed 9 years ago

EvanKrall commented 9 years ago

I have a very simple module (lives at module/main.tf):

variable "in" {}
output "out" {
    value = "${var.in}"
}

And a very simple config that includes the module above (main.tf):

provider "null" {}

module "a" {
    source = "./module"
    in = "${null_resource.b.id}"
}

resource "null_resource" "b" {}

resource "null_resource" "c" {
    some_input = "${module.a.out}" # no, this part won't work. but we fail before we get to that point.
    depends_on = [
        "null_resource.b",
    ]
}

I would expect this to produce a dependency graph where:

which has no cycles.

Instead, I get this error:

There are warnings and/or errors related to your configuration. Please
fix these before continuing.

Errors:

  * 1 error(s) occurred:

* Cycle: null_resource.c (destroy tainted), null_resource.b (destroy tainted), null_resource.b, module.a (expanded)

Since terraform graph refuses to produce a graph when there's a cycle (which is very frustrating), I can't really figure out what's going on.

Where is my cycle?

mitchellh commented 9 years ago

Sorry for the frustrating experience. We'll look into this.

Terraform right now is fairly conservative with graph cycles. It is better to incorrectly error when there is no cycle then to execute and fail to work with a real cycle. We'll continue to iron out any false cycles over time.

I've tagged this issue and will take a look for the next release.

mitchellh commented 9 years ago

Fixed in master!

EvanKrall commented 9 years ago

Sweet, I'll try it out

ketzacoatl commented 9 years ago

@mitchellh, is it possible for me to determine if this fix will work for a cycle / destroy / tainted error I am getting? My code is a bit more to analyze, but I can post it if that helps. I can also test master, but that will take me time to do (first attempts have been failing me).

mitchellh commented 9 years ago

@ketzacoatl Testing master is the easiest way, what issues have you ran into?

ketzacoatl commented 9 years ago

@mitchellh, I am unable to get past running make updatedeps:

~/go/src/github.com/hashicorp/terraform (master ✔) ᐅ make updatedeps                                         
go get -u github.com/mitchellh/gox
go get -u golang.org/x/tools/cmd/stringer
package golang.org/x/tools/cmd/stringer
    imports go/format: unrecognized import path "go/format"
make: *** [updatedeps] Error 1

I'm on debian, and this is my first attempt building a project in go, so I expect the problem is with me, but the README and http://golang.org/doc/code.html#GOPATH make it seem pretty simple.

mitchellh commented 9 years ago

Hm, I've never seen that, what version of Go? You need 1.4+. Otherwise, I'm unsure.

ketzacoatl commented 9 years ago

bingo, I will get the latest.

ketzacoatl commented 9 years ago

@mitchellh, I got further, but make on master fails:

...
ok      github.com/hashicorp/terraform/terraform    1.079s
make[1]: Entering directory `~/go/gopath/src/github.com/hashicorp/terraform'
go tool vet -asmdecl -atomic -bool -buildtags -copylocks -methods -nilfunc -printf -rangeloops -shift -structtags -unsafeptr .
builtin/providers/aws/resource_aws_s3_bucket.go:93: missing argument for Errorf("%#v"): format reads arg 2, have only 1 args

Vet found suspicious constructs. Please check the reported constructs
and fix them if necessary before submitting the code for reviewal.
make[1]: Leaving directory `~/go/gopath/src/github.com/hashicorp/terraform'

I attempted to drop back to the revision from this issue, and build from there (with make), but that errored out in ways that made me question that path. Thoughts on how best to proceed?

mitchellh commented 9 years ago

That is a harmless error. I'll fix it here shortly but you should be able to compile otherwise: make dev

ketzacoatl commented 9 years ago

Wonderful.. I will build, test, and report back shortly

ketzacoatl commented 9 years ago

Did the behavior of split() change since 0.4.2?

Terraform I built from make dev now complains about: cidr_blocks = "${split(",", var.cidr_blocks)}"

with: * aws_security_group.main: ingress.0.cidr_blocks: should be a list

I imagine the rabbit hole will circle back around to where I started with the dependency cycle error..

mitchellh commented 9 years ago

Put the whole thing in []: ["${split...}"]

ketzacoatl commented 9 years ago

Yes, that's nearly obvious.. sorry.

Ok, so back to the dependency cycle error.. I believe using the latest git has addressed that, though I have a couple of small issues to address with my plan before I'll know for sure. @mitchellh thank you for your help this evening!

ketzacoatl commented 9 years ago

Bah, I spoke too soon.

With v0.4.2, I would get the cycle error with terraform plan. Reviewing the plan will now succeed, with terraform built from master:

/src/terraform-consul-aws-asg/test  ᐅ   tfp           
Refreshing Terraform state prior to plan...
...

+ module.cleader-inbound-sg
    1 resource(s)
+ module.cleaders
    9 resource(s)
+ module.cleaders.cleader-sg
    1 resource(s)
+ module.cminion-inbound-sg
    1 resource(s)
+ module.cminions-a
    7 resource(s)
+ module.cminions-a.cminion-sg
    1 resource(s)

...but errors out when applying the plan:

/src/terraform-consul-aws-asg/test  ᐅ   tfa           
Error applying plan:

1 error(s) occurred:

* Cycle: aws_subnet.c (destroy), aws_subnet.c, module.cleader-sg (expanded), aws_elb.main (destroy), aws_subnet.a (destroy), aws_subnet.a

@mitchellh, Is there a way to inspect the cycle or details of the problem? Would it be easier to review the code behind this error?

gposton commented 9 years ago

I think I may be having a similar problem as @ketzacoatl, here's my error (note this is on the first attempt at terraform apply... if that matters)

Error configuring: 1 error(s) occurred:

* Cycle: aws_instance.nat-backup (destroy tainted), aws_route_table_association.a-private (destroy tainted), aws_route_table.r-private (destroy tainted), aws_instance.nat-primary (destroy tainted), terraform_state.vpc (destroy tainted), terraform_state.vpc, provider.aws, aws_security_group.nat (destroy tainted)
ketzacoatl commented 9 years ago

@mitchellh, I too can confirm the detail @gposton added: I'm applying these resources for the first time, there is no existing state..

Any thoughts on how to proceed?

Apologies if this should be a new/separate issue..?

phinze commented 9 years ago

@ketzacoatl and @gposton if you could each file separate issues with config snippets that would be helpful. I think it's easiest for us to treat each cycle issue as separate until we determine possible relationships. That should allow us to easily work with you 1:1 to determine the issue.

Off the bat - there are some legitimate cycles that can be introduced into the graph by using create_before_destroy. So when you're in the weeds, the first thing to start playing with is commenting out create_before_destroy blocks.

ketzacoatl commented 9 years ago

@phinze, thanks for the guidance here. I'll create a new issue with relevant information.

gposton commented 9 years ago

@phinze, removing the 'create_before_destroy' resolved the cycle. I created a separate issue here: https://github.com/hashicorp/terraform/issues/1636

Thanks!

ketzacoatl commented 9 years ago

FTR, I am not using create_before_destroy anywhere.

wazoo commented 9 years ago

I am also having this issue with a similar configuration, tested with:

Terraform v0.5.0-dev (3d367794e93ce56fb7f6f29f2e5035ea62c6e0d3)

phinze commented 9 years ago

@wazoo are there modules involved in your cycle? If so, have a look at #1637 which should be relevant

wazoo commented 9 years ago

@phinze Yeah I think that is it, I have cyclical security group dependencies too. I ran into this error and it was the root cause on #1326 (dupe of #539) but that had a different error since I wasn't using modules at that point I bet.

ketzacoatl commented 9 years ago

@wazoo, I removed all modules within modules, so I only have a top-level plan that calls other modules, and all of those modules define AWS/etc resources directly, and I have been able to proceed with TF as it is right now.

wazoo commented 9 years ago

Mine was due to cyclic security group dependency I think.

wazoo commented 9 years ago

I just ran into this again, I have this (basically):

module "child_module0" {
source = "./child_module0"
subnet = "1.1.1.1/0"
}
resource "aws_security_group" "nodes" {
...
...
}
output  "sec_nodes" {
value = "${aws_security_group.nodes}"
}
module "child_module1" {
source = "./child_module1"
subnet = "1.1.1.1/0"
allowed_sgrs = "${module.child_module0.sec_nodes}"
}

resource "aws_security_group" "nodes" {
...
security_groups = ["${split(",", var.allowed_sgrs)}"]
...
}

The parent module has two child module definitions (child_module0 and child_module1), the outputs of one child module are used in the second child module definition (security groups). This results in this (made up example):

Errors:

  * module root: module child_module1: depends_on is not a valid parameter
  * 1 error(s) occurred:

* Cycle: module.child_module0 (expanded)

I can create a real set of example TF files if this doesn't really make sense, let me know. I submitted #1742 that would also help solve this by reducing when I need to pass around variables. It's also possible there is a better way to accomplish what I am doing, feel free to let me know.

mikkoc commented 9 years ago

We are running into the same cycle issue with Terraform from master. We are trying to use the create_before_destroy lifecycle for ASG Launch Configurations: commenting that works around the problem.

ketzacoatl commented 9 years ago

@wazoo, @mikkoc, what worked best for me, though slightly less than ideal, flatten the module graph a little. In my case, instead of having a module call another module, I ensure modules all have resources and no modules, and then I feed inputs/outputs to each of the modules that need it, at the root level. This has worked nicely with how TF is as of today, YMMV.

wazoo commented 9 years ago

@ketzacoatl I get graph cycles even with a flattened topology like you described, my actual situation is that instances are only allowed to receive ssh from a bastion SG and the bastion host can only connect to the security groups instances for all of the applications so TF doesn't know which to create first. What it really needs to know to do is create the groups first then modify them with the rules later, this would also be solved if #1620 became a reality I think.

ketzacoatl commented 9 years ago

@wazoo, ah, ok, a little more complicated. If it were me, I would change things around, possibly with duplication of SG if needed. As a fallback, if you really can't change that now, you could create the SG, then use a noop-type resource (I forget the name, you'll have to look up in TF docs) with a local provisioner to issue some shell locally.. then use that to call out to the AWS APIs to make the changes you want.. and use depends_on to string it together. Not sure if all of that will work, but that is what I would pursue if you are not able to refactor the logic so as to avoid the complication to begin with.

Another thought is to have TF run from within a network at AWS, where you can access those private instances, and completely avoid the need of sending TF provisioning through the bastion.

wazoo commented 9 years ago

Interesting, I hadn't thought about using a local provisioner like that. The bastion host is actually just for SSH, this setup I am not using in-guest provisioners, just using TF to do the AWS layout.

ghost commented 4 years ago

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.