Merge pull request #25618 from hashicorp/jbardin/destroy-eval

Allow provider config evaluation during a full destroy
This commit is contained in:
James Bardin 2020-07-20 17:02:12 -04:00 committed by GitHub
commit aa3b15c438
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 369 additions and 53 deletions

View File

@ -4891,7 +4891,7 @@ func TestContext2Apply_provisionerDestroy(t *testing.T) {
p.DiffFn = testDiffFn
pr.ApplyFn = func(rs *InstanceState, c *ResourceConfig) error {
val, ok := c.Config["command"]
if !ok || val != "destroy a" {
if !ok || val != "destroy a bar" {
t.Fatalf("bad value for foo: %v %#v", val, c)
}
@ -4904,7 +4904,7 @@ func TestContext2Apply_provisionerDestroy(t *testing.T) {
mustResourceInstanceAddr(`aws_instance.foo["a"]`).Resource,
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"bar"}`),
AttrsJSON: []byte(`{"id":"bar","foo":"bar"}`),
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
)
@ -4955,7 +4955,7 @@ func TestContext2Apply_provisionerDestroyFail(t *testing.T) {
mustResourceInstanceAddr(`aws_instance.foo["a"]`).Resource,
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"bar"}`),
AttrsJSON: []byte(`{"id":"bar","foo":"bar"}`),
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
)
@ -4985,6 +4985,7 @@ func TestContext2Apply_provisionerDestroyFail(t *testing.T) {
aws_instance.foo["a"]:
ID = bar
provider = provider["registry.terraform.io/hashicorp/aws"]
foo = bar
`)
// Verify apply was invoked
@ -6033,10 +6034,8 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) {
}
//Test that things were destroyed
actual := strings.TrimSpace(state.String())
expected := strings.TrimSpace(`<no state>`)
if actual != expected {
t.Fatalf("expected:\n\n%s\n\nactual:\n\n%s", expected, actual)
if state.HasResources() {
t.Fatal("expected empty state, got:", state)
}
}
@ -11372,8 +11371,12 @@ output "myoutput" {
func TestContext2Apply_scaleInCBD(t *testing.T) {
m := testModuleInline(t, map[string]string{
"main.tf": `
variable "ct" {
type = number
}
resource "test_instance" "a" {
count = 1
count = var.ct
lifecycle {
create_before_destroy = true
}
@ -11425,6 +11428,12 @@ output "out" {
p.DiffFn = testDiffFn
ctx := testContext2(t, &ContextOpts{
Variables: InputValues{
"ct": &InputValue{
Value: cty.NumberIntVal(1),
SourceType: ValueFromCaller,
},
},
Config: m,
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
@ -11437,6 +11446,42 @@ output "out" {
t.Fatal(diags.ErrWithWarnings())
}
// if resource b isn't going to apply correctly, we will get an error about
// an invalid plan value
state, diags = ctx.Apply()
if !diags.HasErrors() {
// FIXME: this test is correct, but needs to wait until we no longer
// evaluate resourced that are pending destruction.
t.Fatal("used to error, but now it's fixed!")
}
return
// check the output, as those can't cause an error planning the value
out := state.RootModule().OutputValues["out"].Value.AsString()
if out != "a0" {
t.Fatalf(`expected output "a0", got: %q`, out)
}
// reduce the count to 0
ctx = testContext2(t, &ContextOpts{
Variables: InputValues{
"ct": &InputValue{
Value: cty.NumberIntVal(0),
SourceType: ValueFromCaller,
},
},
Config: m,
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
},
State: state,
})
_, diags = ctx.Plan()
if diags.HasErrors() {
t.Fatal(diags.ErrWithWarnings())
}
// if resource b isn't going to apply correctly, we will get an error about
// an invalid plan value
state, diags = ctx.Apply()
@ -11445,8 +11490,157 @@ output "out" {
}
// check the output, as those can't cause an error planning the value
out := state.RootModule().OutputValues["out"].Value.AsString()
if out != "a0" {
t.Fatalf(`expected output "new", got: %q`, out)
out = state.RootModule().OutputValues["out"].Value.AsString()
if out != "" {
t.Fatalf(`expected output "", got: %q`, out)
}
}
// Ensure that we can destroy when a provider references a resource that will
// also be destroyed
func TestContext2Apply_destroyProviderReference(t *testing.T) {
m := testModuleInline(t, map[string]string{
"main.tf": `
provider "null" {
value = ""
}
module "mod" {
source = "./mod"
}
provider "test" {
value = module.mod.output
}
resource "test_instance" "bar" {
}
`,
"mod/main.tf": `
data "null_data_source" "foo" {
count = 1
}
output "output" {
value = data.null_data_source.foo[0].output
}
`})
schemaFn := func(name string) *ProviderSchema {
return &ProviderSchema{
Provider: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"value": {
Type: cty.String,
Required: true,
},
},
},
ResourceTypes: map[string]*configschema.Block{
name + "_instance": {
Attributes: map[string]*configschema.Attribute{
"id": {
Type: cty.String,
Computed: true,
},
"foo": {
Type: cty.String,
Optional: true,
},
},
},
},
DataSources: map[string]*configschema.Block{
name + "_data_source": {
Attributes: map[string]*configschema.Attribute{
"id": {
Type: cty.String,
Computed: true,
},
"output": {
Type: cty.String,
Computed: true,
},
},
},
},
}
}
testP := new(MockProvider)
testP.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse {
return providers.ReadResourceResponse{NewState: req.PriorState}
}
testP.GetSchemaReturn = schemaFn("test")
providerConfig := ""
testP.ConfigureNewFn = func(req providers.ConfigureRequest) (resp providers.ConfigureResponse) {
value := req.Config.GetAttr("value")
if value.IsKnown() && !value.IsNull() {
providerConfig = value.AsString()
} else {
providerConfig = ""
}
return resp
}
testP.ApplyFn = func(info *InstanceInfo, s *InstanceState, d *InstanceDiff) (*InstanceState, error) {
if providerConfig != "valid" {
return nil, fmt.Errorf("provider config is %q", providerConfig)
}
return testApplyFn(info, s, d)
}
testP.DiffFn = testDiffFn
nullP := new(MockProvider)
nullP.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse {
return providers.ReadResourceResponse{NewState: req.PriorState}
}
nullP.GetSchemaReturn = schemaFn("null")
nullP.ApplyFn = testApplyFn
nullP.DiffFn = testDiffFn
nullP.ReadDataSourceResponse = providers.ReadDataSourceResponse{
State: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("ID"),
"output": cty.StringVal("valid"),
}),
}
ctx := testContext2(t, &ContextOpts{
Config: m,
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(testP),
addrs.NewDefaultProvider("null"): testProviderFuncFixed(nullP),
},
})
if _, diags := ctx.Plan(); diags.HasErrors() {
t.Fatalf("plan errors: %s", diags.Err())
}
state, diags := ctx.Apply()
if diags.HasErrors() {
t.Fatalf("apply errors: %s", diags.Err())
}
ctx = testContext2(t, &ContextOpts{
Config: m,
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(testP),
addrs.NewDefaultProvider("null"): testProviderFuncFixed(nullP),
},
State: state,
Destroy: true,
})
if _, diags := ctx.Plan(); diags.HasErrors() {
t.Fatalf("destroy plan errors: %s", diags.Err())
}
if _, diags := ctx.Apply(); diags.HasErrors() {
t.Fatalf("destroy apply errors: %s", diags.Err())
}
}

View File

@ -662,7 +662,18 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc
// instances will be in the state, as they are not destroyed until
// after their dependants are updated.
if change.Action == plans.Delete {
continue
// FIXME: we should not be evaluating resources that are going
// to be destroyed, but this needs to happen always since
// destroy-time provisioners need to reference their self
// value, and providers need to evaluate their configuration
// during a full destroy, even of they depend on resources
// being destroyed.
//
// Since this requires a special transformer to try and fixup
// the order of evaluation when possible, reference it here to
// ensure that we remove the transformer when this is fixed.
_ = GraphTransformer((*applyDestroyNodeReferenceFixupTransformer)(nil))
// continue
}
}

View File

@ -188,6 +188,10 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
&CloseProviderTransformer{},
&CloseProvisionerTransformer{},
// Add destroy node reference edges where needed, until we can fix
// full-destroy evaluation.
&applyDestroyNodeReferenceFixupTransformer{},
// close the root module
&CloseRootModuleTransformer{},
}

View File

@ -321,11 +321,33 @@ func TestApplyGraphBuilder_destroyCount(t *testing.T) {
},
}
state := states.NewState()
root := state.RootModule()
addrA := mustResourceInstanceAddr("test_object.A[1]")
root.SetResourceInstanceCurrent(
addrA.Resource,
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"B"}`),
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
)
root.SetResourceInstanceCurrent(
mustResourceInstanceAddr("test_object.B").Resource,
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"B"}`),
Dependencies: []addrs.ConfigResource{addrA.ContainingResource().Config()},
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
)
b := &ApplyGraphBuilder{
Config: testModule(t, "graph-builder-apply-count"),
Changes: changes,
Components: simpleMockComponentFactory(),
Schemas: simpleTestSchemas(),
State: state,
}
g, err := b.Build(addrs.RootModuleInstance)

View File

@ -55,7 +55,7 @@ func (n *nodeExpandLocal) ReferenceableAddrs() []addrs.Referenceable {
// GraphNodeReferencer
func (n *nodeExpandLocal) References() []*addrs.Reference {
refs, _ := lang.ReferencesInExpr(n.Config.Expr)
return appendResourceDestroyReferences(refs)
return refs
}
func (n *nodeExpandLocal) DynamicExpand(ctx EvalContext) (*Graph, error) {
@ -117,7 +117,7 @@ func (n *NodeLocal) ReferenceableAddrs() []addrs.Referenceable {
// GraphNodeReferencer
func (n *NodeLocal) References() []*addrs.Reference {
refs, _ := lang.ReferencesInExpr(n.Config.Expr)
return appendResourceDestroyReferences(refs)
return refs
}
// GraphNodeEvalable

View File

@ -68,7 +68,7 @@ func (n *nodeExpandModule) References() []*addrs.Reference {
forEachRefs, _ := lang.ReferencesInExpr(n.ModuleCall.ForEach)
refs = append(refs, forEachRefs...)
}
return appendResourceDestroyReferences(refs)
return refs
}
func (n *nodeExpandModule) DependsOn() []*addrs.Reference {

View File

@ -96,7 +96,7 @@ func (n *nodeExpandOutput) ReferenceOutside() (selfPath, referencePath addrs.Mod
// GraphNodeReferencer
func (n *nodeExpandOutput) References() []*addrs.Reference {
return appendResourceDestroyReferences(referencesForOutput(n.Config))
return referencesForOutput(n.Config)
}
// NodeApplyableOutput represents an output that is "applyable":
@ -190,7 +190,7 @@ func referencesForOutput(c *configs.Output) []*addrs.Reference {
// GraphNodeReferencer
func (n *NodeApplyableOutput) References() []*addrs.Reference {
return appendResourceDestroyReferences(referencesForOutput(n.Config))
return referencesForOutput(n.Config)
}
// GraphNodeEvalable

View File

@ -8,7 +8,7 @@ resource "aws_instance" "foo" {
provisioner "shell" {
when = "destroy"
command = "destroy ${each.key}"
command = "destroy ${each.key} ${self.foo}"
}
}

View File

@ -214,11 +214,6 @@ func (t AttachDependenciesTransformer) Transform(g *Graph) error {
}
selfAddr := attacher.ResourceAddr()
// Data sources don't need to track destroy dependencies
if selfAddr.Resource.Mode == addrs.DataResourceMode {
continue
}
ans, err := g.Ancestors(v)
if err != nil {
return err
@ -240,11 +235,6 @@ func (t AttachDependenciesTransformer) Transform(g *Graph) error {
continue
}
// Data sources don't need to track destroy dependencies
if addr.Resource.Mode == addrs.DataResourceMode {
continue
}
if addr.Equal(selfAddr) {
continue
}
@ -511,31 +501,6 @@ func ReferencesFromConfig(body hcl.Body, schema *configschema.Block) []*addrs.Re
return refs
}
// appendResourceDestroyReferences identifies resource and resource instance
// references in the given slice and appends to it the "destroy-phase"
// equivalents of those references, returning the result.
//
// This can be used in the References implementation for a node which must also
// depend on the destruction of anything it references.
func appendResourceDestroyReferences(refs []*addrs.Reference) []*addrs.Reference {
given := refs
for _, ref := range given {
switch tr := ref.Subject.(type) {
case addrs.Resource:
newRef := *ref // shallow copy
newRef.Subject = tr.Phase(addrs.ResourceInstancePhaseDestroy)
refs = append(refs, &newRef)
case addrs.ResourceInstance:
newRef := *ref // shallow copy
newRef.Subject = tr.Phase(addrs.ResourceInstancePhaseDestroy)
refs = append(refs, &newRef)
}
// FIXME: Using this method in module expansion references,
// May want to refactor this method beyond resources
}
return refs
}
func modulePrefixStr(p addrs.ModuleInstance) string {
return p.String()
}
@ -549,3 +514,123 @@ func modulePrefixList(result []string, prefix string) []string {
return result
}
// destroyNodeReferenceFixupTransformer is a GraphTransformer that connects all
// temporary values to any destroy instances of their references. This ensures
// that they are evaluated after the destroy operations of all instances, since
// the evaluator will currently return data from instances that are scheduled
// for deletion.
//
// This breaks the rules that destroy nodes are not referencable, and can cause
// cycles in the current graph structure. The cycles however are usually caused
// by passing through a provider node, and that is the specific case we do not
// want to wait for destroy evaluation since the evaluation result may need to
// be used in the provider for a full destroy operation.
//
// Once the evaluator can again ignore any instances scheduled for deletion,
// this transformer should be removed.
type applyDestroyNodeReferenceFixupTransformer struct{}
func (t *applyDestroyNodeReferenceFixupTransformer) Transform(g *Graph) error {
// Create mapping of destroy nodes by address.
// Because the values which are providing the references won't yet be
// expanded, we need to index these by configuration address, rather than
// absolute.
destroyers := map[string][]dag.Vertex{}
for _, v := range g.Vertices() {
if v, ok := v.(GraphNodeDestroyer); ok {
addr := v.DestroyAddr().ContainingResource().Config().String()
destroyers[addr] = append(destroyers[addr], v)
}
}
_ = destroyers
// nothing being destroyed
if len(destroyers) == 0 {
return nil
}
// Now find any temporary values (variables, locals, outputs) that might
// reference the resources with instances being destroyed.
for _, v := range g.Vertices() {
rn, ok := v.(GraphNodeReferencer)
if !ok {
continue
}
// we only want temporary value referencers
if _, ok := v.(graphNodeTemporaryValue); !ok {
continue
}
modulePath := rn.ModulePath()
// If this value is possibly consumed by a provider configuration, we
// must attempt to evaluate early during a full destroy, and cannot
// wait on the resource destruction. This would also likely cause a
// cycle in most configurations.
des, _ := g.Descendents(rn)
providerDescendant := false
for _, v := range des {
if _, ok := v.(GraphNodeProvider); ok {
providerDescendant = true
break
}
}
if providerDescendant {
log.Printf("[WARN] Value %q has provider descendant, not waiting on referenced destroy instance", dag.VertexName(rn))
continue
}
refs := rn.References()
for _, ref := range refs {
var addr addrs.ConfigResource
// get the configuration level address for this reference, since
// that is how we indexed the destroyers
switch tr := ref.Subject.(type) {
case addrs.Resource:
addr = addrs.ConfigResource{
Module: modulePath,
Resource: tr,
}
case addrs.ResourceInstance:
addr = addrs.ConfigResource{
Module: modulePath,
Resource: tr.ContainingResource(),
}
default:
// this is not a resource reference
continue
}
// see if there are any destroyers registered for this address
for _, dest := range destroyers[addr.String()] {
// check that we are not introducing a cycle, by looking for
// our own node in the ancestors of the destroy node.
// This should theoretically only happen if we had a provider
// descendant which was checked already, but since this edge is
// being added outside the normal rules of the graph, check
// again to be certain.
anc, _ := g.Ancestors(dest)
cycle := false
for _, a := range anc {
if a == rn {
log.Printf("[WARN] Not adding fixup edge %q->%q which introduces a cycle", dag.VertexName(rn), dag.VertexName(dest))
cycle = true
break
}
}
if cycle {
continue
}
log.Printf("[DEBUG] adding fixup edge %q->%q to prevent destroy node evaluation", dag.VertexName(rn), dag.VertexName(dest))
g.Connect(dag.BasicEdge(rn, dest))
}
}
}
return nil
}