correct logic for plan skipping

This also differentiates between to_dir and from_dir plan errors.
minamijoyo · Aug 6, 2023 · 8163471 · 8163471
1 parent f05adc9
commit 8163471
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 8 deletions.
diff --git a/tfmigrate/multi_state_migrator.go b/tfmigrate/multi_state_migrator.go
@@ -152,41 +152,41 @@ func (m *MultiStateMigrator) plan(ctx context.Context) (*tfexec.State, *tfexec.S
 	}
 
 	if m.fromSkipPlan {
+		log.Printf("[INFO] [migrator@%s] skipping check diffs\n", m.fromTf.Dir())
+	} else {
 		// check if a plan in fromDir has no changes.
 		log.Printf("[INFO] [migrator@%s] check diffs\n", m.fromTf.Dir())
 		_, err = m.fromTf.Plan(ctx, fromCurrentState, planOpts...)
 		if err != nil {
 			if exitErr, ok := err.(tfexec.ExitError); ok && exitErr.ExitCode() == 2 {
 				if !m.force {
 					log.Printf("[ERROR] [migrator@%s] unexpected diffs\n", m.fromTf.Dir())
-					return nil, nil, fmt.Errorf("terraform plan command returns unexpected diffs: %s", err)
+					return nil, nil, fmt.Errorf("terraform plan command returns unexpected diffs in %s from_dir: %s", m.fromTf.Dir(), err)
 				}
 				log.Printf("[INFO] [migrator@%s] unexpected diffs, ignoring as force option is true: %s", m.fromTf.Dir(), err)
 			} else {
 				return nil, nil, err
 			}
 		}
-	} else {
-		log.Printf("[INFO] [migrator@%s] skipping check diffs\n", m.fromTf.Dir())
 	}
 
 	if m.toSkipPlan {
+		log.Printf("[INFO] [migrator@%s] skipping check diffs\n", m.toTf.Dir())
+	} else {
 		// check if a plan in toDir has no changes.
 		log.Printf("[INFO] [migrator@%s] check diffs\n", m.toTf.Dir())
 		_, err = m.toTf.Plan(ctx, toCurrentState, planOpts...)
 		if err != nil {
 			if exitErr, ok := err.(tfexec.ExitError); ok && exitErr.ExitCode() == 2 {
 				if !m.force {
 					log.Printf("[ERROR] [migrator@%s] unexpected diffs\n", m.toTf.Dir())
-					return nil, nil, fmt.Errorf("terraform plan command returns unexpected diffs: %s", err)
+					return nil, nil, fmt.Errorf("terraform plan command returns unexpected diffs in %s to_dir: %s", m.toTf.Dir(), err)
 				}
 				log.Printf("[INFO] [migrator@%s] unexpected diffs, ignoring as force option is true: %s", m.toTf.Dir(), err)
 			} else {
 				return nil, nil, err
 			}
 		}
-	} else {
-		log.Printf("[INFO] [migrator@%s] skipping check diffs\n", m.toTf.Dir())
 	}
 
 	return fromCurrentState, toCurrentState, nil

diff --git a/tfmigrate/multi_state_migrator_test.go b/tfmigrate/multi_state_migrator_test.go
@@ -310,8 +310,8 @@ resource "null_resource" "qux" {}
 	if err != nil {
 		t.Fatalf("failed to run PlanHasChange in fromDir: %s", err)
 	}
-	if !fromChanged {
-		t.Fatalf("expect to have changes in fromDir")
+	if fromChanged {
+		t.Fatalf("expect not to have changes in fromDir")
 	}
 
 	toChanged, err := toTf.PlanHasChange(ctx, nil)