-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
terraform: inner-count dependencies work [GH-1540] #1587
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
resource "aws_instance" "foo" { | ||
count = 3 | ||
value = "${aws_instance.foo.0.value}" | ||
} | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
resource "aws_instance" "foo" { | ||
count = 2 | ||
|
||
provisioner "local-exec" { | ||
command = "echo ${aws_instance.foo.0.id}" | ||
other = "echo ${aws_instance.foo.id}" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -134,8 +134,34 @@ func (n *graphNodeExpandedResource) DependableName() []string { | |
|
||
// GraphNodeDependent impl. | ||
func (n *graphNodeExpandedResource) DependentOn() []string { | ||
config := &GraphNodeConfigResource{Resource: n.Resource} | ||
return config.DependentOn() | ||
configNode := &GraphNodeConfigResource{Resource: n.Resource} | ||
result := configNode.DependentOn() | ||
|
||
// Walk the variables to find any count-specific variables we depend on. | ||
configNode.VarWalk(func(v config.InterpolatedVariable) { | ||
rv, ok := v.(*config.ResourceVariable) | ||
if !ok { | ||
return | ||
} | ||
|
||
// We only want ourselves | ||
if rv.ResourceId() != n.Resource.Id() { | ||
return | ||
} | ||
|
||
// If this isn't a multi-access (which shouldn't be allowed but | ||
// is verified elsewhere), then we depend on the specific count | ||
// of this resource, ignoring ourself (which again should be | ||
// validated elsewhere). | ||
if rv.Index > -1 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be equivalently and more expressively written as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh if rv, ok := v.(*config.ResourceVariable); ok {
if rv.Multi && rv.Index >= 0 {
// Skip self references
if rv.ResourceId() == n.Resource.Id() && rv.Index == n.Index {
continue
}
deps = append(deps, fmt.Sprintf("%s.%d", rv.ResourceId(), rv.Index))
}
} Maybe tell me what concepts I had missing there, if any? Just for the learnings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually nothing really looks wrong there and it looks like we could use |
||
id := fmt.Sprintf("%s.%d", rv.ResourceId(), rv.Index) | ||
if id != n.stateId() && id != n.stateId()+".0" { | ||
result = append(result, id) | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is pretty sad, but ANY modification to any part of this code causes a test failure, so that's the silver lining. |
||
}) | ||
|
||
return result | ||
} | ||
|
||
// GraphNodeProviderConsumer | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,11 +37,33 @@ func TestResourceCountTransformer_countNegative(t *testing.T) { | |
} | ||
} | ||
|
||
func TestResourceCountTransformer_deps(t *testing.T) { | ||
cfg := testModule(t, "transform-resource-count-deps").Config() | ||
resource := cfg.Resources[0] | ||
|
||
g := Graph{Path: RootModulePath} | ||
{ | ||
tf := &ResourceCountTransformer{Resource: resource} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Related to the above comment about test cases, I'm not sure how this ever happened. |
||
if err := tf.Transform(&g); err != nil { | ||
t.Fatalf("err: %s", err) | ||
} | ||
} | ||
|
||
actual := strings.TrimSpace(g.String()) | ||
expected := strings.TrimSpace(testResourceCountTransformDepsStr) | ||
if actual != expected { | ||
t.Fatalf("bad:\n\n%s", actual) | ||
} | ||
} | ||
|
||
const testResourceCountTransformStr = ` | ||
aws_instance.foo #0 | ||
aws_instance.foo #2 | ||
aws_instance.foo #1 | ||
aws_instance.foo #2 | ||
aws_instance.foo #2 | ||
aws_instance.foo #2 | ||
` | ||
|
||
const testResourceCountTransformDepsStr = ` | ||
aws_instance.foo #0 | ||
aws_instance.foo #1 | ||
aws_instance.foo #0 | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly not sure what this was testing, also the resulting expectation was wrong.