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

What is the dependency cycle in this? #1475

Closed
EvanKrall opened this issue Apr 10, 2015 · 33 comments · Fixed by #1523
Closed

What is the dependency cycle in this? #1475

EvanKrall opened this issue Apr 10, 2015 · 33 comments · Fixed by #1523
Assignees

Comments

@EvanKrall
Copy link
Contributor

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:

  • C depends on A
  • C depends on B
  • A depends on B

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
Copy link
Contributor

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
Copy link
Contributor

Fixed in master!

@EvanKrall
Copy link
Contributor Author

Sweet, I'll try it out

catsby added a commit that referenced this issue Apr 15, 2015
* master: (511 commits)
  Update CHANGELOG.md
  core: avoid diff mismatch on NewRemoved fields during -/+
  Update CHANGELOG.md
  update CHANGELOG
  Fix minor error in index/count docs
  terraform: remove debug
  terraform: when pruning destroy, only match exact nodes, or exact counts
  up version for dev
  update CHANGELOG
  terraform: prune tainted destroys if no tainted in state [GH-1475]
  update CHANGELOG
  config/lang: support math on variables through implicits
  update CHANGELOG
  update cHANGELOG
  update cHANGELOG
  providers/aws: set id outside if/esle
  providers/aws: set ID after creation
  core: remove dead code from pre-deposed refactor
  website: update LC docs to note name is optional
  security_groups field expects a list of Security Group Group Names, not IDs
  ...
@ketzacoatl
Copy link
Contributor

@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
Copy link
Contributor

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

@ketzacoatl
Copy link
Contributor

@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
Copy link
Contributor

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

@ketzacoatl
Copy link
Contributor

bingo, I will get the latest.

@ketzacoatl
Copy link
Contributor

@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
Copy link
Contributor

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

@ketzacoatl
Copy link
Contributor

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

@ketzacoatl
Copy link
Contributor

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
Copy link
Contributor

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

@ketzacoatl
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

gposton commented Apr 22, 2015

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
Copy link
Contributor

@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
Copy link
Contributor

phinze commented Apr 22, 2015

@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
Copy link
Contributor

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

@gposton
Copy link
Contributor

gposton commented Apr 22, 2015

@phinze, removing the 'create_before_destroy' resolved the cycle. I created a separate issue here: #1636

Thanks!

@ketzacoatl
Copy link
Contributor

FTR, I am not using create_before_destroy anywhere.

@wazoo
Copy link

wazoo commented Apr 27, 2015

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

Terraform v0.5.0-dev (3d36779)

@phinze
Copy link
Contributor

phinze commented Apr 27, 2015

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

@wazoo
Copy link

wazoo commented Apr 27, 2015

@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
Copy link
Contributor

@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
Copy link

wazoo commented Apr 28, 2015

Mine was due to cyclic security group dependency I think.

@wazoo
Copy link

wazoo commented Apr 29, 2015

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
Copy link

mikkoc commented Apr 30, 2015

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.

mikkoc pushed a commit to mikkoc/tsuru-terraform-example that referenced this issue Apr 30, 2015
We were getting some cycle dependencies error when applying the config without existing .tfstate.
We double checked and we have no cycles at all.
For reference, see:
  * [What is the dependency cycle in this? #1475](hashicorp/terraform#1475)
  * [Dependency cycle errors with modules #1637](hashicorp/terraform#1637)
@ketzacoatl
Copy link
Contributor

@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
Copy link

wazoo commented Apr 30, 2015

@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
Copy link
Contributor

@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
Copy link

wazoo commented Apr 30, 2015

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.

mikkoc pushed a commit to alphagov/paas-alpha-tsuru-terraform that referenced this issue May 1, 2015
We were getting some cycle dependencies error when applying the config without existing .tfstate.
We double checked and we have no cycles at all.
For reference, see:
  * [What is the dependency cycle in this? #1475](hashicorp/terraform#1475)
  * [Dependency cycle errors with modules #1637](hashicorp/terraform#1637)
mikkoc pushed a commit to alphagov/paas-alpha-tsuru-terraform that referenced this issue May 1, 2015
We were getting some cycle dependencies error when applying the config without existing .tfstate.
We double checked and we have no cycles at all.
For reference, see:
  * [What is the dependency cycle in this? #1475](hashicorp/terraform#1475)
  * [Dependency cycle errors with modules #1637](hashicorp/terraform#1637)
@ghost
Copy link

ghost commented May 3, 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 May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants