From 3d4cbace376efce89c3abbfef66d109a5b57b65d Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 21 Sep 2016 13:49:34 -0700 Subject: [PATCH] Vault diff --- nomad/structs/diff.go | 78 ++++++++---- nomad/structs/diff_test.go | 238 +++++++++++++++++++++++++++++++++++++ 2 files changed, 296 insertions(+), 20 deletions(-) diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index a5ae6ccd8..8066b936f 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -83,6 +83,10 @@ func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) { return nil, fmt.Errorf("can not diff jobs with different IDs: %q and %q", j.ID, other.ID) } + if !reflect.DeepEqual(j, other) { + diff.Type = DiffTypeEdited + } + jUpdate = &j.Update otherUpdate = &other.Update oldPrimitiveFlat = flatmap.Flatten(j, filter, true) @@ -94,7 +98,7 @@ func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) { diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, false) // Datacenters diff - if setDiff := stringSetDiff(j.Datacenters, other.Datacenters, "Datacenters"); setDiff != nil { + if setDiff := stringSetDiff(j.Datacenters, other.Datacenters, "Datacenters", contextual); setDiff != nil { diff.Objects = append(diff.Objects, setDiff) } @@ -126,20 +130,6 @@ func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) { diff.Objects = append(diff.Objects, pDiff) } - // If the job is not a delete or add, determine if there are edits. - if diff.Type == DiffTypeNone { - tgEdit := false - for _, tg := range diff.TaskGroups { - if tg.Type != DiffTypeNone { - tgEdit = true - break - } - } - if tgEdit || len(diff.Fields)+len(diff.Objects) != 0 { - diff.Type = DiffTypeEdited - } - } - return diff, nil } @@ -396,6 +386,12 @@ func (t *Task) Diff(other *Task, contextual bool) (*TaskDiff, error) { diff.Objects = append(diff.Objects, sDiffs...) } + // Vault diff + vDiff := vaultDiff(t.Vault, other.Vault, contextual) + if vDiff != nil { + diff.Objects = append(diff.Objects, vDiff) + } + return diff, nil } @@ -589,6 +585,39 @@ func serviceCheckDiffs(old, new []*ServiceCheck, contextual bool) []*ObjectDiff return diffs } +// vaultDiff returns the diff of two vault objects. If contextual diff is +// enabled, all fields will be returned, even if no diff occurred. +func vaultDiff(old, new *Vault, contextual bool) *ObjectDiff { + diff := &ObjectDiff{Type: DiffTypeNone, Name: "Vault"} + var oldPrimitiveFlat, newPrimitiveFlat map[string]string + + if reflect.DeepEqual(old, new) { + return nil + } else if old == nil { + old = &Vault{} + diff.Type = DiffTypeAdded + newPrimitiveFlat = flatmap.Flatten(new, nil, true) + } else if new == nil { + new = &Vault{} + diff.Type = DiffTypeDeleted + oldPrimitiveFlat = flatmap.Flatten(old, nil, true) + } else { + diff.Type = DiffTypeEdited + oldPrimitiveFlat = flatmap.Flatten(old, nil, true) + newPrimitiveFlat = flatmap.Flatten(new, nil, true) + } + + // Diff the primitive fields. + diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, contextual) + + // Policies diffs + if setDiff := stringSetDiff(old.Policies, new.Policies, "Policies", contextual); setDiff != nil { + diff.Objects = append(diff.Objects, setDiff) + } + + return diff +} + // Diff returns a diff of two resource objects. If contextual diff is enabled, // non-changed fields will still be returned. func (r *Resources) Diff(other *Resources, contextual bool) *ObjectDiff { @@ -944,7 +973,7 @@ func fieldDiffs(old, new map[string]string, contextual bool) []*FieldDiff { } // stringSetDiff diffs two sets of strings with the given name. -func stringSetDiff(old, new []string, name string) *ObjectDiff { +func stringSetDiff(old, new []string, name string, contextual bool) *ObjectDiff { oldMap := make(map[string]struct{}, len(old)) newMap := make(map[string]struct{}, len(new)) for _, o := range old { @@ -953,7 +982,7 @@ func stringSetDiff(old, new []string, name string) *ObjectDiff { for _, n := range new { newMap[n] = struct{}{} } - if reflect.DeepEqual(oldMap, newMap) { + if reflect.DeepEqual(oldMap, newMap) && !contextual { return nil } @@ -961,14 +990,16 @@ func stringSetDiff(old, new []string, name string) *ObjectDiff { var added, removed bool for k := range oldMap { if _, ok := newMap[k]; !ok { - diff.Fields = append(diff.Fields, fieldDiff(k, "", name, false)) + diff.Fields = append(diff.Fields, fieldDiff(k, "", name, contextual)) removed = true + } else if contextual { + diff.Fields = append(diff.Fields, fieldDiff(k, k, name, contextual)) } } for k := range newMap { if _, ok := oldMap[k]; !ok { - diff.Fields = append(diff.Fields, fieldDiff("", k, name, false)) + diff.Fields = append(diff.Fields, fieldDiff("", k, name, contextual)) added = true } } @@ -980,8 +1011,15 @@ func stringSetDiff(old, new []string, name string) *ObjectDiff { diff.Type = DiffTypeEdited } else if added { diff.Type = DiffTypeAdded - } else { + } else if removed { diff.Type = DiffTypeDeleted + } else { + // Diff of an empty set + if len(diff.Fields) == 0 { + return nil + } + + diff.Type = DiffTypeNone } return diff diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 7cb03ecee..87d5c4b44 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -396,6 +396,39 @@ func TestJobDiff(t *testing.T) { }, }, }, + { + // Datacenter contextual + Contextual: true, + Old: &Job{ + Datacenters: []string{"foo", "bar"}, + }, + New: &Job{ + Datacenters: []string{"foo", "bar"}, + }, + Expected: &JobDiff{ + Type: DiffTypeNone, + Objects: []*ObjectDiff{ + { + Type: DiffTypeNone, + Name: "Datacenters", + Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Datacenters", + Old: "bar", + New: "bar", + }, + { + Type: DiffTypeNone, + Name: "Datacenters", + Old: "foo", + New: "foo", + }, + }, + }, + }, + }, + }, { // Update strategy edited Old: &Job{ @@ -2965,6 +2998,211 @@ func TestTaskDiff(t *testing.T) { }, }, }, + { + // Vault added + Old: &Task{}, + New: &Task{ + Vault: &Vault{ + Policies: []string{"foo", "bar"}, + Env: true, + }, + }, + Expected: &TaskDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Vault", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Env", + Old: "", + New: "true", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Policies", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Policies", + Old: "", + New: "bar", + }, + { + Type: DiffTypeAdded, + Name: "Policies", + Old: "", + New: "foo", + }, + }, + }, + }, + }, + }, + }, + }, + { + // Vault deleted + Old: &Task{ + Vault: &Vault{ + Policies: []string{"foo", "bar"}, + Env: true, + }, + }, + New: &Task{}, + Expected: &TaskDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeDeleted, + Name: "Vault", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "Env", + Old: "true", + New: "", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeDeleted, + Name: "Policies", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "Policies", + Old: "bar", + New: "", + }, + { + Type: DiffTypeDeleted, + Name: "Policies", + Old: "foo", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, + { + // Vault edited + Old: &Task{ + Vault: &Vault{ + Policies: []string{"foo", "bar"}, + Env: true, + }, + }, + New: &Task{ + Vault: &Vault{ + Policies: []string{"bar", "baz"}, + Env: false, + }, + }, + Expected: &TaskDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Vault", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Env", + Old: "true", + New: "false", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Policies", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Policies", + Old: "", + New: "baz", + }, + { + Type: DiffTypeDeleted, + Name: "Policies", + Old: "foo", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, + { + // LogConfig edited with context + Contextual: true, + Old: &Task{ + Vault: &Vault{ + Policies: []string{"foo", "bar"}, + Env: true, + }, + }, + New: &Task{ + Vault: &Vault{ + Policies: []string{"bar", "baz"}, + Env: true, + }, + }, + Expected: &TaskDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Vault", + Fields: []*FieldDiff{ + { + Type: DiffTypeNone, + Name: "Env", + Old: "true", + New: "true", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Policies", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Policies", + Old: "", + New: "baz", + }, + { + Type: DiffTypeNone, + Name: "Policies", + Old: "bar", + New: "bar", + }, + { + Type: DiffTypeDeleted, + Name: "Policies", + Old: "foo", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, } for i, c := range cases {