From 81b137b5f902132c8b64a9012d08f74a7256b41b Mon Sep 17 00:00:00 2001 From: Fraser Waters Date: Fri, 28 Oct 2022 17:51:42 +0100 Subject: [PATCH 1/2] 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. --- pkg/backend/apply.go | 9 +++-- pkg/backend/backend.go | 2 -- pkg/cmd/pulumi/destroy.go | 1 + pkg/cmd/pulumi/import.go | 1 + pkg/cmd/pulumi/preview.go | 1 + pkg/cmd/pulumi/query.go | 4 ++- pkg/cmd/pulumi/refresh.go | 1 + pkg/cmd/pulumi/up.go | 8 +++-- pkg/cmd/pulumi/watch.go | 1 + pkg/engine/lifecycletest/update_plan_test.go | 38 ++++++++++---------- pkg/engine/update.go | 5 ++- 11 files changed, 43 insertions(+), 28 deletions(-) diff --git a/pkg/backend/apply.go b/pkg/backend/apply.go index 368567ca5064..6ba7816a14d2 100644 --- 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 index e45fc94e6be6..de5c154fb4e6 100644 --- 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 index 01d348832253..55e7ceee8f00 100644 --- 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 index 5dea64c51398..8a22e82e94fb 100644 --- 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 index e9f141722c18..0b4987de6195 100644 --- 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 index a1b203012368..2e422d932a54 100644 --- 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 index 25ead2fe2cb4..d2ac6b3a669b 100644 --- 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 index 7e109e3c89fa..f77f22542861 100644 --- 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 index eeb1df36ace8..0af305e87850 100644 --- 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 index 875bd52b2a9a..9ee30cbd0c7c 100644 --- 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 index bc7fa1fbabd2..16f2ccf9cf9b 100644 --- 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. From 0b3fdaf141c6f47500443de2c95c1fa84fb35cd5 Mon Sep 17 00:00:00 2001 From: Fraser Waters Date: Wed, 2 Nov 2022 14:02:13 +0000 Subject: [PATCH 2/2] Handle None being passed to register_resource_outputs --- ...handle-none-being-passed-to-register_resource_outputs.yaml | 4 ++++ sdk/python/lib/pulumi/runtime/resource.py | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changelog/pending/20221102--sdk-python--handle-none-being-passed-to-register_resource_outputs.yaml diff --git a/changelog/pending/20221102--sdk-python--handle-none-being-passed-to-register_resource_outputs.yaml b/changelog/pending/20221102--sdk-python--handle-none-being-passed-to-register_resource_outputs.yaml new file mode 100644 index 000000000000..08c8d2111bb8 --- /dev/null +++ b/changelog/pending/20221102--sdk-python--handle-none-being-passed-to-register_resource_outputs.yaml @@ -0,0 +1,4 @@ +changes: +- type: fix + scope: sdk/python + description: Handle None being passed to register_resource_outputs. diff --git a/sdk/python/lib/pulumi/runtime/resource.py b/sdk/python/lib/pulumi/runtime/resource.py index 6941325f1468..a9a310e0e966 100644 --- a/sdk/python/lib/pulumi/runtime/resource.py +++ b/sdk/python/lib/pulumi/runtime/resource.py @@ -682,7 +682,9 @@ def register_resource_outputs( ): async def do_register_resource_outputs(): urn = await res.urn.future() - serialized_props = await rpc.serialize_properties(outputs, {}) + # serialize_properties expects a collection (empty is fine) but not None, but this is called pretty + # much directly by users who could pass None in (although the type hints say they shouldn't). + serialized_props = await rpc.serialize_properties(outputs or {}, {}) log.debug( f"register resource outputs prepared: urn={urn}, props={serialized_props}" )