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

Bug/support running apply destroy in reverse order #2445

Conversation

jlepere-everlaw
Copy link
Contributor

@jlepere-everlaw jlepere-everlaw commented Feb 10, 2023

Description

Invert module group ordering for apply -destroy commands, mirroring the functionality for destroy commands.

Testing

This has been tested by:

  • $ go test -v ./...
  • Validating the group ordering is now correct with $ go run main.go run-all apply -destroy --terragrunt-working-dir ...

TODOs

I'm not sure if documentation should be updated. Maybe somewhere here?

Release Notes (draft)

Added support for inverting module group ordering for apply -destroy commands.

@jlepere-everlaw jlepere-everlaw force-pushed the bug/support-running-apply-destroy-in-reverse-order branch from 5127f5d to 0f67971 Compare February 10, 2023 17:39
@jlepere-everlaw jlepere-everlaw changed the title [WIP] Bug/support running apply destroy in reverse order Bug/support running apply destroy in reverse order Feb 10, 2023
@denis256
Copy link
Member

[INFO] Initializing environment for https://github.com/gruntwork-io/pre-commit.
Terraform fmt............................................................Passed
goimports................................................................Failed
- hook id: goimports
- exit code: 2

configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
configstack/stack_test.go:119:51: expected type, found newline
[terragrunt run-all apply -auto-approve --terragrunt-log-level debug --terragrunt-non-interactive --terragrunt-working-dir C:\test\infrastructure-modules\fixture-source-map\multiple-match]
time=2023-02-12T20:29:50Z level=error msg=failed to get console mode: The handle is invalid.

time=2023-02-12T20:29:50Z level=error msg=failed to get console mode: The handle is invalid.

time=2023-02-12T20:29:50Z level=error msg=failed to get console mode: The handle is invalid.

@jlepere-everlaw jlepere-everlaw force-pushed the bug/support-running-apply-destroy-in-reverse-order branch from 0f67971 to f77d299 Compare February 13, 2023 21:17
@jlepere-everlaw
Copy link
Contributor Author

jlepere-everlaw commented Feb 13, 2023

@denis256, do you mind rerunning the tests? I don't believe I have access to.

@denis256
Copy link
Member

I was wondering in which case is required to invert module group ordering?
In case if is required to destroy first and apply back modules?

@jlepere-everlaw
Copy link
Contributor Author

jlepere-everlaw commented Feb 14, 2023

I was wondering in which case is required to invert module group ordering? In case if is required to destroy first and apply back modules?

I think we want to invert on any "destroy", right? destroy is actually an alias for apply -destroy.

@kansberry-kr
Copy link

kansberry-kr commented Feb 17, 2023

We are running into this same issue. Our workflows perform a "run-all plan" with the destroy option and then use the "destroy" plan generated to do a "run-all apply", which should delete module state in the reverse order in which they were created. I posted issue #2453 on this yesterday. I will close that issue as I can see you are already working on this problem.

@jlepere-everlaw
Copy link
Contributor Author

@denis256, light bump on this. Thanks!

@giladdi-tr
Copy link

Any updates?

@tomcart90
Copy link

We ran into this today too whilst doing a run-all plan -destroy.

@yhakbar
Copy link
Collaborator

yhakbar commented Aug 1, 2024

Hey @jlepere-everlaw

Sorry we didn't get to this PR before it went stale. It does seem like a very sensible adjustment to how Terragrunt works, and would make it more consistent.

If you would still like to see this make its way to into Terragrunt, please create a new pull request.

To maximize the likelihood that your next pull request is merged, please make sure that:

  1. You create an issue documenting the problem.
  2. All tests pass (including pre-commits).
  3. Docs are updated in addition to adding the functionality you desire.

@yhakbar yhakbar closed this Aug 1, 2024
@giladdi-tr
Copy link

giladdi-tr commented Aug 1, 2024

Could you please share the link to the new issue here?
@yhakbar @jlepere-everlaw

@jlepere-everlaw jlepere-everlaw deleted the bug/support-running-apply-destroy-in-reverse-order branch August 1, 2024 15:56
@riccardomassullo
Copy link

So? Any news about this bug? Using Terragrunt version 0.70.0, in my case, still results in a broken order for destroy and apply -destroy.

@yhakbar
Copy link
Collaborator

yhakbar commented Aug 29, 2024

I didn't create an issue to report this, but if someone from the community would like to do so and submit a pull request to address it, I would review it. I'm fairly confident that @jlepere-everlaw 's implementation here will meet the need, so if someone can re-implement it following the guidance outlined here:
#2445 (comment)

That will help it get merged.

@riccardomassullo
Copy link

I didn't create an issue to report this, but if someone from the community would like to do so and submit a pull request to address it, I would review it. I'm fairly confident that @jlepere-everlaw 's implementation here will meet the need, so if someone can re-implement it following the guidance outlined here:
#2445 (comment)

That will help it get merged.

Issue reopened as #3371

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants