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

Use precomputed terraform plan for bundle deploy #1640

Merged
merged 89 commits into from
Jul 31, 2024

Conversation

shreyas-goenka
Copy link
Contributor

@shreyas-goenka shreyas-goenka commented Jul 31, 2024

Changes

With #1413 we started to compute and partially print the plan if it contained deletion of UC schemas. This PR uses the precomputed plan to avoid double planning when actually doing the terraform plan.

This fixes a performance regression introduced in #1413.

Tests

Tested manually.

  1. Verified bundle deployment still works and deploys resources.
  2. Verified that the precomputed plan is indeed being used by attaching a debugger and removing the plan file right before the terraform apply process is spawned and asserting that terraform apply fails because the plan is not found.

…ction and modify all diagnostics paths to be relative to the bundle root path
@shreyas-goenka
Copy link
Contributor Author

Triggered a round of nightlies on this PR.

@shreyas-goenka shreyas-goenka marked this pull request as ready for review July 31, 2024 12:36
@shreyas-goenka
Copy link
Contributor Author

Waiting for the nightlies to pass before merging.

@shreyas-goenka
Copy link
Contributor Author

The test for deploying empty bundles failed. This is because we used to always run terraform apply, which generated a barebones tfstate, but now we do not run terraform apply if the plan is empty.

Fixed by handing the case of state file missing in the terraform.StatePush mutator.

Triggering another round of the nightlies.

@shreyas-goenka
Copy link
Contributor Author

The night are green now.

@shreyas-goenka shreyas-goenka added this pull request to the merge queue Jul 31, 2024
Merged via the queue into main with commit c454c2f Jul 31, 2024
5 checks passed
@shreyas-goenka shreyas-goenka deleted the fix-double-planning branch July 31, 2024 14:16
andrewnester added a commit that referenced this pull request Jul 31, 2024
Bundles:
 * Add resource for UC schemas to DABs ([#1413](#1413)).

Internal:
 * Use dynamic walking to validate unique resource keys ([#1614](#1614)).
 * Regenerate TF schema ([#1635](#1635)).
 * Add upgrade and upgrade eager flags to pip install call ([#1636](#1636)).
 * Added test for negation pattern in sync include exclude section ([#1637](#1637)).
 * Use precomputed terraform plan for `bundle deploy` ([#1640](#1640)).
@andrewnester andrewnester mentioned this pull request Jul 31, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jul 31, 2024
Bundles:
* Add resource for UC schemas to DABs
([#1413](#1413)).

Internal:
* Use dynamic walking to validate unique resource keys
([#1614](#1614)).
* Regenerate TF schema
([#1635](#1635)).
* Add upgrade and upgrade eager flags to pip install call
([#1636](#1636)).
* Added test for negation pattern in sync include exclude section
([#1637](#1637)).
* Use precomputed terraform plan for `bundle deploy`
([#1640](#1640)).
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.

2 participants