maxmustermann/pulumi
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Do not propose replacement of providers in preview. (#2088)

Browse source 
Providers with unknown properties are currently considered to require
replacement. This was intended to indicate that we could not be sure
whether or not replacement was reqiuired. Unfortunately, this was not a
good user experience, as replacement would never be required at runtime.
This caused quite a bit of confusion--never proposing replacement seems
to be the better option.
This commit is contained in:
Pat Gavlin 2018-10-23 10:23:28 -07:00 committed by GitHub
parent f465fc0a48
commit 72a6ed320c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 2 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
pkg/resource/plugin/provider_plugin.go
View file

@ -109,39 +109,13 @@ func (p *provider) DiffConfig(olds, news resource.PropertyMap) (DiffResult, erro
// 1. Configuration differences in which all properties are known
// 2. Configuration differences in which some new property is unknown.
//
// Despite the fact that in both scenarios we know that configuration has changed, they differ in whether or not
// they return a diff result that indicates that the provider should be replaced (and thus require that any
// existing resource managed by the provider are also replaced).
//
// In the first case, we return a diff result that indicates that the provider _should not_ be replaced. We may
// encounter this scenario during any update or any preview in which all properties are known. Although this
// In both cases, we return a diff result that indicates that the provider _should not_ be replaced. Although this
// decision is not conservative--indeed, the conservative decision would be to always require replacement of a
// provider if any input has changed--we believe that it results in the best possible user experience for providers
// that do not implement DiffConfig functionality. If we took the conservative route here, any change to a
// provider's configuration (no matter how inconsequential) would cause all of its resources to be replaced. This
// is clearly a bad experience, and differs from how things worked prior to first-class providers.
//
// In the second case, we return a diff result that indicates that the provider _should_ be replaced. This may
// occur during any preview, but will never occur during an update. This decision is conservative because we
// believe that it is unlikely that a provider property that may be unknown is very likely to be a property that
// is fundamental to the provider's ability to manage its existing resources.
//
// The different decisions we make here cause a bit of a sharp edge: a provider with unknown configuration during
// preview will appear to require replacement, but will never actually require replacement during an update, as by
// that point all of its configuration will necessarily be known.
var replaceKeys []resource.PropertyKey
for k, v := range news {
// These are ensured during Check().
contract.Assert(v.IsString() || v.IsComputed())
// As per the note above, any unknown properties require replacement.
if v.IsComputed() {
replaceKeys = append(replaceKeys, k)
}
}
return DiffResult{Changes: DiffUnknown, ReplaceKeys: replaceKeys}, nil
return DiffResult{Changes: DiffUnknown, ReplaceKeys: nil}, nil
}
// getClient returns the client, and ensures that the target provider has been configured. This just makes it safer