add context to action error messages

We want to be informative about how to remidy the issues actions ran
into.
pull/37403/head
Daniel Schmidt 7 months ago
parent 17e7338b0f
commit b810d3d2f8

@ -23,9 +23,10 @@ func TestContext2Apply_actions(t *testing.T) {
module map[string]string
mode plans.Mode
prevRunState *states.State
events []providers.InvokeActionEvent
events func(req providers.InvokeActionRequest) []providers.InvokeActionEvent
callingInvokeReturnsDiagnostics func(providers.InvokeActionRequest) tfdiags.Diagnostics
planOpts *PlanOpts
planOpts *PlanOpts
expectInvokeActionCalled bool
expectInvokeActionCalls []providers.InvokeActionRequest
@ -160,23 +161,79 @@ resource "test_object" "a" {
`,
},
expectInvokeActionCalled: true,
events: []providers.InvokeActionEvent{
providers.InvokeActionEvent_Completed{
Diagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing",
),
events: func(req providers.InvokeActionRequest) []providers.InvokeActionEvent {
return []providers.InvokeActionEvent{
providers.InvokeActionEvent_Completed{
Diagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing",
),
},
},
},
}
},
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing",
"Failed to apply actions before test_object.a",
"An error occured while invoking action action.act_unlinked.hello: test case for failing: this simulates a provider failing\n",
),
},
},
"before_create failing with successfully completed actions": {
module: map[string]string{
"main.tf": `
action "act_unlinked" "hello" {}
action "act_unlinked" "world" {}
action "act_unlinked" "failure" {
config {
attr = "failure"
}
}
resource "test_object" "a" {
lifecycle {
action_trigger {
events = [before_create]
actions = [action.act_unlinked.hello, action.act_unlinked.world, action.act_unlinked.failure]
}
}
}
`,
},
expectInvokeActionCalled: true,
events: func(req providers.InvokeActionRequest) []providers.InvokeActionEvent {
if !req.PlannedActionData.IsNull() && req.PlannedActionData.GetAttr("attr").AsString() == "failure" {
return []providers.InvokeActionEvent{
providers.InvokeActionEvent_Completed{
Diagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing",
),
},
},
}
} else {
return []providers.InvokeActionEvent{
providers.InvokeActionEvent_Completed{},
}
}
},
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"Failed to apply actions before test_object.a",
`An error occured while invoking action action.act_unlinked.failure: test case for failing: this simulates a provider failing
The following actions were successfully invoked:
- action.act_unlinked.hello
- action.act_unlinked.world
As the resource did not change, these actions will be re-invoked in the next apply.`,
),
},
},
@ -208,8 +265,104 @@ resource "test_object" "a" {
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing before the action is invoked",
"Failed to apply actions before test_object.a",
"An error occured while invoking action action.act_unlinked.hello: test case for failing: this simulates a provider failing before the action is invoked\n",
),
},
},
"after_create failing": {
module: map[string]string{
"main.tf": `
action "act_unlinked" "hello" {}
resource "test_object" "a" {
lifecycle {
action_trigger {
events = [after_create]
actions = [action.act_unlinked.hello]
}
}
}
`,
},
expectInvokeActionCalled: true,
events: func(req providers.InvokeActionRequest) []providers.InvokeActionEvent {
return []providers.InvokeActionEvent{
providers.InvokeActionEvent_Completed{
Diagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing",
),
},
},
}
},
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"Failed to apply actions after test_object.a",
`An error occured while invoking action action.act_unlinked.hello: test case for failing: this simulates a provider failing
The following actions were not yet invoked:
- action.act_unlinked.hello
These actions will not be triggered in the next apply, please run "terraform invoke" to invoke them.`,
),
},
},
"after_create failing with successfully completed actions": {
module: map[string]string{
"main.tf": `
action "act_unlinked" "hello" {}
action "act_unlinked" "world" {}
action "act_unlinked" "failure" {
config {
attr = "failure"
}
}
resource "test_object" "a" {
lifecycle {
action_trigger {
events = [after_create]
actions = [action.act_unlinked.hello, action.act_unlinked.world, action.act_unlinked.failure]
}
}
}
`,
},
expectInvokeActionCalled: true,
events: func(req providers.InvokeActionRequest) []providers.InvokeActionEvent {
if !req.PlannedActionData.IsNull() && req.PlannedActionData.GetAttr("attr").AsString() == "failure" {
return []providers.InvokeActionEvent{
providers.InvokeActionEvent_Completed{
Diagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing",
),
},
},
}
} else {
return []providers.InvokeActionEvent{
providers.InvokeActionEvent_Completed{},
}
}
},
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"Failed to apply actions after test_object.a",
`An error occured while invoking action action.act_unlinked.failure: test case for failing: this simulates a provider failing
The following actions were not yet invoked:
- action.act_unlinked.failure
These actions will not be triggered in the next apply, please run "terraform invoke" to invoke them.`,
),
},
},
@ -242,7 +395,7 @@ resource "test_object" "a" {
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing before the action is invoked",
"this simulates a provider failing",
),
}
}
@ -251,10 +404,14 @@ resource "test_object" "a" {
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing before the action is invoked",
"Failed to apply actions before test_object.a",
`An error occured while invoking action action.act_unlinked.failure: test case for failing: this simulates a provider failing
The following actions were successfully invoked:
- action.act_unlinked.hello
As the resource did not change, these actions will be re-invoked in the next apply.`,
),
},
// We expect two calls but not the third one, because the second action fails
expectInvokeActionCalls: []providers.InvokeActionRequest{{
ActionType: "act_unlinked",
@ -305,7 +462,7 @@ resource "test_object" "a" {
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing before the action is invoked",
"this simulates a provider failing",
),
}
}
@ -314,8 +471,11 @@ resource "test_object" "a" {
expectDiagnostics: tfdiags.Diagnostics{
tfdiags.Sourceless(
tfdiags.Error,
"test case for failing",
"this simulates a provider failing before the action is invoked",
"Failed to apply actions before test_object.a",
`An error occured while invoking action action.act_unlinked.failure: test case for failing: this simulates a provider failing
The following actions were successfully invoked:
- action.act_unlinked.hello
As the resource did not change, these actions will be re-invoked in the next apply.`,
),
},
// We expect two calls but not the third one, because the second action fails
@ -524,8 +684,8 @@ resource "test_object" "a" {
defaultEvents = append(defaultEvents, providers.InvokeActionEvent_Completed{})
events := defaultEvents
if len(tc.events) > 0 {
events = tc.events
if tc.events != nil {
events = tc.events(req)
}
return providers.InvokeActionResponse{
@ -571,7 +731,6 @@ resource "test_object" "a" {
},
},
},
Unlinked: &providers.UnlinkedAction{},
},
},

@ -6,9 +6,12 @@ package terraform
import (
"fmt"
"sort"
"strings"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/actions"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/dag"
"github.com/hashicorp/terraform/internal/plans"
"github.com/hashicorp/terraform/internal/plans/objchange"
@ -39,10 +42,16 @@ func (n *nodeActionApply) DotNode(string, *dag.DotOpts) *dag.DotNode {
}
func (n *nodeActionApply) Execute(ctx EvalContext, _ walkOperation) (diags tfdiags.Diagnostics) {
return invokeActions(ctx, n.ActionInvocations)
return invokeActionsWithEnhancedDiagnostics(ctx, n.ActionInvocations, &n.TriggeringResourceaddrs)
}
func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationInstanceSrc) tfdiags.Diagnostics {
func invokeActionsWithEnhancedDiagnostics(ctx EvalContext, actionInvocations []*plans.ActionInvocationInstanceSrc, triggeringResourceAddrs *addrs.AbsResourceInstance) tfdiags.Diagnostics {
finishedActionInvocations, diags := invokeActions(ctx, actionInvocations)
return betterDiags(finishedActionInvocations, actionInvocations, diags, triggeringResourceAddrs)
}
func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationInstanceSrc) ([]*plans.ActionInvocationInstance, tfdiags.Diagnostics) {
var finishedActionInvocations []*plans.ActionInvocationInstance
var diags tfdiags.Diagnostics
// First we order the action invocations by their trigger block index and events list index.
// This way we have the correct order of execution.
@ -56,7 +65,7 @@ func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationI
fmt.Sprintf("Failed to find action invocation instance %s in changes.", ai.Addr),
fmt.Sprintf("The action invocation instance %s was not found in the changes for %s.", ai.Addr, ai.TriggeringResourceAddr.String()),
))
return diags
return finishedActionInvocations, diags
}
orderedActionInvocations = append(orderedActionInvocations, ai)
@ -78,7 +87,7 @@ func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationI
"Action instance not found",
"Could not find action instance for address "+invocation.Addr.String(),
))
return diags
return finishedActionInvocations, diags
}
orderedActionData[i] = ai
@ -99,7 +108,7 @@ func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationI
fmt.Sprintf("Failed to get provider for %s", ai.Addr),
fmt.Sprintf("Failed to get provider: %s", err),
))
return diags
return finishedActionInvocations, diags
}
actionSchema, ok := schema.Actions[ai.Addr.Action.Action.Type]
@ -110,7 +119,7 @@ func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationI
fmt.Sprintf("Action %s not found in provider schema", ai.Addr),
fmt.Sprintf("The action %s was not found in the provider schema for %s", ai.Addr.Action.Action.Type, actionData.ProviderAddr),
))
return diags
return finishedActionInvocations, diags
}
// We don't want to send the marks, but all marks are okay in the context of an action invocation.
@ -147,7 +156,7 @@ func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationI
diags = diags.Append(resp.Diagnostics)
if resp.Diagnostics.HasErrors() {
return diags
return finishedActionInvocations, diags
}
for event := range resp.Events {
@ -162,11 +171,9 @@ func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationI
return h.CompleteAction(hookIdentity, ev.Diagnostics.Err())
})
if ev.Diagnostics.HasErrors() {
// TODO: We would want to add some warning / error telling the user how to recover
// from this, or maybe attach this info to the diagnostics sent by the provider.
// For now we just return the diagnostics.
return diags
return finishedActionInvocations, diags
} else {
finishedActionInvocations = append(finishedActionInvocations, ai)
}
default:
panic(fmt.Sprintf("unexpected action event type %T", ev))
@ -174,7 +181,7 @@ func invokeActions(ctx EvalContext, actionInvocations []*plans.ActionInvocationI
}
}
return diags
return finishedActionInvocations, diags
}
func (n *nodeActionApply) ModulePath() addrs.Module {
@ -201,3 +208,99 @@ func (n *nodeActionApply) Actions() []addrs.ConfigAction {
}
return ret
}
func betterDiags(finishedActionInvocations []*plans.ActionInvocationInstance, allActionInvocations []*plans.ActionInvocationInstanceSrc, diags tfdiags.Diagnostics, triggeringResourceAddrs *addrs.AbsResourceInstance) tfdiags.Diagnostics {
// If everything went well, we can return the diagnostics as is.
if !diags.HasErrors() {
return diags
}
if triggeringResourceAddrs == nil {
panic("We currently don't support actions without triggering resources in this code path, this is a bug.")
}
// Something went wrong, the user might need to take action so that
// - the actions that failed or that were not executed can be retried
// - the user can undo side-effects of actions that were executed successfully before the
// failure and will be re-run in the next apply.
// We know that the last not run action invocation is the one that triggered the failure, so we can use that to inform the user.
// TODO: We should have a source range on the action invocation to use in the subject of the diagnostic.
failingActionInvocation := allActionInvocations[len(finishedActionInvocations)]
if areBeforeActionInvocations(allActionInvocations) {
// Before actions need to let the user know that they will be re-run in the next apply
alreadyRunActionText := ""
if len(finishedActionInvocations) > 0 {
alreadyRunActions := []string{}
for _, ai := range finishedActionInvocations {
alreadyRunActions = append(alreadyRunActions, fmt.Sprintf("- %s", ai.Addr))
}
alreadyRunActionText = fmt.Sprintf(`The following actions were successfully invoked:
%s
As the resource did not change, these actions will be re-invoked in the next apply.`,
strings.Join(alreadyRunActions, "\n"))
}
return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Failed to apply actions before %s", triggeringResourceAddrs),
Detail: fmt.Sprintf(
`An error occured while invoking action %s: %s
%s`,
failingActionInvocation.Addr.String(),
diags.ErrWithWarnings(),
alreadyRunActionText,
),
// TODO: Add subject here (we need to record the source range in the action invocation)
})
} else {
missingActionInvocations := allActionInvocations[len(finishedActionInvocations):]
missingActions := []string{}
for _, ai := range missingActionInvocations {
missingActions = append(missingActions, fmt.Sprintf("- %s", ai.Addr))
}
return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Failed to apply actions after %s", triggeringResourceAddrs),
Detail: fmt.Sprintf(
`An error occured while invoking action %s: %s
The following actions were not yet invoked:
%s
These actions will not be triggered in the next apply, please run "terraform invoke" to invoke them.`,
failingActionInvocation.Addr.String(),
diags.ErrWithWarnings(),
strings.Join(missingActions, "\n"),
),
// TODO: Add subject here (we need to record the source range in the action invocation)
})
}
}
// areBeforeActionInvocations checks if all action invocations are for before actions.
// It panics if the action invocations are empty or if they have different trigger events.
func areBeforeActionInvocations(actionInvocations []*plans.ActionInvocationInstanceSrc) bool {
if len(actionInvocations) == 0 {
panic("areBeforeActionInvocations called with empty actionInvocations")
}
firstEvent := actionInvocations[0].TriggerEvent
for _, ai := range actionInvocations {
if ai.TriggerEvent != firstEvent {
panic(fmt.Sprintf("areBeforeActionInvocations called with action invocations with different trigger events: %s != %s", firstEvent, ai.TriggerEvent))
}
}
switch firstEvent {
case configs.BeforeCreate, configs.BeforeUpdate, configs.BeforeDestroy:
return true
case configs.AfterCreate, configs.AfterUpdate, configs.AfterDestroy:
return false
default:
panic(fmt.Sprintf("areBeforeActionInvocations called with unknown trigger event: %s", firstEvent))
}
}

@ -213,7 +213,7 @@ func (n *NodeApplyableResourceInstance) managedResourceExecute(ctx EvalContext)
var createBeforeDestroyEnabled bool
var deposedKey states.DeposedKey
diags = diags.Append(invokeActions(ctx, n.beforeActionInvocations))
diags = diags.Append(invokeActionsWithEnhancedDiagnostics(ctx, n.beforeActionInvocations, &n.Addr))
if diags.HasErrors() {
return diags
}

Loading…
Cancel
Save