diff --git a/states/instance_object.go b/states/instance_object.go index 78e1dda93..1642b4569 100644 --- a/states/instance_object.go +++ b/states/instance_object.go @@ -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. diff --git a/states/instance_object_src.go b/states/instance_object_src.go index a18cf313c..ff56bcfb8 100644 --- a/states/instance_object_src.go +++ b/states/instance_object_src.go @@ -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 } @@ -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 } diff --git a/states/state_deepcopy.go b/states/state_deepcopy.go index 7d7a7ef10..89082fb8e 100644 --- a/states/state_deepcopy.go +++ b/states/state_deepcopy.go @@ -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, } } diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index 6e221e026..8eebecf0d 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -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{ diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index d4fb95260..ed2bafa69 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -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 diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index 23b6e2a75..637309fad 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -161,7 +161,6 @@ func (t *DiffTransformer) Transform(g *Graph) error { NodeAbstractResourceInstance: abstract, DeposedKey: dk, } - node.(*NodeDestroyResourceInstance).ModifyCreateBeforeDestroy(createBeforeDestroy) } else { node = &NodeDestroyDeposedResourceInstanceObject{ NodeAbstractResourceInstance: abstract,