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: validate graph w/ diff during plan phase #1816

Merged
merged 1 commit into from
May 6, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions terraform/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,12 @@ func (c *Context) Plan() (*Plan, error) {
}
p.Diff = c.diff

// Now that we have a diff, we can build the exact graph that Apply will use
// and catch any possible cycles during the Plan phase.
if _, err := c.Graph(&ContextGraphOpts{Validate: true}); err != nil {
return nil, err
}

return p, nil
}

Expand Down Expand Up @@ -406,12 +412,11 @@ func (c *Context) Validate() ([]string, []error) {
}
}

// Build a Verbose version of the graph so we can catch any potential cycles
// in the validate stage
graph, err := c.Graph(&ContextGraphOpts{
Validate: true,
Verbose: false,
})
// Build the graph so we can walk it and run Validate on nodes.
// We also validate the graph generated here, but this graph doesn't
// necessarily match the graph that Plan will generate, so we'll validate the
// graph again later after Planning.
graph, err := c.Graph(&ContextGraphOpts{Validate: true})
if err != nil {
return nil, []error{err}
}
Expand Down
9 changes: 6 additions & 3 deletions terraform/graph_config_node_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,10 +426,13 @@ func (n *graphNodeResourceDestroy) destroyIncludePrimary(
// to include this resource.
prefix := n.Original.Resource.Id()

// If we're present in the diff proper, then keep it.
// If we're present in the diff proper, then keep it. We're looking
// only for resources in the diff that match our resource or a count-index
// of our resource that are marked for destroy.
if d != nil {
for k, _ := range d.Resources {
if strings.HasPrefix(k, prefix) {
for k, d := range d.Resources {
match := k == prefix || strings.HasPrefix(k, prefix+".")
if match && d.Destroy {
return true
}
}
Expand Down
4 changes: 1 addition & 3 deletions terraform/transform_destroy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func TestPruneDestroyTransformer_diff(t *testing.T) {
actual := strings.TrimSpace(g.String())
expected := strings.TrimSpace(testTransformPruneDestroyBasicDiffStr)
if actual != expected {
t.Fatalf("bad:\n\n%s", actual)
t.Fatalf("expected:\n\n%s\n\nbad:\n\n%s", expected, actual)
}
}

Expand Down Expand Up @@ -421,9 +421,7 @@ aws_instance.foo

const testTransformPruneDestroyBasicDiffStr = `
aws_instance.bar
aws_instance.bar (destroy)
aws_instance.foo
aws_instance.bar (destroy)
aws_instance.foo
`

Expand Down