From d18638bab33bfbb58a7a3996283fcd94abb86978 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 25 Sep 2024 11:45:57 -0400 Subject: [PATCH 1/2] expose StopContext in EvalContext The existing Stopped method predates the use of a context, returning a simple channel for signaling cancellation. The method was not used however, so we can just update it to return a context to give us access to cancellation for ephemeral values. --- internal/terraform/eval_context.go | 8 +++++--- internal/terraform/eval_context_builtin.go | 6 +++--- internal/terraform/eval_context_mock.go | 12 +++++++----- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/internal/terraform/eval_context.go b/internal/terraform/eval_context.go index b78fec4c0e..e88e24f2c0 100644 --- a/internal/terraform/eval_context.go +++ b/internal/terraform/eval_context.go @@ -4,6 +4,8 @@ package terraform import ( + "context" + "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" @@ -28,9 +30,9 @@ import ( // EvalContext is the interface that is given to eval nodes to execute. type EvalContext interface { - // Stopped returns a channel that is closed when evaluation is stopped - // via Terraform.Context.Stop() - Stopped() <-chan struct{} + // Stopped returns a context that is canceled when evaluation is stopped via + // Terraform.Context.Stop() + StopCtx() context.Context // Path is the current module path. Path() addrs.ModuleInstance diff --git a/internal/terraform/eval_context_builtin.go b/internal/terraform/eval_context_builtin.go index bad4a5c7df..2376d32961 100644 --- a/internal/terraform/eval_context_builtin.go +++ b/internal/terraform/eval_context_builtin.go @@ -102,13 +102,13 @@ func (ctx *BuiltinEvalContext) withScope(scope evalContextScope) EvalContext { return &newCtx } -func (ctx *BuiltinEvalContext) Stopped() <-chan struct{} { +func (ctx *BuiltinEvalContext) StopCtx() context.Context { // This can happen during tests. During tests, we just block forever. if ctx.StopContext == nil { - return nil + return context.TODO() } - return ctx.StopContext.Done() + return ctx.StopContext } func (ctx *BuiltinEvalContext) Hook(fn func(Hook) (HookAction, error)) error { diff --git a/internal/terraform/eval_context_mock.go b/internal/terraform/eval_context_mock.go index 48015b729d..236d7260cc 100644 --- a/internal/terraform/eval_context_mock.go +++ b/internal/terraform/eval_context_mock.go @@ -4,6 +4,8 @@ package terraform import ( + "context" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcldec" "github.com/zclconf/go-cty/cty" @@ -31,8 +33,8 @@ import ( // MockEvalContext is a mock version of EvalContext that can be used // for tests. type MockEvalContext struct { - StoppedCalled bool - StoppedValue <-chan struct{} + StopCtxCalled bool + StopCtxValue context.Context HookCalled bool HookHook Hook @@ -164,9 +166,9 @@ type MockEvalContext struct { // MockEvalContext implements EvalContext var _ EvalContext = (*MockEvalContext)(nil) -func (c *MockEvalContext) Stopped() <-chan struct{} { - c.StoppedCalled = true - return c.StoppedValue +func (c *MockEvalContext) StopCtx() context.Context { + c.StopCtxCalled = true + return c.StopCtxValue } func (c *MockEvalContext) Hook(fn func(Hook) (HookAction, error)) error { From cd2631d657c1d22fc1c61de0b8c51230f53255b7 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 25 Sep 2024 12:17:04 -0400 Subject: [PATCH 2/2] make ephemeral calls cancellable --- internal/resources/ephemeral/ephemeral_resources.go | 7 +++++-- internal/terraform/eval_context_mock.go | 5 ++++- internal/terraform/node_module_expand.go | 3 +-- internal/terraform/node_resource_ephemeral.go | 10 +++------- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/resources/ephemeral/ephemeral_resources.go b/internal/resources/ephemeral/ephemeral_resources.go index 3474bc000f..773a3d59b3 100644 --- a/internal/resources/ephemeral/ephemeral_resources.go +++ b/internal/resources/ephemeral/ephemeral_resources.go @@ -140,8 +140,11 @@ func (r *Resources) Close(ctx context.Context) tfdiags.Diagnostics { r.mu.Lock() defer r.mu.Unlock() - // We might be closing due to a context cancellation, but we still need - // to be able to make non-canceled Close requests. + // We might be closing due to a context cancellation, but we still need to + // be able to make non-canceled Close requests. + // + // TODO: if we're going to ignore the cancellation to ensure that Close is + // always called, should we add some sort of timeout? ctx = context.WithoutCancel(ctx) var diags tfdiags.Diagnostics diff --git a/internal/terraform/eval_context_mock.go b/internal/terraform/eval_context_mock.go index 236d7260cc..47daaceadb 100644 --- a/internal/terraform/eval_context_mock.go +++ b/internal/terraform/eval_context_mock.go @@ -168,7 +168,10 @@ var _ EvalContext = (*MockEvalContext)(nil) func (c *MockEvalContext) StopCtx() context.Context { c.StopCtxCalled = true - return c.StopCtxValue + if c.StopCtxValue != nil { + return c.StopCtxValue + } + return context.TODO() } func (c *MockEvalContext) Hook(fn func(Hook) (HookAction, error)) error { diff --git a/internal/terraform/node_module_expand.go b/internal/terraform/node_module_expand.go index 5a86c7f4f9..e18a135fbc 100644 --- a/internal/terraform/node_module_expand.go +++ b/internal/terraform/node_module_expand.go @@ -4,7 +4,6 @@ package terraform import ( - "context" "log" "github.com/hashicorp/terraform/internal/addrs" @@ -215,7 +214,7 @@ func (n *nodeCloseModule) Execute(ctx EvalContext, op walkOperation) (diags tfdi diags = diags.Append(ctx.ClosePlugins()) // We also close up the ephemeral resource manager - diags = diags.Append(ctx.EphemeralResources().Close(context.TODO())) + diags = diags.Append(ctx.EphemeralResources().Close(ctx.StopCtx())) switch op { case walkApply, walkDestroy: diff --git a/internal/terraform/node_resource_ephemeral.go b/internal/terraform/node_resource_ephemeral.go index 01798506aa..ef9a9029bd 100644 --- a/internal/terraform/node_resource_ephemeral.go +++ b/internal/terraform/node_resource_ephemeral.go @@ -124,12 +124,8 @@ func ephemeralResourceOpen(ctx EvalContext, inp ephemeralResourceInput) tfdiags. provider: provider, internal: resp.InternalContext, } - // TODO: What can we use as a signal to cancel the context we're passing in - // here, so that the object will stop renewing things when we start shutting - // down? - // TODO: The context Stopped channel should probably be updated - // finally to a Context - ephemerals.RegisterInstance(context.TODO(), inp.addr, ephemeral.ResourceInstanceRegistration{ + + ephemerals.RegisterInstance(ctx.StopCtx(), inp.addr, ephemeral.ResourceInstanceRegistration{ Value: resultVal, ConfigBody: config.Config, Impl: impl, @@ -172,7 +168,7 @@ func (n *nodeEphemeralResourceClose) ModulePath() addrs.Module { func (n *nodeEphemeralResourceClose) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics { log.Printf("[TRACE] nodeEphemeralResourceClose: closing all instances of %s", n.addr) resources := ctx.EphemeralResources() - return resources.CloseInstances(context.TODO(), n.addr) + return resources.CloseInstances(ctx.StopCtx(), n.addr) } // ephemeralResourceInstImpl implements ephemeral.ResourceInstance as an