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

Feature request: ability to skip plan on (e.g. force) migrations #53

Closed
dominics opened this issue Nov 18, 2021 · 6 comments
Closed

Feature request: ability to skip plan on (e.g. force) migrations #53

dominics opened this issue Nov 18, 2021 · 6 comments

Comments

@dominics
Copy link

dominics commented Nov 18, 2021

Thanks for the very useful tool.

Background

I'm using the force option in most of my migrations, because while I appreciate the safety of planning and looking for no changes, I find I often want to break up my migration files, and end up applying more than one for the same TF code change (i.e. the TF code has changes A, B, C, and there are three migration files that correspond; if tfmigrate goes to plan before all have happened it'll show "expected" diff changes).

Another piece of context: I have a Terraform configuration that can take three minutes to perform terraform plan (it's quite slow 😢 ), even with TF_CLI_ARGS_plan="-target=module.blah.module.blah" to try to speed it up (🤔 I guess I should try TF_CLI_ARGS_plan="-refresh=false" too while I'm at it - most of the plan time is state refresh time.)

Request

  1. While tfmigrate is running plan, there's no log feedback, even at trace level (it looks hung at [INFO] [migrator@.] check diffs, even though it's busy running plan in the background), and tfmigrate doesn't output what terraform plan commands it is running.
    • It would be nice to have some indication of progress
  2. (I assume) the plan information isn't used anyway (because force = true) - and this happens for each migration (so if I'm splitting a migration into 3 logical "parts", to apply all 3 will cost me 3 times the terraform plan time)
    • It would be nice to be able to skip the plan, and have a mode to use tfmigrate to only apply state migrations (without checking their effects in the same tool)

Workaround

In the meantime, I figure I can make an exec wrapper that just exits when asked to plan, and use it with TFMIGRATE_EXEC_PATH - so I'm not completely blocked or anything

@minamijoyo
Copy link
Owner

@dominics Thank you for your proposal!

For 2, I feel skipping a plan phase is unsafe. If you don't need a plan, why don't you use just terraform state commands directly? I'd like to discuss why the plan is useless in your case. If we had a feature for applying multiple migrations in a single transaction, we could validate the plan once at the end. Does it make sense for you?

For 1, asynchronous feedback for terraform command sounds great, but I can image that it requires redesigning internal implementations and it would be a non-trivial work. If you really need it, please split it into a new issue so that we can track it separately.

P.S. A tip for speeding up a refresh phase on terraform plan:
Have you tried terraform plan -parallelism=n flag? https://www.terraform.io/docs/cli/commands/plan.html#parallelism-n
Although it highly depends on your environment, it's worth trying.

@dominics
Copy link
Author

why don't you use just terraform state commands directly?

We still get a lot of mileage out of:

  • having the migration commands specified in a nice format that's right there beside the TF code being affected (as opposed to e.g. terraform state commands in bash I guess? We weren't really doing tf gitops for state migrations before adopting tfmigrate - just comments in PRs about what commands were necessary - so we find it useful just for that, even before we do CI/CD integration)
  • having the history mode figure out which migrations need to be run (although we could use a generic non-tf-aware framework for that)

If we had a feature for applying multiple migrations in a single transaction, we could validate the plan once at the end. Does it make sense for you?

Yes, this would be a nice improvement

For 1, asynchronous feedback for terraform command sounds great, but I can image that it requires redesigning internal implementations and it would be a non-trivial work. If you really need it, please split it into a new issue so that we can track it separately.

I'll open another issue if it continues to be an annoyance - it's fine for now, and I don't really need it.

I guess the only thing I'd ask is: might it be possible to output the debug message about the command that's being run before that command is finished running (not all the output, just the command being exec'd?) - that would have been sufficient to reassure me that the "pause" I was observing was expected/normal/plan time (ps -f also reassured me anyway, so again not a very urgent bit of feedback 😄 )

2021/11/18 16:49:37 [INFO] [migrator@.] check diffs
2021/11/18 16:51:00 [DEBUG] [executor@.]$ terraform plan -state=<tmp> [...]

-parallelism

Thanks for the prompt to retry -parallelism in my environment (we had it tuned down due to provider bugs) - TF_CLI_ARGS_plan='-refresh=false -target=module.something -parallelism=20' reduces plan time by 90%+ (parallelism is the smallest of those improving flags, but every bit helps)

minamijoyo added a commit that referenced this issue Nov 24, 2021
Derived from:
#53 (comment)

If teraform plan takes a long time, it looks like tfmigrate plan is hanging.
To help understand what's happening, log a command string before executing it.
@minamijoyo
Copy link
Owner

I guess the only thing I'd ask is: might it be possible to output the debug message about the command that's being run before that command is finished running (not all the output, just the command being exec'd?)

It sounds a reasonable improvement. I've fixed it in #55.

If we had a feature for applying multiple migrations in a single transaction, we could validate the plan once at the end. Does it make sense for you?

Yes, this would be a nice improvement

OK. Although I can't start working on this feature right away, I'll plan to extend internal architecture so that multiple migrations can be applied in a single transaction.

@minamijoyo
Copy link
Owner

@dominics I've cut a new release v0.2.12, which includes #55.

@jonatan-b-kr
Copy link

OK. Although I can't start working on this feature right away, I'll plan to extend internal architecture so that multiple migrations can be applied in a single transaction.

Please. this is something that we also need on my team, as we have resources in multiple states in azure storage backends that we need to put into a single state on terraform cloud.

this backends are separated by: resource group, subnet, aks cluster, kubernetes resources in different root modules/folders.
we also have this set of backends multiplied by cluster (we have a few).
This feature would allow us to migrate to TFC very easy from our current terraform architecture to a more unified one.

@minamijoyo
Copy link
Owner

I am hesitant to implement the ability to skip the plan phase as originally proposed, but grouping multiple migrations into a single transaction is worth considering. Even though I can't start working on this feature right away, please separate this as a new feature request if necessarily. Thanks!

@minamijoyo minamijoyo closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2023
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

No branches or pull requests

3 participants