Update command respects --target-dependents (#8395)

* Update command respects `--target-dependents`

* Update CHANGELOG_PENDING.md

* Depend on parent and provider

* Add tests for new feature

* Separate predicate and mutation in code

* Remove `targetDependentsForUpdate`

* Refactor `isTargetedForUpdate`

* Add very important nil check
This commit is contained in:
Ian Wahbe 2021-11-16 17:12:36 -08:00 committed by GitHub
parent a1339277f0
commit 3329d81c1a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 77 additions and 11 deletions

View file

@ -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)

View file

@ -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{}

View file

@ -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