resilien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #24956 from hashicorp/jbardin/cbd-state 

CreateBeforeDestroy inheritence and state serializaton
This commit is contained in:
James Bardin 2020-05-18 11:46:29 -04:00 committed by GitHub
parent eb4a3cafc6 08128fe775
commit 78d8af18c2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 148 additions and 145 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
command/command_test.go
View File

@ -265,7 +265,6 @@ func testState() *states.State {
AttrsJSON: []byte("{\n \"id\": \"bar\"\n }"),
Status: states.ObjectReady,
Dependencies: []addrs.ConfigResource{},
DependsOn: []addrs.Referenceable{},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),

4
command/refresh_test.go
View File

@ -278,7 +278,6 @@ func TestRefresh_defaultState(t *testing.T) {
Status: states.ObjectReady,
AttrsJSON: []byte("{\n \"ami\": null,\n \"id\": \"yes\"\n }"),
Dependencies: []addrs.ConfigResource{},
DependsOn: []addrs.Referenceable{},
}
if !reflect.DeepEqual(actual, expected) {
t.Fatalf("wrong new object\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(expected))
@ -343,7 +342,6 @@ func TestRefresh_outPath(t *testing.T) {
Status: states.ObjectReady,
AttrsJSON: []byte("{\n \"ami\": null,\n \"id\": \"yes\"\n }"),
Dependencies: []addrs.ConfigResource{},
DependsOn: []addrs.Referenceable{},
}
if !reflect.DeepEqual(actual, expected) {
t.Fatalf("wrong new object\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(expected))
@ -573,7 +571,6 @@ func TestRefresh_backup(t *testing.T) {
Status: states.ObjectReady,
AttrsJSON: []byte("{\n \"ami\": null,\n \"id\": \"changed\"\n }"),
Dependencies: []addrs.ConfigResource{},
DependsOn: []addrs.Referenceable{},
}
if !reflect.DeepEqual(actual, expected) {
t.Fatalf("wrong new object\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(expected))
@ -640,7 +637,6 @@ func TestRefresh_disableBackup(t *testing.T) {
Status: states.ObjectReady,
AttrsJSON: []byte("{\n \"ami\": null,\n \"id\": \"yes\"\n }"),
Dependencies: []addrs.ConfigResource{},
DependsOn: []addrs.Referenceable{},
}
if !reflect.DeepEqual(actual, expected) {
t.Fatalf("wrong new object\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(expected))

1
command/show_test.go
View File

@ -83,7 +83,6 @@ func TestShow_aliasedProvider(t *testing.T) {
AttrsJSON: []byte("{\n \"id\": \"bar\"\n }"),
Status: states.ObjectReady,
Dependencies: []addrs.ConfigResource{},
DependsOn: []addrs.Referenceable{},
},
addrs.RootModuleInstance.ProviderConfigAliased(addrs.NewDefaultProvider("test"), "alias"),
)

4
helper/resource/state_shim.go
View File

@ -87,10 +87,6 @@ func shimNewState(newState *states.State, providers map[string]terraform.Resourc
resState.Primary.Meta["schema_version"] = i.Current.SchemaVersion
}
for _, dep := range i.Current.DependsOn {
resState.Dependencies = append(resState.Dependencies, dep.String())
}
// convert the indexes to the old style flapmap indexes
idx := ""
switch key.(type) {

36
helper/resource/state_shim_test.go
View File

@ -31,15 +31,6 @@ func TestStateShim(t *testing.T) {
Status: states.ObjectReady,
AttrsFlat: map[string]string{"id": "foo", "bazzle": "dazzle"},
SchemaVersion: 7,
DependsOn: []addrs.Referenceable{
addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: 'M',
Type: "test_thing",
Name: "baz",
},
},
},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
@ -55,7 +46,6 @@ func TestStateShim(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsFlat: map[string]string{"id": "baz", "bazzle": "dazzle"},
DependsOn: []addrs.Referenceable{},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
@ -74,7 +64,6 @@ func TestStateShim(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id": "bar", "fuzzle":"wuzzle"}`),
DependsOn: []addrs.Referenceable{},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
@ -90,15 +79,6 @@ func TestStateShim(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id": "bar", "fizzle":"wizzle"}`),
DependsOn: []addrs.Referenceable{
addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: 'D',
Type: "test_data_thing",
Name: "foo",
},
},
},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
@ -116,15 +96,6 @@ func TestStateShim(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsFlat: map[string]string{"id": "old", "fizzle": "wizzle"},
DependsOn: []addrs.Referenceable{
addrs.ResourceInstance{
Resource: addrs.Resource{
Mode: 'D',
Type: "test_data_thing",
Name: "foo",
},
},
},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
@ -141,7 +112,6 @@ func TestStateShim(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsFlat: map[string]string{"id": "0", "bazzle": "dazzle"},
DependsOn: []addrs.Referenceable{},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
@ -157,7 +127,6 @@ func TestStateShim(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectTainted,
AttrsFlat: map[string]string{"id": "1", "bazzle": "dazzle"},
DependsOn: []addrs.Referenceable{},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
@ -174,7 +143,6 @@ func TestStateShim(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id": "single", "bazzle":"dazzle"}`),
DependsOn: []addrs.Referenceable{},
},
addrs.AbsProviderConfig{
Provider: addrs.NewDefaultProvider("test"),
@ -223,7 +191,6 @@ func TestStateShim(t *testing.T) {
"schema_version": 7,
},
},
Dependencies: []string{"test_thing.baz"},
},
},
},
@ -249,7 +216,6 @@ func TestStateShim(t *testing.T) {
},
},
},
Dependencies: []string{"data.test_data_thing.foo"},
},
"data.test_data_thing.foo": &terraform.ResourceState{
Type: "test_data_thing",
@ -386,7 +352,6 @@ func TestShimLegacyState(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsFlat: map[string]string{"id": "baz", "bazzle": "dazzle"},
DependsOn: []addrs.Referenceable{},
Dependencies: []addrs.ConfigResource{},
},
addrs.AbsProviderConfig{
@ -404,7 +369,6 @@ func TestShimLegacyState(t *testing.T) {
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsFlat: map[string]string{"id": "bar", "fizzle": "wizzle"},
DependsOn: []addrs.Referenceable{},
Dependencies: []addrs.ConfigResource{},
},
addrs.AbsProviderConfig{

16
states/instance_object.go
View File

@ -42,11 +42,6 @@ type ResourceInstanceObject struct {
// 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.
DependsOn []addrs.Referenceable
}
// ObjectStatus represents the status of a RemoteObject.
@ -109,11 +104,12 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res
}
return &ResourceInstanceObjectSrc{
SchemaVersion: schemaVersion,
AttrsJSON: src,
Private: o.Private,
Status: o.Status,
Dependencies: o.Dependencies,
SchemaVersion: schemaVersion,
AttrsJSON: src,
Private: o.Private,
Status: o.Status,
Dependencies: o.Dependencies,
CreateBeforeDestroy: o.CreateBeforeDestroy,
}, nil
}

3
states/instance_object_src.go
View File

@ -55,8 +55,6 @@ type ResourceInstanceObjectSrc struct {
Status ObjectStatus
Dependencies []addrs.ConfigResource
CreateBeforeDestroy bool
// deprecated
DependsOn []addrs.Referenceable
}
// Decode unmarshals the raw representation of the object attributes. Pass the
@ -89,7 +87,6 @@ func (os *ResourceInstanceObjectSrc) Decode(ty cty.Type) (*ResourceInstanceObjec
Value: val,
Status: os.Status,
Dependencies: os.Dependencies,
DependsOn: os.DependsOn,
Private: os.Private,
CreateBeforeDestroy: os.CreateBeforeDestroy,
}, nil

7
states/state_deepcopy.go
View File

@ -158,12 +158,6 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc {
copy(dependencies, obj.Dependencies)
}
var dependsOn []addrs.Referenceable
if obj.DependsOn != nil {
dependsOn = make([]addrs.Referenceable, len(obj.DependsOn))
copy(dependsOn, obj.DependsOn)
}
return &ResourceInstanceObjectSrc{
Status: obj.Status,
SchemaVersion: obj.SchemaVersion,
@ -171,7 +165,6 @@ func (obj *ResourceInstanceObjectSrc) DeepCopy() *ResourceInstanceObjectSrc {
AttrsFlat: attrsFlat,
AttrsJSON: attrsJSON,
Dependencies: dependencies,
DependsOn: dependsOn,
CreateBeforeDestroy: obj.CreateBeforeDestroy,
}
}

34
states/statefile/testdata/roundtrip/v4-cbd.in.tfstate vendored Normal file
View File

@ -0,0 +1,34 @@
{
"version": 4,
"serial": 0,
"lineage": "f2968801-fa14-41ab-a044-224f3a4adf04",
"terraform_version": "0.12.0",
"outputs": {
"numbers": {
"type": "string",
"value": "0,1"
}
},
"resources": [
{
"module": "module.modA",
"mode": "managed",
"type": "null_resource",
"name": "resource",
"provider": "provider[\"registry.terraform.io/-/null\"]",
"instances": [
{
"schema_version": 0,
"attributes": {
"id": "4639265839606265182",
"triggers": {
"input": "test"
}
},
"create_before_destroy": true,
"private": "bnVsbA=="
}
]
}
]
}

1
states/statefile/testdata/roundtrip/v4-cbd.out.tfstate vendored Symbolic link
View File

@ -0,0 +1 @@
v4-cbd.in.tfstate

1
states/statefile/version3_upgrade.go
View File

@ -370,7 +370,6 @@ func upgradeInstanceObjectV3ToV4(rsOld *resourceStateV2, isOld *instanceStateV2,
Status: status,
Deposed: string(deposedKey),
AttributesFlat: attributes,
DependsOn: dependencies,
SchemaVersion: schemaVersion,
PrivateRaw: privateJSON,
}, nil

57
states/statefile/version4.go
View File

@ -130,7 +130,8 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) {
instAddr := rAddr.Instance(key)
obj := &states.ResourceInstanceObjectSrc{
SchemaVersion: isV4.SchemaVersion,
SchemaVersion: isV4.SchemaVersion,
CreateBeforeDestroy: isV4.CreateBeforeDestroy,
}
{
@ -171,34 +172,6 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) {
obj.Private = raw
}
{
// Allow both the deprecated `depends_on` and new
// `dependencies` to coexist for now so resources can be
// upgraded as they are refreshed.
depsRaw := isV4.DependsOn
deps := make([]addrs.Referenceable, 0, len(depsRaw))
for _, depRaw := range depsRaw {
ref, refDiags := addrs.ParseRefStr(depRaw)
diags = diags.Append(refDiags)
if refDiags.HasErrors() {
continue
}
if len(ref.Remaining) != 0 {
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Error,
"Invalid resource instance metadata in state",
fmt.Sprintf("Instance %s declares dependency on %q, which is not a reference to a dependable object.", instAddr.Absolute(moduleAddr), depRaw),
))
}
if ref.Subject == nil {
// Should never happen
panic(fmt.Sprintf("parsing dependency %q for instance %s returned a nil address", depRaw, instAddr.Absolute(moduleAddr)))
}
deps = append(deps, ref.Subject)
}
obj.DependsOn = deps
}
{
depsRaw := isV4.Dependencies
deps := make([]addrs.ConfigResource, 0, len(depsRaw))
@ -462,11 +435,6 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc
deps[i] = depAddr.String()
}
depOn := make([]string, len(obj.DependsOn))
for i, depAddr := range obj.DependsOn {
depOn[i] = depAddr.String()
}
var rawKey interface{}
switch tk := key.(type) {
case addrs.IntKey:
@ -484,15 +452,15 @@ func appendInstanceObjectStateV4(rs *states.Resource, is *states.ResourceInstanc
}
return append(isV4s, instanceObjectStateV4{
IndexKey: rawKey,
Deposed: string(deposed),
Status: status,
SchemaVersion: obj.SchemaVersion,
AttributesFlat: obj.AttrsFlat,
AttributesRaw: obj.AttrsJSON,
PrivateRaw: privateRaw,
Dependencies: deps,
DependsOn: depOn,
IndexKey: rawKey,
Deposed: string(deposed),
Status: status,
SchemaVersion: obj.SchemaVersion,
AttributesFlat: obj.AttrsFlat,
AttributesRaw: obj.AttrsJSON,
PrivateRaw: privateRaw,
Dependencies: deps,
CreateBeforeDestroy: obj.CreateBeforeDestroy,
}), diags
}
@ -543,7 +511,8 @@ type instanceObjectStateV4 struct {
PrivateRaw []byte `json:"private,omitempty"`
Dependencies []string `json:"dependencies,omitempty"`
DependsOn []string `json:"depends_on,omitempty"`
CreateBeforeDestroy bool `json:"create_before_destroy,omitempty"`
}
// stateVersionV4 is a weird special type we use to produce our hard-coded

41
terraform/context_apply_test.go
View File

@ -11011,3 +11011,44 @@ module.mod2:
t.Fatalf("expected:\n%s\ngot:\n%s\n", expected, state)
}
}
func TestContext2Apply_inheritAndStoreCBD(t *testing.T) {
m := testModuleInline(t, map[string]string{
"main.tf": `
resource "aws_instance" "foo" {
}
resource "aws_instance" "cbd" {
foo = aws_instance.foo.id
lifecycle {
create_before_destroy = true
}
}
`,
})
p := testProvider("aws")
p.ApplyFn = testApplyFn
p.DiffFn = testDiffFn
ctx := testContext2(t, &ContextOpts{
Config: m,
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
},
})
_, diags := ctx.Plan()
if diags.HasErrors() {
t.Fatal(diags.ErrWithWarnings())
}
state, diags := ctx.Apply()
if diags.HasErrors() {
t.Fatal(diags.ErrWithWarnings())
}
foo := state.ResourceInstance(mustResourceInstanceAddr("aws_instance.foo"))
if !foo.Current.CreateBeforeDestroy {
t.Fatal("aws_instance.foo should also be create_before_destroy")
}
}

30
terraform/eval_apply.go
View File

@ -22,17 +22,18 @@ import (
// EvalApply is an EvalNode implementation that writes the diff to
// the full diff.
type EvalApply struct {
Addr addrs.ResourceInstance
Config *configs.Resource
State **states.ResourceInstanceObject
Change **plans.ResourceInstanceChange
ProviderAddr addrs.AbsProviderConfig
Provider *providers.Interface
ProviderMetas map[addrs.Provider]*configs.ProviderMeta
ProviderSchema **ProviderSchema
Output **states.ResourceInstanceObject
CreateNew *bool
Error *error
Addr addrs.ResourceInstance
Config *configs.Resource
State **states.ResourceInstanceObject
Change **plans.ResourceInstanceChange
ProviderAddr addrs.AbsProviderConfig
Provider *providers.Interface
ProviderMetas map[addrs.Provider]*configs.ProviderMeta
ProviderSchema **ProviderSchema
Output **states.ResourceInstanceObject
CreateNew *bool
Error *error
CreateBeforeDestroy bool
}
// TODO: test
@ -305,9 +306,10 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
var newState *states.ResourceInstanceObject
if !newVal.IsNull() { // null value indicates that the object is deleted, so we won't set a new state in that case
newState = &states.ResourceInstanceObject{
Status: newStatus,
Value: newVal,
Private: resp.Private,
Status: newStatus,
Value: newVal,
Private: resp.Private,
CreateBeforeDestroy: n.CreateBeforeDestroy,
}
}

1
terraform/eval_refresh.go
View File

@ -119,6 +119,7 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) {
newState.Value = resp.NewState
newState.Private = resp.Private
newState.Dependencies = state.Dependencies
newState.CreateBeforeDestroy = state.CreateBeforeDestroy
// Call post-refresh hook
err = ctx.Hook(func(h Hook) (HookAction, error) {

4
terraform/graph_builder_apply.go
View File

@ -143,6 +143,10 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
&ReferenceTransformer{},
&AttachDependenciesTransformer{},
// Detect when create_before_destroy must be forced on for a particular
// node due to dependency edges, to avoid graph cycles during apply.
&ForcedCBDTransformer{},
// Destruction ordering
&DestroyEdgeTransformer{
Config: b.Config,

52
terraform/node_resource_apply_instance.go
View File

@ -25,6 +25,10 @@ type NodeApplyableResourceInstance struct {
destroyNode GraphNodeDestroyerCBD
graphNodeDeposer // implementation of GraphNodeDeposerConfig
// If this node is forced to be CreateBeforeDestroy, we need to record that
// in the state to.
ForceCreateBeforeDestroy bool
}
var (
@ -42,20 +46,27 @@ func (n *NodeApplyableResourceInstance) AttachDestroyNode(d GraphNodeDestroyerCB
n.destroyNode = d
}
// createBeforeDestroy checks this nodes config status and the status af any
// CreateBeforeDestroy checks this nodes config status and the status af any
// companion destroy node for CreateBeforeDestroy.
func (n *NodeApplyableResourceInstance) createBeforeDestroy() bool {
cbd := false
func (n *NodeApplyableResourceInstance) CreateBeforeDestroy() bool {
if n.ForceCreateBeforeDestroy {
return n.ForceCreateBeforeDestroy
}
if n.Config != nil && n.Config.Managed != nil {
cbd = n.Config.Managed.CreateBeforeDestroy
return n.Config.Managed.CreateBeforeDestroy
}
if n.destroyNode != nil {
cbd = cbd || n.destroyNode.CreateBeforeDestroy()
return n.destroyNode.CreateBeforeDestroy()
}
return cbd
return false
}
func (n *NodeApplyableResourceInstance) ModifyCreateBeforeDestroy(v bool) error {
n.ForceCreateBeforeDestroy = v
return nil
}
// GraphNodeCreator
@ -78,7 +89,7 @@ func (n *NodeApplyableResourceInstance) References() []*addrs.Reference {
// would create a dependency cycle. We make a compromise here of requiring
// changes to be updated across two applies in this case, since the first
// plan will use the old values.
if !n.createBeforeDestroy() {
if !n.CreateBeforeDestroy() {
for _, ref := range ret {
switch tr := ref.Subject.(type) {
case addrs.ResourceInstance:
@ -254,7 +265,7 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
if diffApply != nil {
destroy = (diffApply.Action == plans.Delete || diffApply.Action.IsReplace())
}
if destroy && n.createBeforeDestroy() {
if destroy && n.CreateBeforeDestroy() {
createBeforeDestroyEnabled = true
}
return createBeforeDestroyEnabled, nil
@ -346,17 +357,18 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
Change: &diffApply,
},
&EvalApply{
Addr: addr.Resource,
Config: n.Config,
State: &state,
Change: &diffApply,
Provider: &provider,
ProviderAddr: n.ResolvedProvider,
ProviderMetas: n.ProviderMetas,
ProviderSchema: &providerSchema,
Output: &state,
Error: &err,
CreateNew: &createNew,
Addr: addr.Resource,
Config: n.Config,
State: &state,
Change: &diffApply,
Provider: &provider,
ProviderAddr: n.ResolvedProvider,
ProviderMetas: n.ProviderMetas,
ProviderSchema: &providerSchema,
Output: &state,
Error: &err,
CreateNew: &createNew,
CreateBeforeDestroy: n.CreateBeforeDestroy(),
},
&EvalMaybeTainted{
Addr: addr.Resource,
@ -411,7 +423,7 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe
if !diff.Action.IsReplace() {
return true, nil
}
if !n.createBeforeDestroy() {
if !n.CreateBeforeDestroy() {
return true, nil
}
return false, nil