Refine resource replacement logic for providers (#2767)

This commit touches an intersection of a few different provider-oriented
features that combined to cause a particularly severe bug that made it
impossible for users to upgrade provider versions without seeing
replacements with their resources.

For some context, Pulumi models all providers as resources and places
them in the snapshot like any other resource. Every resource has a
reference to the provider that created it. If a Pulumi program does not
specify a particular provider to use when performing a resource
operation, the Pulumi engine injects one automatically; these are called
"default providers" and are the most common ways that users end up with
providers in their snapshot. Default providers can be identified by
their name, which is always prefixed with "default".

Recently, in an effort to make the Pulumi engine more flexible with
provider versions, it was made possible for the engine to have multiple
default providers active for a provider of a particular type, which was
previously not possible. Because a provider is identified as a tuple of
package name and version, it was difficult to find a name for these
duplicate default providers that did not cause additional problems. The
provider versioning PR gave these default providers a name that was
derived from the version of the package. This proved to be a problem,
because when users upgraded from one version of a package to another,
this changed the name of their default provider which in turn caused all
of their resources created using that provider (read: everything) to be
replaced.

To combat this, this PR introduces a rule that the engine will apply
when diffing a resource to determine whether or not it needs to be
replaced: "If a resource's provider changes, and both old and new
providers are default providers whose properties do not require
replacement, proceed as if there were no diff." This allows the engine
to gracefully recognize and recover when a resource's default provider changes
names, as long as the provider's config has not changed.
This commit is contained in:
Sean Gillespie 2019-06-03 12:16:31 -07:00 committed by Pat Gavlin
parent 9c4ee43041
commit 2870518a64
7 changed files with 350 additions and 53 deletions

View file

@ -7,6 +7,8 @@
- Support for referencing the outputs of other Pulumi stacks has been added to the Pulumi Python libraries via the
`StackReference` type.
- Add CI system detection for Bitbucket Pipelines.
- Pulumi now tolerates changes in default providers in certain cases, which fixes an issue where users would see
unexpected replaces when upgrading a Pulumi package.
- Add support for renaming resources via the `aliases` resource option. Adding aliases allows new resources to match
resources from previous deployments which used different names, maintaining the identity of the resource and avoiding
replacements or re-creation of the resource.

View file

@ -58,9 +58,8 @@ const (
)
type JournalEntry struct {
Kind JournalEntryKind
Step deploy.Step
Resource *resource.State
Kind JournalEntryKind
Step deploy.Step
}
type Journal struct {
@ -166,7 +165,7 @@ func (j *Journal) Snap(base *deploy.Snapshot) *deploy.Snapshot {
dones[e.Step.Old()] = true
}
case deploy.OpRemovePendingReplace:
dones[e.Resource] = true
dones[e.Step.Old()] = true
}
}
}
@ -2971,6 +2970,212 @@ func TestSingleResourceIgnoreChanges(t *testing.T) {
}, []string{"a"}, []deploy.StepOp{deploy.OpUpdate})
}
// TestDefaultProviderDiff tests that the engine can gracefully recover whenever a resource's default provider changes
// and there is no diff in the provider's inputs.
func TestDefaultProviderDiff(t *testing.T) {
const resName, resBName = "resA", "resB"
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.10"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.11"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.12"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
runProgram := func(base *deploy.Snapshot, versionA, versionB string, expectedStep deploy.StepOp) *deploy.Snapshot {
program := deploytest.NewLanguageRuntime(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, _, _, err := monitor.RegisterResource("pkgA:m:typA", resName, true, "", false, nil, "",
resource.PropertyMap{}, nil, false, versionA, nil, nil)
assert.NoError(t, err)
_, _, _, err = monitor.RegisterResource("pkgA:m:typA", resBName, true, "", false, nil, "",
resource.PropertyMap{}, nil, false, versionB, nil, nil)
assert.NoError(t, err)
return nil
})
host := deploytest.NewPluginHost(nil, nil, program, loaders...)
p := &TestPlan{
Options: UpdateOptions{host: host},
Steps: []TestStep{
{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, j *Journal,
events []Event, res result.Result) result.Result {
for _, entry := range j.Entries {
if entry.Kind != JournalEntrySuccess {
continue
}
switch entry.Step.URN().Name().String() {
case resName, resBName:
assert.Equal(t, expectedStep, entry.Step.Op())
}
}
return res
},
},
},
}
return p.Run(t, base)
}
// This test simulates the upgrade scenario of old-style default providers to new-style versioned default providers.
//
// The first update creates a stack using a language host that does not report a version to the engine. As a result,
// the engine makes up a default provider for "pkgA" and calls it "default". It then creates the two resources that
// we are creating and associates them with the default provider.
snap := runProgram(nil, "", "", deploy.OpCreate)
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
assert.Equal(t, "default", res.URN.Name().String())
case res.URN.Name().String() == resName || res.URN.Name().String() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
assert.Equal(t, "default", provRef.URN().Name().String())
}
}
// The second update switches to a language host that does report a version to the engine. As a result, the engine
// uses this version to make a new provider, with a different URN, and uses that provider to operate on resA and
// resB.
//
// Despite switching out the provider, the engine should still generate a Same step for resA. It is vital that the
// engine gracefully react to changes in the default provider in this manner. See pulumi/pulumi#2753 for what
// happens when it doesn't.
snap = runProgram(snap, "0.17.10", "0.17.10", deploy.OpSame)
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
assert.Equal(t, "default_0_17_10", res.URN.Name().String())
case res.URN.Name().String() == resName || res.URN.Name().String() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
assert.Equal(t, "default_0_17_10", provRef.URN().Name().String())
}
}
// The third update changes the version that the language host reports to the engine. This simulates a scenario in
// which a user updates their SDK to a new version of a provider package. In order to simulate side-by-side
// packages with different versions, this update requests distinct package versions for resA and resB.
snap = runProgram(snap, "0.17.11", "0.17.12", deploy.OpSame)
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
assert.True(t, res.URN.Name().String() == "default_0_17_11" || res.URN.Name().String() == "default_0_17_12")
case res.URN.Name().String() == resName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
assert.Equal(t, "default_0_17_11", provRef.URN().Name().String())
case res.URN.Name().String() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
assert.Equal(t, "default_0_17_12", provRef.URN().Name().String())
}
}
}
// TestDefaultProviderDiffReplacement tests that, when replacing a default provider for a resource, the engine will
// replace the resource if DiffConfig on the new provider returns a diff for the provider's new state.
func TestDefaultProviderDiffReplacement(t *testing.T) {
const resName, resBName = "resA", "resB"
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.10"), func() (plugin.Provider, error) {
return &deploytest.Provider{
// This implementation of DiffConfig always requests replacement.
DiffConfigF: func(_ resource.URN, olds, news resource.PropertyMap, _ bool) (plugin.DiffResult, error) {
keys := []resource.PropertyKey{}
for k := range news {
keys = append(keys, k)
}
return plugin.DiffResult{
Changes: plugin.DiffSome,
ReplaceKeys: keys,
}, nil
},
}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.11"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
runProgram := func(base *deploy.Snapshot, versionA, versionB string, expectedSteps ...deploy.StepOp) *deploy.Snapshot {
program := deploytest.NewLanguageRuntime(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, _, _, err := monitor.RegisterResource("pkgA:m:typA", resName, true, "", false, nil, "",
resource.PropertyMap{}, nil, false, versionA, nil, nil)
assert.NoError(t, err)
_, _, _, err = monitor.RegisterResource("pkgA:m:typA", resBName, true, "", false, nil, "",
resource.PropertyMap{}, nil, false, versionB, nil, nil)
assert.NoError(t, err)
return nil
})
host := deploytest.NewPluginHost(nil, nil, program, loaders...)
p := &TestPlan{
Options: UpdateOptions{host: host},
Steps: []TestStep{
{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, j *Journal,
events []Event, res result.Result) result.Result {
for _, entry := range j.Entries {
if entry.Kind != JournalEntrySuccess {
continue
}
switch entry.Step.URN().Name().String() {
case resName:
assert.Subset(t, expectedSteps, []deploy.StepOp{entry.Step.Op()})
case resBName:
assert.Subset(t,
[]deploy.StepOp{deploy.OpCreate, deploy.OpSame}, []deploy.StepOp{entry.Step.Op()})
}
}
return res
},
},
},
}
return p.Run(t, base)
}
// This test simulates the upgrade scenario of default providers, except that the requested upgrade results in the
// provider getting replaced. Because of this, the engine should decide to replace resA. It should not decide to
// replace resB, as its change does not require replacement.
snap := runProgram(nil, "", "", deploy.OpCreate)
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
assert.Equal(t, "default", res.URN.Name().String())
case res.URN.Name().String() == resName || res.URN.Name().String() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
assert.Equal(t, "default", provRef.URN().Name().String())
}
}
// Upon update, now that the language host is sending a version, DiffConfig reports that there's a diff between the
// old and new provider and so we must replace resA.
snap = runProgram(snap, "0.17.10", "0.17.11", deploy.OpCreateReplacement, deploy.OpReplace, deploy.OpDeleteReplaced)
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
assert.True(t, res.URN.Name().String() == "default_0_17_10" || res.URN.Name().String() == "default_0_17_11")
case res.URN.Name().String() == resName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
assert.Equal(t, "default_0_17_10", provRef.URN().Name().String())
case res.URN.Name().String() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
assert.Equal(t, "default_0_17_11", provRef.URN().Name().String())
}
}
}
// Resource is an abstract representation of a resource graph
type Resource struct {
t tokens.Type

View file

@ -16,7 +16,6 @@ package engine
import (
"context"
"strings"
"sync"
"github.com/opentracing/opentracing-go"
@ -351,6 +350,5 @@ func assertSeen(seen map[resource.URN]deploy.Step, step deploy.Step) {
}
func isDefaultProviderStep(step deploy.Step) bool {
urn := step.URN()
return providers.IsProviderType(urn.Type()) && strings.HasPrefix(urn.Name().String(), "default")
return providers.IsDefaultProvider(step.URN())
}

View file

@ -91,10 +91,12 @@ func (host *pluginHost) Provider(pkg tokens.Package, version *semver.Version) (p
continue
}
if version != nil && l.version.LT(*version) {
continue
}
if best == nil || l.version.GT(best.version) {
if version != nil {
if l.version.EQ(*version) {
best = l
break
}
} else if best == nil || l.version.GT(best.version) {
best = l
}
}

View file

@ -41,6 +41,11 @@ func IsProviderType(typ tokens.Type) bool {
return typ.Module() == "pulumi:providers" && typ.Name() != ""
}
// IsDefaultProvider returns true if this URN refers to a default Pulumi provider.
func IsDefaultProvider(urn resource.URN) bool {
return IsProviderType(urn.Type()) && strings.HasPrefix(urn.Name().String(), "default")
}
// MakeProviderType returns the provider type token for the given package.
func MakeProviderType(pkg tokens.Package) tokens.Type {
return tokens.Type("pulumi:providers:" + pkg)

View file

@ -494,21 +494,18 @@ func (rm *resmon) getProvider(req providers.ProviderRequest, rawProviderRef stri
}
func (rm *resmon) parseProviderRequest(pkg tokens.Package, version string) (providers.ProviderRequest, error) {
return providers.NewProviderRequest(nil, pkg), nil
if version == "" {
logging.V(5).Infof("parseProviderRequest(%s): semver version is the empty string", pkg)
return providers.NewProviderRequest(nil, pkg), nil
}
// TODO[pulumi/pulumi#2753]: We should re-enable this code once we have a solution for #2753.
// if version == "" {
// logging.V(5).Infof("parseProviderRequest(%s): semver version is the empty string", pkg)
// return providers.NewProviderRequest(nil, pkg), nil
// }
parsedVersion, err := semver.Parse(version)
if err != nil {
logging.V(5).Infof("parseProviderRequest(%s, %s): semver version string is invalid: %v", pkg, version, err)
return providers.ProviderRequest{}, err
}
// parsedVersion, err := semver.Parse(version)
// if err != nil {
// logging.V(5).Infof("parseProviderRequest(%s, %s): semver version string is invalid: %v", pkg, version, err)
// return providers.ProviderRequest{}, err
// }
// return providers.NewProviderRequest(&parsedVersion, pkg), nil
return providers.NewProviderRequest(&parsedVersion, pkg), nil
}
func (rm *resmon) SupportsFeature(ctx context.Context,

View file

@ -15,6 +15,8 @@
package deploy
import (
"github.com/pkg/errors"
"github.com/pulumi/pulumi/pkg/diag"
"github.com/pulumi/pulumi/pkg/resource"
"github.com/pulumi/pulumi/pkg/resource/deploy/providers"
@ -34,14 +36,15 @@ type stepGenerator struct {
plan *Plan // the plan to which this step generator belongs
opts Options // options for this step generator
urns map[resource.URN]bool // set of URNs discovered for this plan
reads map[resource.URN]bool // set of URNs read for this plan
deletes map[resource.URN]bool // set of URNs deleted in this plan
replaces map[resource.URN]bool // set of URNs replaced in this plan
updates map[resource.URN]bool // set of URNs updated in this plan
creates map[resource.URN]bool // set of URNs created in this plan
sames map[resource.URN]bool // set of URNs that were not changed in this plan
pendingDeletes map[*resource.State]bool // set of resources (not URNs!) that are pending deletion
urns map[resource.URN]bool // set of URNs discovered for this plan
reads map[resource.URN]bool // set of URNs read for this plan
deletes map[resource.URN]bool // set of URNs deleted in this plan
replaces map[resource.URN]bool // set of URNs replaced in this plan
updates map[resource.URN]bool // set of URNs updated in this plan
creates map[resource.URN]bool // set of URNs created in this plan
sames map[resource.URN]bool // set of URNs that were not changed in this plan
pendingDeletes map[*resource.State]bool // set of resources (not URNs!) that are pending deletion
providers map[resource.URN]*resource.State // URN map of providers that we have seen so far.
// a map from URN to a list of property keys that caused the replacement of a dependent resource during a
// delete-before-replace.
dependentReplaceKeys map[resource.URN][]resource.PropertyKey
@ -161,6 +164,12 @@ func (sg *stepGenerator) GenerateSteps(event RegisterResourceEvent) ([]Step, res
goal.Dependencies, goal.InitErrors, goal.Provider, goal.PropertyDependencies, false,
goal.AdditionalSecretOutputs, goal.Aliases)
// Is this thing a provider resource? If so, stash it - we might need it later when calculating replacement
// of resources that use this provider.
if providers.IsProviderType(goal.Type) {
sg.providers[urn] = new
}
// Fetch the provider for this resource.
prov, res := sg.loadResourceProvider(urn, goal.Custom, goal.Provider, goal.Type)
if res != nil {
@ -284,25 +293,17 @@ func (sg *stepGenerator) GenerateSteps(event RegisterResourceEvent) ([]Step, res
// be replaced, we do so. If it does not, we update the resource in place.
if hasOld {
contract.Assert(old != nil)
var diff plugin.DiffResult
if old.Provider != new.Provider {
diff = plugin.DiffResult{Changes: plugin.DiffSome, ReplaceKeys: []resource.PropertyKey{"provider"}}
} else {
// Determine whether the change resulted in a diff.
d, diffErr := sg.diff(urn, old.ID, oldInputs, oldOutputs, inputs, prov, allowUnknowns)
if diffErr != nil {
// If the plugin indicated that the diff is unavailable, assume that the resource will be updated and
// report the message contained in the error.
//nolint
if _, ok := diffErr.(plugin.DiffUnavailableError); ok {
d = plugin.DiffResult{Changes: plugin.DiffSome}
sg.plan.ctx.Diag.Warningf(diag.RawMessage(urn, diffErr.Error()))
} else {
return nil, result.FromError(diffErr)
}
diff, err := sg.diff(urn, old, new, oldInputs, oldOutputs, inputs, prov, allowUnknowns)
if err != nil {
// If the plugin indicated that the diff is unavailable, assume that the resource will be updated and
// report the message contained in the error.
if _, ok := err.(plugin.DiffUnavailableError); ok {
diff = plugin.DiffResult{Changes: plugin.DiffSome}
sg.plan.ctx.Diag.Warningf(diag.RawMessage(urn, err.Error()))
} else {
return nil, result.FromError(err)
}
diff = d
}
// Ensure that we received a sensible response.
@ -595,9 +596,95 @@ func (sg *stepGenerator) ScheduleDeletes(deleteSteps []Step) []antichain {
return antichains
}
// providerChanged diffs the Provider field of old and new resources, returning true if the rest of the step generator
// should consider there to be a diff between these two resources.
func (sg *stepGenerator) providerChanged(urn resource.URN, old, new *resource.State) (bool, error) {
// If a resource's Provider field has changed, we may need to show a diff and we may not. This is subtle. See
// pulumi/pulumi#2753 for more details.
//
// Recent versions of Pulumi allow for language hosts to pass a plugin version to the engine. The purpose of this is
// to ensure that the plugin that the engine uses for a particular resource is *exactly equal* to the version of the
// SDK that the language host used to produce the resource registration. This is critical for correct versioning
// semantics; it is generally an error for a language SDK to produce a registration that is serviced by a
// differently versioned plugin, since the two version in complete lockstep and there is no guarantee that the two
// will work correctly together when not the same version.
if old.Provider == new.Provider {
return false, nil
}
logging.V(stepExecutorLogLevel).Infof("sg.diffProvider(%s, ...): observed provider diff", urn)
logging.V(stepExecutorLogLevel).Infof("sg.diffProvider(%s, ...): %s => %s", urn, old.Provider, new.Provider)
oldRef, err := providers.ParseReference(old.Provider)
if err != nil {
return false, err
}
newRef, err := providers.ParseReference(new.Provider)
if err != nil {
return false, err
}
// If one or both of these providers are not default providers, we will need to accept the diff and replace
// everything. This might not be strictly necessary, but it is conservatively correct.
if !providers.IsDefaultProvider(oldRef.URN()) || !providers.IsDefaultProvider(newRef.URN()) {
logging.V(stepExecutorLogLevel).Infof(
"sg.diffProvider(%s, ...): reporting provider diff due to change in default provider status", urn)
logging.V(stepExecutorLogLevel).Infof(
"sg.diffProvider(%s, ...): old provider %q is default: %v",
urn, oldRef.URN(), providers.IsDefaultProvider(oldRef.URN()))
logging.V(stepExecutorLogLevel).Infof(
"sg.diffProvider(%s, ...): new provider %q is default: %v",
urn, newRef.URN(), providers.IsDefaultProvider(newRef.URN()))
return true, err
}
// If both of these providers are default providers, use the *new provider* to diff the config and determine if
// this provider requires replacement.
//
// Note that, if we have many resources managed by the same provider that is getting replaced in this manner,
// this will call DiffConfig repeatedly with the same arguments for every resource. If this becomes a
// performance problem, this result can be cached.
newProv, ok := sg.plan.providers.GetProvider(newRef)
if !ok {
return false, errors.Errorf("failed to resolve provider reference: %q", oldRef.String())
}
oldRes, ok := sg.plan.olds[oldRef.URN()]
contract.Assertf(ok, "old state didn't have provider, despite resource using it?")
newRes, ok := sg.providers[newRef.URN()]
contract.Assertf(ok, "new plan didn't have provider, despite resource using it?")
diff, err := newProv.DiffConfig(newRef.URN(), oldRes.Inputs, newRes.Inputs, true)
if err != nil {
return false, err
}
// If there is a replacement diff, we must also replace this resource.
if diff.Replace() {
logging.V(stepExecutorLogLevel).Infof(
"sg.diffProvider(%s, ...): new provider's DiffConfig reported replacement", urn)
return true, nil
}
// Otherwise, it's safe to allow this new provider to replace our old one.
logging.V(stepExecutorLogLevel).Infof(
"sg.diffProvider(%s, ...): both providers are default, proceeding with resource diff", urn)
return false, nil
}
// diff returns a DiffResult for the given resource.
func (sg *stepGenerator) diff(urn resource.URN, id resource.ID, oldInputs, oldOutputs, newInputs resource.PropertyMap,
prov plugin.Provider, allowUnknowns bool) (plugin.DiffResult, error) {
func (sg *stepGenerator) diff(urn resource.URN, old, new *resource.State, oldInputs, oldOutputs,
newInputs resource.PropertyMap, prov plugin.Provider, allowUnknowns bool) (plugin.DiffResult, error) {
// Before diffing the resource, diff the provider field. If the provider field changes, we may or may
// not need to replace the resource.
providerChanged, err := sg.providerChanged(urn, old, new)
if err != nil {
return plugin.DiffResult{}, err
} else if providerChanged {
return plugin.DiffResult{Changes: plugin.DiffSome, ReplaceKeys: []resource.PropertyKey{"provider"}}, nil
}
// Workaround #1251: unexpected replaces.
//
@ -618,7 +705,7 @@ func (sg *stepGenerator) diff(urn resource.URN, id resource.ID, oldInputs, oldOu
// Grab the diff from the provider. At this point we know that there were changes to the Pulumi inputs, so if the
// provider returns an "unknown" diff result, pretend it returned "diffs exist".
diff, err := prov.Diff(urn, id, oldOutputs, newInputs, allowUnknowns)
diff, err := prov.Diff(urn, old.ID, oldOutputs, newInputs, allowUnknowns)
if err != nil {
return diff, err
}
@ -801,6 +888,7 @@ func newStepGenerator(plan *Plan, opts Options) *stepGenerator {
updates: make(map[resource.URN]bool),
deletes: make(map[resource.URN]bool),
pendingDeletes: make(map[*resource.State]bool),
providers: make(map[resource.URN]*resource.State),
dependentReplaceKeys: make(map[resource.URN][]resource.PropertyKey),
aliased: make(map[resource.URN]resource.URN),
}