Merge pull request #10455 from hashicorp/b-non-cbd-promote
terraform: when promoting non-CBD to CBD, mark the config as such
This commit is contained in:
commit
08a56304bb
|
@ -881,6 +881,73 @@ func TestContext2Apply_createBeforeDestroyUpdate(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// This tests that when a CBD resource depends on a non-CBD resource,
|
||||
// we can still properly apply changes that require new for both.
|
||||
func TestContext2Apply_createBeforeDestroy_dependsNonCBD(t *testing.T) {
|
||||
m := testModule(t, "apply-cbd-depends-non-cbd")
|
||||
p := testProvider("aws")
|
||||
p.ApplyFn = testApplyFn
|
||||
p.DiffFn = testDiffFn
|
||||
state := &State{
|
||||
Modules: []*ModuleState{
|
||||
&ModuleState{
|
||||
Path: rootModulePath,
|
||||
Resources: map[string]*ResourceState{
|
||||
"aws_instance.bar": &ResourceState{
|
||||
Type: "aws_instance",
|
||||
Primary: &InstanceState{
|
||||
ID: "bar",
|
||||
Attributes: map[string]string{
|
||||
"require_new": "abc",
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
"aws_instance.foo": &ResourceState{
|
||||
Type: "aws_instance",
|
||||
Primary: &InstanceState{
|
||||
ID: "foo",
|
||||
Attributes: map[string]string{
|
||||
"require_new": "abc",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
ctx := testContext2(t, &ContextOpts{
|
||||
Module: m,
|
||||
Providers: map[string]ResourceProviderFactory{
|
||||
"aws": testProviderFuncFixed(p),
|
||||
},
|
||||
State: state,
|
||||
})
|
||||
|
||||
if p, err := ctx.Plan(); err != nil {
|
||||
t.Fatalf("err: %s", err)
|
||||
} else {
|
||||
t.Logf(p.String())
|
||||
}
|
||||
|
||||
state, err := ctx.Apply()
|
||||
if err != nil {
|
||||
t.Fatalf("err: %s", err)
|
||||
}
|
||||
|
||||
checkStateString(t, state, `
|
||||
aws_instance.bar:
|
||||
ID = foo
|
||||
require_new = yes
|
||||
type = aws_instance
|
||||
value = foo
|
||||
aws_instance.foo:
|
||||
ID = foo
|
||||
require_new = yes
|
||||
type = aws_instance
|
||||
`)
|
||||
}
|
||||
|
||||
func TestContext2Apply_createBeforeDestroy_hook(t *testing.T) {
|
||||
h := new(MockHook)
|
||||
m := testModule(t, "apply-good-create-before")
|
||||
|
|
|
@ -30,6 +30,20 @@ func (n *NodeDestroyResource) CreateBeforeDestroy() bool {
|
|||
return n.Config.Lifecycle.CreateBeforeDestroy
|
||||
}
|
||||
|
||||
// GraphNodeDestroyerCBD
|
||||
func (n *NodeDestroyResource) ModifyCreateBeforeDestroy(v bool) error {
|
||||
// If we have no config, do nothing since it won't affect the
|
||||
// create step anyways.
|
||||
if n.Config == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Set CBD to true
|
||||
n.Config.Lifecycle.CreateBeforeDestroy = true
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// GraphNodeReferenceable, overriding NodeAbstractResource
|
||||
func (n *NodeDestroyResource) ReferenceableName() []string {
|
||||
result := n.NodeAbstractResource.ReferenceableName()
|
||||
|
|
|
@ -0,0 +1,12 @@
|
|||
resource "aws_instance" "foo" {
|
||||
require_new = "yes"
|
||||
}
|
||||
|
||||
resource "aws_instance" "bar" {
|
||||
require_new = "yes"
|
||||
value = "${aws_instance.foo.id}"
|
||||
|
||||
lifecycle {
|
||||
create_before_destroy = true
|
||||
}
|
||||
}
|
|
@ -1,6 +1,9 @@
|
|||
package terraform
|
||||
|
||||
import "github.com/hashicorp/terraform/dag"
|
||||
import (
|
||||
"github.com/hashicorp/terraform/config"
|
||||
"github.com/hashicorp/terraform/dag"
|
||||
)
|
||||
|
||||
// GraphNodeDestroyable is the interface that nodes that can be destroyed
|
||||
// must implement. This is used to automatically handle the creation of
|
||||
|
@ -151,8 +154,28 @@ func (t *CreateBeforeDestroyTransformer) Transform(g *Graph) error {
|
|||
}
|
||||
|
||||
// If the node doesn't need to create before destroy, then continue
|
||||
if !dn.CreateBeforeDestroy() && noCreateBeforeDestroyAncestors(g, dn) {
|
||||
continue
|
||||
if !dn.CreateBeforeDestroy() {
|
||||
if noCreateBeforeDestroyAncestors(g, dn) {
|
||||
continue
|
||||
}
|
||||
|
||||
// PURPOSELY HACKY FIX SINCE THIS TRANSFORM IS DEPRECATED.
|
||||
// This is a hacky way to fix GH-10439. For a detailed description
|
||||
// of the fix, see CBDEdgeTransformer, which is the equivalent
|
||||
// transform used by the new graphs.
|
||||
//
|
||||
// This transform is deprecated because it is only used by the
|
||||
// old graphs which are going to be removed.
|
||||
var update *config.Resource
|
||||
if dn, ok := v.(*graphNodeResourceDestroy); ok {
|
||||
update = dn.Original.Resource
|
||||
}
|
||||
if dn, ok := v.(*graphNodeResourceDestroyFlat); ok {
|
||||
update = dn.Original.Resource
|
||||
}
|
||||
if update != nil {
|
||||
update.Lifecycle.CreateBeforeDestroy = true
|
||||
}
|
||||
}
|
||||
|
||||
// Get the creation side of this node
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
package terraform
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"log"
|
||||
|
||||
"github.com/hashicorp/terraform/config/module"
|
||||
|
@ -15,6 +16,11 @@ type GraphNodeDestroyerCBD interface {
|
|||
// CreateBeforeDestroy returns true if this node represents a node
|
||||
// that is doing a CBD.
|
||||
CreateBeforeDestroy() bool
|
||||
|
||||
// ModifyCreateBeforeDestroy is called when the CBD state of a node
|
||||
// is changed dynamically. This can return an error if this isn't
|
||||
// allowed.
|
||||
ModifyCreateBeforeDestroy(bool) error
|
||||
}
|
||||
|
||||
// CBDEdgeTransformer modifies the edges of CBD nodes that went through
|
||||
|
@ -48,7 +54,23 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error {
|
|||
}
|
||||
|
||||
if !dn.CreateBeforeDestroy() {
|
||||
continue
|
||||
// If there are no CBD ancestors (dependent nodes), then we
|
||||
// do nothing here.
|
||||
if !t.hasCBDAncestor(g, v) {
|
||||
continue
|
||||
}
|
||||
|
||||
// If this isn't naturally a CBD node, this means that an ancestor is
|
||||
// and we need to auto-upgrade this node to CBD. We do this because
|
||||
// a CBD node depending on non-CBD will result in cycles. To avoid this,
|
||||
// we always attempt to upgrade it.
|
||||
if err := dn.ModifyCreateBeforeDestroy(true); err != nil {
|
||||
return fmt.Errorf(
|
||||
"%s: must have create before destroy enabled because "+
|
||||
"a dependent resource has CBD enabled. However, when "+
|
||||
"attempting to automatically do this, an error occurred: %s",
|
||||
dag.VertexName(v), err)
|
||||
}
|
||||
}
|
||||
|
||||
// Find the destroy edge. There should only be one.
|
||||
|
@ -189,3 +211,26 @@ func (t *CBDEdgeTransformer) depMap(
|
|||
|
||||
return depMap, nil
|
||||
}
|
||||
|
||||
// hasCBDAncestor returns true if any ancestor (node that depends on this)
|
||||
// has CBD set.
|
||||
func (t *CBDEdgeTransformer) hasCBDAncestor(g *Graph, v dag.Vertex) bool {
|
||||
s, _ := g.Ancestors(v)
|
||||
if s == nil {
|
||||
return true
|
||||
}
|
||||
|
||||
for _, v := range s.List() {
|
||||
dn, ok := v.(GraphNodeDestroyerCBD)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
|
||||
if dn.CreateBeforeDestroy() {
|
||||
// some ancestor is CreateBeforeDestroy, so we need to follow suit
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
|
|
@ -36,6 +36,38 @@ func TestCBDEdgeTransformer(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCBDEdgeTransformer_depNonCBD(t *testing.T) {
|
||||
g := Graph{Path: RootModulePath}
|
||||
g.Add(&graphNodeCreatorTest{AddrString: "test.A"})
|
||||
g.Add(&graphNodeCreatorTest{AddrString: "test.B"})
|
||||
g.Add(&graphNodeDestroyerTest{AddrString: "test.A"})
|
||||
g.Add(&graphNodeDestroyerTest{AddrString: "test.B", CBD: true})
|
||||
|
||||
module := testModule(t, "transform-destroy-edge-basic")
|
||||
|
||||
{
|
||||
tf := &DestroyEdgeTransformer{
|
||||
Module: module,
|
||||
}
|
||||
if err := tf.Transform(&g); err != nil {
|
||||
t.Fatalf("err: %s", err)
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
tf := &CBDEdgeTransformer{Module: module}
|
||||
if err := tf.Transform(&g); err != nil {
|
||||
t.Fatalf("err: %s", err)
|
||||
}
|
||||
}
|
||||
|
||||
actual := strings.TrimSpace(g.String())
|
||||
expected := strings.TrimSpace(testTransformCBDEdgeDepNonCBDStr)
|
||||
if actual != expected {
|
||||
t.Fatalf("bad:\n\n%s", actual)
|
||||
}
|
||||
}
|
||||
|
||||
const testTransformCBDEdgeBasicStr = `
|
||||
test.A
|
||||
test.A (destroy)
|
||||
|
@ -43,3 +75,14 @@ test.A (destroy)
|
|||
test.B
|
||||
test.B
|
||||
`
|
||||
|
||||
const testTransformCBDEdgeDepNonCBDStr = `
|
||||
test.A
|
||||
test.A (destroy) (modified)
|
||||
test.A
|
||||
test.B
|
||||
test.B (destroy)
|
||||
test.B
|
||||
test.B (destroy)
|
||||
test.B
|
||||
`
|
||||
|
|
|
@ -113,10 +113,25 @@ func (n *graphNodeCreatorTest) CreateAddr() *ResourceAddress {
|
|||
type graphNodeDestroyerTest struct {
|
||||
AddrString string
|
||||
CBD bool
|
||||
Modified bool
|
||||
}
|
||||
|
||||
func (n *graphNodeDestroyerTest) Name() string {
|
||||
result := n.DestroyAddr().String() + " (destroy)"
|
||||
if n.Modified {
|
||||
result += " (modified)"
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
func (n *graphNodeDestroyerTest) Name() string { return n.DestroyAddr().String() + " (destroy)" }
|
||||
func (n *graphNodeDestroyerTest) CreateBeforeDestroy() bool { return n.CBD }
|
||||
|
||||
func (n *graphNodeDestroyerTest) ModifyCreateBeforeDestroy(v bool) error {
|
||||
n.Modified = true
|
||||
return nil
|
||||
}
|
||||
|
||||
func (n *graphNodeDestroyerTest) DestroyAddr() *ResourceAddress {
|
||||
addr, err := ParseResourceAddress(n.AddrString)
|
||||
if err != nil {
|
||||
|
|
Loading…
Reference in New Issue