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

Add CreateBeforeDestroy to instance state #24084

Merged
merged 5 commits into from
Mar 9, 2020
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
7 changes: 7 additions & 0 deletions states/instance_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ type ResourceInstanceObject struct {
// altogether, or is now deposed.
Dependencies []addrs.AbsResource

// CreateBeforeDestroy reflects the status of the lifecycle
// create_before_destroy option when this instance was last updated.
// Because create_before_destroy also effects the overall ordering of the
// destroy operations, we need to record the status to ensure a resource
// removed from the config will still be destroyed in the same manner.
CreateBeforeDestroy bool

// DependsOn corresponds to the deprecated `depends_on` field in the state.
// This field contained the configuration `depends_on` values, and some of
// the references from within a single module.
Expand Down
18 changes: 10 additions & 8 deletions states/instance_object_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ type ResourceInstanceObjectSrc struct {

// These fields all correspond to the fields of the same name on
// ResourceInstanceObject.
Private []byte
Status ObjectStatus
Dependencies []addrs.AbsResource
Private []byte
Status ObjectStatus
Dependencies []addrs.AbsResource
CreateBeforeDestroy bool
// deprecated
DependsOn []addrs.Referenceable
}
Expand Down Expand Up @@ -85,11 +86,12 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec
}

return &ResourceInstanceObject{
Value: val,
Status: os.Status,
Dependencies: os.Dependencies,
DependsOn: os.DependsOn,
Private: os.Private,
Value: val,
Status: os.Status,
Dependencies: os.Dependencies,
DependsOn: os.DependsOn,
Private: os.Private,
CreateBeforeDestroy: os.CreateBeforeDestroy,
}, nil
}

Expand Down
15 changes: 8 additions & 7 deletions states/state_deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,14 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc {
}

return &ResourceInstanceObjectSrc{
Status: obj.Status,
SchemaVersion: obj.SchemaVersion,
Private: private,
AttrsFlat: attrsFlat,
AttrsJSON: attrsJSON,
Dependencies: dependencies,
DependsOn: dependsOn,
Status: obj.Status,
SchemaVersion: obj.SchemaVersion,
Private: private,
AttrsFlat: attrsFlat,
AttrsJSON: attrsJSON,
Dependencies: dependencies,
DependsOn: dependsOn,
CreateBeforeDestroy: obj.CreateBeforeDestroy,
}
}

Expand Down
101 changes: 101 additions & 0 deletions terraform/graph_builder_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,107 @@ test_object.b
}
}

// Ensure that an update resulting from the removal of a resource happens before
// a CBD resource is destroyed.
func TestApplyGraphBuilder_updateFromCBDOrphan(t *testing.T) {
schemas := simpleTestSchemas()
instanceSchema := schemas.Providers[addrs.NewLegacyProvider("test")].ResourceTypes["test_object"]

bBefore, _ := plans.NewDynamicValue(
cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("b_id"),
"test_string": cty.StringVal("a_id"),
}), instanceSchema.ImpliedType())
bAfter, _ := plans.NewDynamicValue(
cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("b_id"),
"test_string": cty.StringVal("changed"),
}), instanceSchema.ImpliedType())

changes := &plans.Changes{
Resources: []*plans.ResourceInstanceChangeSrc{
{
Addr: mustResourceInstanceAddr("test_object.a"),
ChangeSrc: plans.ChangeSrc{
Action: plans.Delete,
},
},
{
Addr: mustResourceInstanceAddr("test_object.b"),
ChangeSrc: plans.ChangeSrc{
Action: plans.Update,
Before: bBefore,
After: bAfter,
},
},
},
}

state := states.NewState()
root := state.EnsureModule(addrs.RootModuleInstance)
root.SetResourceInstanceCurrent(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test_object",
Name: "a",
}.Instance(addrs.NoKey),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"a_id"}`),
CreateBeforeDestroy: true,
},
mustProviderConfig(`provider["registry.terraform.io/-/test"]`),
)
root.SetResourceInstanceCurrent(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test_object",
Name: "b",
}.Instance(addrs.NoKey),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"b_id","test_string":"a_id"}`),
Dependencies: []addrs.AbsResource{
addrs.AbsResource{
Resource: addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test_object",
Name: "a",
},
Module: root.Addr,
},
},
},
mustProviderConfig(`provider["registry.terraform.io/-/test"]`),
)

b := &ApplyGraphBuilder{
Config: testModule(t, "graph-builder-apply-orphan-update"),
Changes: changes,
Components: simpleMockComponentFactory(),
Schemas: schemas,
State: state,
}

g, err := b.Build(addrs.RootModuleInstance)
if err != nil {
t.Fatalf("err: %s", err)
}

expected := strings.TrimSpace(`
test_object.a (destroy)
test_object.b
test_object.b
`)

instanceGraph := filterInstances(g)
got := strings.TrimSpace(instanceGraph.String())

if got != expected {
t.Fatalf("expected:\n%s\ngot:\n%s", expected, got)
}
}

// The orphan clean up node should not be connected to a provider
func TestApplyGraphBuilder_orphanedWithProvider(t *testing.T) {
changes := &plans.Changes{
Expand Down
17 changes: 13 additions & 4 deletions terraform/node_resource_destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,21 @@ func (n *NodeDestroyResourceInstance) CreateBeforeDestroy() bool {
return *n.CreateBeforeDestroyOverride
}

// If we have no config, we just assume no
if n.Config == nil || n.Config.Managed == nil {
return false
// Config takes precedence
if n.Config != nil && n.Config.Managed != nil {
return n.Config.Managed.CreateBeforeDestroy
}

return n.Config.Managed.CreateBeforeDestroy
// Otherwise check the state for a stored destroy order
if rs := n.ResourceState; rs != nil {
if s := rs.Instance(n.InstanceKey); s != nil {
if s.Current != nil {
return s.Current.CreateBeforeDestroy
}
}
}

return false
}

// GraphNodeDestroyerCBD
Expand Down
1 change: 0 additions & 1 deletion terraform/transform_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ func (t *DiffTransformer) Transform(g *Graph) error {
NodeAbstractResourceInstance: abstract,
DeposedKey: dk,
}
node.(*NodeDestroyResourceInstance).ModifyCreateBeforeDestroy(createBeforeDestroy)
} else {
node = &NodeDestroyDeposedResourceInstanceObject{
NodeAbstractResourceInstance: abstract,
Expand Down