Fix panic related to update with refresh (#5980)

The step generator was incorrectly tracking goal states for
old resources, which could lead to a panic if the resource
was removed in the update. This fix only generates goal
states for resources that exist in the updated program.
This commit is contained in:
Levi Blackstone 2020-12-19 00:03:40 -07:00 committed by GitHub
parent 268e50d00e
commit 8a9b381767
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 60 additions and 8 deletions

View file

@ -3,6 +3,9 @@ CHANGELOG
## HEAD (Unreleased)
- Fix a bug in the core engine where deleting/renaming a resource would panic on update + refresh.
[#5980](https://github.com/pulumi/pulumi/pull/5980)
- Fix a bug in the core engine that caused `ignoreChanges` to fail for resources being imported.
[#5976](https://github.com/pulumi/pulumi/pull/5976)

View file

@ -776,7 +776,7 @@ func TestRefreshStepWillPersistUpdatedIDs(t *testing.T) {
}
program := deploytest.NewLanguageRuntime(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, _, _, err := monitor.RegisterResource("pkgA:m:typA", "resA", false)
_, _, _, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
assert.NoError(t, err)
return nil
})
@ -813,3 +813,49 @@ func TestRefreshStepWillPersistUpdatedIDs(t *testing.T) {
}
}
}
// TestRefreshUpdateWithDeletedResource validates that the engine handles a deleted resource without error on an
// update with refresh.
func TestRefreshUpdateWithDeletedResource(t *testing.T) {
p := &TestPlan{}
resURN := p.NewURN("pkgA:m:typA", "resA", "")
idBefore := resource.ID("myid")
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{
ReadF: func(
urn resource.URN, id resource.ID, inputs, state resource.PropertyMap,
) (plugin.ReadResult, resource.Status, error) {
return plugin.ReadResult{}, resource.StatusOK, nil
},
}, nil
}),
}
program := deploytest.NewLanguageRuntime(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
return nil
})
host := deploytest.NewPluginHost(nil, nil, program, loaders...)
p.Options.Host = host
p.Options.Refresh = true
old := &deploy.Snapshot{
Resources: []*resource.State{
{
Type: resURN.Type(),
URN: resURN,
Custom: true,
ID: idBefore,
Inputs: resource.PropertyMap{},
Outputs: resource.PropertyMap{},
},
},
}
p.Steps = []TestStep{{Op: Update}}
snap := p.Run(t, old)
assert.Equal(t, 0, len(snap.Resources))
}

View file

@ -142,9 +142,10 @@ type Deployment struct {
source Source // the source of new resources.
localPolicyPackPaths []string // the policy packs to run during this deployment's generation.
preview bool // true if this deployment is to be previewed.
depGraph *graph.DependencyGraph // the dependency graph of the old snapshot
depGraph *graph.DependencyGraph // the dependency graph of the old snapshot.
providers *providers.Registry // the provider registry for this deployment.
news *resourceMap // the set of new resources generated by the deployment
goals map[resource.URN]*resource.Goal // the set of resource goals generated by the deployment.
news *resourceMap // the set of new resources generated by the deployment.
}
// addDefaultProviders adds any necessary default provider definitions and references to the given snapshot. Version
@ -304,6 +305,9 @@ func NewDeployment(ctx *plugin.Context, target *Target, prev *Snapshot, source S
// Build the dependency graph for the old resources.
depGraph := graph.NewDependencyGraph(oldResources)
// Create a goals map for the deployment.
newGoals := make(map[resource.URN]*resource.Goal)
// Create a resource map for the deployment.
newResources := &resourceMap{}
@ -328,6 +332,7 @@ func NewDeployment(ctx *plugin.Context, target *Target, prev *Snapshot, source S
preview: preview,
depGraph: depGraph,
providers: reg,
goals: newGoals,
news: newResources,
}, nil
}

View file

@ -293,7 +293,7 @@ func (se *stepExecutor) executeStep(workerID int, step Step) error {
}
// If this is not a resource that is managed by Pulumi, then we can ignore it.
if !newState.External && se.deployment.news != nil {
if _, hasGoal := se.deployment.goals[newState.URN]; hasGoal {
se.deployment.news.set(newState.URN, newState)
}
}

View file

@ -59,7 +59,6 @@ type stepGenerator struct {
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.
resourceGoals map[resource.URN]*resource.Goal // URN map of goals for ALL resources 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.
@ -253,7 +252,7 @@ func (sg *stepGenerator) generateSteps(event RegisterResourceEvent) ([]Step, res
// Mark the URN/resource as having been seen. So we can run analyzers on all resources seen, as well as
// lookup providers for calculating replacement of resources that use the provider.
sg.resourceGoals[urn] = goal
sg.deployment.goals[urn] = goal
if providers.IsProviderType(goal.Type) {
sg.providers[urn] = new
}
@ -1287,7 +1286,7 @@ func (sg *stepGenerator) calculateDependentReplacements(root *resource.State) ([
func (sg *stepGenerator) AnalyzeResources() result.Result {
var resources []plugin.AnalyzerStackResource
sg.deployment.news.mapRange(func(urn resource.URN, v *resource.State) bool {
goal := sg.resourceGoals[urn]
goal := sg.deployment.goals[urn]
resource := plugin.AnalyzerStackResource{
AnalyzerResource: plugin.AnalyzerResource{
URN: v.URN,
@ -1368,7 +1367,6 @@ func newStepGenerator(
skippedCreates: make(map[resource.URN]bool),
pendingDeletes: make(map[*resource.State]bool),
providers: make(map[resource.URN]*resource.State),
resourceGoals: make(map[resource.URN]*resource.Goal),
dependentReplaceKeys: make(map[resource.URN][]resource.PropertyKey),
aliased: make(map[resource.URN]resource.URN),
}