Merge pull request #24084 from hashicorp/jbardin/cbd-instance-state

Add CreateBeforeDestroy to instance state

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.

states/instance_object_src.go

@@ -54,6 +54,7 @@ type ResourceInstanceObjectSrc struct {
 	Private             []byte
 	Status              ObjectStatus
 	Dependencies        []addrs.AbsResource
+	CreateBeforeDestroy bool
 	// deprecated
 	DependsOn []addrs.Referenceable
 }
@@ -90,6 +91,7 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec
 		Dependencies:        os.Dependencies,
 		DependsOn:           os.DependsOn,
 		Private:             os.Private,
+		CreateBeforeDestroy: os.CreateBeforeDestroy,
 	}, nil
 }
 

states/state_deepcopy.go

@@ -173,6 +173,7 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc {
 		AttrsJSON:           attrsJSON,
 		Dependencies:        dependencies,
 		DependsOn:           dependsOn,
+		CreateBeforeDestroy: obj.CreateBeforeDestroy,
 	}
 }
 

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{

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
+	// Config takes precedence
-	if n.Config == nil || n.Config.Managed == nil {
+	if n.Config != nil && n.Config.Managed != nil {
-		return false
+		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

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,