make the diagnostics handling of apply easier

Now that we don't need to track separate error values for the apply
logic, it's easier to reorganize the diagnostic handling to use a single
set of diagnostics.

terraform/node_resource_apply_instance.go

@@ -265,34 +265,36 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 	}
 
 	state, applyDiags := n.apply(ctx, state, diffApply, n.Config, n.CreateBeforeDestroy())
-	// keep the applyDiags separate to handle the error case independently
-	// we must add these into diags before returning
+	diags = diags.Append(applyDiags)
 
 	// We clear the change out here so that future nodes don't see a change
 	// that is already complete.
-	diags = diags.Append(n.writeChange(ctx, nil, ""))
-
-	state = maybeTainted(addr.Absolute(ctx.Path()), state, diffApply, applyDiags.Err())
-
-	diags = diags.Append(n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState))
-	if diags.HasErrors() {
-		return diags.Append(applyDiags)
+	err = n.writeChange(ctx, nil, "")
+	if err != nil {
+		return diags.Append(err)
 	}
 
+	state = maybeTainted(addr.Absolute(ctx.Path()), state, diffApply, diags.Err())
+
+	err = n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState)
+	if err != nil {
+		return diags.Append(err)
+	}
+
+	// Run Provisioners
 	createNew := (diffApply.Action == plans.Create || diffApply.Action.IsReplace())
 	applyProvisionersDiags := n.evalApplyProvisioners(ctx, state, createNew, configs.ProvisionerWhenCreate)
 	// the provisioner errors count as port of the apply error, so we can bundle the diags
-	applyDiags = applyDiags.Append(applyProvisionersDiags)
+	diags = diags.Append(applyProvisionersDiags)
 
-	applyErr := applyDiags.Err()
-	state = maybeTainted(addr.Absolute(ctx.Path()), state, diffApply, applyErr)
+	state = maybeTainted(addr.Absolute(ctx.Path()), state, diffApply, diags.Err())
 
-	diags = diags.Append(n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState))
-	if diags.HasErrors() {
-		return diags.Append(applyDiags)
+	err = n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState)
+	if err != nil {
+		return diags.Append(err)
 	}
 
-	if createBeforeDestroyEnabled && applyErr != nil {
+	if createBeforeDestroyEnabled && diags.HasErrors() {
 		if deposedKey == states.NotDeposed {
 			// This should never happen, and so it always indicates a bug.
 			// We should evaluate this node only if we've previously deposed
@@ -327,7 +329,6 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
 	}
 
 	diags = diags.Append(n.postApplyHook(ctx, state, diags.Err()))
-	diags = diags.Append(applyDiags)
 	diags = diags.Append(updateStateHook(ctx))
 	return diags
 }

terraform/node_resource_destroy.go

@@ -171,37 +171,32 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 		return diags
 	}
 
-	var applyDiags tfdiags.Diagnostics
-	var applyProvisionersDiags tfdiags.Diagnostics
 	// Run destroy provisioners if not tainted
 	if state != nil && state.Status != states.ObjectTainted {
-		applyProvisionersDiags = n.evalApplyProvisioners(ctx, state, false, configs.ProvisionerWhenDestroy)
+		applyProvisionersDiags := n.evalApplyProvisioners(ctx, state, false, configs.ProvisionerWhenDestroy)
+		diags = diags.Append(applyProvisionersDiags)
 		// keep the diags separate from the main set until we handle the cleanup
 
-		provisionerErr := applyProvisionersDiags.Err()
-		if provisionerErr != nil {
+		if diags.HasErrors() {
 			// If we have a provisioning error, then we just call
 			// the post-apply hook now.
-			diags = diags.Append(n.postApplyHook(ctx, state, provisionerErr))
-			return diags.Append(applyProvisionersDiags)
+			diags = diags.Append(n.postApplyHook(ctx, state, diags.Err()))
+			return diags
 		}
 	}
 
-	// provisioner and apply diags are handled together from here down
-	applyDiags = applyDiags.Append(applyProvisionersDiags)
-
 	// Managed resources need to be destroyed, while data sources
 	// are only removed from state.
 	if addr.Resource.Resource.Mode == addrs.ManagedResourceMode {
 		// we pass a nil configuration to apply because we are destroying
 		s, d := n.apply(ctx, state, changeApply, nil, false)
-		state, applyDiags = s, applyDiags.Append(d)
+		state, diags = s, diags.Append(d)
 		// we must keep applyDiags separate until returning in order to process
 		// the error independently
 
-		diags = diags.Append(n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState))
-		if diags.HasErrors() {
-			return diags.Append(applyDiags)
+		err := n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState)
+		if err != nil {
+			return diags.Append(err)
 		}
 	} else {
 		log.Printf("[TRACE] NodeDestroyResourceInstance: removing state object for %s", n.Addr)
@@ -210,9 +205,7 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 	}
 
 	// create the err value for postApplyHook
-	diags = diags.Append(n.postApplyHook(ctx, state, applyDiags.Err()))
-
-	diags = diags.Append(applyDiags)
+	diags = diags.Append(n.postApplyHook(ctx, state, diags.Err()))
 	diags = diags.Append(updateStateHook(ctx))
 	return diags
 }

terraform/node_resource_destroy_deposed.go

@@ -155,9 +155,8 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w
 
 	// Read the state for the deposed resource instance
 	state, err := n.readResourceInstanceStateDeposed(ctx, n.Addr, n.DeposedKey)
-	diags = diags.Append(err)
-	if diags.HasErrors() {
-		return diags
+	if err != nil {
+		return diags.Append(err)
 	}
 
 	change, destroyPlanDiags := n.planDestroy(ctx, state, n.DeposedKey)
@@ -174,19 +173,18 @@ func (n *NodeDestroyDeposedResourceInstanceObject) Execute(ctx EvalContext, op w
 
 	// we pass a nil configuration to apply because we are destroying
 	state, applyDiags := n.apply(ctx, state, change, nil, false)
-	// we need to keep the apply diagnostics separate until we return, so that
-	// we can handle the apply error case independently
+	diags = diags.Append(applyDiags)
+	// don't return immediately on errors, we need to handle the state
 
 	// Always write the resource back to the state deposed. If it
 	// was successfully destroyed it will be pruned. If it was not, it will
 	// be caught on the next run.
-	diags = diags.Append(n.writeResourceInstanceState(ctx, state))
-	if diags.HasErrors() {
-		return diags.Append(applyDiags)
+	err = n.writeResourceInstanceState(ctx, state)
+	if err != nil {
+		return diags.Append(err)
 	}
 
-	diags = diags.Append(n.postApplyHook(ctx, state, applyDiags.Err()))
-	diags = diags.Append(applyDiags)
+	diags = diags.Append(n.postApplyHook(ctx, state, diags.Err()))
 
 	return diags.Append(updateStateHook(ctx))
 }