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

Basic support for provisioners in "removed" blocks #34753

Closed
wants to merge 3 commits into from

Conversation

apparentlymart
Copy link
Contributor

During the apply phase, we'll check if there are provisioners either in the matching resource block or the matching removed block -- whichever of the two is present -- and execute the destroy-time subset of them either way.

This also establishes a standard way to attach a removed block to a NodeResourceAbstract when one is defined, which is likely to be useful for supporting other resource-related meta arguments in removed blocks in future.

removed {
  from = aws_instance.example

  provisioner "local-exec" {
    # All provisioners must have when = destroy in this context,
    # because "removed" resources can only be destroyed.
    when = destroy

    # ...
  }
}

This code is currently accessible only to modules that opt in to the removed_provisioners language experiment, and therefore only available in alpha releases.


One known limitation and design question from this initial implementation is:

How should each.key, each.value, and count.index behave when used as part of a provisioner configuration in a removed block?

This is a tricky question because whereas a resource block allows us to determine from the configuration whether we're using count, for_each, or neither -- and treat invalid references as an error -- removed blocks must accept whatever happens to be in the state and so in unusual cases there might even be a mixture of numeric instance keys and string instance keys for the same resource, making it impossible to write a provisioner configuration that would work with both.


Other notes/questions for future work:

  • Should it be invalid to combine a destroy-time provisioner with destroy = false? Or should that instead mean "run the provisioner just before forgetting the object"?
  • Can we and should we provide a new command like terraform rm aws_instance.example that would automatically replace the resource "aws_instance" "example" block with a matching removed block, and copy over its destroy-time provisioners, to avoid the need to migrate them over manually?

This is motivated by #34711 and part of #13549 .

This new experiment keyword represents being able to declare provisioners
in "removed" blocks, so that it's possible to retain a destroy-time
provisioner even when the resource it belonged to is no longer declared
in the configuration.

However, that's not yet implemented as of this commit. The implementation
associated with this keyword will follow in subsequent commits.
When the removed_provisioners experiment is active, removed blocks
referring to managed resources are allowed to include "connection" and
"provisioner" blocks, as long as all of the "provisioner" blocks specify
when = destroy to indicate that they should execute as part of the
resource's "destroy" action.

This commit only deals with parsing the configuration. The logic to react
to this during the apply phase will follow in later commits.
During the apply phase, we'll check if there are provisioners either in
the matching "resource" block or the matching "removed" block -- whichever
of the two is present -- and execute the destroy-time subset of them
either way.

This also establishes a standard way to attach a "removed" block to a
NodeResourceAbstract when one is defined, which is likely to be useful
for supporting other resource-related meta arguments in "removed" blocks
in future.

This code is currently accessible only to modules that opt in to the
"removed_provisioners" language experiment, and therefore only available
in alpha releases.

One known limitation and design question from this initial implementation
is: how should each.key, each.value, and count.index behave when used as
part of a provisioner configuration in a "removed" block? This is a tricky
question because whereas a "resource" block allows us to determine from
the configuration whether we're using count, for_each, or neither, removed
blocks must accept whatever happens to be in the state and so in unusual
cases there might even be a mixture of numeric instance keys and string
instance keys for the same resource, making it impossible to write a
provisioner configuration that would work with both.
@apparentlymart
Copy link
Contributor Author

Others on the team are considering implementing something like what I prototyped here, so I'm going to close this draft with the hope that it'll be replaced with a more mergeable equivalent soon. 😀

Copy link

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 Jun 21, 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.

3 participants