Decouple the generation of plans from the automatic use of plans

This threads a new option "Experimental" through to the engine which is used to decided if plans should be used or not. This also generates plans for any preview with --save-plan, but also now generates plans during the preview phase of up.
pulumi · Oct 28, 2022 · 81b137b · 81b137b
1 parent bd87211
commit 81b137b
Show file tree

Hide file tree

Showing 11 changed files with 43 additions and 28 deletions.
diff --git a/pkg/backend/apply.go b/pkg/backend/apply.go
@@ -220,12 +220,14 @@ func PreviewThenPromptThenExecute(ctx context.Context, kind apitype.UpdateKind,
 			return changes, res
 		}
 
-		// If we had an original plan use it, else use the newly generated plan (might be nil if we've turned
+		// If we had an original plan use it, else if experimental use the newly generated plan (might be nil if we've turned
 		// plan generation off)
 		if originalPlan != nil {
 			op.Opts.Engine.Plan = originalPlan
-		} else {
+		} else if op.Opts.Engine.Experimental {
 			op.Opts.Engine.Plan = plan
+		} else {
+			op.Opts.Engine.Plan = nil
 		}
 	}
 
@@ -235,6 +237,9 @@ func PreviewThenPromptThenExecute(ctx context.Context, kind apitype.UpdateKind,
 		DryRun:   false,
 		ShowLink: true,
 	}
+	// No need to generate a plan at this stage, there's no way for the system or user to extract the plan
+	// after here.
+	op.Opts.Engine.GeneratePlan = false
 	_, changes, res := apply(ctx, kind, stack, op, opts, nil /*events*/)
 	return changes, res
 }

diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go
@@ -258,8 +258,6 @@ type UpdateOptions struct {
 	AutoApprove bool
 	// SkipPreview, when true, causes the preview step to be skipped.
 	SkipPreview bool
-	// GeneratePlan when true cause plans to be generated.
-	GeneratePlan bool
 }
 
 // QueryOptions configures a query to operate against a backend and the engine.

diff --git a/pkg/cmd/pulumi/destroy.go b/pkg/cmd/pulumi/destroy.go
@@ -246,6 +246,7 @@ func newDestroyCmd() *cobra.Command {
 				DisableProviderPreview:    disableProviderPreview(),
 				DisableResourceReferences: disableResourceReferences(),
 				DisableOutputValues:       disableOutputValues(),
+				Experimental:              hasExperimentalCommands(),
 			}
 
 			_, res := s.Destroy(ctx, backend.UpdateOperation{

diff --git a/pkg/cmd/pulumi/import.go b/pkg/cmd/pulumi/import.go
@@ -512,6 +512,7 @@ func newImportCmd() *cobra.Command {
 				Parallel:      parallel,
 				Debug:         debug,
 				UseLegacyDiff: useLegacyDiff(),
+				Experimental:  hasExperimentalCommands(),
 			}
 
 			_, res := s.Import(ctx, backend.UpdateOperation{

diff --git a/pkg/cmd/pulumi/preview.go b/pkg/cmd/pulumi/preview.go
@@ -230,6 +230,7 @@ func newPreviewCmd() *cobra.Command {
 					// If we're trying to save a plan then we _need_ to generate it. We also turn this on in
 					// experimental mode to just get more testing of it.
 					GeneratePlan: hasExperimentalCommands() || planFilePath != "",
+					Experimental: hasExperimentalCommands(),
 				},
 				Display: displayOpts,
 			}

diff --git a/pkg/cmd/pulumi/query.go b/pkg/cmd/pulumi/query.go
@@ -66,7 +66,9 @@ func newQueryCmd() *cobra.Command {
 				return result.FromError(err)
 			}
 
-			opts.Engine = engine.UpdateOptions{}
+			opts.Engine = engine.UpdateOptions{
+				Experimental: hasExperimentalCommands(),
+			}
 
 			res := b.Query(ctx, backend.QueryOperation{
 				Proj:   proj,

diff --git a/pkg/cmd/pulumi/refresh.go b/pkg/cmd/pulumi/refresh.go
@@ -256,6 +256,7 @@ func newRefreshCmd() *cobra.Command {
 				DisableResourceReferences: disableResourceReferences(),
 				DisableOutputValues:       disableOutputValues(),
 				RefreshTargets:            deploy.NewUrnTargets(targetUrns),
+				Experimental:              hasExperimentalCommands(),
 			}
 
 			changes, res := s.Refresh(ctx, backend.UpdateOperation{

diff --git a/pkg/cmd/pulumi/up.go b/pkg/cmd/pulumi/up.go
@@ -156,9 +156,10 @@ func newUpCmd() *cobra.Command {
 			DisableOutputValues:       disableOutputValues(),
 			UpdateTargets:             deploy.NewUrnTargets(targetURNs),
 			TargetDependents:          targetDependents,
-			// If we're in experimental mode then we trigger a plan to be generated during the preview phase
-			// which will be constrained to during the update phase.
-			GeneratePlan: hasExperimentalCommands(),
+			// Trigger a plan to be generated during the preview phase which can be constrained to during the
+			// update phase.
+			GeneratePlan: true,
+			Experimental: hasExperimentalCommands(),
 		}
 
 		if planFilePath != "" {
@@ -360,6 +361,7 @@ func newUpCmd() *cobra.Command {
 			// If we're in experimental mode then we trigger a plan to be generated during the preview phase
 			// which will be constrained to during the update phase.
 			GeneratePlan: hasExperimentalCommands(),
+			Experimental: hasExperimentalCommands(),
 		}
 
 		// TODO for the URL case:

diff --git a/pkg/cmd/pulumi/watch.go b/pkg/cmd/pulumi/watch.go
@@ -136,6 +136,7 @@ func newWatchCmd() *cobra.Command {
 				DisableProviderPreview:    disableProviderPreview(),
 				DisableResourceReferences: disableResourceReferences(),
 				DisableOutputValues:       disableOutputValues(),
+				Experimental:              hasExperimentalCommands(),
 			}
 
 			res := s.Watch(ctx, backend.UpdateOperation{

diff --git a/pkg/engine/lifecycletest/update_plan_test.go b/pkg/engine/lifecycletest/update_plan_test.go
@@ -61,7 +61,7 @@ func TestPlannedUpdate(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -173,7 +173,7 @@ func TestUnplannedCreate(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -240,7 +240,7 @@ func TestUnplannedDelete(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -312,7 +312,7 @@ func TestExpectedDelete(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -381,7 +381,7 @@ func TestExpectedCreate(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -443,7 +443,7 @@ func TestPropertySetChange(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -494,7 +494,7 @@ func TestExpectedUnneededCreate(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -559,7 +559,7 @@ func TestExpectedUnneededDelete(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -636,7 +636,7 @@ func TestResoucesWithSames(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -727,7 +727,7 @@ func TestPlannedPreviews(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -812,7 +812,7 @@ func TestPlannedUpdateChangedStack(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -895,7 +895,7 @@ func TestPlannedOutputChanges(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -962,7 +962,7 @@ func TestPlannedInputOutputDifferences(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -1046,7 +1046,7 @@ func TestAliasWithPlans(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -1111,7 +1111,7 @@ func TestComputedCanBeDropped(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -1263,7 +1263,7 @@ func TestPlannedUpdateWithNondeterministicCheck(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -1337,7 +1337,7 @@ func TestPlannedUpdateWithCheckFailure(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -1397,7 +1397,7 @@ func TestPluginsAreDownloaded(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()
@@ -1474,7 +1474,7 @@ func TestProviderDeterministicPreview(t *testing.T) {
 	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
 
 	p := &TestPlan{
-		Options: UpdateOptions{Host: host, GeneratePlan: true},
+		Options: UpdateOptions{Host: host, GeneratePlan: true, Experimental: true},
 	}
 
 	project := p.GetProject()

diff --git a/pkg/engine/update.go b/pkg/engine/update.go
@@ -149,8 +149,11 @@ type UpdateOptions struct {
 	// The plan to use for the update, if any.
 	Plan *deploy.Plan
 
-	// true if plans should be generated.
+	// GeneratePlan when true cause plans to be generated, we skip this if we know their not needed (e.g. during up)
 	GeneratePlan bool
+
+	// Experimental is true if the engine is in experimental mode (i.e. PULUMI_EXPERIMENTAL was set)
+	Experimental bool
 }
 
 // HasChanges returns true if there are any non-same changes in the resulting summary.