From c454c2fd10534abe666c879b23de3859b738af79 Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Wed, 31 Jul 2024 19:37:25 +0530 Subject: [PATCH] Use precomputed terraform plan for `bundle deploy` (#1640) # Changes With https://github.com/databricks/cli/pull/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 https://github.com/databricks/cli/pull/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. --- bundle/deploy/terraform/apply.go | 21 ++++++------ bundle/deploy/terraform/destroy.go | 46 --------------------------- bundle/deploy/terraform/state_push.go | 8 +++++ bundle/phases/deploy.go | 5 ++- bundle/phases/destroy.go | 2 +- 5 files changed, 25 insertions(+), 57 deletions(-) delete mode 100644 bundle/deploy/terraform/destroy.go diff --git a/bundle/deploy/terraform/apply.go b/bundle/deploy/terraform/apply.go index e4acda85..e52d0ca8 100644 --- a/bundle/deploy/terraform/apply.go +++ b/bundle/deploy/terraform/apply.go @@ -4,7 +4,6 @@ import ( "context" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" "github.com/hashicorp/terraform-exec/tfexec" @@ -17,28 +16,32 @@ func (w *apply) Name() string { } func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + // return early if plan is empty + if b.Plan.IsEmpty { + log.Debugf(ctx, "No changes in plan. Skipping terraform apply.") + return nil + } + tf := b.Terraform if tf == nil { return diag.Errorf("terraform not initialized") } - cmdio.LogString(ctx, "Deploying resources...") - - err := tf.Init(ctx, tfexec.Upgrade(true)) - if err != nil { - return diag.Errorf("terraform init: %v", err) + if b.Plan.Path == "" { + return diag.Errorf("no plan found") } - err = tf.Apply(ctx) + // Apply terraform according to the computed plan + err := tf.Apply(ctx, tfexec.DirOrPlan(b.Plan.Path)) if err != nil { return diag.Errorf("terraform apply: %v", err) } - log.Infof(ctx, "Resource deployment completed") + log.Infof(ctx, "terraform apply completed") return nil } -// Apply returns a [bundle.Mutator] that runs the equivalent of `terraform apply` +// Apply returns a [bundle.Mutator] that runs the equivalent of `terraform apply ./plan` // from the bundle's ephemeral working directory for Terraform. func Apply() bundle.Mutator { return &apply{} diff --git a/bundle/deploy/terraform/destroy.go b/bundle/deploy/terraform/destroy.go deleted file mode 100644 index 9c63a0b3..00000000 --- a/bundle/deploy/terraform/destroy.go +++ /dev/null @@ -1,46 +0,0 @@ -package terraform - -import ( - "context" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/log" - "github.com/hashicorp/terraform-exec/tfexec" -) - -type destroy struct{} - -func (w *destroy) Name() string { - return "terraform.Destroy" -} - -func (w *destroy) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - // return early if plan is empty - if b.Plan.IsEmpty { - log.Debugf(ctx, "No resources to destroy in plan. Skipping destroy.") - return nil - } - - tf := b.Terraform - if tf == nil { - return diag.Errorf("terraform not initialized") - } - - if b.Plan.Path == "" { - return diag.Errorf("no plan found") - } - - // Apply terraform according to the computed destroy plan - err := tf.Apply(ctx, tfexec.DirOrPlan(b.Plan.Path)) - if err != nil { - return diag.Errorf("terraform destroy: %v", err) - } - return nil -} - -// Destroy returns a [bundle.Mutator] that runs the conceptual equivalent of -// `terraform destroy ./plan` from the bundle's ephemeral working directory for Terraform. -func Destroy() bundle.Mutator { - return &destroy{} -} diff --git a/bundle/deploy/terraform/state_push.go b/bundle/deploy/terraform/state_push.go index b50983bd..6cdde137 100644 --- a/bundle/deploy/terraform/state_push.go +++ b/bundle/deploy/terraform/state_push.go @@ -2,6 +2,8 @@ package terraform import ( "context" + "errors" + "io/fs" "os" "path/filepath" @@ -34,6 +36,12 @@ func (l *statePush) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic // Expect the state file to live under dir. local, err := os.Open(filepath.Join(dir, TerraformStateFileName)) + if errors.Is(err, fs.ErrNotExist) { + // The state file can be absent if terraform apply is skipped because + // there are no changes to apply in the plan. + log.Debugf(ctx, "Local terraform state file does not exist.") + return nil + } if err != nil { return diag.FromErr(err) } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index c68153f2..6929f74b 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -92,7 +92,10 @@ func Deploy() bundle.Mutator { // Core mutators that CRUD resources and modify deployment state. These // mutators need informed consent if they are potentially destructive. deployCore := bundle.Defer( - terraform.Apply(), + bundle.Seq( + bundle.LogString("Deploying resources..."), + terraform.Apply(), + ), bundle.Seq( terraform.StatePush(), terraform.Load(), diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index bd99af78..01b27667 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -82,7 +82,7 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle) (bool, error) { func Destroy() bundle.Mutator { // Core destructive mutators for destroy. These require informed user consent. destroyCore := bundle.Seq( - terraform.Destroy(), + terraform.Apply(), terraform.StatePush(), files.Delete(), bundle.LogString("Destroy complete!"),