Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: ephemeral resources #35764

Merged
merged 9 commits into from
Sep 25, 2024
Merged

core: ephemeral resources #35764

merged 9 commits into from
Sep 25, 2024

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Sep 23, 2024

Enable the use of ephemeral resources is Terraform core.

One step closer to supporting ephemeral resources, this adds the core support implementation, and connects a lot of the individual pieces needed for evaluation.

A few key points which we can address separately are

  • Deferrals are not yet accounted for in ephemeral resources
  • The prune transformer needs to take care of ephemerals too in order to continue partial destroys. This is always a very difficult part of graph resolution, so I left that as a separate task.

Implement the core nodes for ephemeral resources. This constitutes
what's needed for execution, but does not yet include all the necessary
graph transformations.
There's still some duplicated code in here, we'll try and sort that out
in a later refactor. Ephemeral resources are not currently working with
deferrals.
TODOs:
 - deferrals (still)
 - The prune transformer needs to take care of ephemerals too in order
   to continue partial destroys
 - ValidateEphemeralResourceConfig is not yet called during validate
   walk.
Call ValidateEphemeralResourcConfig during validation walk.

Found that marks don't show up during validation, which may be an HCL
bug
// First, we're going to load any instances that we have written into the
// deferrals system. A deferred resource overrides anything that might be
// in the state for the resource, so we do this first.
for key, value := range d.Evaluator.Deferrals.GetDeferredResourceInstances(addr.Absolute(d.ModulePath)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would run into an issue here since the ephemeral resource mode is not supported by the deferrals package. Or is that what you referred to in the description that deferrals are not accounted for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, this function was taken directly from the original POC. I have not considered deferrals yet in any way, so it's not expected to work at all if they are active.

@@ -213,6 +214,9 @@ func (n *nodeCloseModule) Execute(ctx EvalContext, op walkOperation) (diags tfdi
// any running plugins
diags = diags.Append(ctx.ClosePlugins())

// We also close up the ephemeral resource manager
diags = diags.Append(ctx.EphemeralResources().Close(context.TODO()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With context.TODO() we won't be able to cancel the close calls, right? Is that intended?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll have a followup to connect the cancelation context with ephemeral resources. Because this still has some of the original architecture that predates the context package, they were never connected here. (plugins sort of get the context through a side-channel rather than inline in each rpc call)

Includes a fix for this PR, we will update to a tagged version before
release.
@jbardin jbardin requested review from a team as code owners September 24, 2024 20:15
@jbardin jbardin requested review from a team and removed request for a team September 24, 2024 20:16
@jbardin
Copy link
Member Author

jbardin commented Sep 24, 2024

Since the hcl fix was merged first, I updated this PR to use the latest hcl commit rather than work around the evaluation bugs.

@jbardin jbardin merged commit a224a45 into main Sep 25, 2024
6 checks passed
@jbardin jbardin deleted the jbardin/ephemeral-core branch September 25, 2024 12:43
Copy link
Contributor

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants