Add `replaceOnChanges` resource option #7226

lukehoban · 2021-06-07T00:38:11Z

Adds a new resource option to force replacement when certain properties report changes, even if the resource provider itself does not require a replacement.

Also supports replaceOnChanges: ["*"] to force any update to be a replacement instead. This includes updates triggered by initialization errors on previous create/update operations even when no inputs have changed (as a more general fix for things like pulumi/pulumi-kubernetes#856).

Fixes #6753.
Addresses pulumi/pulumi-kubernetes#1007 (though via a resource option instead of a Kubernetes-specific annotation).

TODO:

Add Go, .NET SDK support
Add language-SDK-level test cases for TypeScript, Go and .NET
Review versioning (new CLI + old SDK, old SDK + new CLI) considerations

Future work:

Add support to codegen (likely along with adding all the remaining resource options not yet supported there)

Without replaceOnChanges:

const bucket = new aws.s3.Bucket("my-bucket", {
   website: {
       indexDocument: "index2.html",
   },
});

$ pulumi preview --non-interactive
Previewing update (dev)
 
View Live: https://app.pulumi.com/lukehoban/replaceonupdate/dev/previews/abebc9fb-2f9d-4ad8-802e-f5da834562d7
 
 
   pulumi:pulumi:Stack replaceonupdate-dev running
~  aws:s3:Bucket my-bucket update [diff: ~website]
   pulumi:pulumi:Stack replaceonupdate-dev 

Resources:
   ~ 1 to update
   1 unchanged

With replaceOnChanges:

const bucket = new aws.s3.Bucket("my-bucket", {
   website: {
       indexDocument: "index2.html",
   },
}, { replaceOnChanges: ["website.indexDocument"] });

$ pulumi preview --non-interactive
Previewing update (dev)
 
View Live: https://app.pulumi.com/lukehoban/replaceonupdate/dev/previews/c886484a-6719-40f8-8007-b328ea4c62d0
 
 
   pulumi:pulumi:Stack replaceonupdate-dev running
++ aws:s3:Bucket my-bucket create replacement [diff: ~website]
+- aws:s3:Bucket my-bucket replace [diff: ~website]
-- aws:s3:Bucket my-bucket delete original [diff: ~website]
   pulumi:pulumi:Stack replaceonupdate-dev 

Outputs:
 ~ bucketName: "my-bucket-d60afc3" => output<string>
 
Resources:
   +-1 to replace
   1 unchanged

With replaceOnChanges and deleteBeforeReplace:

const bucket = new aws.s3.Bucket("my-bucket", {
    website: {
        indexDocument: "index2.html",
    },
}, { replaceOnChanges: ["website.indexDocument"], deleteBeforeReplace: true });

$ pulumi preview --non-interactive
Previewing update (dev)

View Live: https://app.pulumi.com/lukehoban/replaceonupdate/dev/previews/de0427c0-37b1-4e7c-92c9-31243546cce1


    pulumi:pulumi:Stack replaceonupdate-dev running 
 -- aws:s3:Bucket my-bucket delete original 
 +- aws:s3:Bucket my-bucket replace [diff: ~website]
 ++ aws:s3:Bucket my-bucket create replacement [diff: ~website]
    pulumi:pulumi:Stack replaceonupdate-dev  
 
Outputs:
  ~ bucketName: "my-bucket-d60afc3" => output<string>

Resources:
    +-1 to replace
    1 unchanged

Siteproxy

lukehoban · 2021-06-07T00:40:39Z

pkg/engine/lifeycletest/pulumi_test.go

+	loaders := []*deploytest.ProviderLoader{
+		deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
+			return &deploytest.Provider{
+				DiffF: func(urn resource.URN, id resource.ID,


It seems like we should have something built-in to produce a plugin.DiffResult from two resource.PropertyMaps (and potentially a set of replace keys). I was surprised I had to implement all this myself. Did I miss where this is defined? cc @pgavlin

It looks like there is code like this in most of our native providers: https://github.com/pulumi/pulumi-azure-native/blob/ae7f380268c97b951a76848ec45b3bafa22d4338/provider/pkg/provider/diff.go. It's mostly the same sort of thing implemented here - but with some slightly special-cased to pull the "set of replace keys" from the specific provider schema. It feels like that implementation could/should be pulled into the core and generalized so it doesn't have to be reimplemented in every provider (and could also be reused in tests here).

pkg/resource/deploy/step_generator.go

pkg/resource/stack/deployment.go

pkg/engine/lifeycletest/pulumi_test.go

pkg/resource/deploy/step_generator.go

pkg/resource/stack/deployment.go

pkg/resource/deploy/step_generator.go

lblackstone · 2021-06-15T23:47:54Z

@lukehoban All of the SDK changes + tests should be finished now. It seems like the diff code deduplication could be addressed in a followup PR so we can finish out the immediate work stream.

I'm not as familiar with the version compatibility considerations, so @pgavlin will probably need to verify that.

lukehoban · 2021-06-17T03:32:24Z

@pgavlin This looks to be ready for your review.

It seems like the diff code deduplication could be addressed in a followup PR so we can finish out the immediate work stream.

Opened #7326 on this one.

I'm not as familiar with the version compatibility considerations, so @pgavlin will probably need to verify that.

Since we are not storing this in the state file (at least until #7285), I believe the two scenarios here are just:

New CLI, old SDK: There is no way to requests replaceOnChanges, so the new CLI behaviour will never be triggered.
Old CLI, new SDK: SDK could request replaceOnChanges, but the CLI won't know about it. It will be ignored by the deployment.

This second case is potentially concerning. I am actually not sure how we handle this generally. Ideally the SDKs could require this support in the CLI and fail telling the user that this feature isn't supported in their CLI if they try to use it? Do we have support for that kind of version negotiation? Curious @pgavlin thoughts on this one.

lblackstone · 2021-06-21T21:15:44Z

@lukehoban I was trying to test this out with the k8s provider and noticed that the DiffRequest doesn't include the replaceOnChanges field. Is this expected to work without that change?

pgavlin · 2021-06-22T18:52:08Z

@lblackstone this might be asking a lot, but would it be possible to split the property-path-related changes into a separate PR? It's a bit difficult to see the forest for the trees at the moment.

pgavlin · 2021-06-22T18:53:16Z

Old CLI, new SDK: SDK could request replaceOnChanges, but the CLI won't know about it. It will be ignored by the deployment.

This second case is potentially concerning. I am actually not sure how we handle this generally. Ideally the SDKs could require this support in the CLI and fail telling the user that this feature isn't supported in their CLI if they try to use it? Do we have support for that kind of version negotiation? Curious @pgavlin thoughts on this one.

The resource monitor provides the SupportsFeature gRPC API for this. The SDKs can call that method and fail gracefully if replaceOnChanges is not supported.

lukehoban · 2021-06-22T19:26:35Z

would it be possible to split the property-path-related changes into a separate PR? It's a bit difficult to see the forest for the trees at the moment.

Do you mean just the changes in sdk/go/common/resource/properties_path.go?

lukehoban · 2021-06-22T19:29:48Z

I was trying to test this out with the k8s provider and noticed that the DiffRequest doesn't include the replaceOnChanges field. Is this expected to work without that change?

@lblackstone The engine does not pass replaceOnChanges to providers. It manages the requested replaceOnChanges entirely as a modification to the diff returned by the provider. This ensures that providers do not need to be updated to deal with this.

pgavlin · 2021-06-22T19:56:13Z

And the tests, yes, assuming that those are the only affected files.

lblackstone · 2021-06-22T21:37:59Z

Split out the prop path changes into #7354

lblackstone · 2021-06-24T15:18:15Z

Rebased and split up commits into logical chunks

pgavlin

A couple suggestions, but LGTM overall

pgavlin · 2021-06-25T19:58:39Z

pkg/resource/deploy/step_generator.go

+		diff, err = sg.applyReplaceOnChanges(diff, goal.ReplaceOnChanges, hasInitErrors)
+		if err != nil {
+			return nil, result.FromError(err)
+		}


This change to the logic is a bit concerning: we'll now be examining the diff even if there are no changes but there are init errors, where previously we would skip looking at the diff if it had no changes but there were init errors. What if we instead had applyReplaceOnChanges run outside this check?

pgavlin · 2021-06-25T19:59:30Z

pkg/resource/deploy/step_generator.go

+func (sg *stepGenerator) applyReplaceOnChanges(diff plugin.DiffResult,
+	replaceOnChanges []string, hasInitErrors bool) (plugin.DiffResult, error) {
+
+	var err error


If we take my earlier suggestion re: rearranging the logic in generateStepsFromDiff, then this function should early-out if diff.Changes != plugin.DiffSome && !hasInitErrors

pkg/resource/deploy/step_generator.go

sdk/go/common/resource/plugin/provider.go

lblackstone · 2021-06-28T20:23:03Z

@pgavlin I attempted to address your feedback. Can you double-check that the changes to the diff logic look like you expect?

pkg/resource/deploy/step_generator.go

Adds a new resource option to force replacement when certain properties report changes, even if the resource provider itself does not require a replacement. Fixes #6753.

Squashed commits: [bba44ac92] Modify changes logic for init error case [c07ece4b3] Further feedback from Pat [50c7c16da] Address naming feedback [8b61b6ccf] Address diff feedback

# Conflicts: # CHANGELOG_PENDING.md

lblackstone · 2021-07-01T16:14:37Z

@lukehoban Any final comments before this gets merged?

lukehoban · 2021-07-01T19:18:18Z

LGTM 🚀

I did some manual validation against a few "real world" use cases last night, and this worked great!

The diff rendering logic tests whether the DetailedDiff is nil to determine whether to use it for rendering or defer to older older supported approaches to computing diffs. The new logic added in #7226 could lead to replacing a nil DetailedDiff with an empty DetailedDiff, whcih would make the rendering logic believe that a DetailedDiff was present but empty, instead of missing entirely. This change ensures that we maintain nil-ness of the DetailedDiff when we transform it for handling of replaceOnChanges. Also adds tests for this and other cases on applyReplaceOnChanges. Fixes #7486.

Adds a new resource option to force replacement when certain properties report changes, even if the resource provider itself does not require a replacement. Fixes pulumi/pulumi#6753. Co-authored-by: Levi Blackstone <levi@pulumi.com>

lukehoban commented Jun 7, 2021

View reviewed changes

lukehoban requested review from pgavlin and lblackstone June 7, 2021 00:52

pgavlin self-assigned this Jun 7, 2021

lblackstone assigned pgavlin and lblackstone and unassigned pgavlin Jun 7, 2021

lblackstone mentioned this pull request Jun 7, 2021

Add replaceOnUpdate annotation to force resource replacement pulumi/pulumi-kubernetes#1563

Closed

lblackstone reviewed Jun 7, 2021

View reviewed changes

pkg/engine/lifeycletest/pulumi_test.go Outdated Show resolved Hide resolved

pkg/resource/deploy/step_generator.go Outdated Show resolved Hide resolved

pkg/resource/stack/deployment.go Outdated Show resolved Hide resolved

lukehoban force-pushed the lukehoban/6753 branch from b4064b4 to 05f2768 Compare June 13, 2021 04:04

lblackstone reviewed Jun 15, 2021

View reviewed changes

pkg/resource/deploy/step_generator.go Show resolved Hide resolved

lukehoban marked this pull request as ready for review June 17, 2021 03:23

lukehoban mentioned this pull request Jun 17, 2021

Add a standard implementation for converting resource.ObjectDiff to rpc.DetailedDiff #7326

Open

lblackstone force-pushed the lukehoban/6753 branch from 0ca5c63 to a472baf Compare June 24, 2021 15:17

pgavlin approved these changes Jun 25, 2021

View reviewed changes

pgavlin reviewed Jun 28, 2021

View reviewed changes

pkg/resource/deploy/step_generator.go Outdated Show resolved Hide resolved

lblackstone added 3 commits June 30, 2021 10:37

Protobuf changes

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lblackstone Levi Blackstone

GPG key ID: 4616849FDD4FE32E
Expired

Verified
Learn about vigilant mode

190e82b

Go tests

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lblackstone Levi Blackstone

GPG key ID: 4616849FDD4FE32E
Expired

Verified
Learn about vigilant mode

abe293c

lblackstone added 5 commits June 30, 2021 10:47

C# SDK

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lblackstone Levi Blackstone

GPG key ID: 4616849FDD4FE32E
Expired

Verified
Learn about vigilant mode

3dc2923

NodeJS SDK

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lblackstone Levi Blackstone

GPG key ID: 4616849FDD4FE32E
Expired

Verified
Learn about vigilant mode

911fbaf

Python SDK

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lblackstone Levi Blackstone

GPG key ID: 4616849FDD4FE32E
Expired

Verified
Learn about vigilant mode

281ac53

changelog

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lblackstone Levi Blackstone

GPG key ID: 4616849FDD4FE32E
Expired

Verified
Learn about vigilant mode

f67ec7b

lblackstone force-pushed the lukehoban/6753 branch from 5e318f0 to 14027da Compare June 30, 2021 16:48

mikhailshilkov mentioned this pull request Jul 1, 2021

Pulumi update issues with AKS clusters pulumi/pulumi-azure-native#959

Closed

pgavlin approved these changes Jul 1, 2021

View reviewed changes

Merge branch 'master' into lukehoban/6753

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lblackstone Levi Blackstone

GPG key ID: 4616849FDD4FE32E
Expired

Verified
Learn about vigilant mode

e9f50d3

# Conflicts: # CHANGELOG_PENDING.md

lblackstone merged commit eb32039 into master Jul 1, 2021

pulumi-bot deleted the lukehoban/6753 branch July 1, 2021 19:32

lukehoban mentioned this pull request Jul 9, 2021

Add docs for replaceOnChanges pulumi/pulumi-hugo#398

Closed

lukehoban mentioned this pull request Jul 13, 2021

Fix rendering of diffs for resource without DetailedDiffs #7500

Merged

This was referenced Jul 14, 2021

Give the possibility to set an annotation that forces resource re-creation pulumi/pulumi-kubernetes#1007

Closed

replaceOnChanges option treats "" as a wildcard #7556

Closed

lblackstone mentioned this pull request Aug 19, 2021

[schema] Add replaceOnChanges option to resource properties #7798

Closed

iwahbe mentioned this pull request Sep 15, 2021

Iwahbe/7881/thread replace on chages through sdks #7967

Merged

3 tasks

This was referenced Mar 9, 2022

Schema should support replaceOnChanges on the resource instead/in addition to property scope #9158

Open

Add option to treat configmaps as mutable pulumi/pulumi-kubernetes#1926

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `replaceOnChanges` resource option #7226

Add `replaceOnChanges` resource option #7226

lukehoban commented Jun 7, 2021 •

edited

Loading

Siteproxy

搜索引擎

常用网站

新闻网站

海外论坛

lukehoban Jun 7, 2021

lukehoban Jun 13, 2021

lblackstone commented Jun 15, 2021

lukehoban commented Jun 17, 2021

lblackstone commented Jun 21, 2021

pgavlin commented Jun 22, 2021

pgavlin commented Jun 22, 2021

lukehoban commented Jun 22, 2021

lukehoban commented Jun 22, 2021

pgavlin commented Jun 22, 2021

lblackstone commented Jun 22, 2021

lblackstone commented Jun 24, 2021

pgavlin left a comment

pgavlin Jun 25, 2021

pgavlin Jun 25, 2021

lblackstone commented Jun 28, 2021

lblackstone commented Jul 1, 2021

lukehoban commented Jul 1, 2021

Add replaceOnChanges resource option #7226

Add replaceOnChanges resource option #7226

Conversation

lukehoban commented Jun 7, 2021 • edited Loading

Siteproxy

搜索引擎

常用网站

新闻网站

海外论坛

lukehoban Jun 7, 2021

Choose a reason for hiding this comment

lukehoban Jun 13, 2021

Choose a reason for hiding this comment

lblackstone commented Jun 15, 2021

lukehoban commented Jun 17, 2021

lblackstone commented Jun 21, 2021

pgavlin commented Jun 22, 2021

pgavlin commented Jun 22, 2021

lukehoban commented Jun 22, 2021

lukehoban commented Jun 22, 2021

pgavlin commented Jun 22, 2021

lblackstone commented Jun 22, 2021

lblackstone commented Jun 24, 2021

pgavlin left a comment

Choose a reason for hiding this comment

pgavlin Jun 25, 2021

Choose a reason for hiding this comment

pgavlin Jun 25, 2021

Choose a reason for hiding this comment

lblackstone commented Jun 28, 2021

lblackstone commented Jul 1, 2021

lukehoban commented Jul 1, 2021

Add `replaceOnChanges` resource option #7226

Add `replaceOnChanges` resource option #7226

lukehoban commented Jun 7, 2021 •

edited

Loading