core: fix provider/close provider race when targeting

When targeting prunes out all the resource nodes between a provider and
its close node, there was no dependency to ensure the close happened
after the configure. Needed to add an explicit dependency from the close
to the provider.

This tweak highlighted the fact that CloseProviderTransformer needed to
happen after DisableProviderTransformer, since
DisableProviderTransformer inspects up-edges to decide what to disable,
and CloseProviderTransformer adds an up-edge.

fixes #2495
This commit is contained in:
Paul Hinze 2015-06-26 14:21:03 -05:00
parent 2626c179a5
commit 2d6a8c1f39
3 changed files with 74 additions and 79 deletions

View File

@ -116,8 +116,8 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer {
// Provider-related transformations
&MissingProviderTransformer{Providers: b.Providers},
&ProviderTransformer{},
&CloseProviderTransformer{},
&DisableProviderTransformer{},
&CloseProviderTransformer{},
// Provisioner-related transformations
&MissingProvisionerTransformer{Provisioners: b.Provisioners},

View File

@ -113,28 +113,41 @@ func (t *ProviderTransformer) Transform(g *Graph) error {
type CloseProviderTransformer struct{}
func (t *CloseProviderTransformer) Transform(g *Graph) error {
m := closeProviderVertexMap(g)
pm := providerVertexMap(g)
cpm := closeProviderVertexMap(g)
var err error
for _, v := range g.Vertices() {
if pv, ok := v.(GraphNodeProviderConsumer); ok {
for _, p := range pv.ProvidedBy() {
source := m[p]
source := cpm[p]
if source == nil {
// Create a new graphNodeCloseProvider and add it to the graph
source = &graphNodeCloseProvider{ProviderNameValue: p}
g.Add(source)
// Close node needs to depend on provider
provider, ok := pm[p]
if !ok {
err = multierror.Append(err, fmt.Errorf(
"%s: provider %s couldn't be found",
dag.VertexName(v), p))
continue
}
g.Connect(dag.BasicEdge(source, provider))
// Make sure we also add the new graphNodeCloseProvider to the map
// so we don't create and add any duplicate graphNodeCloseProviders.
m[p] = source
cpm[p] = source
}
// Close node depends on all nodes provided by the provider
g.Connect(dag.BasicEdge(source, v))
}
}
}
return nil
return err
}
// MissingProviderTransformer is a GraphTransformer that adds nodes

View File

@ -62,6 +62,36 @@ func TestCloseProviderTransformer(t *testing.T) {
}
}
func TestCloseProviderTransformer_withTargets(t *testing.T) {
mod := testModule(t, "transform-provider-basic")
g := Graph{Path: RootModulePath}
transforms := []GraphTransformer{
&ConfigTransformer{Module: mod},
&ProviderTransformer{},
&CloseProviderTransformer{},
&TargetsTransformer{
Targets: []string{"something.else"},
},
}
for _, tr := range transforms {
if err := tr.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
actual := strings.TrimSpace(g.String())
expected := strings.TrimSpace(`
provider.aws
provider.aws (close)
provider.aws
`)
if actual != expected {
t.Fatalf("expected:%s\n\ngot:\n\n%s", expected, actual)
}
}
func TestMissingProviderTransformer(t *testing.T) {
mod := testModule(t, "transform-provider-missing")
@ -144,44 +174,17 @@ func TestDisableProviderTransformer(t *testing.T) {
mod := testModule(t, "transform-provider-disable")
g := Graph{Path: RootModulePath}
{
tf := &ConfigTransformer{Module: mod}
if err := tf.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
transforms := []GraphTransformer{
&ConfigTransformer{Module: mod},
&MissingProviderTransformer{Providers: []string{"aws"}},
&ProviderTransformer{},
&DisableProviderTransformer{},
&CloseProviderTransformer{},
&PruneProviderTransformer{},
}
{
transform := &MissingProviderTransformer{Providers: []string{"aws"}}
if err := transform.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
{
transform := &ProviderTransformer{}
if err := transform.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
{
transform := &CloseProviderTransformer{}
if err := transform.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
{
transform := &PruneProviderTransformer{}
if err := transform.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
{
transform := &DisableProviderTransformer{}
if err := transform.Transform(&g); err != nil {
for _, tr := range transforms {
if err := tr.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
@ -189,7 +192,7 @@ func TestDisableProviderTransformer(t *testing.T) {
actual := strings.TrimSpace(g.String())
expected := strings.TrimSpace(testTransformDisableProviderBasicStr)
if actual != expected {
t.Fatalf("bad:\n\n%s", actual)
t.Fatalf("expected:\n%s\n\ngot:\n%s\n", expected, actual)
}
}
@ -197,44 +200,17 @@ func TestDisableProviderTransformer_keep(t *testing.T) {
mod := testModule(t, "transform-provider-disable-keep")
g := Graph{Path: RootModulePath}
{
tf := &ConfigTransformer{Module: mod}
if err := tf.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
transforms := []GraphTransformer{
&ConfigTransformer{Module: mod},
&MissingProviderTransformer{Providers: []string{"aws"}},
&ProviderTransformer{},
&DisableProviderTransformer{},
&CloseProviderTransformer{},
&PruneProviderTransformer{},
}
{
transform := &MissingProviderTransformer{Providers: []string{"aws"}}
if err := transform.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
{
transform := &ProviderTransformer{}
if err := transform.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
{
transform := &CloseProviderTransformer{}
if err := transform.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
{
transform := &PruneProviderTransformer{}
if err := transform.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
{
transform := &DisableProviderTransformer{}
if err := transform.Transform(&g); err != nil {
for _, tr := range transforms {
if err := tr.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
@ -242,7 +218,7 @@ func TestDisableProviderTransformer_keep(t *testing.T) {
actual := strings.TrimSpace(g.String())
expected := strings.TrimSpace(testTransformDisableProviderKeepStr)
if actual != expected {
t.Fatalf("bad:\n\n%s", actual)
t.Fatalf("expected:\n%s\n\ngot:\n%s\n", expected, actual)
}
}
@ -271,6 +247,7 @@ aws_instance.web
provider.aws
provider.aws (close)
aws_instance.web
provider.aws
`
const testTransformMissingProviderBasicStr = `
@ -279,9 +256,11 @@ foo_instance.web
provider.aws
provider.aws (close)
aws_instance.web
provider.aws
provider.foo
provider.foo (close)
foo_instance.web
provider.foo
`
const testTransformPruneProviderBasicStr = `
@ -290,6 +269,7 @@ foo_instance.web
provider.foo
provider.foo (close)
foo_instance.web
provider.foo
`
const testTransformDisableProviderBasicStr = `
@ -298,6 +278,7 @@ module.child
var.foo
provider.aws (close)
module.child
provider.aws (disabled)
provider.aws (disabled)
var.foo
`
@ -312,5 +293,6 @@ provider.aws
provider.aws (close)
aws_instance.foo
module.child
provider.aws
var.foo
`