Filter diff keys based on provider info (#2526)

If a provider returns information about the top-level properties that
differ, use those keys to filter the diffs that are rendered to the
user.

Fixes #2453.
This commit is contained in:
Pat Gavlin 2019-03-06 16:41:19 -08:00 committed by GitHub
parent 82c4df84e5
commit 4b33a45561
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 134 additions and 74 deletions

View file

@ -4,6 +4,8 @@
- Show `brew upgrade pulumi` as the upgrade message when the currently running `pulumi` executable
is running on macOS from the brew install directory.
- Resource diffs that are rendered to the console are now filtered to properties that have semantically-meaningful
changes where possible.
## 0.17.1 (Released March 6, 2019)

View file

@ -81,6 +81,8 @@ type StepEventMetadata struct {
// Keys causing a replacement (only applicable for "create" and "replace" Ops).
Keys []string `json:"keys,omitempty"`
// Keys that changed with this step.
Diffs []string `json:"diffs"`
// Logical is set if the step is a logical operation in the program.
Logical bool `json:"logical"`
// Provider actually performing the step.

View file

@ -425,9 +425,32 @@ func (data *resourceRowData) getDiffInfo(step engine.StepEventMetadata) string {
updates[k] = resource.PropertyValue{}
}
writePropertyKeys(changesBuf, diff.Adds, deploy.OpCreate)
writePropertyKeys(changesBuf, diff.Deletes, deploy.OpDelete)
writePropertyKeys(changesBuf, updates, deploy.OpUpdate)
filteredKeys := func(m resource.PropertyMap) []string {
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, string(k))
}
return keys
}
if include := step.Diffs; include != nil {
includeSet := make(map[resource.PropertyKey]bool)
for _, k := range include {
includeSet[k] = true
}
filteredKeys = func(m resource.PropertyMap) []string {
var filteredKeys []string
for k := range m {
if includeSet[k] {
filteredKeys = append(filteredKeys, string(k))
}
}
return filteredKeys
}
}
writePropertyKeys(changesBuf, filteredKeys(diff.Adds), deploy.OpCreate)
writePropertyKeys(changesBuf, filteredKeys(diff.Deletes), deploy.OpDelete)
writePropertyKeys(changesBuf, filteredKeys(updates), deploy.OpUpdate)
}
}
@ -435,23 +458,17 @@ func (data *resourceRowData) getDiffInfo(step engine.StepEventMetadata) string {
return changesBuf.String()
}
func writePropertyKeys(b *bytes.Buffer, propMap resource.PropertyMap, op deploy.StepOp) {
if len(propMap) > 0 {
func writePropertyKeys(b *bytes.Buffer, keys []string, op deploy.StepOp) {
if len(keys) > 0 {
writeString(b, strings.Trim(op.Prefix(), " "))
keys := make([]string, 0, len(propMap))
for k := range propMap {
keys = append(keys, string(k))
}
sort.Strings(keys)
index := 0
for _, k := range keys {
for index, k := range keys {
if index != 0 {
writeString(b, ",")
}
writeString(b, k)
index++
}
writeString(b, colors.Reset)

View file

@ -316,6 +316,10 @@ func convertStepEventMetadata(md engine.StepEventMetadata) apitype.StepEventMeta
for i, v := range md.Keys {
keys[i] = string(v)
}
var diffs []string
for _, v := range md.Diffs {
diffs = append(diffs, string(v))
}
return apitype.StepEventMetadata{
Op: string(md.Op),
@ -327,6 +331,7 @@ func convertStepEventMetadata(md engine.StepEventMetadata) apitype.StepEventMeta
Res: convertStepEventStateMetadata(md.Res),
Keys: keys,
Diffs: diffs,
Logical: md.Logical,
Provider: md.Provider,
}

View file

@ -328,8 +328,8 @@ func TestVexingDeployment(t *testing.T) {
cPrime := NewResource(string(c.URN), bPrime.URN)
// mocking out the behavior of a provider indicating that this resource needs to be deleted
createReplacement := deploy.NewCreateReplacementStep(nil, MockRegisterResourceEvent{}, c, cPrime, nil, true)
replace := deploy.NewReplaceStep(nil, c, cPrime, nil, true)
createReplacement := deploy.NewCreateReplacementStep(nil, MockRegisterResourceEvent{}, c, cPrime, nil, nil, true)
replace := deploy.NewReplaceStep(nil, c, cPrime, nil, nil, true)
c.Delete = true
applyStep(createReplacement)
@ -338,7 +338,7 @@ func TestVexingDeployment(t *testing.T) {
// cPrime now exists, c is now pending deletion
// dPrime now depends on cPrime, which got replaced
dPrime := NewResource(string(d.URN), cPrime.URN)
applyStep(deploy.NewUpdateStep(nil, MockRegisterResourceEvent{}, d, dPrime, nil))
applyStep(deploy.NewUpdateStep(nil, MockRegisterResourceEvent{}, d, dPrime, nil, nil))
lastSnap := sp.SavedSnapshots[len(sp.SavedSnapshots)-1]
assert.Len(t, lastSnap.Resources, 6)
@ -498,7 +498,7 @@ func TestRecordingUpdateSuccess(t *testing.T) {
})
manager, sp := MockSetup(t, snap)
step := deploy.NewUpdateStep(nil, &MockRegisterResourceEvent{}, resourceA, resourceANew, nil)
step := deploy.NewUpdateStep(nil, &MockRegisterResourceEvent{}, resourceA, resourceANew, nil, nil)
mutation, err := manager.BeginMutation(step)
if !assert.NoError(t, err) {
t.FailNow()
@ -537,7 +537,7 @@ func TestRecordingUpdateFailure(t *testing.T) {
})
manager, sp := MockSetup(t, snap)
step := deploy.NewUpdateStep(nil, &MockRegisterResourceEvent{}, resourceA, resourceANew, nil)
step := deploy.NewUpdateStep(nil, &MockRegisterResourceEvent{}, resourceA, resourceANew, nil, nil)
mutation, err := manager.BeginMutation(step)
if !assert.NoError(t, err) {
t.FailNow()

View file

@ -196,9 +196,9 @@ func GetResourcePropertiesDetails(
printObject(&b, old.Inputs, planning, indent, step.Op, false, debug)
}
} else if len(new.Outputs) > 0 {
printOldNewDiffs(&b, old.Outputs, new.Outputs, planning, indent, step.Op, summary, debug)
printOldNewDiffs(&b, old.Outputs, new.Outputs, nil, planning, indent, step.Op, summary, debug)
} else {
printOldNewDiffs(&b, old.Inputs, new.Inputs, planning, indent, step.Op, summary, debug)
printOldNewDiffs(&b, old.Inputs, new.Inputs, step.Diffs, planning, indent, step.Op, summary, debug)
}
return b.String()
@ -218,7 +218,7 @@ func printObject(
b *bytes.Buffer, props resource.PropertyMap, planning bool,
indent int, op deploy.StepOp, prefix bool, debug bool) {
// Compute the maximum with of property keys so we can justify everything.
// Compute the maximum width of property keys so we can justify everything.
keys := props.StableKeys()
maxkey := maxKey(keys)
@ -444,12 +444,12 @@ func shortHash(hash string) string {
}
func printOldNewDiffs(
b *bytes.Buffer, olds resource.PropertyMap, news resource.PropertyMap,
b *bytes.Buffer, olds resource.PropertyMap, news resource.PropertyMap, include []resource.PropertyKey,
planning bool, indent int, op deploy.StepOp, summary bool, debug bool) {
// Get the full diff structure between the two, and print it (recursively).
if diff := olds.Diff(news); diff != nil {
printObjectDiff(b, *diff, planning, indent, summary, debug)
printObjectDiff(b, *diff, include, planning, indent, summary, debug)
} else {
// If there's no diff, report the op as Same - there's no diff to render
// so it should be rendered as if nothing changed.
@ -457,13 +457,27 @@ func printOldNewDiffs(
}
}
func printObjectDiff(b *bytes.Buffer, diff resource.ObjectDiff,
func printObjectDiff(b *bytes.Buffer, diff resource.ObjectDiff, include []resource.PropertyKey,
planning bool, indent int, summary bool, debug bool) {
contract.Assert(indent > 0)
// Compute the maximum with of property keys so we can justify everything.
// Compute the maximum width of property keys so we can justify everything. If an include set was given, filter out
// any properties that are not in the set.
keys := diff.Keys()
if include != nil {
includeSet := make(map[resource.PropertyKey]bool)
for _, k := range include {
includeSet[k] = true
}
var filteredKeys []resource.PropertyKey
for _, k := range keys {
if includeSet[k] {
filteredKeys = append(filteredKeys, k)
}
}
keys = filteredKeys
}
maxkey := maxKey(keys)
// To print an object diff, enumerate the keys in stable order, and print each property independently.
@ -525,7 +539,7 @@ func printPropertyValueDiff(
} else if diff.Object != nil {
titleFunc(op, true)
writeVerbatim(b, op, "{\n")
printObjectDiff(b, *diff.Object, planning, indent+1, summary, debug)
printObjectDiff(b, *diff.Object, nil, planning, indent+1, summary, debug)
writeWithIndentNoPrefix(b, indent, op, "}\n")
} else {
shouldPrintOld := shouldPrintPropertyValue(diff.Old, false)

View file

@ -107,6 +107,7 @@ type StepEventMetadata struct {
New *StepEventStateMetadata // the state of the resource after performing this step.
Res *StepEventStateMetadata // the latest state for the resource that is known (worst case, old).
Keys []resource.PropertyKey // the keys causing replacement (only for CreateStep and ReplaceStep).
Diffs []resource.PropertyKey // the keys causing diffs
Logical bool // true if this step represents a logical operation in the program.
Provider string // the provider that performed this step.
}
@ -177,11 +178,12 @@ type eventEmitter struct {
func makeStepEventMetadata(op deploy.StepOp, step deploy.Step, debug bool) StepEventMetadata {
contract.Assert(op == step.Op() || step.Op() == deploy.OpRefresh)
var keys []resource.PropertyKey
if step.Op() == deploy.OpCreateReplacement {
keys = step.(*deploy.CreateStep).Keys()
} else if step.Op() == deploy.OpReplace {
keys = step.(*deploy.ReplaceStep).Keys()
var keys, diffs []resource.PropertyKey
if keyer, hasKeys := step.(interface{ Keys() []resource.PropertyKey }); hasKeys {
keys = keyer.Keys()
}
if differ, hasDiffs := step.(interface{ Diffs() []resource.PropertyKey }); hasDiffs {
diffs = differ.Diffs()
}
return StepEventMetadata{
@ -189,6 +191,7 @@ func makeStepEventMetadata(op deploy.StepOp, step deploy.Step, debug bool) StepE
URN: step.URN(),
Type: step.Type(),
Keys: keys,
Diffs: diffs,
Old: makeStepEventStateMetadata(step.Old(), debug),
New: makeStepEventStateMetadata(step.New(), debug),
Res: makeStepEventStateMetadata(step.Res(), debug),

View file

@ -104,6 +104,7 @@ type CreateStep struct {
old *resource.State // the state of the existing resource (only for replacements).
new *resource.State // the state of the resource after this step.
keys []resource.PropertyKey // the keys causing replacement (only for replacements).
diffs []resource.PropertyKey // the keys causing a diff (only for replacements).
replacing bool // true if this is a create due to a replacement.
pendingDelete bool // true if this replacement should create a pending delete.
}
@ -126,7 +127,7 @@ func NewCreateStep(plan *Plan, reg RegisterResourceEvent, new *resource.State) S
}
func NewCreateReplacementStep(plan *Plan, reg RegisterResourceEvent,
old *resource.State, new *resource.State, keys []resource.PropertyKey, pendingDelete bool) Step {
old *resource.State, new *resource.State, keys, diffs []resource.PropertyKey, pendingDelete bool) Step {
contract.Assert(reg != nil)
contract.Assert(old != nil)
contract.Assert(old.URN != "")
@ -145,6 +146,7 @@ func NewCreateReplacementStep(plan *Plan, reg RegisterResourceEvent,
old: old,
new: new,
keys: keys,
diffs: diffs,
replacing: true,
pendingDelete: pendingDelete,
}
@ -156,15 +158,16 @@ func (s *CreateStep) Op() StepOp {
}
return OpCreate
}
func (s *CreateStep) Plan() *Plan { return s.plan }
func (s *CreateStep) Type() tokens.Type { return s.new.Type }
func (s *CreateStep) Provider() string { return s.new.Provider }
func (s *CreateStep) URN() resource.URN { return s.new.URN }
func (s *CreateStep) Old() *resource.State { return s.old }
func (s *CreateStep) New() *resource.State { return s.new }
func (s *CreateStep) Res() *resource.State { return s.new }
func (s *CreateStep) Keys() []resource.PropertyKey { return s.keys }
func (s *CreateStep) Logical() bool { return !s.replacing }
func (s *CreateStep) Plan() *Plan { return s.plan }
func (s *CreateStep) Type() tokens.Type { return s.new.Type }
func (s *CreateStep) Provider() string { return s.new.Provider }
func (s *CreateStep) URN() resource.URN { return s.new.URN }
func (s *CreateStep) Old() *resource.State { return s.old }
func (s *CreateStep) New() *resource.State { return s.new }
func (s *CreateStep) Res() *resource.State { return s.new }
func (s *CreateStep) Keys() []resource.PropertyKey { return s.keys }
func (s *CreateStep) Diffs() []resource.PropertyKey { return s.diffs }
func (s *CreateStep) Logical() bool { return !s.replacing }
func (s *CreateStep) Apply(preview bool) (resource.Status, StepCompleteFunc, error) {
var resourceError error
@ -341,12 +344,13 @@ type UpdateStep struct {
old *resource.State // the state of the existing resource.
new *resource.State // the newly computed state of the resource after updating.
stables []resource.PropertyKey // an optional list of properties that won't change during this update.
diffs []resource.PropertyKey // the keys causing a diff.
}
var _ Step = (*UpdateStep)(nil)
func NewUpdateStep(plan *Plan, reg RegisterResourceEvent, old *resource.State,
new *resource.State, stables []resource.PropertyKey) Step {
new *resource.State, stables, diffs []resource.PropertyKey) Step {
contract.Assert(old != nil)
contract.Assert(old.URN != "")
contract.Assert(old.ID != "" || !old.Custom)
@ -365,18 +369,20 @@ func NewUpdateStep(plan *Plan, reg RegisterResourceEvent, old *resource.State,
old: old,
new: new,
stables: stables,
diffs: diffs,
}
}
func (s *UpdateStep) Op() StepOp { return OpUpdate }
func (s *UpdateStep) Plan() *Plan { return s.plan }
func (s *UpdateStep) Type() tokens.Type { return s.old.Type }
func (s *UpdateStep) Provider() string { return s.old.Provider }
func (s *UpdateStep) URN() resource.URN { return s.old.URN }
func (s *UpdateStep) Old() *resource.State { return s.old }
func (s *UpdateStep) New() *resource.State { return s.new }
func (s *UpdateStep) Res() *resource.State { return s.new }
func (s *UpdateStep) Logical() bool { return true }
func (s *UpdateStep) Op() StepOp { return OpUpdate }
func (s *UpdateStep) Plan() *Plan { return s.plan }
func (s *UpdateStep) Type() tokens.Type { return s.old.Type }
func (s *UpdateStep) Provider() string { return s.old.Provider }
func (s *UpdateStep) URN() resource.URN { return s.old.URN }
func (s *UpdateStep) Old() *resource.State { return s.old }
func (s *UpdateStep) New() *resource.State { return s.new }
func (s *UpdateStep) Res() *resource.State { return s.new }
func (s *UpdateStep) Logical() bool { return true }
func (s *UpdateStep) Diffs() []resource.PropertyKey { return s.diffs }
func (s *UpdateStep) Apply(preview bool) (resource.Status, StepCompleteFunc, error) {
// Always propagate the URN and ID, even in previews and refreshes.
@ -429,13 +435,14 @@ type ReplaceStep struct {
old *resource.State // the state of the existing resource.
new *resource.State // the new state snapshot.
keys []resource.PropertyKey // the keys causing replacement.
diffs []resource.PropertyKey // the keys causing a diff.
pendingDelete bool // true if a pending deletion should happen.
}
var _ Step = (*ReplaceStep)(nil)
func NewReplaceStep(plan *Plan, old *resource.State, new *resource.State,
keys []resource.PropertyKey, pendingDelete bool) Step {
keys, diffs []resource.PropertyKey, pendingDelete bool) Step {
contract.Assert(old != nil)
contract.Assert(old.URN != "")
contract.Assert(old.ID != "" || !old.Custom)
@ -449,20 +456,22 @@ func NewReplaceStep(plan *Plan, old *resource.State, new *resource.State,
old: old,
new: new,
keys: keys,
diffs: diffs,
pendingDelete: pendingDelete,
}
}
func (s *ReplaceStep) Op() StepOp { return OpReplace }
func (s *ReplaceStep) Plan() *Plan { return s.plan }
func (s *ReplaceStep) Type() tokens.Type { return s.old.Type }
func (s *ReplaceStep) Provider() string { return s.old.Provider }
func (s *ReplaceStep) URN() resource.URN { return s.old.URN }
func (s *ReplaceStep) Old() *resource.State { return s.old }
func (s *ReplaceStep) New() *resource.State { return s.new }
func (s *ReplaceStep) Res() *resource.State { return s.new }
func (s *ReplaceStep) Keys() []resource.PropertyKey { return s.keys }
func (s *ReplaceStep) Logical() bool { return true }
func (s *ReplaceStep) Op() StepOp { return OpReplace }
func (s *ReplaceStep) Plan() *Plan { return s.plan }
func (s *ReplaceStep) Type() tokens.Type { return s.old.Type }
func (s *ReplaceStep) Provider() string { return s.old.Provider }
func (s *ReplaceStep) URN() resource.URN { return s.old.URN }
func (s *ReplaceStep) Old() *resource.State { return s.old }
func (s *ReplaceStep) New() *resource.State { return s.new }
func (s *ReplaceStep) Res() *resource.State { return s.new }
func (s *ReplaceStep) Keys() []resource.PropertyKey { return s.keys }
func (s *ReplaceStep) Diffs() []resource.PropertyKey { return s.diffs }
func (s *ReplaceStep) Logical() bool { return true }
func (s *ReplaceStep) Apply(preview bool) (resource.Status, StepCompleteFunc, error) {
// If this is a pending delete, we should have marked the old resource for deletion in the CreateReplacement step.

View file

@ -89,7 +89,7 @@ func (sg *stepGenerator) GenerateReadSteps(event ReadResourceEvent) ([]Step, *re
sg.replaces[urn] = true
return []Step{
NewReadReplacementStep(sg.plan, event, old, newState),
NewReplaceStep(sg.plan, old, newState, nil, true),
NewReplaceStep(sg.plan, old, newState, nil, nil, true),
}, nil
}
@ -222,8 +222,8 @@ func (sg *stepGenerator) GenerateSteps(event RegisterResourceEvent) ([]Step, *re
sg.replaces[urn] = true
keys := sg.dependentReplaceKeys[urn]
return []Step{
NewReplaceStep(sg.plan, old, new, nil, false),
NewCreateReplacementStep(sg.plan, event, old, new, keys, false),
NewReplaceStep(sg.plan, old, new, nil, nil, false),
NewCreateReplacementStep(sg.plan, event, old, new, keys, nil, false),
}, nil
}
@ -243,8 +243,8 @@ func (sg *stepGenerator) GenerateSteps(event RegisterResourceEvent) ([]Step, *re
}
return []Step{
NewCreateReplacementStep(sg.plan, event, old, new, nil, true),
NewReplaceStep(sg.plan, old, new, nil, true),
NewCreateReplacementStep(sg.plan, event, old, new, nil, nil, true),
NewReplaceStep(sg.plan, old, new, nil, nil, true),
}, nil
}
@ -363,14 +363,14 @@ func (sg *stepGenerator) GenerateSteps(event RegisterResourceEvent) ([]Step, *re
return append(steps,
NewDeleteReplacementStep(sg.plan, old, true),
NewReplaceStep(sg.plan, old, new, diff.ReplaceKeys, false),
NewCreateReplacementStep(sg.plan, event, old, new, diff.ReplaceKeys, false),
NewReplaceStep(sg.plan, old, new, diff.ReplaceKeys, diff.ChangedKeys, false),
NewCreateReplacementStep(sg.plan, event, old, new, diff.ReplaceKeys, diff.ChangedKeys, false),
), nil
}
return []Step{
NewCreateReplacementStep(sg.plan, event, old, new, diff.ReplaceKeys, true),
NewReplaceStep(sg.plan, old, new, diff.ReplaceKeys, true),
NewCreateReplacementStep(sg.plan, event, old, new, diff.ReplaceKeys, diff.ChangedKeys, true),
NewReplaceStep(sg.plan, old, new, diff.ReplaceKeys, diff.ChangedKeys, true),
// note that the delete step is generated "later" on, after all creates/updates finish.
}, nil
}
@ -380,14 +380,14 @@ func (sg *stepGenerator) GenerateSteps(event RegisterResourceEvent) ([]Step, *re
if logging.V(7) {
logging.V(7).Infof("Planner decided to update '%v' (oldprops=%v inputs=%v", urn, oldInputs, new.Inputs)
}
return []Step{NewUpdateStep(sg.plan, event, old, new, diff.StableKeys)}, nil
return []Step{NewUpdateStep(sg.plan, event, old, new, diff.StableKeys, diff.ChangedKeys)}, nil
}
// If resource was unchanged, but there were initialization errors, generate an empty update
// step to attempt to "continue" awaiting initialization.
if len(old.InitErrors) > 0 {
sg.updates[urn] = true
return []Step{NewUpdateStep(sg.plan, event, old, new, diff.StableKeys)}, nil
return []Step{NewUpdateStep(sg.plan, event, old, new, diff.StableKeys, nil)}, nil
}
// No need to update anything, the properties didn't change.

View file

@ -100,6 +100,7 @@ type DiffResult struct {
Changes DiffChanges // true if this diff represents a changed resource.
ReplaceKeys []resource.PropertyKey // an optional list of replacement keys.
StableKeys []resource.PropertyKey // an optional list of property keys that are stable.
ChangedKeys []resource.PropertyKey // an optional list of keys that changed.
DeleteBeforeReplace bool // if true, this resource must be deleted before recreating it.
}

View file

@ -298,14 +298,21 @@ func (p *provider) Diff(urn resource.URN, id resource.ID,
for _, stable := range resp.GetStables() {
stables = append(stables, resource.PropertyKey(stable))
}
var diffs []resource.PropertyKey
for _, diff := range resp.GetDiffs() {
diffs = append(diffs, resource.PropertyKey(diff))
}
changes := resp.GetChanges()
deleteBeforeReplace := resp.GetDeleteBeforeReplace()
logging.V(7).Infof("%s success: changes=%d #replaces=%v #stables=%v delbefrepl=%v",
label, changes, replaces, stables, deleteBeforeReplace)
logging.V(7).Infof("%s success: changes=%d #replaces=%v #stables=%v delbefrepl=%v, diffs=#%v",
label, changes, replaces, stables, deleteBeforeReplace, diffs)
return DiffResult{
Changes: DiffChanges(changes),
ReplaceKeys: replaces,
StableKeys: stables,
ChangedKeys: diffs,
DeleteBeforeReplace: deleteBeforeReplace,
}, nil
}