diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index c79a31188..6f6861c0d 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -33,5 +33,8 @@ - [engine] - Compute dependents correctly during targeted deletes. [#8360](https://github.com/pulumi/pulumi/pull/8360) +- [cli/engine] - Update command respects `--target-dependents` + [#8395](https://github.com/pulumi/pulumi/pull/8395) + - [docs] - Fix broken lists in dotnet docs [docs#6558](https://github.com/pulumi/docs/issues/6558) diff --git a/pkg/engine/lifeycletest/target_test.go b/pkg/engine/lifeycletest/target_test.go index c9ba4b1cc..6604337d4 100644 --- a/pkg/engine/lifeycletest/target_test.go +++ b/pkg/engine/lifeycletest/target_test.go @@ -145,17 +145,20 @@ func TestUpdateTarget(t *testing.T) { // limit to up to 3 resources to destroy. This keeps the test running time under // control as it only generates a few hundred combinations instead of several thousand. if len(subset) <= 3 { - updateSpecificTargets(t, subset) + updateSpecificTargets(t, subset, false /*targetDependents*/) } } - updateSpecificTargets(t, []string{"A"}) + updateSpecificTargets(t, []string{"A"}, false /*targetDependents*/) // Also update a target that doesn't exist to make sure we don't crash or otherwise go off the rails. updateInvalidTarget(t) + + // We want to check that targetDependents is respected + updateSpecificTargets(t, []string{"C"}, true /*targetDependents*/) } -func updateSpecificTargets(t *testing.T, targets []string) { +func updateSpecificTargets(t *testing.T, targets []string, targetDependents bool) { // A // _________|_________ // B C D @@ -193,6 +196,7 @@ func updateSpecificTargets(t *testing.T, targets []string) { } p.Options.Host = deploytest.NewPluginHost(nil, nil, program, loaders...) + p.Options.TargetDependents = targetDependents updateTargets := []resource.URN{} for _, target := range targets { @@ -228,6 +232,28 @@ func updateSpecificTargets(t *testing.T, targets []string) { assert.Contains(t, updated, target) } + if !targetDependents { + // We should only perform updates on the entries we have targeted. + for _, target := range p.Options.UpdateTargets { + assert.Contains(t, targets, target.Name().String()) + } + } else { + // We expect to find at least one other resource updates. + + // NOTE: The test is limited to only passing a subset valid behavior. By specifying + // a URN with no dependents, no other urns will be updated and the test will fail + // (incorrectly). + found := false + updateList := []string{} + for target := range updated { + updateList = append(updateList, target.Name().String()) + if !contains(targets, target.Name().String()) { + found = true + } + } + assert.True(t, found, "Updates: %v", updateList) + } + for _, target := range p.Options.UpdateTargets { assert.NotContains(t, sames, target) } @@ -238,6 +264,15 @@ func updateSpecificTargets(t *testing.T, targets []string) { p.Run(t, old) } +func contains(list []string, entry string) bool { + for _, e := range list { + if e == entry { + return true + } + } + return false +} + func updateInvalidTarget(t *testing.T) { p := &TestPlan{} diff --git a/pkg/resource/deploy/step_generator.go b/pkg/resource/deploy/step_generator.go index ee8f3865c..4b6a95eee 100644 --- a/pkg/resource/deploy/step_generator.go +++ b/pkg/resource/deploy/step_generator.go @@ -72,8 +72,33 @@ func (sg *stepGenerator) isTargetedUpdate() bool { return sg.updateTargetsOpt != nil || sg.replaceTargetsOpt != nil } -func (sg *stepGenerator) isTargetedForUpdate(urn resource.URN) bool { - return sg.updateTargetsOpt == nil || sg.updateTargetsOpt[urn] +// isTargetedForUpdate returns if `res` is targeted for update. The function accommodates +// `--target-dependents`. `targetDependentsForUpdate` should probably be called if this function +// returns true. +func (sg *stepGenerator) isTargetedForUpdate(res *resource.State) bool { + if sg.updateTargetsOpt == nil || sg.updateTargetsOpt[res.URN] { + return true + } else if !sg.opts.TargetDependents { + return false + } + if res.Provider != "" { + res, err := providers.ParseReference(res.Provider) + contract.AssertNoError(err) + if sg.updateTargetsOpt[res.URN()] { + return true + } + } + if res.Parent != "" { + if sg.updateTargetsOpt[res.Parent] { + return true + } + } + for _, dep := range res.Dependencies { + if dep != "" && sg.updateTargetsOpt[dep] { + return true + } + } + return false } func (sg *stepGenerator) isTargetedReplace(urn resource.URN) bool { @@ -158,7 +183,7 @@ func (sg *stepGenerator) GenerateSteps(event RegisterResourceEvent) ([]Step, res return steps, nil } - // We got a set of steps to perfom during a targeted update. If any of the steps are not same steps and depend on + // We got a set of steps to perform during a targeted update. If any of the steps are not same steps and depend on // creates we skipped because they were not in the --target list, issue an error that that the create was necessary // and that the user must target the resource to create. for _, step := range steps { @@ -421,6 +446,11 @@ func (sg *stepGenerator) generateSteps(event RegisterResourceEvent) ([]Step, res }, nil } + isTargeted := sg.isTargetedForUpdate(new) + if isTargeted && sg.updateTargetsOpt != nil { + sg.updateTargetsOpt[urn] = true + } + // Case 3: hasOld // In this case, the resource we are operating upon now exists in the old snapshot. // It must be an update or a replace. Which operation we do depends on the the specific change made to the @@ -439,8 +469,8 @@ func (sg *stepGenerator) generateSteps(event RegisterResourceEvent) ([]Step, res contract.Assert(old != nil) // If the user requested only specific resources to update, and this resource was not in - // that set, then do nothin but create a SameStep for it. - if !sg.isTargetedForUpdate(urn) { + // that set, then do nothing but create a SameStep for it. + if !isTargeted { logging.V(7).Infof( "Planner decided not to update '%v' due to not being in target group (same) (inputs=%v)", urn, new.Inputs) } else { @@ -490,9 +520,7 @@ func (sg *stepGenerator) generateSteps(event RegisterResourceEvent) ([]Step, res // We will also not record this non-created resource into the checkpoint as it doesn't actually // exist. - if !sg.isTargetedForUpdate(urn) && - !providers.IsProviderType(goal.Type) { - + if !isTargeted && !providers.IsProviderType(goal.Type) { sg.sames[urn] = true sg.skippedCreates[urn] = true return []Step{NewSkippedCreateStep(sg.deployment, event, new)}, nil