From e7b1c309ef00597f3d63bd858bc23add910be57c Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 2 May 2016 18:32:23 -0700 Subject: [PATCH 01/17] Job diff infrastructure --- nomad/structs/diff/job.go | 1037 ++++++++++++++++++++++++++++++++ nomad/structs/diff/job_test.go | 469 +++++++++++++++ 2 files changed, 1506 insertions(+) create mode 100644 nomad/structs/diff/job.go create mode 100644 nomad/structs/diff/job_test.go diff --git a/nomad/structs/diff/job.go b/nomad/structs/diff/job.go new file mode 100644 index 000000000..0eaccb9c3 --- /dev/null +++ b/nomad/structs/diff/job.go @@ -0,0 +1,1037 @@ +package diff + +import ( + "fmt" + "reflect" + + "github.com/hashicorp/nomad/nomad/structs" + "github.com/mitchellh/hashstructure" +) + +// The below are the set of primitive fields that can be diff'd automatically +// using FieldDiff. +var ( + jobPrimitiveFields = []string{ + "Region", + "ID", + "ParentID", + "Name", + "Type", + "Priority", + "AllAtOnce", + } + + constraintFields = []string{ + "LTarget", + "RTarget", + "Operand", + } + + updateStrategyFields = []string{ + "Stagger", + "MaxParallel", + } + + periodicConfigFields = []string{ + "Enabled", + "Spec", + "SpecType", + "ProhibitOverlap", + } + + taskGroupPrimitiveFields = []string{ + "Name", + "Count", + } + + restartPolicyFields = []string{ + "Attempts", + "Interval", + "Delay", + "Mode", + } + + taskPrimitiveFields = []string{ + "Name", + "Driver", + "User", + "KillTimeout", + } + + logConfigFields = []string{ + "MaxFiles", + "MaxFileSizeMB", + } + + servicePrimitiveFields = []string{ + "Name", + "PortLabel", + } + + serviceCheckPrimitiveFields = []string{ + "Name", + "Type", + "Command", + "Path", + "Protocol", + "Interval", + "Timeout", + } + + taskArtifactPrimitiveFields = []string{ + "GetterSource", + "RelativeDest", + } + + resourcesPrimitiveFields = []string{ + "CPU", + "MemoryMB", + "DiskMB", + "IOPS", + } + + networkResourcePrimitiveFields = []string{ + "Device", + "CIDR", + "IP", + "MBits", + } + + portFields = []string{ + "Label", + "Value", + } +) + +// DiffType is the high-level type of the diff. +type DiffType string + +const ( + DiffTypeNone DiffType = "Equal" + DiffTypeAdded = "Added" + DiffTypeDeleted = "Deleted" + DiffTypeEdited = "Edited" +) + +// DiffEntry contains information about a diff. +type DiffEntry struct { + Type DiffType + Annotations []string +} + +func (d *DiffEntry) SetDiffType(old, new interface{}) { + if reflect.DeepEqual(old, new) { + d.Type = DiffTypeNone + } else if old == nil { + d.Type = DiffTypeAdded + } else if new == nil { + d.Type = DiffTypeDeleted + } else { + d.Type = DiffTypeEdited + } +} + +// JobDiff contains the set of changes betwen two Jobs. +type JobDiff struct { + PrimitiveStructDiff + Constraints []*PrimitiveStructDiff + Datacenters *StringSetDiff + Update *PrimitiveStructDiff + Periodic *PrimitiveStructDiff + Meta *StringMapDiff + TaskGroups *TaskGroupsDiff +} + +// TaskGroupsDiff contains the set of Task Groups that were changed. +type TaskGroupsDiff struct { + DiffEntry + Added, Deleted []*structs.TaskGroup + Edited []*TaskGroupDiff +} + +// TaskGroupsDiff contains the set of changes between two Task Groups. +type TaskGroupDiff struct { + PrimitiveStructDiff + Constraints []*PrimitiveStructDiff + RestartPolicy *PrimitiveStructDiff + Meta *StringMapDiff + Tasks *TasksDiff +} + +// TasksDiff contains the set of Tasks that were changed. +type TasksDiff struct { + DiffEntry + Added, Deleted []*structs.Task + Edited []*TaskDiff +} + +// TaskDiff contains the changes between two Tasks. +type TaskDiff struct { + PrimitiveStructDiff + Constraints []*PrimitiveStructDiff + LogConfig *PrimitiveStructDiff + Env *StringMapDiff + Meta *StringMapDiff + Services *ServicesDiff + Artifacts *TaskArtifactsDiff + Resources *ResourcesDiff +} + +// ServicesDiff contains the set of Services that were changed. +type ServicesDiff struct { + DiffEntry + Added, Deleted []*structs.Service + Edited []*ServiceDiff +} + +// ServiceDiff contains the changes between two Services. +type ServiceDiff struct { + PrimitiveStructDiff + Tags *StringSetDiff + Checks *ServiceChecksDiff +} + +// ServiceChecksDiff contains the set of Service Checks that were changed. +type ServiceChecksDiff struct { + DiffEntry + Added, Deleted []*structs.ServiceCheck + Edited []*ServiceCheckDiff +} + +// ServiceCheckDiff contains the changes between two Service Checks. +type ServiceCheckDiff struct { + PrimitiveStructDiff + Args *StringSetDiff +} + +// TaskArtifactsDiff contains the set of Task Artifacts that were changed. +type TaskArtifactsDiff struct { + DiffEntry + Added, Deleted []*structs.TaskArtifact + Edited []*TaskArtifactDiff +} + +// TaskArtifactDiff contains the diff between two Task Artifacts. +type TaskArtifactDiff struct { + PrimitiveStructDiff + GetterOptions *StringMapDiff +} + +// ResourcesDiff contains the diff between two Resources. +type ResourcesDiff struct { + PrimitiveStructDiff + Networks *NetworkResourcesDiff +} + +// NetworkResourcesDiff contains the set of Network Resources that were changed. +type NetworkResourcesDiff struct { + DiffEntry + Added, Deleted []*NetworkResourceDiff +} + +// NetworkResourceDiff contains the diff between two Network Resources. +type NetworkResourceDiff struct { + PrimitiveStructDiff + ReservedPorts *PortsDiff + DynamicPorts *PortsDiff +} + +// PortsDiff contains the difference between two sets of Ports. +type PortsDiff struct { + DiffEntry + Added, Deleted []structs.Port + Edited []*PrimitiveStructDiff +} + +// PrimitiveStructDiff contains the diff of two structs that only contain +// primitive fields. +type PrimitiveStructDiff struct { + DiffEntry + PrimitiveFields []*FieldDiff +} + +func (p *PrimitiveStructDiff) DiffFields(old, new interface{}, fields []string) { + for _, field := range fields { + oldV := getField(old, field) + newV := getField(new, field) + pDiff := NewFieldDiff(field, oldV, newV) + if pDiff != nil { + p.Type = DiffTypeEdited + p.PrimitiveFields = append(p.PrimitiveFields, pDiff) + } + } +} + +// FieldDiff contains the diff between an old and new version of a field. +type FieldDiff struct { + DiffEntry + Name string + OldValue interface{} + NewValue interface{} +} + +// StringSetDiff captures the changes that occured between two sets of strings +type StringSetDiff struct { + DiffEntry + Added, Deleted []string +} + +// StringMapDiff captures the changes that occured between two string maps +type StringMapDiff struct { + DiffEntry + Added, Deleted map[string]string + Edited map[string]StringValueDelta +} + +// StringValueDelta holds the old and new value of a string. +type StringValueDelta struct { + DiffEntry + Old, New string +} + +// NewJobDiff returns the diff between two jobs. If there is no difference, nil +// is returned. +func NewJobDiff(old, new *structs.Job) *JobDiff { + if old == nil && new == nil || reflect.DeepEqual(old, new) { + return nil + } + + // Get the diffs of the primitive fields + diff := &JobDiff{} + diff.DiffFields(old, new, jobPrimitiveFields) + + // Get the diff of the datacenters + diff.Datacenters = NewStringSetDiff(old.Datacenters, new.Datacenters) + + // Get the diff of the constraints. + diff.Constraints = setDiffPrimitiveStructs( + interfaceSlice(old.Constraints), + interfaceSlice(new.Constraints), + constraintFields) + + // Get the update strategy diff + diff.Update = NewPrimitiveStructDiff(old.Update, new.Update, updateStrategyFields) + + // Get the update strategy diff + diff.Periodic = NewPrimitiveStructDiff(old.Periodic, new.Periodic, periodicConfigFields) + + // Get the meta diff + diff.Meta = NewStringMapDiff(old.Meta, new.Meta) + + // Get the task group diff + diff.TaskGroups = setDiffTaskGroups(old.TaskGroups, new.TaskGroups) + + // If there are no changes return nil + if len(diff.PrimitiveFields)+len(diff.Constraints) == 0 && + diff.Datacenters == nil && + diff.Update == nil && + diff.Periodic == nil && + diff.Meta == nil && + diff.TaskGroups == nil { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewTaskGroupDiff returns the diff between two task groups. If there is no +// difference, nil is returned. +func NewTaskGroupDiff(old, new *structs.TaskGroup) *TaskGroupDiff { + if old == nil && new == nil || reflect.DeepEqual(old, new) { + return nil + } + + // Get the diffs of the primitive fields + diff := &TaskGroupDiff{} + diff.DiffFields(old, new, taskGroupPrimitiveFields) + + // Get the diff of the constraints. + diff.Constraints = setDiffPrimitiveStructs( + interfaceSlice(old.Constraints), + interfaceSlice(new.Constraints), + constraintFields) + + // Get the restart policy diff + diff.RestartPolicy = NewPrimitiveStructDiff(old.RestartPolicy, new.RestartPolicy, restartPolicyFields) + + // Get the meta diff + diff.Meta = NewStringMapDiff(old.Meta, new.Meta) + + // Get the task diff + diff.Tasks = setDiffTasks(old.Tasks, new.Tasks) + + // If there are no changes return nil + if len(diff.PrimitiveFields)+len(diff.Constraints) == 0 && + diff.Tasks == nil && + diff.RestartPolicy == nil && + diff.Meta == nil { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewTaskDiff returns the diff between two tasks. If there is no difference, +// nil is returned. +func NewTaskDiff(old, new *structs.Task) *TaskDiff { + if old == nil && new == nil || reflect.DeepEqual(old, new) { + return nil + } + + // Get the diffs of the primitive fields + diff := &TaskDiff{} + diff.DiffFields(old, new, taskPrimitiveFields) + + // Get the diff of the constraints. + diff.Constraints = setDiffPrimitiveStructs( + interfaceSlice(old.Constraints), + interfaceSlice(new.Constraints), + constraintFields) + + // Get the meta and env diff + diff.Meta = NewStringMapDiff(old.Meta, new.Meta) + diff.Env = NewStringMapDiff(old.Env, new.Env) + + // Get the log config diff + diff.LogConfig = NewPrimitiveStructDiff(old.LogConfig, new.LogConfig, logConfigFields) + + // Get the services diff + diff.Services = setDiffServices(old.Services, new.Services) + + // Get the artifacts diff + diff.Artifacts = setDiffTaskArtifacts(old.Artifacts, new.Artifacts) + + // Get the resource diff + diff.Resources = NewResourcesDiff(old.Resources, new.Resources) + + // Get the task config diff + // TODO: Flat map the config and diff it. + + // If there are no changes return nil + if len(diff.PrimitiveFields)+len(diff.Constraints) == 0 && + diff.Artifacts == nil && + diff.LogConfig == nil && + diff.Services == nil && + diff.Env == nil && + diff.Meta == nil { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewServiceDiff returns the diff between two services. If there is no +// difference, nil is returned. +func NewServiceDiff(old, new *structs.Service) *ServiceDiff { + if old == nil && new == nil || reflect.DeepEqual(old, new) { + return nil + } + + // Get the diffs of the primitive fields + diff := &ServiceDiff{} + diff.DiffFields(old, new, servicePrimitiveFields) + + // Get the tags diff + diff.Tags = NewStringSetDiff(old.Tags, new.Tags) + + // Get the checks diff + diff.Checks = setDiffServiceChecks(old.Checks, new.Checks) + + // If there are no changes return nil + if len(diff.PrimitiveFields) == 0 && + diff.Checks == nil && + diff.Tags == nil { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewServiceCheckDiff returns the diff between two service checks. If there is +// no difference, nil is returned. +func NewServiceCheckDiff(old, new *structs.ServiceCheck) *ServiceCheckDiff { + if old == nil && new == nil || reflect.DeepEqual(old, new) { + return nil + } + + // Get the diffs of the primitive fields + diff := &ServiceCheckDiff{} + diff.DiffFields(old, new, serviceCheckPrimitiveFields) + + // Get the args diff + diff.Args = NewStringSetDiff(old.Args, new.Args) + + // If there are no changes return nil + if len(diff.PrimitiveFields) == 0 && + diff.Args == nil { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewTaskArtifactDiff returns the diff between two task artifacts. If there is +// no difference, nil is returned. +func NewTaskArtifactDiff(old, new *structs.TaskArtifact) *TaskArtifactDiff { + if old == nil && new == nil || reflect.DeepEqual(old, new) { + return nil + } + + // Get the diffs of the primitive fields + diff := &TaskArtifactDiff{} + diff.DiffFields(old, new, taskArtifactPrimitiveFields) + + // Get the args diff + diff.GetterOptions = NewStringMapDiff(old.GetterOptions, new.GetterOptions) + + // If there are no changes return nil + if len(diff.PrimitiveFields) == 0 && + diff.GetterOptions == nil { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewResourcesDiff returns the diff between two resources. If there is no +// difference, nil is returned. +func NewResourcesDiff(old, new *structs.Resources) *ResourcesDiff { + if old == nil && new == nil || reflect.DeepEqual(old, new) { + return nil + } + + // Get the diffs of the primitive fields + diff := &ResourcesDiff{} + diff.DiffFields(old, new, resourcesPrimitiveFields) + + // Get the network resource diff + diff.Networks = setDiffNetworkResources(old.Networks, new.Networks) + + // If there are no changes return nil + if len(diff.PrimitiveFields) == 0 && + diff.Networks == nil { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewNetworkResourceDiff returns the diff between two network resources. If +// there is no difference, nil is returned. +func NewNetworkResourceDiff(old, new *structs.NetworkResource) *NetworkResourceDiff { + if old == nil && new == nil || reflect.DeepEqual(old, new) { + return nil + } + + // Get the diffs of the primitive fields + diff := &NetworkResourceDiff{} + diff.DiffFields(old, new, networkResourcePrimitiveFields) + + // Get the port diffs + diff.ReservedPorts = setDiffPorts(old.ReservedPorts, new.ReservedPorts) + diff.DynamicPorts = setDiffPorts(old.DynamicPorts, new.DynamicPorts) + + // If there are no changes return nil + if len(diff.PrimitiveFields) == 0 && + diff.DynamicPorts == nil && + diff.ReservedPorts == nil { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewPrimitiveStructDiff returns the diff between two structs containing only +// primitive fields. The list of fields to be diffed is passed via the fields +// parameter. If there is no difference, nil is returned. +func NewPrimitiveStructDiff(old, new interface{}, fields []string) *PrimitiveStructDiff { + if reflect.DeepEqual(old, new) { + return nil + } + + // Diff the individual fields + diff := &PrimitiveStructDiff{} + diff.DiffFields(old, new, fields) + if len(diff.PrimitiveFields) == 0 { + return nil + } + + diff.SetDiffType(old, new) + return diff +} + +// NewFieldDiff returns the diff between two fields. If there is no difference, +// nil is returned. +func NewFieldDiff(name string, old, new interface{}) *FieldDiff { + diff := &FieldDiff{Name: name} + if reflect.DeepEqual(old, new) { + return nil + } else if old == nil { + diff.Type = DiffTypeAdded + diff.NewValue = new + } else if new == nil { + diff.Type = DiffTypeDeleted + diff.OldValue = old + } else { + diff.Type = DiffTypeEdited + diff.OldValue = old + diff.NewValue = new + } + + return diff +} + +// NewStringSetDiff returns the diff between two sets of strings. If there is no +// difference, nil is returned. +func NewStringSetDiff(old, new []string) *StringSetDiff { + diff := &StringSetDiff{} + lOld, lNew := len(old), len(new) + if reflect.DeepEqual(old, new) { + return nil + } else if lOld == 0 && lNew > 0 { + diff.Type = DiffTypeAdded + return diff + } else if lNew == 0 && lOld > 0 { + diff.Type = DiffTypeDeleted + return diff + } + + diff.Type = DiffTypeEdited + makeMap := func(inputs []string) map[string]interface{} { + m := make(map[string]interface{}) + for _, in := range inputs { + m[in] = struct{}{} + } + return m + } + + added, deleted, _, _ := keyedSetDifference(makeMap(old), makeMap(new)) + for k := range added { + diff.Added = append(diff.Added, k) + } + for k := range deleted { + diff.Deleted = append(diff.Deleted, k) + } + return diff +} + +// NewStringMapDiff returns the diff between two maps of strings. If there is no +// difference, nil is returned. +func NewStringMapDiff(old, new map[string]string) *StringMapDiff { + diff := &StringMapDiff{} + lOld, lNew := len(old), len(new) + if reflect.DeepEqual(old, new) { + return nil + } else if lOld == 0 && lNew > 0 { + diff.Type = DiffTypeAdded + return diff + } else if lNew == 0 && lOld > 0 { + diff.Type = DiffTypeDeleted + return diff + } + + diff.Type = DiffTypeEdited + diff.Added = make(map[string]string) + diff.Deleted = make(map[string]string) + diff.Edited = make(map[string]StringValueDelta) + + for k, v := range old { + if _, ok := new[k]; !ok { + diff.Deleted[k] = v + } + } + for k, newV := range new { + oldV, ok := old[k] + if !ok { + diff.Added[k] = newV + continue + } + + // Key is in both, check if they have been edited. + if newV != oldV { + d := StringValueDelta{Old: oldV, New: newV} + d.Type = DiffTypeEdited + diff.Edited[k] = d + } + } + + if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { + return nil + } + return diff +} + +// Set helpers + +// setDiffTaskGroups does a set difference of task groups using the task group +// name as a key. +func setDiffTaskGroups(old, new []*structs.TaskGroup) *TaskGroupsDiff { + diff := &TaskGroupsDiff{} + + oldMap := make(map[string]*structs.TaskGroup) + newMap := make(map[string]*structs.TaskGroup) + for _, tg := range old { + oldMap[tg.Name] = tg + } + for _, tg := range new { + newMap[tg.Name] = tg + } + + for k, v := range oldMap { + if _, ok := newMap[k]; !ok { + diff.Deleted = append(diff.Deleted, v) + } + } + for k, newV := range newMap { + oldV, ok := oldMap[k] + if !ok { + diff.Added = append(diff.Added, newV) + continue + } + + // Key is in both, check if they have been edited. + if !reflect.DeepEqual(oldV, newV) { + tgdiff := NewTaskGroupDiff(oldV, newV) + diff.Edited = append(diff.Edited, tgdiff) + } + } + + if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { + return nil + } + return diff +} + +// setDiffTasks does a set difference of tasks using the task name as a key. +func setDiffTasks(old, new []*structs.Task) *TasksDiff { + diff := &TasksDiff{} + + oldMap := make(map[string]*structs.Task) + newMap := make(map[string]*structs.Task) + for _, task := range old { + oldMap[task.Name] = task + } + for _, task := range new { + newMap[task.Name] = task + } + + for k, v := range oldMap { + if _, ok := newMap[k]; !ok { + diff.Deleted = append(diff.Deleted, v) + } + } + for k, newV := range newMap { + oldV, ok := oldMap[k] + if !ok { + diff.Added = append(diff.Added, newV) + continue + } + + // Key is in both, check if they have been edited. + if !reflect.DeepEqual(oldV, newV) { + tdiff := NewTaskDiff(oldV, newV) + diff.Edited = append(diff.Edited, tdiff) + } + } + + if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { + return nil + } + return diff +} + +// setDiffServices does a set difference of Services using the service name as a +// key. +func setDiffServices(old, new []*structs.Service) *ServicesDiff { + diff := &ServicesDiff{} + + oldMap := make(map[string]*structs.Service) + newMap := make(map[string]*structs.Service) + for _, s := range old { + oldMap[s.Name] = s + } + for _, s := range new { + newMap[s.Name] = s + } + + for k, v := range oldMap { + if _, ok := newMap[k]; !ok { + diff.Deleted = append(diff.Deleted, v) + } + } + for k, newV := range newMap { + oldV, ok := oldMap[k] + if !ok { + diff.Added = append(diff.Added, newV) + continue + } + + // Key is in both, check if they have been edited. + if !reflect.DeepEqual(oldV, newV) { + sdiff := NewServiceDiff(oldV, newV) + diff.Edited = append(diff.Edited, sdiff) + } + } + + if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { + return nil + } + return diff +} + +// setDiffServiceChecks does a set difference of service checks using the check +// name as a key. +func setDiffServiceChecks(old, new []*structs.ServiceCheck) *ServiceChecksDiff { + diff := &ServiceChecksDiff{} + + oldMap := make(map[string]*structs.ServiceCheck) + newMap := make(map[string]*structs.ServiceCheck) + for _, s := range old { + oldMap[s.Name] = s + } + for _, s := range new { + newMap[s.Name] = s + } + + for k, v := range oldMap { + if _, ok := newMap[k]; !ok { + diff.Deleted = append(diff.Deleted, v) + } + } + for k, newV := range newMap { + oldV, ok := oldMap[k] + if !ok { + diff.Added = append(diff.Added, newV) + continue + } + + // Key is in both, check if they have been edited. + if !reflect.DeepEqual(oldV, newV) { + sdiff := NewServiceCheckDiff(oldV, newV) + diff.Edited = append(diff.Edited, sdiff) + } + } + + if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { + return nil + } + return diff +} + +// setDiffTaskArtifacts does a set difference of task artifacts using the geter +// source as a key. +func setDiffTaskArtifacts(old, new []*structs.TaskArtifact) *TaskArtifactsDiff { + diff := &TaskArtifactsDiff{} + + oldMap := make(map[string]*structs.TaskArtifact) + newMap := make(map[string]*structs.TaskArtifact) + for _, ta := range old { + oldMap[ta.GetterSource] = ta + } + for _, ta := range new { + newMap[ta.GetterSource] = ta + } + + for k, v := range oldMap { + if _, ok := newMap[k]; !ok { + diff.Deleted = append(diff.Deleted, v) + } + } + for k, newV := range newMap { + oldV, ok := oldMap[k] + if !ok { + diff.Added = append(diff.Added, newV) + continue + } + + // Key is in both, check if they have been edited. + if !reflect.DeepEqual(oldV, newV) { + tdiff := NewTaskArtifactDiff(oldV, newV) + diff.Edited = append(diff.Edited, tdiff) + } + } + + if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { + return nil + } + return diff +} + +// setDiffNetworkResources does a set difference of network resources. +func setDiffNetworkResources(old, new []*structs.NetworkResource) *NetworkResourcesDiff { + diff := &NetworkResourcesDiff{} + + added, del := setDifference(interfaceSlice(old), interfaceSlice(new)) + for _, a := range added { + nDiff := NewNetworkResourceDiff(nil, a.(*structs.NetworkResource)) + diff.Added = append(diff.Added, nDiff) + } + for _, d := range del { + nDiff := NewNetworkResourceDiff(d.(*structs.NetworkResource), nil) + diff.Added = append(diff.Deleted, nDiff) + } + + return diff +} + +// setDiffPorts does a set difference of ports using the label as a key. +func setDiffPorts(old, new []structs.Port) *PortsDiff { + diff := &PortsDiff{} + + oldMap := make(map[string]structs.Port) + newMap := make(map[string]structs.Port) + for _, p := range old { + oldMap[p.Label] = p + } + for _, p := range new { + newMap[p.Label] = p + } + + for k, v := range oldMap { + if _, ok := newMap[k]; !ok { + diff.Deleted = append(diff.Deleted, v) + } + } + for k, newV := range newMap { + oldV, ok := oldMap[k] + if !ok { + diff.Added = append(diff.Added, newV) + continue + } + + // Key is in both, check if they have been edited. + if !reflect.DeepEqual(oldV, newV) { + pdiff := NewPrimitiveStructDiff(oldV, newV, portFields) + diff.Edited = append(diff.Edited, pdiff) + } + } + + if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { + return nil + } + return diff +} + +// setDiffPrimitiveStructs does a set difference on primitive structs. The +// caller must pass the primitive structs fields. +func setDiffPrimitiveStructs(old, new []interface{}, fields []string) []*PrimitiveStructDiff { + var diffs []*PrimitiveStructDiff + + added, del := setDifference(old, new) + for _, a := range added { + pDiff := NewPrimitiveStructDiff(nil, a, fields) + diffs = append(diffs, pDiff) + } + for _, d := range del { + pDiff := NewPrimitiveStructDiff(d, nil, fields) + diffs = append(diffs, pDiff) + } + + return diffs +} + +// Reflective helpers. + +// setDifference does a set difference on two sets of interfaces and returns the +// values that were added or deleted when comparing the new to old. +func setDifference(old, new []interface{}) (added, deleted []interface{}) { + makeSet := func(objects []interface{}) map[string]interface{} { + objMap := make(map[string]interface{}, len(objects)) + for _, obj := range objects { + hash, err := hashstructure.Hash(obj, nil) + if err != nil { + panic(err) + } + objMap[fmt.Sprintf("%d", hash)] = obj + } + + return objMap + } + + addedMap, deletedMap, _, _ := keyedSetDifference(makeSet(old), makeSet(new)) + flatten := func(in map[string]interface{}) []interface{} { + out := make([]interface{}, 0, len(in)) + for _, v := range in { + out = append(out, v) + } + return out + } + + return flatten(addedMap), flatten(deletedMap) +} + +// keyedSetDifference does a set difference on keyed object and returns the +// objects that have been added, deleted, edited and unmodified when comparing +// the new to old set. +func keyedSetDifference(old, new map[string]interface{}) ( + added, deleted map[string]interface{}, edited, unmodified []string) { + + added = make(map[string]interface{}) + deleted = make(map[string]interface{}) + + for k, v := range old { + if _, ok := new[k]; !ok { + deleted[k] = v + } + } + for k, newV := range new { + oldV, ok := old[k] + if !ok { + added[k] = newV + continue + } + + // Key is in both, check if they have been edited. + if reflect.DeepEqual(oldV, newV) { + unmodified = append(unmodified, k) + } else { + edited = append(edited, k) + } + } + + return added, deleted, edited, unmodified +} + +// interfaceSlice is a helper method that takes a slice of typed elements and +// returns a slice of interface. This method will panic if given a non-slice +// input. +func interfaceSlice(slice interface{}) []interface{} { + s := reflect.ValueOf(slice) + if s.Kind() != reflect.Slice { + panic("InterfaceSlice() given a non-slice type") + } + + ret := make([]interface{}, s.Len()) + + for i := 0; i < s.Len(); i++ { + ret[i] = s.Index(i).Interface() + } + + return ret +} + +// getField is a helper that returns the passed fields value of the given +// object. This method will panic if the field does not exist in the passed +// object. +func getField(obj interface{}, field string) interface{} { + if obj == nil { + return nil + } + + r := reflect.ValueOf(obj) + r = reflect.Indirect(r) + if !r.IsValid() { + return nil + } + + f := r.FieldByName(field) + return f.Interface() +} diff --git a/nomad/structs/diff/job_test.go b/nomad/structs/diff/job_test.go new file mode 100644 index 000000000..2b3608648 --- /dev/null +++ b/nomad/structs/diff/job_test.go @@ -0,0 +1,469 @@ +package diff + +import ( + "reflect" + "sort" + "testing" + + "github.com/davecgh/go-spew/spew" + "github.com/hashicorp/nomad/nomad/mock" + "github.com/hashicorp/nomad/nomad/structs" +) + +func TestNewJobDiff_Same(t *testing.T) { + job1 := mock.Job() + job2 := mock.Job() + job2.ID = job1.ID + + diff := NewJobDiff(job1, job2) + if diff != nil { + t.Fatalf("expected nil job diff; got %s", spew.Sdump(diff)) + } +} + +func TestNewJobDiff_Constraints(t *testing.T) { + c1 := &structs.Constraint{LTarget: "foo"} + c2 := &structs.Constraint{LTarget: "bar"} + c3 := &structs.Constraint{LTarget: "baz"} + + // Test the added case. + j1 := &structs.Job{Constraints: []*structs.Constraint{c1, c2}} + j2 := &structs.Job{Constraints: []*structs.Constraint{c1, c2, c3}} + + diff := NewJobDiff(j1, j2) + if diff == nil { + t.Fatalf("expected non-nil job diff") + } + + if diff.Type != DiffTypeEdited { + t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeEdited) + } + + if len(diff.Constraints) != 1 { + t.Fatalf("expected one constraint diff; got %v", diff.Constraints) + } + + cdiff := diff.Constraints[0] + if cdiff.Type != DiffTypeAdded { + t.Fatalf("expected constraint to be added: %#v", cdiff) + } + if len(cdiff.PrimitiveFields) != 3 { + t.Fatalf("bad: %#v", cdiff) + } + + // Test the deleted case. + j1 = &structs.Job{Constraints: []*structs.Constraint{c1, c2}} + j2 = &structs.Job{Constraints: []*structs.Constraint{c1}} + + diff = NewJobDiff(j1, j2) + if diff == nil { + t.Fatalf("expected non-nil job diff") + } + + if diff.Type != DiffTypeEdited { + t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeEdited) + } + + if len(diff.Constraints) != 1 { + t.Fatalf("expected one constraint diff; got %v", diff.Constraints) + } + + cdiff = diff.Constraints[0] + if cdiff.Type != DiffTypeDeleted { + t.Fatalf("expected constraint to be deleted: %#v", cdiff) + } + if len(cdiff.PrimitiveFields) != 3 { + t.Fatalf("bad: %#v", cdiff) + } +} + +func TestNewJobDiff_Datacenters(t *testing.T) { + j1 := &structs.Job{Datacenters: []string{"a", "b"}} + j2 := &structs.Job{Datacenters: []string{"b", "c"}} + + diff := NewJobDiff(j1, j2) + if diff == nil { + t.Fatalf("expected non-nil job diff") + } + + if diff.Type != DiffTypeEdited { + t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeEdited) + } + + dd := diff.Datacenters + if dd == nil { + t.Fatalf("expected datacenter diff") + } + + if !reflect.DeepEqual(dd.Added, []string{"c"}) || + !reflect.DeepEqual(dd.Deleted, []string{"a"}) { + t.Fatalf("bad: %#v", dd) + } + +} + +func TestNewJobDiff_TaskGroups(t *testing.T) { + tg1 := &structs.TaskGroup{Name: "foo"} + tg2 := &structs.TaskGroup{Name: "bar"} + tg2_2 := &structs.TaskGroup{Name: "bar", Count: 2} + tg3 := &structs.TaskGroup{Name: "baz"} + + j1 := &structs.Job{TaskGroups: []*structs.TaskGroup{tg1, tg2}} + j2 := &structs.Job{TaskGroups: []*structs.TaskGroup{tg2_2, tg3}} + + diff := NewJobDiff(j1, j2) + if diff == nil { + t.Fatalf("expected non-nil job diff") + } + + if diff.Type != DiffTypeEdited { + t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeEdited) + } + + tgd := diff.TaskGroups + if tgd == nil { + t.Fatalf("expected task group diff") + } + + if !reflect.DeepEqual(tgd.Added, []*structs.TaskGroup{tg3}) || + !reflect.DeepEqual(tgd.Deleted, []*structs.TaskGroup{tg1}) { + t.Fatalf("bad: %#v", tgd) + } + + if len(tgd.Edited) != 1 { + t.Fatalf("expect one edited task group: %#v", tgd) + } + if e := tgd.Edited[0]; tgd.Type != DiffTypeEdited && len(e.PrimitiveFields) != 1 { + t.Fatalf("bad: %#v", e) + } + +} + +func TestNewPrimitiveStructDiff(t *testing.T) { + p1 := structs.Port{Label: "1"} + p2 := structs.Port{Label: "2"} + p3 := structs.Port{} + + pdiff := NewPrimitiveStructDiff(nil, nil, portFields) + if pdiff != nil { + t.Fatalf("expected no diff: %#v", pdiff) + } + + pdiff = NewPrimitiveStructDiff(p1, p1, portFields) + if pdiff != nil { + t.Fatalf("expected no diff: %#v", pdiff) + } + + pdiff = NewPrimitiveStructDiff(p1, p2, portFields) + if pdiff == nil { + t.Fatalf("expected diff") + } + + if pdiff.Type != DiffTypeEdited { + t.Fatalf("unexpected type: got %v; want %v", pdiff.Type, DiffTypeEdited) + } + + if len(pdiff.PrimitiveFields) != 1 { + t.Fatalf("unexpected number of field diffs: %#v", pdiff.PrimitiveFields) + } + + f := pdiff.PrimitiveFields[0] + if f.Type != DiffTypeEdited { + t.Fatalf("unexpected type: got %v; want %v", f.Type, DiffTypeEdited) + } + if !reflect.DeepEqual(f.OldValue, "1") || !reflect.DeepEqual(f.NewValue, "2") { + t.Fatalf("bad: %#v", f) + } + + pdiff = NewPrimitiveStructDiff(p1, p3, portFields) + if pdiff == nil { + t.Fatalf("expected diff") + } + + if pdiff.Type != DiffTypeEdited { + t.Fatalf("unexpected type: got %v; want %v", pdiff.Type, DiffTypeEdited) + } + + if len(pdiff.PrimitiveFields) != 1 { + t.Fatalf("unexpected number of field diffs: %#v", pdiff.PrimitiveFields) + } + + f = pdiff.PrimitiveFields[0] + if f.Type != DiffTypeEdited { + t.Fatalf("unexpected type: got %v; want %v", f.Type, DiffTypeEdited) + } + if !reflect.DeepEqual(f.OldValue, "1") || !reflect.DeepEqual(f.NewValue, "") { + t.Fatalf("bad: %#v", f) + } +} + +func TestSetDiffPrimitiveStructs(t *testing.T) { + p1 := structs.Port{Label: "1"} + p2 := structs.Port{Label: "2"} + p3 := structs.Port{Label: "3"} + p4 := structs.Port{Label: "4"} + p5 := structs.Port{Label: "5"} + p6 := structs.Port{Label: "6"} + + old := []structs.Port{p1, p2, p3, p4} + new := []structs.Port{p3, p4, p5, p6} + + diffs := setDiffPrimitiveStructs(interfaceSlice(old), interfaceSlice(new), portFields) + if len(diffs) != 4 { + t.Fatalf("expected four diffs: %#v", diffs) + } + + var added, deleted int + for _, diff := range diffs { + switch diff.Type { + case DiffTypeAdded: + added++ + case DiffTypeDeleted: + deleted++ + default: + t.Fatalf("unexpected diff type: %#v", diff.Type) + } + } + + if added != 2 && deleted != 2 { + t.Fatalf("incorrect counts") + } +} + +func TestNewFieldDiff(t *testing.T) { + cases := []struct { + NilExpected bool + Old, New interface{} + Expected DiffType + }{ + { + NilExpected: true, + Old: 1, + New: 1, + }, + { + NilExpected: true, + Old: true, + New: true, + }, + { + NilExpected: true, + Old: "foo", + New: "foo", + }, + { + NilExpected: true, + Old: 2.23, + New: 2.23, + }, + { + Old: 1, + New: 4, + Expected: DiffTypeEdited, + }, + { + Old: true, + New: false, + Expected: DiffTypeEdited, + }, + { + Old: "foo", + New: "bar", + Expected: DiffTypeEdited, + }, + { + Old: 2.23, + New: 12.511, + Expected: DiffTypeEdited, + }, + { + Old: nil, + New: 4, + Expected: DiffTypeAdded, + }, + { + Old: nil, + New: true, + Expected: DiffTypeAdded, + }, + { + Old: nil, + New: "bar", + Expected: DiffTypeAdded, + }, + { + Old: nil, + New: 12.511, + Expected: DiffTypeAdded, + }, + { + Old: 4, + New: nil, + Expected: DiffTypeDeleted, + }, + { + Old: true, + New: nil, + Expected: DiffTypeDeleted, + }, + { + Old: "bar", + New: nil, + Expected: DiffTypeDeleted, + }, + { + Old: 12.511, + New: nil, + Expected: DiffTypeDeleted, + }, + } + + for i, c := range cases { + diff := NewFieldDiff("foo", c.Old, c.New) + if diff == nil { + if !c.NilExpected { + t.Fatalf("case %d: diff was nil and unexpected", i) + } + continue + } + + if diff.Type != c.Expected { + t.Fatalf("case %d: wanted type %v; got %v", diff.Type, c.Expected) + } + } +} + +func TestStringSetDiff(t *testing.T) { + values := []string{"1", "2", "3", "4", "5", "6"} + + setDiff := NewStringSetDiff(values[:4], values[2:]) + if setDiff.Type != DiffTypeEdited { + t.Fatalf("expected edited") + } + + addedExp := []string{"5", "6"} + deletedExp := []string{"1", "2"} + sort.Strings(setDiff.Added) + sort.Strings(setDiff.Deleted) + + if !reflect.DeepEqual(addedExp, setDiff.Added) || + !reflect.DeepEqual(deletedExp, setDiff.Deleted) { + t.Fatalf("bad: %#v", setDiff) + } +} + +func TestStringMapDiff(t *testing.T) { + m1 := map[string]string{ + "a": "aval", + "b": "bval", + } + m2 := map[string]string{ + "b": "bval2", + "c": "cval", + } + expected := &StringMapDiff{ + DiffEntry: DiffEntry{ + Type: DiffTypeEdited, + }, + Added: map[string]string{"c": "cval"}, + Deleted: map[string]string{"a": "aval"}, + Edited: map[string]StringValueDelta{ + "b": StringValueDelta{Old: "bval", + DiffEntry: DiffEntry{ + Type: DiffTypeEdited, + }, + New: "bval2", + }, + }, + } + + act := NewStringMapDiff(m1, m2) + if !reflect.DeepEqual(act, expected) { + t.Fatalf("got %#v; want %#v", act, expected) + } +} + +func TestSetDifference(t *testing.T) { + old := []interface{}{1, 2} + new := []interface{}{2, 3} + added, deleted := setDifference(old, new) + + if len(added) != 1 && len(deleted) != 1 { + t.Fatalf("bad: %#v %#v", added, deleted) + } + + a, ok := added[0].(int) + if !ok || a != 3 { + t.Fatalf("bad: %v %v", a, ok) + } + + d, ok := deleted[0].(int) + if !ok || d != 1 { + t.Fatalf("bad: %v %v", a, ok) + } +} + +func TestKeyedSetDifference(t *testing.T) { + oldMap := map[string]interface{}{ + "a": 1, + "b": 2, + "c": 3, + } + newMap := map[string]interface{}{ + "b": 3, + "c": 3, + "d": 4, + } + + added, deleted, edited, unmodified := keyedSetDifference(oldMap, newMap) + + if v, ok := added["d"]; !ok || v.(int) != 4 { + t.Fatalf("bad: %#v", added) + } + if v, ok := deleted["a"]; !ok || v.(int) != 1 { + t.Fatalf("bad: %#v", deleted) + } + if l := len(edited); l != 1 || edited[0] != "b" { + t.Fatalf("bad: %#v", edited) + } + if l := len(unmodified); l != 1 || unmodified[0] != "c" { + t.Fatalf("bad: %#v", edited) + } +} + +func TestInterfaceSlice(t *testing.T) { + j1 := mock.Job() + j2 := mock.Job() + jobs := []*structs.Job{j1, j2} + + slice := interfaceSlice(jobs) + if len(slice) != 2 { + t.Fatalf("bad: %#v", slice) + } + + f := slice[0] + actJob1, ok := f.(*structs.Job) + if !ok { + t.Fatalf("unexpected type: %v", actJob1) + } + + if !reflect.DeepEqual(actJob1, j1) { + t.Fatalf("got %#v, want %#v", actJob1, j1) + } +} + +func TestGetField(t *testing.T) { + j := mock.Job() + exp := "foo" + j.Type = "foo" + + i := getField(j, "Type") + act, ok := i.(string) + if !ok { + t.Fatalf("expected to get string type back") + } + + if act != exp { + t.Fatalf("got %v; want %v", act, exp) + } +} From fa042c45b942b1d23dbc6b574afbde5ddc7af4f3 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 2 May 2016 22:25:06 -0700 Subject: [PATCH 02/17] flatmap for walking the task config --- nomad/structs/diff/flatmap.go | 95 +++++++++++++++++++ nomad/structs/diff/flatmap_test.go | 143 +++++++++++++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 nomad/structs/diff/flatmap.go create mode 100644 nomad/structs/diff/flatmap_test.go diff --git a/nomad/structs/diff/flatmap.go b/nomad/structs/diff/flatmap.go new file mode 100644 index 000000000..cb9748382 --- /dev/null +++ b/nomad/structs/diff/flatmap.go @@ -0,0 +1,95 @@ +package diff + +import ( + "fmt" + "reflect" +) + +// Flatten takes an object and returns a flat map of the object. The keys of the +// map is the path of the field names until a primitive field is reached and the +// value is a string representation of the terminal field. +func Flatten(obj interface{}) map[string]string { + flat := make(map[string]string) + v := reflect.ValueOf(obj) + if !v.IsValid() { + return nil + } + + flatten("", v, flat) + return flat +} + +// flatten recursively calls itself to create a flatmap representation of the +// passed value. The results are stored into the output map and the keys are +// the fields prepended with the passed prefix. +// XXX: A current restriction is that maps only support string keys. +func flatten(prefix string, v reflect.Value, output map[string]string) { + switch v.Kind() { + case reflect.Bool: + output[prefix] = fmt.Sprintf("%v", v.Bool()) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + output[prefix] = fmt.Sprintf("%v", v.Int()) + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: + output[prefix] = fmt.Sprintf("%v", v.Uint()) + case reflect.Float32, reflect.Float64: + output[prefix] = fmt.Sprintf("%v", v.Float()) + case reflect.Complex64, reflect.Complex128: + output[prefix] = fmt.Sprintf("%v", v.Complex()) + case reflect.String: + output[prefix] = fmt.Sprintf("%v", v.String()) + case reflect.Invalid: + output[prefix] = "nil" + case reflect.Ptr: + e := v.Elem() + if !e.IsValid() { + output[prefix] = "nil" + } + flatten(prefix, e, output) + case reflect.Map: + for _, k := range v.MapKeys() { + if k.Kind() == reflect.Interface { + k = k.Elem() + } + + if k.Kind() != reflect.String { + panic(fmt.Sprintf("%q: map key is not string: %s", prefix, k)) + } + + flatten(fmt.Sprintf("%s.%s", prefix, k.String()), v.MapIndex(k), output) + } + case reflect.Struct: + t := v.Type() + for i := 0; i < v.NumField(); i++ { + name := t.Field(i).Name + val := v.Field(i) + if val.Kind() == reflect.Interface && !val.IsNil() { + val = val.Elem() + } + newPrefix := "" + if prefix != "" { + newPrefix = fmt.Sprintf("%s.%s", prefix, name) + } else { + newPrefix = fmt.Sprintf("%s", name) + } + + flatten(newPrefix, val, output) + } + case reflect.Interface: + e := v.Elem() + if !e.IsValid() { + output[prefix] = "nil" + return + } + flatten(prefix, e, output) + case reflect.Array, reflect.Slice: + if v.Kind() == reflect.Slice && v.IsNil() { + output[prefix] = "nil" + return + } + for i := 0; i < v.Len(); i++ { + flatten(fmt.Sprintf("%s[%d]", prefix, i), v.Index(i), output) + } + default: + panic(fmt.Sprintf("prefix %q; unsupported type %v", prefix, v.Kind())) + } +} diff --git a/nomad/structs/diff/flatmap_test.go b/nomad/structs/diff/flatmap_test.go new file mode 100644 index 000000000..47cd25e94 --- /dev/null +++ b/nomad/structs/diff/flatmap_test.go @@ -0,0 +1,143 @@ +package diff + +import ( + "reflect" + "testing" +) + +type simpleTypes struct { + b bool + i int + i8 int8 + i16 int16 + i32 int32 + i64 int64 + ui uint + ui8 uint8 + ui16 uint16 + ui32 uint32 + ui64 uint64 + f32 float32 + f64 float64 + c64 complex64 + c128 complex128 + s string +} + +type linkedList struct { + value string + next *linkedList +} + +type containers struct { + myslice []int + mymap map[string]linkedList +} + +type interfaceHolder struct { + value interface{} +} + +func TestFlatMap(t *testing.T) { + cases := []struct { + Input interface{} + Expected map[string]string + }{ + { + Input: nil, + Expected: nil, + }, + { + Input: &simpleTypes{ + b: true, + i: -10, + i8: 88, + i16: 1616, + i32: 3232, + i64: 6464, + ui: 10, + ui8: 88, + ui16: 1616, + ui32: 3232, + ui64: 6464, + f32: 3232, + f64: 6464, + c64: 64, + c128: 128, + s: "foobar", + }, + Expected: map[string]string{ + "b": "true", + "i": "-10", + "i8": "88", + "i16": "1616", + "i32": "3232", + "i64": "6464", + "ui": "10", + "ui8": "88", + "ui16": "1616", + "ui32": "3232", + "ui64": "6464", + "f32": "3232", + "f64": "6464", + "c64": "(64+0i)", + "c128": "(128+0i)", + "s": "foobar", + }, + }, + { + Input: &linkedList{ + value: "foo", + next: &linkedList{ + value: "bar", + next: nil, + }, + }, + Expected: map[string]string{ + "value": "foo", + "next.value": "bar", + "next.next": "nil", + }, + }, + { + Input: &containers{ + myslice: []int{1, 2}, + mymap: map[string]linkedList{ + "foo": linkedList{ + value: "l1", + }, + "bar": linkedList{ + value: "l2", + }, + }, + }, + Expected: map[string]string{ + "myslice[0]": "1", + "myslice[1]": "2", + "mymap.foo.value": "l1", + "mymap.foo.next": "nil", + "mymap.bar.value": "l2", + "mymap.bar.next": "nil", + }, + }, + { + Input: &interfaceHolder{ + value: &linkedList{ + value: "foo", + next: nil, + }, + }, + Expected: map[string]string{ + "value.value": "foo", + "value.next": "nil", + }, + }, + } + + for i, c := range cases { + act := Flatten(c.Input) + if !reflect.DeepEqual(act, c.Expected) { + t.Fatalf("case %d: got %#v; want %#v", i, act, c.Expected) + } + } +} From 082d6901f7f24e98108404fd9df5b531bca5ff67 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 3 May 2016 09:50:49 -0700 Subject: [PATCH 03/17] visitor --- nomad/structs/diff/job.go | 2 ++ nomad/structs/diff/job_visitor.go | 52 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 nomad/structs/diff/job_visitor.go diff --git a/nomad/structs/diff/job.go b/nomad/structs/diff/job.go index 0eaccb9c3..9bbcab955 100644 --- a/nomad/structs/diff/job.go +++ b/nomad/structs/diff/job.go @@ -250,6 +250,8 @@ type PrimitiveStructDiff struct { PrimitiveFields []*FieldDiff } +// DiffFields performs the diff of the passed fields against the old and new +// object. func (p *PrimitiveStructDiff) DiffFields(old, new interface{}, fields []string) { for _, field := range fields { oldV := getField(old, field) diff --git a/nomad/structs/diff/job_visitor.go b/nomad/structs/diff/job_visitor.go new file mode 100644 index 000000000..c9ef9a4cc --- /dev/null +++ b/nomad/structs/diff/job_visitor.go @@ -0,0 +1,52 @@ +package diff + +// JobVisitor is the set of types a visitor must implement to traverse a JobDiff +// structure. +type JobVisitor interface { + VisitJob(*JobDiff) + VisitTaskGroups(*TaskGroupsDiff) + VisitTaskGroup(*TaskGroupDiff) + VisitTasks(*TasksDiff) + VisitTask(*TaskDiff) + VisitServices(*ServicesDiff) + VisitService(*ServiceDiff) + VisitServiceChecks(*ServiceChecksDiff) + VisitServiceCheck(*ServiceCheckDiff) + VisitTaskArtifactsDiff(*TaskArtifactsDiff) + VisitTaskArtifactDiff(*TaskArtifactDiff) + VisitResources(*ResourcesDiff) + VisitNetworkResources(*NetworkResourcesDiff) + VisitNetworkResource(*NetworkResourceDiff) + VisitPorts(*PortsDiff) + VisitPrimitiveStruct(*PrimitiveStructDiff) + VisitField(*FieldDiff) + VisitStringSet(*StringSetDiff) + VisitStringMap(*StringMapDiff) + VisitStringValueDelta(*StringValueDelta) +} + +// JobDiffComponent is the method a diff component must implement to be visited. +type JobDiffComponent interface { + Accept(JobVisitor) +} + +func (j *JobDiff) Accept(v JobVisitor) { v.VisitJob(j) } +func (t *TaskGroupsDiff) Accept(v JobVisitor) { v.VisitTaskGroups(t) } +func (t *TaskGroupDiff) Accept(v JobVisitor) { v.VisitTaskGroup(t) } +func (t *TasksDiff) Accept(v JobVisitor) { v.VisitTasks(t) } +func (t *TaskDiff) Accept(v JobVisitor) { v.VisitTask(t) } +func (s *ServicesDiff) Accept(v JobVisitor) { v.VisitServices(s) } +func (s *ServiceDiff) Accept(v JobVisitor) { v.VisitService(s) } +func (s *ServiceChecksDiff) Accept(v JobVisitor) { v.VisitServiceChecks(s) } +func (s *ServiceCheckDiff) Accept(v JobVisitor) { v.VisitServiceCheck(s) } +func (t *TaskArtifactsDiff) Accept(v JobVisitor) { v.VisitTaskArtifactsDiff(t) } +func (t *TaskArtifactDiff) Accept(v JobVisitor) { v.VisitTaskArtifactDiff(t) } +func (r *ResourcesDiff) Accept(v JobVisitor) { v.VisitResources(r) } +func (n *NetworkResourcesDiff) Accept(v JobVisitor) { v.VisitNetworkResources(n) } +func (n *NetworkResourceDiff) Accept(v JobVisitor) { v.VisitNetworkResource(n) } +func (p *PortsDiff) Accept(v JobVisitor) { v.VisitPorts(p) } +func (p *PrimitiveStructDiff) Accept(v JobVisitor) { v.VisitPrimitiveStruct(p) } +func (f *FieldDiff) Accept(v JobVisitor) { v.VisitField(f) } +func (s *StringSetDiff) Accept(v JobVisitor) { v.VisitStringSet(s) } +func (s *StringMapDiff) Accept(v JobVisitor) { v.VisitStringMap(s) } +func (s *StringValueDelta) Accept(v JobVisitor) { v.VisitStringValueDelta(s) } From bea01efa5da881641417595052b17edc2990b98a Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 3 May 2016 14:23:44 -0700 Subject: [PATCH 04/17] Diff Task Configs, handle the nil cases, better type setting and more tests --- nomad/structs/diff/flatmap.go | 22 +-- nomad/structs/diff/flatmap_test.go | 2 +- nomad/structs/diff/job.go | 201 +++++++++++++++++++++------- nomad/structs/diff/job_test.go | 208 ++++++++++++++++++++++++++++- 4 files changed, 373 insertions(+), 60 deletions(-) diff --git a/nomad/structs/diff/flatmap.go b/nomad/structs/diff/flatmap.go index cb9748382..f3f2d9493 100644 --- a/nomad/structs/diff/flatmap.go +++ b/nomad/structs/diff/flatmap.go @@ -55,7 +55,7 @@ func flatten(prefix string, v reflect.Value, output map[string]string) { panic(fmt.Sprintf("%q: map key is not string: %s", prefix, k)) } - flatten(fmt.Sprintf("%s.%s", prefix, k.String()), v.MapIndex(k), output) + flatten(getSubPrefix(prefix, k.String()), v.MapIndex(k), output) } case reflect.Struct: t := v.Type() @@ -65,14 +65,8 @@ func flatten(prefix string, v reflect.Value, output map[string]string) { if val.Kind() == reflect.Interface && !val.IsNil() { val = val.Elem() } - newPrefix := "" - if prefix != "" { - newPrefix = fmt.Sprintf("%s.%s", prefix, name) - } else { - newPrefix = fmt.Sprintf("%s", name) - } - flatten(newPrefix, val, output) + flatten(getSubPrefix(prefix, name), val, output) } case reflect.Interface: e := v.Elem() @@ -93,3 +87,15 @@ func flatten(prefix string, v reflect.Value, output map[string]string) { panic(fmt.Sprintf("prefix %q; unsupported type %v", prefix, v.Kind())) } } + +// getSubPrefix takes the current prefix and the next subfield and returns an +// appropriate prefix. +func getSubPrefix(curPrefix, subField string) string { + newPrefix := "" + if curPrefix != "" { + newPrefix = fmt.Sprintf("%s.%s", curPrefix, subField) + } else { + newPrefix = fmt.Sprintf("%s", subField) + } + return newPrefix +} diff --git a/nomad/structs/diff/flatmap_test.go b/nomad/structs/diff/flatmap_test.go index 47cd25e94..f5b82e7a7 100644 --- a/nomad/structs/diff/flatmap_test.go +++ b/nomad/structs/diff/flatmap_test.go @@ -137,7 +137,7 @@ func TestFlatMap(t *testing.T) { for i, c := range cases { act := Flatten(c.Input) if !reflect.DeepEqual(act, c.Expected) { - t.Fatalf("case %d: got %#v; want %#v", i, act, c.Expected) + t.Fatalf("case %d: got %#v; want %#v", i+1, act, c.Expected) } } } diff --git a/nomad/structs/diff/job.go b/nomad/structs/diff/job.go index 9bbcab955..31496e5b2 100644 --- a/nomad/structs/diff/job.go +++ b/nomad/structs/diff/job.go @@ -119,12 +119,19 @@ type DiffEntry struct { Annotations []string } +// SetDiffType sets the diff type. The inputs must be a pointer. func (d *DiffEntry) SetDiffType(old, new interface{}) { if reflect.DeepEqual(old, new) { d.Type = DiffTypeNone - } else if old == nil { + return + } + + oldV := reflect.ValueOf(old) + newV := reflect.ValueOf(new) + + if oldV.IsNil() { d.Type = DiffTypeAdded - } else if new == nil { + } else if newV.IsNil() { d.Type = DiffTypeDeleted } else { d.Type = DiffTypeEdited @@ -175,6 +182,7 @@ type TaskDiff struct { Services *ServicesDiff Artifacts *TaskArtifactsDiff Resources *ResourcesDiff + Config *StringMapDiff } // ServicesDiff contains the set of Services that were changed. @@ -258,7 +266,6 @@ func (p *PrimitiveStructDiff) DiffFields(old, new interface{}, fields []string) newV := getField(new, field) pDiff := NewFieldDiff(field, oldV, newV) if pDiff != nil { - p.Type = DiffTypeEdited p.PrimitiveFields = append(p.PrimitiveFields, pDiff) } } @@ -294,14 +301,24 @@ type StringValueDelta struct { // NewJobDiff returns the diff between two jobs. If there is no difference, nil // is returned. func NewJobDiff(old, new *structs.Job) *JobDiff { - if old == nil && new == nil || reflect.DeepEqual(old, new) { + diff := &JobDiff{} + diff.SetDiffType(old, new) + if diff.Type == DiffTypeNone { return nil } // Get the diffs of the primitive fields - diff := &JobDiff{} diff.DiffFields(old, new, jobPrimitiveFields) + // Protect accessing nil fields, this occurs after diffing the primitives so + // that we can properly detect Added/Deleted fields. + if old == nil { + old = &structs.Job{} + } + if new == nil { + new = &structs.Job{} + } + // Get the diff of the datacenters diff.Datacenters = NewStringSetDiff(old.Datacenters, new.Datacenters) @@ -333,21 +350,30 @@ func NewJobDiff(old, new *structs.Job) *JobDiff { return nil } - diff.SetDiffType(old, new) return diff } // NewTaskGroupDiff returns the diff between two task groups. If there is no // difference, nil is returned. func NewTaskGroupDiff(old, new *structs.TaskGroup) *TaskGroupDiff { - if old == nil && new == nil || reflect.DeepEqual(old, new) { + diff := &TaskGroupDiff{} + diff.SetDiffType(old, new) + if diff.Type == DiffTypeNone { return nil } // Get the diffs of the primitive fields - diff := &TaskGroupDiff{} diff.DiffFields(old, new, taskGroupPrimitiveFields) + // Protect accessing nil fields, this occurs after diffing the primitives so + // that we can properly detect Added/Deleted fields. + if old == nil { + old = &structs.TaskGroup{} + } + if new == nil { + new = &structs.TaskGroup{} + } + // Get the diff of the constraints. diff.Constraints = setDiffPrimitiveStructs( interfaceSlice(old.Constraints), @@ -371,21 +397,30 @@ func NewTaskGroupDiff(old, new *structs.TaskGroup) *TaskGroupDiff { return nil } - diff.SetDiffType(old, new) return diff } // NewTaskDiff returns the diff between two tasks. If there is no difference, // nil is returned. func NewTaskDiff(old, new *structs.Task) *TaskDiff { - if old == nil && new == nil || reflect.DeepEqual(old, new) { + diff := &TaskDiff{} + diff.SetDiffType(old, new) + if diff.Type == DiffTypeNone { return nil } // Get the diffs of the primitive fields - diff := &TaskDiff{} diff.DiffFields(old, new, taskPrimitiveFields) + // Protect accessing nil fields, this occurs after diffing the primitives so + // that we can properly detect Added/Deleted fields. + if old == nil { + old = &structs.Task{} + } + if new == nil { + new = &structs.Task{} + } + // Get the diff of the constraints. diff.Constraints = setDiffPrimitiveStructs( interfaceSlice(old.Constraints), @@ -409,10 +444,11 @@ func NewTaskDiff(old, new *structs.Task) *TaskDiff { diff.Resources = NewResourcesDiff(old.Resources, new.Resources) // Get the task config diff - // TODO: Flat map the config and diff it. + diff.Config = NewStringMapDiff(Flatten(old.Config), Flatten(new.Config)) // If there are no changes return nil if len(diff.PrimitiveFields)+len(diff.Constraints) == 0 && + diff.Config == nil && diff.Artifacts == nil && diff.LogConfig == nil && diff.Services == nil && @@ -421,21 +457,30 @@ func NewTaskDiff(old, new *structs.Task) *TaskDiff { return nil } - diff.SetDiffType(old, new) return diff } // NewServiceDiff returns the diff between two services. If there is no // difference, nil is returned. func NewServiceDiff(old, new *structs.Service) *ServiceDiff { - if old == nil && new == nil || reflect.DeepEqual(old, new) { + diff := &ServiceDiff{} + diff.SetDiffType(old, new) + if diff.Type == DiffTypeNone { return nil } // Get the diffs of the primitive fields - diff := &ServiceDiff{} diff.DiffFields(old, new, servicePrimitiveFields) + // Protect accessing nil fields, this occurs after diffing the primitives so + // that we can properly detect Added/Deleted fields. + if old == nil { + old = &structs.Service{} + } + if new == nil { + new = &structs.Service{} + } + // Get the tags diff diff.Tags = NewStringSetDiff(old.Tags, new.Tags) @@ -449,21 +494,30 @@ func NewServiceDiff(old, new *structs.Service) *ServiceDiff { return nil } - diff.SetDiffType(old, new) return diff } // NewServiceCheckDiff returns the diff between two service checks. If there is // no difference, nil is returned. func NewServiceCheckDiff(old, new *structs.ServiceCheck) *ServiceCheckDiff { - if old == nil && new == nil || reflect.DeepEqual(old, new) { + diff := &ServiceCheckDiff{} + diff.SetDiffType(old, new) + if diff.Type == DiffTypeNone { return nil } // Get the diffs of the primitive fields - diff := &ServiceCheckDiff{} diff.DiffFields(old, new, serviceCheckPrimitiveFields) + // Protect accessing nil fields, this occurs after diffing the primitives so + // that we can properly detect Added/Deleted fields. + if old == nil { + old = &structs.ServiceCheck{} + } + if new == nil { + new = &structs.ServiceCheck{} + } + // Get the args diff diff.Args = NewStringSetDiff(old.Args, new.Args) @@ -473,21 +527,30 @@ func NewServiceCheckDiff(old, new *structs.ServiceCheck) *ServiceCheckDiff { return nil } - diff.SetDiffType(old, new) return diff } // NewTaskArtifactDiff returns the diff between two task artifacts. If there is // no difference, nil is returned. func NewTaskArtifactDiff(old, new *structs.TaskArtifact) *TaskArtifactDiff { - if old == nil && new == nil || reflect.DeepEqual(old, new) { + diff := &TaskArtifactDiff{} + diff.SetDiffType(old, new) + if diff.Type == DiffTypeNone { return nil } // Get the diffs of the primitive fields - diff := &TaskArtifactDiff{} diff.DiffFields(old, new, taskArtifactPrimitiveFields) + // Protect accessing nil fields, this occurs after diffing the primitives so + // that we can properly detect Added/Deleted fields. + if old == nil { + old = &structs.TaskArtifact{} + } + if new == nil { + new = &structs.TaskArtifact{} + } + // Get the args diff diff.GetterOptions = NewStringMapDiff(old.GetterOptions, new.GetterOptions) @@ -497,21 +560,30 @@ func NewTaskArtifactDiff(old, new *structs.TaskArtifact) *TaskArtifactDiff { return nil } - diff.SetDiffType(old, new) return diff } // NewResourcesDiff returns the diff between two resources. If there is no // difference, nil is returned. func NewResourcesDiff(old, new *structs.Resources) *ResourcesDiff { - if old == nil && new == nil || reflect.DeepEqual(old, new) { + diff := &ResourcesDiff{} + diff.SetDiffType(old, new) + if diff.Type == DiffTypeNone { return nil } // Get the diffs of the primitive fields - diff := &ResourcesDiff{} diff.DiffFields(old, new, resourcesPrimitiveFields) + // Protect accessing nil fields, this occurs after diffing the primitives so + // that we can properly detect Added/Deleted fields. + if old == nil { + old = &structs.Resources{} + } + if new == nil { + new = &structs.Resources{} + } + // Get the network resource diff diff.Networks = setDiffNetworkResources(old.Networks, new.Networks) @@ -521,21 +593,30 @@ func NewResourcesDiff(old, new *structs.Resources) *ResourcesDiff { return nil } - diff.SetDiffType(old, new) return diff } // NewNetworkResourceDiff returns the diff between two network resources. If // there is no difference, nil is returned. func NewNetworkResourceDiff(old, new *structs.NetworkResource) *NetworkResourceDiff { - if old == nil && new == nil || reflect.DeepEqual(old, new) { + diff := &NetworkResourceDiff{} + diff.SetDiffType(old, new) + if diff.Type == DiffTypeNone { return nil } // Get the diffs of the primitive fields - diff := &NetworkResourceDiff{} diff.DiffFields(old, new, networkResourcePrimitiveFields) + // Protect accessing nil fields, this occurs after diffing the primitives so + // that we can properly detect Added/Deleted fields. + if old == nil { + old = &structs.NetworkResource{} + } + if new == nil { + new = &structs.NetworkResource{} + } + // Get the port diffs diff.ReservedPorts = setDiffPorts(old.ReservedPorts, new.ReservedPorts) diff.DynamicPorts = setDiffPorts(old.DynamicPorts, new.DynamicPorts) @@ -547,7 +628,6 @@ func NewNetworkResourceDiff(old, new *structs.NetworkResource) *NetworkResourceD return nil } - diff.SetDiffType(old, new) return diff } @@ -566,7 +646,27 @@ func NewPrimitiveStructDiff(old, new interface{}, fields []string) *PrimitiveStr return nil } - diff.SetDiffType(old, new) + var added, deleted bool + for _, f := range diff.PrimitiveFields { + switch f.Type { + case DiffTypeEdited: + diff.Type = DiffTypeEdited + return diff + case DiffTypeAdded: + added = true + case DiffTypeDeleted: + deleted = true + } + } + + if added && deleted { + diff.Type = DiffTypeEdited + } else if added { + diff.Type = DiffTypeAdded + } else { + diff.Type = DiffTypeDeleted + } + return diff } @@ -594,19 +694,11 @@ func NewFieldDiff(name string, old, new interface{}) *FieldDiff { // NewStringSetDiff returns the diff between two sets of strings. If there is no // difference, nil is returned. func NewStringSetDiff(old, new []string) *StringSetDiff { - diff := &StringSetDiff{} - lOld, lNew := len(old), len(new) if reflect.DeepEqual(old, new) { return nil - } else if lOld == 0 && lNew > 0 { - diff.Type = DiffTypeAdded - return diff - } else if lNew == 0 && lOld > 0 { - diff.Type = DiffTypeDeleted - return diff } - diff.Type = DiffTypeEdited + diff := &StringSetDiff{} makeMap := func(inputs []string) map[string]interface{} { m := make(map[string]interface{}) for _, in := range inputs { @@ -622,25 +714,29 @@ func NewStringSetDiff(old, new []string) *StringSetDiff { for k := range deleted { diff.Deleted = append(diff.Deleted, k) } + + la, ld := len(added), len(deleted) + if la+ld == 0 { + return nil + } else if ld == 0 { + diff.Type = DiffTypeAdded + } else if la == 0 { + diff.Type = DiffTypeDeleted + } else { + diff.Type = DiffTypeEdited + } + return diff } // NewStringMapDiff returns the diff between two maps of strings. If there is no // difference, nil is returned. func NewStringMapDiff(old, new map[string]string) *StringMapDiff { - diff := &StringMapDiff{} - lOld, lNew := len(old), len(new) if reflect.DeepEqual(old, new) { return nil - } else if lOld == 0 && lNew > 0 { - diff.Type = DiffTypeAdded - return diff - } else if lNew == 0 && lOld > 0 { - diff.Type = DiffTypeDeleted - return diff } - diff.Type = DiffTypeEdited + diff := &StringMapDiff{} diff.Added = make(map[string]string) diff.Deleted = make(map[string]string) diff.Edited = make(map[string]StringValueDelta) @@ -665,9 +761,18 @@ func NewStringMapDiff(old, new map[string]string) *StringMapDiff { } } - if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { + la, ld, le := len(diff.Added), len(diff.Deleted), len(diff.Edited) + if la+ld+le == 0 { return nil } + + if le != 0 || la > 0 && ld > 0 { + diff.Type = DiffTypeEdited + } else if ld == 0 { + diff.Type = DiffTypeAdded + } else if la == 0 { + diff.Type = DiffTypeDeleted + } return diff } diff --git a/nomad/structs/diff/job_test.go b/nomad/structs/diff/job_test.go index 2b3608648..ac8d627d5 100644 --- a/nomad/structs/diff/job_test.go +++ b/nomad/structs/diff/job_test.go @@ -21,6 +21,29 @@ func TestNewJobDiff_Same(t *testing.T) { } } +func TestNewJobDiff_NilCases(t *testing.T) { + j := mock.Job() + + // Old job nil + diff := NewJobDiff(nil, j) + if diff == nil { + t.Fatalf("expected non-nil job diff") + } + if diff.Type != DiffTypeAdded { + //t.Fatalf("got diff type %v; want %v; %s", diff.Type, DiffTypeAdded, spew.Sdump(diff)) + t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeAdded) + } + + // New job nil + diff = NewJobDiff(j, nil) + if diff == nil { + t.Fatalf("expected non-nil job diff") + } + if diff.Type != DiffTypeDeleted { + t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeDeleted) + } +} + func TestNewJobDiff_Constraints(t *testing.T) { c1 := &structs.Constraint{LTarget: "foo"} c2 := &structs.Constraint{LTarget: "bar"} @@ -136,7 +159,141 @@ func TestNewJobDiff_TaskGroups(t *testing.T) { if e := tgd.Edited[0]; tgd.Type != DiffTypeEdited && len(e.PrimitiveFields) != 1 { t.Fatalf("bad: %#v", e) } +} +func TestNewTaskDiff_Config(t *testing.T) { + c1 := map[string]interface{}{ + "command": "/bin/date", + "args": []string{"1", "2"}, + } + + c2 := map[string]interface{}{ + "args": []string{"1", "2"}, + } + + c3 := map[string]interface{}{ + "command": "/bin/date", + "args": []string{"1", "2"}, + "nested": &structs.Port{ + Label: "http", + Value: 80, + }, + } + + c4 := map[string]interface{}{ + "command": "/bin/bash", + "args": []string{"1", "2"}, + } + + // No old case + t1 := &structs.Task{Config: c1} + diff := NewTaskDiff(nil, t1) + if diff == nil { + t.Fatalf("expected non-nil diff") + } + if diff.Config == nil { + t.Fatalf("expected Config diff: %#v", diff) + } + + cdiff := diff.Config + if cdiff.Type != DiffTypeAdded { + t.Fatalf("expected Config diff type %v; got %#v", DiffTypeAdded, cdiff.Type) + } + + // No new case + diff = NewTaskDiff(t1, nil) + if diff == nil { + t.Fatalf("expected non-nil diff") + } + if diff.Config == nil { + t.Fatalf("expected Config diff: %#v", diff) + } + + cdiff = diff.Config + if cdiff.Type != DiffTypeDeleted { + t.Fatalf("expected Config diff type %v; got %#v", DiffTypeDeleted, cdiff.Type) + } + + // Same case + diff = NewTaskDiff(t1, t1) + if diff != nil { + t.Fatalf("expected nil diff") + } + + // Deleted case + t2 := &structs.Task{Config: c2} + diff = NewTaskDiff(t1, t2) + if diff == nil { + t.Fatalf("expected non-nil diff") + } + if diff.Config == nil { + t.Fatalf("expected Config diff: %#v", diff) + } + + cdiff = diff.Config + if cdiff.Type != DiffTypeDeleted { + t.Fatalf("expected Config diff type %v; got %#v", DiffTypeDeleted, cdiff.Type) + } + + if len(cdiff.Added)+len(cdiff.Edited) != 0 && len(cdiff.Deleted) != 1 { + t.Fatalf("unexpected config diffs: %#v", cdiff) + } + + if v, ok := cdiff.Deleted["command"]; !ok || v != "/bin/date" { + t.Fatalf("bad: %#v", cdiff.Deleted) + } + + // Added case + t3 := &structs.Task{Config: c3} + diff = NewTaskDiff(t1, t3) + if diff == nil { + t.Fatalf("expected non-nil diff") + } + if diff.Config == nil { + t.Fatalf("expected Config diff: %#v", diff) + } + + cdiff = diff.Config + if cdiff.Type != DiffTypeAdded { + t.Fatalf("expected Config diff type %v; got %#v", DiffTypeAdded, cdiff.Type) + } + + if len(cdiff.Deleted)+len(cdiff.Edited) != 0 && len(cdiff.Added) != 2 { + t.Fatalf("unexpected config diffs: %#v", cdiff) + } + + if v, ok := cdiff.Added["nested.Value"]; !ok || v != "80" { + t.Fatalf("bad: %#v", cdiff.Added) + } + if v, ok := cdiff.Added["nested.Label"]; !ok || v != "http" { + t.Fatalf("bad: %#v", cdiff.Added) + } + + // Edited case + t4 := &structs.Task{Config: c4} + diff = NewTaskDiff(t1, t4) + if diff == nil { + t.Fatalf("expected non-nil diff") + } + if diff.Config == nil { + t.Fatalf("expected Config diff: %#v", diff) + } + + cdiff = diff.Config + if cdiff.Type != DiffTypeEdited { + t.Fatalf("expected Config diff type %v; got %#v", DiffTypeEdited, cdiff.Type) + } + + if len(cdiff.Deleted)+len(cdiff.Added) != 0 && len(cdiff.Edited) != 1 { + t.Fatalf("unexpected config diffs: %#v", cdiff) + } + + exp := StringValueDelta{Old: "/bin/date", New: "/bin/bash"} + exp.Type = DiffTypeEdited + v, ok := cdiff.Edited["command"] + if !ok || !reflect.DeepEqual(v, exp) { + t.Fatalf("bad: %#v %#v %#v", cdiff.Edited, v, exp) + } } func TestNewPrimitiveStructDiff(t *testing.T) { @@ -154,6 +311,24 @@ func TestNewPrimitiveStructDiff(t *testing.T) { t.Fatalf("expected no diff: %#v", pdiff) } + pdiff = NewPrimitiveStructDiff(nil, p1, portFields) + if pdiff == nil { + t.Fatalf("expected diff") + } + + if pdiff.Type != DiffTypeAdded { + t.Fatalf("unexpected type: got %v; want %v", pdiff.Type, DiffTypeAdded) + } + + pdiff = NewPrimitiveStructDiff(p1, nil, portFields) + if pdiff == nil { + t.Fatalf("expected diff") + } + + if pdiff.Type != DiffTypeDeleted { + t.Fatalf("unexpected type: got %v; want %v", pdiff.Type, DiffTypeDeleted) + } + pdiff = NewPrimitiveStructDiff(p1, p2, portFields) if pdiff == nil { t.Fatalf("expected diff") @@ -322,13 +497,13 @@ func TestNewFieldDiff(t *testing.T) { diff := NewFieldDiff("foo", c.Old, c.New) if diff == nil { if !c.NilExpected { - t.Fatalf("case %d: diff was nil and unexpected", i) + t.Fatalf("case %d: diff was nil and unexpected", i+1) } continue } if diff.Type != c.Expected { - t.Fatalf("case %d: wanted type %v; got %v", diff.Type, c.Expected) + t.Fatalf("case %d: wanted type %v; got %v", i+1, diff.Type, c.Expected) } } } @@ -336,9 +511,10 @@ func TestNewFieldDiff(t *testing.T) { func TestStringSetDiff(t *testing.T) { values := []string{"1", "2", "3", "4", "5", "6"} + // Edited case setDiff := NewStringSetDiff(values[:4], values[2:]) if setDiff.Type != DiffTypeEdited { - t.Fatalf("expected edited") + t.Fatalf("got type %v; want %v", setDiff.Type, DiffTypeEdited) } addedExp := []string{"5", "6"} @@ -350,6 +526,18 @@ func TestStringSetDiff(t *testing.T) { !reflect.DeepEqual(deletedExp, setDiff.Deleted) { t.Fatalf("bad: %#v", setDiff) } + + // Added case + setDiff = NewStringSetDiff(nil, values) + if setDiff.Type != DiffTypeAdded { + t.Fatalf("got type %v; want %v", setDiff.Type, DiffTypeAdded) + } + + // Deleted case + setDiff = NewStringSetDiff(values, nil) + if setDiff.Type != DiffTypeDeleted { + t.Fatalf("got type %v; want %v", setDiff.Type, DiffTypeDeleted) + } } func TestStringMapDiff(t *testing.T) { @@ -361,6 +549,8 @@ func TestStringMapDiff(t *testing.T) { "b": "bval2", "c": "cval", } + + // Edited case expected := &StringMapDiff{ DiffEntry: DiffEntry{ Type: DiffTypeEdited, @@ -381,6 +571,18 @@ func TestStringMapDiff(t *testing.T) { if !reflect.DeepEqual(act, expected) { t.Fatalf("got %#v; want %#v", act, expected) } + + // Added case + diff := NewStringMapDiff(nil, m1) + if diff.Type != DiffTypeAdded { + t.Fatalf("got type %v; want %v", diff.Type, DiffTypeAdded) + } + + // Deleted case + diff = NewStringMapDiff(m1, nil) + if diff.Type != DiffTypeDeleted { + t.Fatalf("got type %v; want %v", diff.Type, DiffTypeDeleted) + } } func TestSetDifference(t *testing.T) { From 11289526b5fdf81daf6a6aeaff4b81af3ab02ef9 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 3 May 2016 18:53:12 -0700 Subject: [PATCH 05/17] change the packages --- .../diff => helper/flatmap}/flatmap.go | 2 +- .../diff => helper/flatmap}/flatmap_test.go | 2 +- nomad/structs/{diff/job.go => job_diff.go} | 108 +++++++++--------- .../{diff/job_test.go => job_diff_test.go} | 91 +++++++-------- .../job_visitor.go => job_diff_visitor.go} | 2 +- nomad/structs/structs_test.go | 78 +------------ nomad/structs/testing.go | 77 +++++++++++++ 7 files changed, 179 insertions(+), 181 deletions(-) rename {nomad/structs/diff => helper/flatmap}/flatmap.go (99%) rename {nomad/structs/diff => helper/flatmap}/flatmap_test.go (99%) rename nomad/structs/{diff/job.go => job_diff.go} (90%) rename nomad/structs/{diff/job_test.go => job_diff_test.go} (87%) rename nomad/structs/{diff/job_visitor.go => job_diff_visitor.go} (99%) create mode 100644 nomad/structs/testing.go diff --git a/nomad/structs/diff/flatmap.go b/helper/flatmap/flatmap.go similarity index 99% rename from nomad/structs/diff/flatmap.go rename to helper/flatmap/flatmap.go index f3f2d9493..94bbc6338 100644 --- a/nomad/structs/diff/flatmap.go +++ b/helper/flatmap/flatmap.go @@ -1,4 +1,4 @@ -package diff +package flatmap import ( "fmt" diff --git a/nomad/structs/diff/flatmap_test.go b/helper/flatmap/flatmap_test.go similarity index 99% rename from nomad/structs/diff/flatmap_test.go rename to helper/flatmap/flatmap_test.go index f5b82e7a7..d84561d1d 100644 --- a/nomad/structs/diff/flatmap_test.go +++ b/helper/flatmap/flatmap_test.go @@ -1,4 +1,4 @@ -package diff +package flatmap import ( "reflect" diff --git a/nomad/structs/diff/job.go b/nomad/structs/job_diff.go similarity index 90% rename from nomad/structs/diff/job.go rename to nomad/structs/job_diff.go index 31496e5b2..01a755c82 100644 --- a/nomad/structs/diff/job.go +++ b/nomad/structs/job_diff.go @@ -1,10 +1,10 @@ -package diff +package structs import ( "fmt" "reflect" - "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/helper/flatmap" "github.com/mitchellh/hashstructure" ) @@ -152,7 +152,7 @@ type JobDiff struct { // TaskGroupsDiff contains the set of Task Groups that were changed. type TaskGroupsDiff struct { DiffEntry - Added, Deleted []*structs.TaskGroup + Added, Deleted []*TaskGroup Edited []*TaskGroupDiff } @@ -168,7 +168,7 @@ type TaskGroupDiff struct { // TasksDiff contains the set of Tasks that were changed. type TasksDiff struct { DiffEntry - Added, Deleted []*structs.Task + Added, Deleted []*Task Edited []*TaskDiff } @@ -188,7 +188,7 @@ type TaskDiff struct { // ServicesDiff contains the set of Services that were changed. type ServicesDiff struct { DiffEntry - Added, Deleted []*structs.Service + Added, Deleted []*Service Edited []*ServiceDiff } @@ -202,7 +202,7 @@ type ServiceDiff struct { // ServiceChecksDiff contains the set of Service Checks that were changed. type ServiceChecksDiff struct { DiffEntry - Added, Deleted []*structs.ServiceCheck + Added, Deleted []*ServiceCheck Edited []*ServiceCheckDiff } @@ -215,7 +215,7 @@ type ServiceCheckDiff struct { // TaskArtifactsDiff contains the set of Task Artifacts that were changed. type TaskArtifactsDiff struct { DiffEntry - Added, Deleted []*structs.TaskArtifact + Added, Deleted []*TaskArtifact Edited []*TaskArtifactDiff } @@ -247,7 +247,7 @@ type NetworkResourceDiff struct { // PortsDiff contains the difference between two sets of Ports. type PortsDiff struct { DiffEntry - Added, Deleted []structs.Port + Added, Deleted []Port Edited []*PrimitiveStructDiff } @@ -300,7 +300,7 @@ type StringValueDelta struct { // NewJobDiff returns the diff between two jobs. If there is no difference, nil // is returned. -func NewJobDiff(old, new *structs.Job) *JobDiff { +func NewJobDiff(old, new *Job) *JobDiff { diff := &JobDiff{} diff.SetDiffType(old, new) if diff.Type == DiffTypeNone { @@ -313,10 +313,10 @@ func NewJobDiff(old, new *structs.Job) *JobDiff { // Protect accessing nil fields, this occurs after diffing the primitives so // that we can properly detect Added/Deleted fields. if old == nil { - old = &structs.Job{} + old = &Job{} } if new == nil { - new = &structs.Job{} + new = &Job{} } // Get the diff of the datacenters @@ -355,7 +355,7 @@ func NewJobDiff(old, new *structs.Job) *JobDiff { // NewTaskGroupDiff returns the diff between two task groups. If there is no // difference, nil is returned. -func NewTaskGroupDiff(old, new *structs.TaskGroup) *TaskGroupDiff { +func NewTaskGroupDiff(old, new *TaskGroup) *TaskGroupDiff { diff := &TaskGroupDiff{} diff.SetDiffType(old, new) if diff.Type == DiffTypeNone { @@ -368,10 +368,10 @@ func NewTaskGroupDiff(old, new *structs.TaskGroup) *TaskGroupDiff { // Protect accessing nil fields, this occurs after diffing the primitives so // that we can properly detect Added/Deleted fields. if old == nil { - old = &structs.TaskGroup{} + old = &TaskGroup{} } if new == nil { - new = &structs.TaskGroup{} + new = &TaskGroup{} } // Get the diff of the constraints. @@ -402,7 +402,7 @@ func NewTaskGroupDiff(old, new *structs.TaskGroup) *TaskGroupDiff { // NewTaskDiff returns the diff between two tasks. If there is no difference, // nil is returned. -func NewTaskDiff(old, new *structs.Task) *TaskDiff { +func NewTaskDiff(old, new *Task) *TaskDiff { diff := &TaskDiff{} diff.SetDiffType(old, new) if diff.Type == DiffTypeNone { @@ -415,10 +415,10 @@ func NewTaskDiff(old, new *structs.Task) *TaskDiff { // Protect accessing nil fields, this occurs after diffing the primitives so // that we can properly detect Added/Deleted fields. if old == nil { - old = &structs.Task{} + old = &Task{} } if new == nil { - new = &structs.Task{} + new = &Task{} } // Get the diff of the constraints. @@ -444,7 +444,7 @@ func NewTaskDiff(old, new *structs.Task) *TaskDiff { diff.Resources = NewResourcesDiff(old.Resources, new.Resources) // Get the task config diff - diff.Config = NewStringMapDiff(Flatten(old.Config), Flatten(new.Config)) + diff.Config = NewStringMapDiff(flatmap.Flatten(old.Config), flatmap.Flatten(new.Config)) // If there are no changes return nil if len(diff.PrimitiveFields)+len(diff.Constraints) == 0 && @@ -462,7 +462,7 @@ func NewTaskDiff(old, new *structs.Task) *TaskDiff { // NewServiceDiff returns the diff between two services. If there is no // difference, nil is returned. -func NewServiceDiff(old, new *structs.Service) *ServiceDiff { +func NewServiceDiff(old, new *Service) *ServiceDiff { diff := &ServiceDiff{} diff.SetDiffType(old, new) if diff.Type == DiffTypeNone { @@ -475,10 +475,10 @@ func NewServiceDiff(old, new *structs.Service) *ServiceDiff { // Protect accessing nil fields, this occurs after diffing the primitives so // that we can properly detect Added/Deleted fields. if old == nil { - old = &structs.Service{} + old = &Service{} } if new == nil { - new = &structs.Service{} + new = &Service{} } // Get the tags diff @@ -499,7 +499,7 @@ func NewServiceDiff(old, new *structs.Service) *ServiceDiff { // NewServiceCheckDiff returns the diff between two service checks. If there is // no difference, nil is returned. -func NewServiceCheckDiff(old, new *structs.ServiceCheck) *ServiceCheckDiff { +func NewServiceCheckDiff(old, new *ServiceCheck) *ServiceCheckDiff { diff := &ServiceCheckDiff{} diff.SetDiffType(old, new) if diff.Type == DiffTypeNone { @@ -512,10 +512,10 @@ func NewServiceCheckDiff(old, new *structs.ServiceCheck) *ServiceCheckDiff { // Protect accessing nil fields, this occurs after diffing the primitives so // that we can properly detect Added/Deleted fields. if old == nil { - old = &structs.ServiceCheck{} + old = &ServiceCheck{} } if new == nil { - new = &structs.ServiceCheck{} + new = &ServiceCheck{} } // Get the args diff @@ -532,7 +532,7 @@ func NewServiceCheckDiff(old, new *structs.ServiceCheck) *ServiceCheckDiff { // NewTaskArtifactDiff returns the diff between two task artifacts. If there is // no difference, nil is returned. -func NewTaskArtifactDiff(old, new *structs.TaskArtifact) *TaskArtifactDiff { +func NewTaskArtifactDiff(old, new *TaskArtifact) *TaskArtifactDiff { diff := &TaskArtifactDiff{} diff.SetDiffType(old, new) if diff.Type == DiffTypeNone { @@ -545,10 +545,10 @@ func NewTaskArtifactDiff(old, new *structs.TaskArtifact) *TaskArtifactDiff { // Protect accessing nil fields, this occurs after diffing the primitives so // that we can properly detect Added/Deleted fields. if old == nil { - old = &structs.TaskArtifact{} + old = &TaskArtifact{} } if new == nil { - new = &structs.TaskArtifact{} + new = &TaskArtifact{} } // Get the args diff @@ -565,7 +565,7 @@ func NewTaskArtifactDiff(old, new *structs.TaskArtifact) *TaskArtifactDiff { // NewResourcesDiff returns the diff between two resources. If there is no // difference, nil is returned. -func NewResourcesDiff(old, new *structs.Resources) *ResourcesDiff { +func NewResourcesDiff(old, new *Resources) *ResourcesDiff { diff := &ResourcesDiff{} diff.SetDiffType(old, new) if diff.Type == DiffTypeNone { @@ -578,10 +578,10 @@ func NewResourcesDiff(old, new *structs.Resources) *ResourcesDiff { // Protect accessing nil fields, this occurs after diffing the primitives so // that we can properly detect Added/Deleted fields. if old == nil { - old = &structs.Resources{} + old = &Resources{} } if new == nil { - new = &structs.Resources{} + new = &Resources{} } // Get the network resource diff @@ -598,7 +598,7 @@ func NewResourcesDiff(old, new *structs.Resources) *ResourcesDiff { // NewNetworkResourceDiff returns the diff between two network resources. If // there is no difference, nil is returned. -func NewNetworkResourceDiff(old, new *structs.NetworkResource) *NetworkResourceDiff { +func NewNetworkResourceDiff(old, new *NetworkResource) *NetworkResourceDiff { diff := &NetworkResourceDiff{} diff.SetDiffType(old, new) if diff.Type == DiffTypeNone { @@ -611,10 +611,10 @@ func NewNetworkResourceDiff(old, new *structs.NetworkResource) *NetworkResourceD // Protect accessing nil fields, this occurs after diffing the primitives so // that we can properly detect Added/Deleted fields. if old == nil { - old = &structs.NetworkResource{} + old = &NetworkResource{} } if new == nil { - new = &structs.NetworkResource{} + new = &NetworkResource{} } // Get the port diffs @@ -780,11 +780,11 @@ func NewStringMapDiff(old, new map[string]string) *StringMapDiff { // setDiffTaskGroups does a set difference of task groups using the task group // name as a key. -func setDiffTaskGroups(old, new []*structs.TaskGroup) *TaskGroupsDiff { +func setDiffTaskGroups(old, new []*TaskGroup) *TaskGroupsDiff { diff := &TaskGroupsDiff{} - oldMap := make(map[string]*structs.TaskGroup) - newMap := make(map[string]*structs.TaskGroup) + oldMap := make(map[string]*TaskGroup) + newMap := make(map[string]*TaskGroup) for _, tg := range old { oldMap[tg.Name] = tg } @@ -818,11 +818,11 @@ func setDiffTaskGroups(old, new []*structs.TaskGroup) *TaskGroupsDiff { } // setDiffTasks does a set difference of tasks using the task name as a key. -func setDiffTasks(old, new []*structs.Task) *TasksDiff { +func setDiffTasks(old, new []*Task) *TasksDiff { diff := &TasksDiff{} - oldMap := make(map[string]*structs.Task) - newMap := make(map[string]*structs.Task) + oldMap := make(map[string]*Task) + newMap := make(map[string]*Task) for _, task := range old { oldMap[task.Name] = task } @@ -857,11 +857,11 @@ func setDiffTasks(old, new []*structs.Task) *TasksDiff { // setDiffServices does a set difference of Services using the service name as a // key. -func setDiffServices(old, new []*structs.Service) *ServicesDiff { +func setDiffServices(old, new []*Service) *ServicesDiff { diff := &ServicesDiff{} - oldMap := make(map[string]*structs.Service) - newMap := make(map[string]*structs.Service) + oldMap := make(map[string]*Service) + newMap := make(map[string]*Service) for _, s := range old { oldMap[s.Name] = s } @@ -896,11 +896,11 @@ func setDiffServices(old, new []*structs.Service) *ServicesDiff { // setDiffServiceChecks does a set difference of service checks using the check // name as a key. -func setDiffServiceChecks(old, new []*structs.ServiceCheck) *ServiceChecksDiff { +func setDiffServiceChecks(old, new []*ServiceCheck) *ServiceChecksDiff { diff := &ServiceChecksDiff{} - oldMap := make(map[string]*structs.ServiceCheck) - newMap := make(map[string]*structs.ServiceCheck) + oldMap := make(map[string]*ServiceCheck) + newMap := make(map[string]*ServiceCheck) for _, s := range old { oldMap[s.Name] = s } @@ -935,11 +935,11 @@ func setDiffServiceChecks(old, new []*structs.ServiceCheck) *ServiceChecksDiff { // setDiffTaskArtifacts does a set difference of task artifacts using the geter // source as a key. -func setDiffTaskArtifacts(old, new []*structs.TaskArtifact) *TaskArtifactsDiff { +func setDiffTaskArtifacts(old, new []*TaskArtifact) *TaskArtifactsDiff { diff := &TaskArtifactsDiff{} - oldMap := make(map[string]*structs.TaskArtifact) - newMap := make(map[string]*structs.TaskArtifact) + oldMap := make(map[string]*TaskArtifact) + newMap := make(map[string]*TaskArtifact) for _, ta := range old { oldMap[ta.GetterSource] = ta } @@ -973,16 +973,16 @@ func setDiffTaskArtifacts(old, new []*structs.TaskArtifact) *TaskArtifactsDiff { } // setDiffNetworkResources does a set difference of network resources. -func setDiffNetworkResources(old, new []*structs.NetworkResource) *NetworkResourcesDiff { +func setDiffNetworkResources(old, new []*NetworkResource) *NetworkResourcesDiff { diff := &NetworkResourcesDiff{} added, del := setDifference(interfaceSlice(old), interfaceSlice(new)) for _, a := range added { - nDiff := NewNetworkResourceDiff(nil, a.(*structs.NetworkResource)) + nDiff := NewNetworkResourceDiff(nil, a.(*NetworkResource)) diff.Added = append(diff.Added, nDiff) } for _, d := range del { - nDiff := NewNetworkResourceDiff(d.(*structs.NetworkResource), nil) + nDiff := NewNetworkResourceDiff(d.(*NetworkResource), nil) diff.Added = append(diff.Deleted, nDiff) } @@ -990,11 +990,11 @@ func setDiffNetworkResources(old, new []*structs.NetworkResource) *NetworkResour } // setDiffPorts does a set difference of ports using the label as a key. -func setDiffPorts(old, new []structs.Port) *PortsDiff { +func setDiffPorts(old, new []Port) *PortsDiff { diff := &PortsDiff{} - oldMap := make(map[string]structs.Port) - newMap := make(map[string]structs.Port) + oldMap := make(map[string]Port) + newMap := make(map[string]Port) for _, p := range old { oldMap[p.Label] = p } diff --git a/nomad/structs/diff/job_test.go b/nomad/structs/job_diff_test.go similarity index 87% rename from nomad/structs/diff/job_test.go rename to nomad/structs/job_diff_test.go index ac8d627d5..044e01ce7 100644 --- a/nomad/structs/diff/job_test.go +++ b/nomad/structs/job_diff_test.go @@ -1,28 +1,24 @@ -package diff +package structs import ( "reflect" "sort" "testing" - - "github.com/davecgh/go-spew/spew" - "github.com/hashicorp/nomad/nomad/mock" - "github.com/hashicorp/nomad/nomad/structs" ) func TestNewJobDiff_Same(t *testing.T) { - job1 := mock.Job() - job2 := mock.Job() + job1 := TestJob() + job2 := TestJob() job2.ID = job1.ID diff := NewJobDiff(job1, job2) if diff != nil { - t.Fatalf("expected nil job diff; got %s", spew.Sdump(diff)) + t.Fatalf("expected nil job diff; got %#v", diff) } } func TestNewJobDiff_NilCases(t *testing.T) { - j := mock.Job() + j := TestJob() // Old job nil diff := NewJobDiff(nil, j) @@ -30,7 +26,6 @@ func TestNewJobDiff_NilCases(t *testing.T) { t.Fatalf("expected non-nil job diff") } if diff.Type != DiffTypeAdded { - //t.Fatalf("got diff type %v; want %v; %s", diff.Type, DiffTypeAdded, spew.Sdump(diff)) t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeAdded) } @@ -45,13 +40,13 @@ func TestNewJobDiff_NilCases(t *testing.T) { } func TestNewJobDiff_Constraints(t *testing.T) { - c1 := &structs.Constraint{LTarget: "foo"} - c2 := &structs.Constraint{LTarget: "bar"} - c3 := &structs.Constraint{LTarget: "baz"} + c1 := &Constraint{LTarget: "foo"} + c2 := &Constraint{LTarget: "bar"} + c3 := &Constraint{LTarget: "baz"} // Test the added case. - j1 := &structs.Job{Constraints: []*structs.Constraint{c1, c2}} - j2 := &structs.Job{Constraints: []*structs.Constraint{c1, c2, c3}} + j1 := &Job{Constraints: []*Constraint{c1, c2}} + j2 := &Job{Constraints: []*Constraint{c1, c2, c3}} diff := NewJobDiff(j1, j2) if diff == nil { @@ -75,8 +70,8 @@ func TestNewJobDiff_Constraints(t *testing.T) { } // Test the deleted case. - j1 = &structs.Job{Constraints: []*structs.Constraint{c1, c2}} - j2 = &structs.Job{Constraints: []*structs.Constraint{c1}} + j1 = &Job{Constraints: []*Constraint{c1, c2}} + j2 = &Job{Constraints: []*Constraint{c1}} diff = NewJobDiff(j1, j2) if diff == nil { @@ -101,8 +96,8 @@ func TestNewJobDiff_Constraints(t *testing.T) { } func TestNewJobDiff_Datacenters(t *testing.T) { - j1 := &structs.Job{Datacenters: []string{"a", "b"}} - j2 := &structs.Job{Datacenters: []string{"b", "c"}} + j1 := &Job{Datacenters: []string{"a", "b"}} + j2 := &Job{Datacenters: []string{"b", "c"}} diff := NewJobDiff(j1, j2) if diff == nil { @@ -126,13 +121,13 @@ func TestNewJobDiff_Datacenters(t *testing.T) { } func TestNewJobDiff_TaskGroups(t *testing.T) { - tg1 := &structs.TaskGroup{Name: "foo"} - tg2 := &structs.TaskGroup{Name: "bar"} - tg2_2 := &structs.TaskGroup{Name: "bar", Count: 2} - tg3 := &structs.TaskGroup{Name: "baz"} + tg1 := &TaskGroup{Name: "foo"} + tg2 := &TaskGroup{Name: "bar"} + tg2_2 := &TaskGroup{Name: "bar", Count: 2} + tg3 := &TaskGroup{Name: "baz"} - j1 := &structs.Job{TaskGroups: []*structs.TaskGroup{tg1, tg2}} - j2 := &structs.Job{TaskGroups: []*structs.TaskGroup{tg2_2, tg3}} + j1 := &Job{TaskGroups: []*TaskGroup{tg1, tg2}} + j2 := &Job{TaskGroups: []*TaskGroup{tg2_2, tg3}} diff := NewJobDiff(j1, j2) if diff == nil { @@ -148,8 +143,8 @@ func TestNewJobDiff_TaskGroups(t *testing.T) { t.Fatalf("expected task group diff") } - if !reflect.DeepEqual(tgd.Added, []*structs.TaskGroup{tg3}) || - !reflect.DeepEqual(tgd.Deleted, []*structs.TaskGroup{tg1}) { + if !reflect.DeepEqual(tgd.Added, []*TaskGroup{tg3}) || + !reflect.DeepEqual(tgd.Deleted, []*TaskGroup{tg1}) { t.Fatalf("bad: %#v", tgd) } @@ -174,7 +169,7 @@ func TestNewTaskDiff_Config(t *testing.T) { c3 := map[string]interface{}{ "command": "/bin/date", "args": []string{"1", "2"}, - "nested": &structs.Port{ + "nested": &Port{ Label: "http", Value: 80, }, @@ -186,7 +181,7 @@ func TestNewTaskDiff_Config(t *testing.T) { } // No old case - t1 := &structs.Task{Config: c1} + t1 := &Task{Config: c1} diff := NewTaskDiff(nil, t1) if diff == nil { t.Fatalf("expected non-nil diff") @@ -221,7 +216,7 @@ func TestNewTaskDiff_Config(t *testing.T) { } // Deleted case - t2 := &structs.Task{Config: c2} + t2 := &Task{Config: c2} diff = NewTaskDiff(t1, t2) if diff == nil { t.Fatalf("expected non-nil diff") @@ -244,7 +239,7 @@ func TestNewTaskDiff_Config(t *testing.T) { } // Added case - t3 := &structs.Task{Config: c3} + t3 := &Task{Config: c3} diff = NewTaskDiff(t1, t3) if diff == nil { t.Fatalf("expected non-nil diff") @@ -270,7 +265,7 @@ func TestNewTaskDiff_Config(t *testing.T) { } // Edited case - t4 := &structs.Task{Config: c4} + t4 := &Task{Config: c4} diff = NewTaskDiff(t1, t4) if diff == nil { t.Fatalf("expected non-nil diff") @@ -297,9 +292,9 @@ func TestNewTaskDiff_Config(t *testing.T) { } func TestNewPrimitiveStructDiff(t *testing.T) { - p1 := structs.Port{Label: "1"} - p2 := structs.Port{Label: "2"} - p3 := structs.Port{} + p1 := Port{Label: "1"} + p2 := Port{Label: "2"} + p3 := Port{} pdiff := NewPrimitiveStructDiff(nil, nil, portFields) if pdiff != nil { @@ -373,15 +368,15 @@ func TestNewPrimitiveStructDiff(t *testing.T) { } func TestSetDiffPrimitiveStructs(t *testing.T) { - p1 := structs.Port{Label: "1"} - p2 := structs.Port{Label: "2"} - p3 := structs.Port{Label: "3"} - p4 := structs.Port{Label: "4"} - p5 := structs.Port{Label: "5"} - p6 := structs.Port{Label: "6"} + p1 := Port{Label: "1"} + p2 := Port{Label: "2"} + p3 := Port{Label: "3"} + p4 := Port{Label: "4"} + p5 := Port{Label: "5"} + p6 := Port{Label: "6"} - old := []structs.Port{p1, p2, p3, p4} - new := []structs.Port{p3, p4, p5, p6} + old := []Port{p1, p2, p3, p4} + new := []Port{p3, p4, p5, p6} diffs := setDiffPrimitiveStructs(interfaceSlice(old), interfaceSlice(new), portFields) if len(diffs) != 4 { @@ -634,9 +629,9 @@ func TestKeyedSetDifference(t *testing.T) { } func TestInterfaceSlice(t *testing.T) { - j1 := mock.Job() - j2 := mock.Job() - jobs := []*structs.Job{j1, j2} + j1 := TestJob() + j2 := TestJob() + jobs := []*Job{j1, j2} slice := interfaceSlice(jobs) if len(slice) != 2 { @@ -644,7 +639,7 @@ func TestInterfaceSlice(t *testing.T) { } f := slice[0] - actJob1, ok := f.(*structs.Job) + actJob1, ok := f.(*Job) if !ok { t.Fatalf("unexpected type: %v", actJob1) } @@ -655,7 +650,7 @@ func TestInterfaceSlice(t *testing.T) { } func TestGetField(t *testing.T) { - j := mock.Job() + j := TestJob() exp := "foo" j.Type = "foo" diff --git a/nomad/structs/diff/job_visitor.go b/nomad/structs/job_diff_visitor.go similarity index 99% rename from nomad/structs/diff/job_visitor.go rename to nomad/structs/job_diff_visitor.go index c9ef9a4cc..a1747fc6f 100644 --- a/nomad/structs/diff/job_visitor.go +++ b/nomad/structs/job_diff_visitor.go @@ -1,4 +1,4 @@ -package diff +package structs // JobVisitor is the set of types a visitor must implement to traverse a JobDiff // structure. diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index fbf3ffd5b..7dbea674d 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -93,82 +93,8 @@ func TestJob_Validate(t *testing.T) { } } -func testJob() *Job { - return &Job{ - Region: "global", - ID: GenerateUUID(), - Name: "my-job", - Type: JobTypeService, - Priority: 50, - AllAtOnce: false, - Datacenters: []string{"dc1"}, - Constraints: []*Constraint{ - &Constraint{ - LTarget: "$attr.kernel.name", - RTarget: "linux", - Operand: "=", - }, - }, - Periodic: &PeriodicConfig{ - Enabled: false, - }, - TaskGroups: []*TaskGroup{ - &TaskGroup{ - Name: "web", - Count: 10, - RestartPolicy: &RestartPolicy{ - Attempts: 3, - Interval: 10 * time.Minute, - Delay: 1 * time.Minute, - }, - Tasks: []*Task{ - &Task{ - Name: "web", - Driver: "exec", - Config: map[string]interface{}{ - "command": "/bin/date", - }, - Env: map[string]string{ - "FOO": "bar", - }, - Artifacts: []*TaskArtifact{ - { - GetterSource: "http://foo.com", - }, - }, - Services: []*Service{ - { - Name: "${TASK}-frontend", - PortLabel: "http", - }, - }, - Resources: &Resources{ - CPU: 500, - MemoryMB: 256, - Networks: []*NetworkResource{ - &NetworkResource{ - MBits: 50, - DynamicPorts: []Port{{Label: "http"}}, - }, - }, - }, - }, - }, - Meta: map[string]string{ - "elb_check_type": "http", - "elb_check_interval": "30s", - "elb_check_min": "3", - }, - }, - }, - Meta: map[string]string{ - "owner": "armon", - }, - } -} - func TestJob_Copy(t *testing.T) { - j := testJob() + j := TestJob() c := j.Copy() if !reflect.DeepEqual(j, c) { t.Fatalf("Copy() returned an unequal Job; got %#v; want %#v", c, j) @@ -528,7 +454,7 @@ func TestEncodeDecode(t *testing.T) { } func BenchmarkEncodeDecode(b *testing.B) { - job := testJob() + job := TestJob() for i := 0; i < b.N; i++ { buf, err := Encode(1, job) diff --git a/nomad/structs/testing.go b/nomad/structs/testing.go new file mode 100644 index 000000000..3893e2369 --- /dev/null +++ b/nomad/structs/testing.go @@ -0,0 +1,77 @@ +package structs + +import "time" + +func TestJob() *Job { + return &Job{ + Region: "global", + ID: GenerateUUID(), + Name: "my-job", + Type: JobTypeService, + Priority: 50, + AllAtOnce: false, + Datacenters: []string{"dc1"}, + Constraints: []*Constraint{ + &Constraint{ + LTarget: "$attr.kernel.name", + RTarget: "linux", + Operand: "=", + }, + }, + Periodic: &PeriodicConfig{ + Enabled: false, + }, + TaskGroups: []*TaskGroup{ + &TaskGroup{ + Name: "web", + Count: 10, + RestartPolicy: &RestartPolicy{ + Attempts: 3, + Interval: 10 * time.Minute, + Delay: 1 * time.Minute, + }, + Tasks: []*Task{ + &Task{ + Name: "web", + Driver: "exec", + Config: map[string]interface{}{ + "command": "/bin/date", + }, + Env: map[string]string{ + "FOO": "bar", + }, + Artifacts: []*TaskArtifact{ + { + GetterSource: "http://foo.com", + }, + }, + Services: []*Service{ + { + Name: "${TASK}-frontend", + PortLabel: "http", + }, + }, + Resources: &Resources{ + CPU: 500, + MemoryMB: 256, + Networks: []*NetworkResource{ + &NetworkResource{ + MBits: 50, + DynamicPorts: []Port{{Label: "http"}}, + }, + }, + }, + }, + }, + Meta: map[string]string{ + "elb_check_type": "http", + "elb_check_interval": "30s", + "elb_check_min": "3", + }, + }, + }, + Meta: map[string]string{ + "owner": "armon", + }, + } +} From 789722bf3f4efbe2d9db7caa94331fbbcab14dc3 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 4 May 2016 14:29:08 -0700 Subject: [PATCH 06/17] Index fields using a map --- nomad/structs/job_diff.go | 8 ++++++-- nomad/structs/job_diff_test.go | 11 +++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/nomad/structs/job_diff.go b/nomad/structs/job_diff.go index 01a755c82..a6bf98bba 100644 --- a/nomad/structs/job_diff.go +++ b/nomad/structs/job_diff.go @@ -255,7 +255,7 @@ type PortsDiff struct { // primitive fields. type PrimitiveStructDiff struct { DiffEntry - PrimitiveFields []*FieldDiff + PrimitiveFields map[string]*FieldDiff } // DiffFields performs the diff of the passed fields against the old and new @@ -266,7 +266,11 @@ func (p *PrimitiveStructDiff) DiffFields(old, new interface{}, fields []string) newV := getField(new, field) pDiff := NewFieldDiff(field, oldV, newV) if pDiff != nil { - p.PrimitiveFields = append(p.PrimitiveFields, pDiff) + if p.PrimitiveFields == nil { + p.PrimitiveFields = make(map[string]*FieldDiff) + } + + p.PrimitiveFields[field] = pDiff } } } diff --git a/nomad/structs/job_diff_test.go b/nomad/structs/job_diff_test.go index 044e01ce7..1db2ebb8e 100644 --- a/nomad/structs/job_diff_test.go +++ b/nomad/structs/job_diff_test.go @@ -337,7 +337,11 @@ func TestNewPrimitiveStructDiff(t *testing.T) { t.Fatalf("unexpected number of field diffs: %#v", pdiff.PrimitiveFields) } - f := pdiff.PrimitiveFields[0] + f, ok := pdiff.PrimitiveFields["Label"] + if !ok { + t.Fatalf("expected diff on field %q", "label") + } + if f.Type != DiffTypeEdited { t.Fatalf("unexpected type: got %v; want %v", f.Type, DiffTypeEdited) } @@ -358,7 +362,10 @@ func TestNewPrimitiveStructDiff(t *testing.T) { t.Fatalf("unexpected number of field diffs: %#v", pdiff.PrimitiveFields) } - f = pdiff.PrimitiveFields[0] + f = pdiff.PrimitiveFields["Label"] + if !ok { + t.Fatalf("expected diff on field %q", "Label") + } if f.Type != DiffTypeEdited { t.Fatalf("unexpected type: got %v; want %v", f.Type, DiffTypeEdited) } From ab0b57a9a131752cc8c9f489d24427211bcc7304 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 5 May 2016 11:21:58 -0700 Subject: [PATCH 07/17] Initial plan endpoint implementation - WIP --- command/agent/job_endpoint.go | 28 ++++ command/agent/job_endpoint_test.go | 36 +++++ nomad/job_endpoint.go | 152 ++++++++++++++---- nomad/structs/diff/structs.go | 25 +++ nomad/structs/structs.go | 55 +++++++ scheduler/annotate.go | 108 +++++++++++++ scheduler/annotate_test.go | 240 +++++++++++++++++++++++++++++ scheduler/generic_sched.go | 15 +- scheduler/generic_sched_test.go | 81 ++++++++++ scheduler/inmem_planner.go | 36 +++++ scheduler/system_sched.go | 15 +- scheduler/system_sched_test.go | 86 +++++++++++ scheduler/util.go | 76 +++++++++ scheduler/util_test.go | 57 +++++++ 14 files changed, 974 insertions(+), 36 deletions(-) create mode 100644 nomad/structs/diff/structs.go create mode 100644 scheduler/annotate.go create mode 100644 scheduler/annotate_test.go create mode 100644 scheduler/inmem_planner.go diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index d66954a3c..74a6e9fff 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -51,6 +51,9 @@ func (s *HTTPServer) JobSpecificRequest(resp http.ResponseWriter, req *http.Requ case strings.HasSuffix(path, "/periodic/force"): jobName := strings.TrimSuffix(path, "/periodic/force") return s.periodicForceRequest(resp, req, jobName) + case strings.HasSuffix(path, "/plan"): + jobName := strings.TrimSuffix(path, "/plan") + return s.jobPlan(resp, req, jobName) default: return s.jobCRUD(resp, req, path) } @@ -74,6 +77,31 @@ func (s *HTTPServer) jobForceEvaluate(resp http.ResponseWriter, req *http.Reques return out, nil } +func (s *HTTPServer) jobPlan(resp http.ResponseWriter, req *http.Request, + jobName string) (interface{}, error) { + if req.Method != "PUT" && req.Method != "POST" { + return nil, CodedError(405, ErrInvalidMethod) + } + + var args structs.JobPlanRequest + if err := decodeBody(req, &args); err != nil { + return nil, CodedError(400, err.Error()) + } + if args.Job == nil { + return nil, CodedError(400, "Job must be specified") + } + if jobName != "" && args.Job.ID != jobName { + return nil, CodedError(400, "Job ID does not match") + } + s.parseRegion(req, &args.Region) + + var out structs.JobPlanResponse + if err := s.agent.RPC("Job.Plan", &args, &out); err != nil { + return nil, err + } + return out, nil +} + func (s *HTTPServer) periodicForceRequest(resp http.ResponseWriter, req *http.Request, jobName string) (interface{}, error) { if req.Method != "PUT" && req.Method != "POST" { diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 98599392d..22c448525 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -483,3 +483,39 @@ func TestHTTP_PeriodicForce(t *testing.T) { } }) } + +func TestHTTP_JobPlan(t *testing.T) { + httpTest(t, nil, func(s *TestServer) { + // Create the job + job := mock.Job() + args := structs.JobPlanRequest{ + Job: job, + Diff: true, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + buf := encodeReq(args) + + // Make the HTTP request + req, err := http.NewRequest("PUT", "/v1/job/"+job.ID+"/plan", buf) + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + // Make the request + obj, err := s.Server.JobSpecificRequest(respW, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Check the response + plan := obj.(structs.JobPlanResponse) + if plan.Plan == nil { + t.Fatalf("bad: %v", plan) + } + + if plan.Diff == nil { + t.Fatalf("bad: %v", plan) + } + }) +} diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 9f3a616b2..b00ccffe5 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/nomad/client/driver" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/watch" + "github.com/hashicorp/nomad/scheduler" ) // Job endpoint is used for job interactions @@ -32,39 +33,11 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis // Initialize the job fields (sets defaults and any necessary init work). args.Job.InitFields() - if err := args.Job.Validate(); err != nil { + // Validate the job. + if err := validateJob(args.Job); err != nil { return err } - // Validate the driver configurations. - var driverErrors multierror.Error - for _, tg := range args.Job.TaskGroups { - for _, task := range tg.Tasks { - d, err := driver.NewDriver( - task.Driver, - driver.NewEmptyDriverContext(), - ) - if err != nil { - msg := "failed to create driver for task %q in group %q for validation: %v" - driverErrors.Errors = append(driverErrors.Errors, fmt.Errorf(msg, tg.Name, task.Name, err)) - continue - } - - if err := d.Validate(task.Config); err != nil { - formatted := fmt.Errorf("group %q -> task %q -> config: %v", tg.Name, task.Name, err) - driverErrors.Errors = append(driverErrors.Errors, formatted) - } - } - } - - if len(driverErrors.Errors) != 0 { - return driverErrors.ErrorOrNil() - } - - if args.Job.Type == structs.JobTypeCore { - return fmt.Errorf("job type cannot be core") - } - // Commit this update via Raft _, index, err := j.srv.raftApply(structs.JobRegisterRequestType, args) if err != nil { @@ -414,3 +387,122 @@ func (j *Job) Evaluations(args *structs.JobSpecificRequest, j.srv.setQueryMeta(&reply.QueryMeta) return nil } + +// Plan is used to cause a dry-run evaluation of the Job and return the results +// with a potential diff containing annotations. +func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) error { + if done, err := j.srv.forward("Job.Plan", args, args, reply); done { + return err + } + defer metrics.MeasureSince([]string{"nomad", "job", "plan"}, time.Now()) + + // Validate the arguments + if args.Job == nil { + return fmt.Errorf("Job required for plan") + } + + // Initialize the job fields (sets defaults and any necessary init work). + args.Job.InitFields() + + // Validate the job. + if err := validateJob(args.Job); err != nil { + return err + } + + // Acquire a snapshot of the state + snap, err := j.srv.fsm.State().Snapshot() + if err != nil { + return err + } + + // Get the original job + oldJob, err := snap.JobByID(args.Job.ID) + if err != nil { + return err + } + + var index uint64 + if oldJob != nil { + index = oldJob.JobModifyIndex + 1 + } + + // Insert the updated Job into the snapshot + snap.UpsertJob(index, args.Job) + + // Create an eval and mark it as requiring annotations and insert that as well + eval := &structs.Evaluation{ + ID: structs.GenerateUUID(), + Priority: args.Job.Priority, + Type: args.Job.Type, + TriggeredBy: structs.EvalTriggerJobRegister, + JobID: args.Job.ID, + JobModifyIndex: index, + Status: structs.EvalStatusPending, + AnnotatePlan: true, + } + + // Create an in-memory Planner that returns no errors and stores the + // submitted plan and created evals. + planner := scheduler.NewInMemoryPlanner() + + // Create the scheduler and run it + sched, err := scheduler.NewScheduler(eval.Type, j.srv.logger, snap, planner) + if err != nil { + return err + } + + if err := sched.Process(eval); err != nil { + return err + } + + // Clear the job from the plan + planner.Plan.Job = nil + + // Annotate and store the diff + if args.Diff { + jobDiff := structs.NewJobDiff(oldJob, args.Job) + scheduler.Annotate(jobDiff) + reply.Diff = jobDiff + } + + reply.Cas = index + reply.Plan = planner.Plan + reply.CreatedEvals = planner.CreatedEvals + return nil +} + +// validateJob validates a Job and task drivers and returns an error if there is +// a validation problem or if the Job is of a type a user is not allowed to +// submit. +func validateJob(job *structs.Job) error { + validationErrors := new(multierror.Error) + if err := job.Validate(); err != nil { + multierror.Append(validationErrors, err) + } + + // Validate the driver configurations. + for _, tg := range job.TaskGroups { + for _, task := range tg.Tasks { + d, err := driver.NewDriver( + task.Driver, + driver.NewEmptyDriverContext(), + ) + if err != nil { + msg := "failed to create driver for task %q in group %q for validation: %v" + multierror.Append(validationErrors, fmt.Errorf(msg, tg.Name, task.Name, err)) + continue + } + + if err := d.Validate(task.Config); err != nil { + formatted := fmt.Errorf("group %q -> task %q -> config: %v", tg.Name, task.Name, err) + multierror.Append(validationErrors, formatted) + } + } + } + + if job.Type == structs.JobTypeCore { + multierror.Append(validationErrors, fmt.Errorf("job type cannot be core")) + } + + return validationErrors.ErrorOrNil() +} diff --git a/nomad/structs/diff/structs.go b/nomad/structs/diff/structs.go new file mode 100644 index 000000000..e92465dfb --- /dev/null +++ b/nomad/structs/diff/structs.go @@ -0,0 +1,25 @@ +package diff + +import "github.com/hashicorp/nomad/nomad/structs" + +// JobPlanResponse is used to respond to a job plan request. Must be in this +// package to avoid a cyclic dependency. +type JobPlanResponse struct { + // Plan holds the decisions the scheduler made. + Plan *structs.Plan + + // The Cas value can be used when running `nomad run` to ensure that the Job + // wasn’t modified since the last plan. If the job is being created, the + // value is zero. + Cas uint64 + + // CreatedEvals is the set of evaluations created by the scheduler. The + // reasons for this can be rolling-updates or blocked evals. + CreatedEvals []*structs.Evaluation + + // Diff contains the diff of the job and annotations on whether the change + // causes an in-place update or create/destroy + Diff *JobDiff + + structs.QueryMeta +} diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index f461c9b05..352e91fd0 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -209,6 +209,14 @@ type JobListRequest struct { QueryOptions } +// JobPlanRequest is used for the Job.Plan endpoint to trigger a dry-run +// evaluation of the Job. +type JobPlanRequest struct { + Job *Job + Diff bool // Toggles an annotated diff + WriteRequest +} + // NodeListRequest is used to parameterize a list request type NodeListRequest struct { QueryOptions @@ -390,6 +398,27 @@ type JobListResponse struct { QueryMeta } +// JobPlanResponse is used to respond to a job plan request +type JobPlanResponse struct { + // Plan holds the decisions the scheduler made. + Plan *Plan + + // The Cas value can be used when running `nomad run` to ensure that the Job + // wasn’t modified since the last plan. If the job is being created, the + // value is zero. + Cas uint64 + + // CreatedEvals is the set of evaluations created by the scheduler. The + // reasons for this can be rolling-updates or blocked evals. + CreatedEvals []*Evaluation + + // Diff contains the diff of the job and annotations on whether the change + // causes an in-place update or create/destroy + Diff *JobDiff + + QueryMeta +} + // SingleAllocResponse is used to return a single allocation type SingleAllocResponse struct { Alloc *Allocation @@ -2577,6 +2606,10 @@ type Evaluation struct { // captured by computed node classes. EscapedComputedClass bool + // AnnotatePlan triggers the scheduler to provide additional annotations + // during the evaluation. This should not be set during normal operations. + AnnotatePlan bool + // Raft Indexes CreateIndex uint64 ModifyIndex uint64 @@ -2721,6 +2754,10 @@ type Plan struct { // but are persisted so that the user can use the feedback // to determine the cause. FailedAllocs []*Allocation + + // Annotations contains annotations by the scheduler to be used by operators + // to understand the decisions made by the scheduler. + Annotations *PlanAnnotations } func (p *Plan) AppendUpdate(alloc *Allocation, status, desc string) { @@ -2817,6 +2854,24 @@ func (p *PlanResult) FullCommit(plan *Plan) (bool, int, int) { return actual == expected, expected, actual } +// PlanAnnotations holds annotations made by the scheduler to give further debug +// information to operators. +type PlanAnnotations struct { + // DesiredTGUpdates is the set of desired updates per task group. + DesiredTGUpdates map[string]*DesiredUpdates +} + +// DesiredUpdates is the set of changes the scheduler would like to make given +// sufficient resources and cluster capacity. +type DesiredUpdates struct { + Ignore uint64 + Place uint64 + Migrate uint64 + Stop uint64 + InPlaceUpdate uint64 + DestructiveUpdate uint64 +} + // msgpackHandle is a shared handle for encoding/decoding of structs var MsgpackHandle = func() *codec.MsgpackHandle { h := &codec.MsgpackHandle{RawToString: true} diff --git a/scheduler/annotate.go b/scheduler/annotate.go new file mode 100644 index 000000000..00209f921 --- /dev/null +++ b/scheduler/annotate.go @@ -0,0 +1,108 @@ +package scheduler + +import "github.com/hashicorp/nomad/nomad/structs" + +const ( + AnnotationForcesCreate = "forces create" + AnnotationForcesDestroy = "forces destroy" + AnnotationForcesInplaceUpdate = "forces in-place update" + AnnotationForcesDestructiveUpdate = "forces create/destroy update" +) + +// Annotate takes the diff between the old and new version of a Job and will add +// annotations to the diff to aide human understanding of the plan. +// +// Currently the things that are annotated are: +// * Count up and count down changes +// * Task changes will be annotated with: +// * forces create/destroy update +// * forces in-place update +func Annotate(diff *structs.JobDiff) { + // No annotation needed as the job was either just submitted or deleted. + if diff.Type != structs.DiffTypeEdited { + return + } + + tgDiffs := diff.TaskGroups + if tgDiffs == nil { + return + } + + for _, tgDiff := range tgDiffs.Edited { + annotateTaskGroup(tgDiff) + } +} + +// annotateTaskGroup takes a task group diff and annotates it. +func annotateTaskGroup(diff *structs.TaskGroupDiff) { + // Don't annotate unless the task group was edited. If it was a + // create/destroy it is not needed. + if diff == nil || diff.Type != structs.DiffTypeEdited { + return + } + + // Annotate the count + annotateCountChange(diff) + + // Annotate the tasks. + taskDiffs := diff.Tasks + if taskDiffs == nil { + return + } + + for _, taskDiff := range taskDiffs.Edited { + annotateTask(taskDiff) + } +} + +// annotateCountChange takes a task group diff and annotates the count +// parameter. +func annotateCountChange(diff *structs.TaskGroupDiff) { + // Don't annotate unless the task was edited. If it was a create/destroy it + // is not needed. + if diff == nil || diff.Type != structs.DiffTypeEdited { + return + } + + countDiff, ok := diff.PrimitiveFields["Count"] + if !ok { + return + } + oldV := countDiff.OldValue.(int) + newV := countDiff.NewValue.(int) + + if oldV < newV { + countDiff.Annotations = append(countDiff.Annotations, AnnotationForcesCreate) + } else if newV < oldV { + countDiff.Annotations = append(countDiff.Annotations, AnnotationForcesDestroy) + } +} + +// annotateCountChange takes a task diff and annotates it. +func annotateTask(diff *structs.TaskDiff) { + // Don't annotate unless the task was edited. If it was a create/destroy it + // is not needed. + if diff == nil || diff.Type != structs.DiffTypeEdited { + return + } + + // All changes to primitive fields result in a destructive update. + destructive := false + if len(diff.PrimitiveFields) != 0 { + destructive = true + } + + if diff.Env != nil || + diff.Meta != nil || + diff.Artifacts != nil || + diff.Resources != nil || + diff.Config != nil { + destructive = true + } + + if destructive { + diff.Annotations = append(diff.Annotations, AnnotationForcesDestructiveUpdate) + } else { + diff.Annotations = append(diff.Annotations, AnnotationForcesInplaceUpdate) + } +} diff --git a/scheduler/annotate_test.go b/scheduler/annotate_test.go new file mode 100644 index 000000000..b22174267 --- /dev/null +++ b/scheduler/annotate_test.go @@ -0,0 +1,240 @@ +package scheduler + +import ( + "reflect" + "testing" + + "github.com/hashicorp/nomad/nomad/structs" +) + +func TestAnnotateCountChange_NonEdited(t *testing.T) { + tg := &structs.TaskGroupDiff{} + tgOrig := &structs.TaskGroupDiff{} + annotateCountChange(tg) + if !reflect.DeepEqual(tgOrig, tg) { + t.Fatalf("annotateCountChange(%#v) should not have caused any annotation: %#v", tgOrig, tg) + } +} + +func TestAnnotateCountChange(t *testing.T) { + up := &structs.FieldDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + Name: "Count", + OldValue: 1, + NewValue: 3, + } + down := &structs.FieldDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + Name: "Count", + OldValue: 3, + NewValue: 1, + } + tgUp := &structs.TaskGroupDiff{ + PrimitiveStructDiff: structs.PrimitiveStructDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + PrimitiveFields: map[string]*structs.FieldDiff{ + "Count": up, + }, + }, + } + tgDown := &structs.TaskGroupDiff{ + PrimitiveStructDiff: structs.PrimitiveStructDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + PrimitiveFields: map[string]*structs.FieldDiff{ + "Count": down, + }, + }, + } + + // Test the up case + annotateCountChange(tgUp) + countDiff := tgUp.PrimitiveFields["Count"] + if len(countDiff.Annotations) != 1 || countDiff.Annotations[0] != AnnotationForcesCreate { + t.Fatalf("incorrect annotation: %#v", tgUp) + } + + // Test the down case + annotateCountChange(tgDown) + countDiff = tgDown.PrimitiveFields["Count"] + if len(countDiff.Annotations) != 1 || countDiff.Annotations[0] != AnnotationForcesDestroy { + t.Fatalf("incorrect annotation: %#v", tgDown) + } +} + +func TestAnnotateTask_NonEdited(t *testing.T) { + td := &structs.TaskDiff{} + tdOrig := &structs.TaskDiff{} + annotateTask(td) + if !reflect.DeepEqual(tdOrig, td) { + t.Fatalf("annotateTask(%#v) should not have caused any annotation: %#v", tdOrig, td) + } +} + +func TestAnnotateTask_Destructive(t *testing.T) { + cases := []*structs.TaskDiff{ + { + PrimitiveStructDiff: structs.PrimitiveStructDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + PrimitiveFields: map[string]*structs.FieldDiff{ + "Driver": &structs.FieldDiff{ + Name: "Driver", + OldValue: "docker", + NewValue: "exec", + }, + }, + }, + }, + { + PrimitiveStructDiff: structs.PrimitiveStructDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + PrimitiveFields: map[string]*structs.FieldDiff{ + "User": &structs.FieldDiff{ + Name: "User", + OldValue: "", + NewValue: "specific", + }, + }, + }, + }, + { + Env: &structs.StringMapDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeAdded, + }, + Added: map[string]string{ + "foo": "bar", + }, + }, + }, + { + Meta: &structs.StringMapDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + Edited: map[string]structs.StringValueDelta{ + "foo": { + Old: "a", + New: "b", + }, + }, + }, + }, + { + Artifacts: &structs.TaskArtifactsDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeAdded, + }, + Added: []*structs.TaskArtifact{ + { + GetterSource: "foo", + }, + }, + }, + }, + { + Resources: &structs.ResourcesDiff{ + PrimitiveStructDiff: structs.PrimitiveStructDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + PrimitiveFields: map[string]*structs.FieldDiff{ + "CPU": &structs.FieldDiff{ + Name: "CPU", + OldValue: 500, + NewValue: 1000, + }, + }, + }, + }, + }, + { + Config: &structs.StringMapDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + Edited: map[string]structs.StringValueDelta{ + "command": { + Old: "/bin/date", + New: "/bin/bash", + }, + }, + }, + }, + } + + for i, c := range cases { + c.Type = structs.DiffTypeEdited + annotateTask(c) + if len(c.Annotations) != 1 || c.Annotations[0] != AnnotationForcesDestructiveUpdate { + t.Fatalf("case %d not properly annotated %#v", i+1, c) + } + } +} + +func TestAnnotateTask_Inplace(t *testing.T) { + cases := []*structs.TaskDiff{ + { + Constraints: []*structs.PrimitiveStructDiff{ + { + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + PrimitiveFields: map[string]*structs.FieldDiff{ + "RTarget": &structs.FieldDiff{ + Name: "RTarget", + OldValue: "linux", + NewValue: "windows", + }, + }, + }, + }, + }, + { + LogConfig: &structs.PrimitiveStructDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + }, + PrimitiveFields: map[string]*structs.FieldDiff{ + "MaxFileSizeMB": &structs.FieldDiff{ + Name: "MaxFileSizeMB", + OldValue: 100, + NewValue: 128, + }, + }, + }, + }, + { + Services: &structs.ServicesDiff{ + DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeAdded, + }, + Added: []*structs.Service{ + { + Name: "foo", + PortLabel: "rpc", + }, + }, + }, + }, + } + + for i, c := range cases { + c.Type = structs.DiffTypeEdited + annotateTask(c) + if len(c.Annotations) != 1 || c.Annotations[0] != AnnotationForcesInplaceUpdate { + t.Fatalf("case %d not properly annotated %#v", i+1, c) + } + } +} diff --git a/scheduler/generic_sched.go b/scheduler/generic_sched.go index 0058be3a3..dd50cc558 100644 --- a/scheduler/generic_sched.go +++ b/scheduler/generic_sched.go @@ -173,8 +173,9 @@ func (s *GenericScheduler) process() (bool, error) { return false, err } - // If the plan is a no-op, we can bail - if s.plan.IsNoOp() { + // If the plan is a no-op, we can bail. If AnnotatePlan is set submit the plan + // anyways to get the annotations. + if s.plan.IsNoOp() && !s.eval.AnnotatePlan { return true, nil } @@ -323,7 +324,15 @@ func (s *GenericScheduler) computeJobAllocs() error { } // Attempt to do the upgrades in place - diff.update = inplaceUpdate(s.ctx, s.eval, s.job, s.stack, diff.update) + destructiveUpdates := inplaceUpdate(s.ctx, s.eval, s.job, s.stack, diff.update) + inplaceUpdates := diff.update[len(destructiveUpdates):] + diff.update = destructiveUpdates + + if s.eval.AnnotatePlan { + s.plan.Annotations = &structs.PlanAnnotations{ + DesiredTGUpdates: desiredUpdates(diff, inplaceUpdates, destructiveUpdates), + } + } // Check if a rolling upgrade strategy is being used limit := len(diff.update) + len(diff.migrate) diff --git a/scheduler/generic_sched_test.go b/scheduler/generic_sched_test.go index 683873348..e3d5e9a84 100644 --- a/scheduler/generic_sched_test.go +++ b/scheduler/generic_sched_test.go @@ -2,6 +2,7 @@ package scheduler import ( "fmt" + "reflect" "testing" "time" @@ -42,6 +43,11 @@ func TestServiceSched_JobRegister(t *testing.T) { } plan := h.Plans[0] + // Ensure the plan doesn't have annotations. + if plan.Annotations != nil { + t.Fatalf("expected no annotations") + } + // Ensure the plan allocated var planned []*structs.Allocation for _, allocList := range plan.NodeAllocation { @@ -76,6 +82,81 @@ func TestServiceSched_JobRegister(t *testing.T) { h.AssertEvalStatus(t, structs.EvalStatusComplete) } +func TestServiceSched_JobRegister_Annotate(t *testing.T) { + h := NewHarness(t) + + // Create some nodes + for i := 0; i < 10; i++ { + node := mock.Node() + noErr(t, h.State.UpsertNode(h.NextIndex(), node)) + } + + // Create a job + job := mock.Job() + noErr(t, h.State.UpsertJob(h.NextIndex(), job)) + + // Create a mock evaluation to register the job + eval := &structs.Evaluation{ + ID: structs.GenerateUUID(), + Priority: job.Priority, + TriggeredBy: structs.EvalTriggerJobRegister, + JobID: job.ID, + AnnotatePlan: true, + } + + // Process the evaluation + err := h.Process(NewServiceScheduler, eval) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Ensure a single plan + if len(h.Plans) != 1 { + t.Fatalf("bad: %#v", h.Plans) + } + plan := h.Plans[0] + + // Ensure the plan allocated + var planned []*structs.Allocation + for _, allocList := range plan.NodeAllocation { + planned = append(planned, allocList...) + } + if len(planned) != 10 { + t.Fatalf("bad: %#v", plan) + } + + // Lookup the allocations by JobID + out, err := h.State.AllocsByJob(job.ID) + noErr(t, err) + + // Ensure all allocations placed + if len(out) != 10 { + t.Fatalf("bad: %#v", out) + } + + h.AssertEvalStatus(t, structs.EvalStatusComplete) + + // Ensure the plan had annotations. + if plan.Annotations == nil { + t.Fatalf("expected annotations") + } + + desiredTGs := plan.Annotations.DesiredTGUpdates + if l := len(desiredTGs); l != 1 { + t.Fatalf("incorrect number of task groups; got %v; want %v", l, 1) + } + + desiredChanges, ok := desiredTGs["web"] + if !ok { + t.Fatalf("expected task group web to have desired changes") + } + + expected := &structs.DesiredUpdates{Place: 10} + if !reflect.DeepEqual(desiredChanges, expected) { + t.Fatalf("Unexpected desired updates; got %#v; want %#v", desiredChanges, expected) + } +} + func TestServiceSched_JobRegister_CountZero(t *testing.T) { h := NewHarness(t) diff --git a/scheduler/inmem_planner.go b/scheduler/inmem_planner.go new file mode 100644 index 000000000..1c9ccbbd6 --- /dev/null +++ b/scheduler/inmem_planner.go @@ -0,0 +1,36 @@ +package scheduler + +import "github.com/hashicorp/nomad/nomad/structs" + +// InMemPlanner is an in-memory Planner that can be used to invoke the scheduler +// without side-effects. +type InMemPlanner struct { + CreatedEvals []*structs.Evaluation + Plan *structs.Plan +} + +// NewInMemoryPlanner returns a new in-memory planner. +func NewInMemoryPlanner() *InMemPlanner { + return &InMemPlanner{} +} + +func (i *InMemPlanner) SubmitPlan(plan *structs.Plan) (*structs.PlanResult, State, error) { + i.Plan = plan + + // Create a fully committed plan result. + result := &structs.PlanResult{ + NodeUpdate: plan.NodeUpdate, + NodeAllocation: plan.NodeAllocation, + } + + return result, nil, nil +} + +func (i *InMemPlanner) UpdateEval(eval *structs.Evaluation) error { + return nil +} + +func (i *InMemPlanner) CreateEval(eval *structs.Evaluation) error { + i.CreatedEvals = append(i.CreatedEvals, eval) + return nil +} diff --git a/scheduler/system_sched.go b/scheduler/system_sched.go index 2b803ba2c..6454c16c2 100644 --- a/scheduler/system_sched.go +++ b/scheduler/system_sched.go @@ -113,8 +113,9 @@ func (s *SystemScheduler) process() (bool, error) { return false, err } - // If the plan is a no-op, we can bail - if s.plan.IsNoOp() { + // If the plan is a no-op, we can bail. If AnnotatePlan is set submit the plan + // anyways to get the annotations. + if s.plan.IsNoOp() && !s.eval.AnnotatePlan { return true, nil } @@ -185,7 +186,15 @@ func (s *SystemScheduler) computeJobAllocs() error { } // Attempt to do the upgrades in place - diff.update = inplaceUpdate(s.ctx, s.eval, s.job, s.stack, diff.update) + destructiveUpdates := inplaceUpdate(s.ctx, s.eval, s.job, s.stack, diff.update) + inplaceUpdates := diff.update[len(destructiveUpdates):] + diff.update = destructiveUpdates + + if s.eval.AnnotatePlan { + s.plan.Annotations = &structs.PlanAnnotations{ + DesiredTGUpdates: desiredUpdates(diff, inplaceUpdates, destructiveUpdates), + } + } // Check if a rolling upgrade strategy is being used limit := len(diff.update) diff --git a/scheduler/system_sched_test.go b/scheduler/system_sched_test.go index cd5fa77a0..526b7cfee 100644 --- a/scheduler/system_sched_test.go +++ b/scheduler/system_sched_test.go @@ -1,6 +1,7 @@ package scheduler import ( + "reflect" "testing" "time" @@ -41,6 +42,11 @@ func TestSystemSched_JobRegister(t *testing.T) { } plan := h.Plans[0] + // Ensure the plan doesn't have annotations. + if plan.Annotations != nil { + t.Fatalf("expected no annotations") + } + // Ensure the plan allocated var planned []*structs.Allocation for _, allocList := range plan.NodeAllocation { @@ -67,6 +73,86 @@ func TestSystemSched_JobRegister(t *testing.T) { h.AssertEvalStatus(t, structs.EvalStatusComplete) } +func TestSystemSched_JobRegister_Annotate(t *testing.T) { + h := NewHarness(t) + + // Create some nodes + for i := 0; i < 10; i++ { + node := mock.Node() + noErr(t, h.State.UpsertNode(h.NextIndex(), node)) + } + + // Create a job + job := mock.SystemJob() + noErr(t, h.State.UpsertJob(h.NextIndex(), job)) + + // Create a mock evaluation to deregister the job + eval := &structs.Evaluation{ + ID: structs.GenerateUUID(), + Priority: job.Priority, + TriggeredBy: structs.EvalTriggerJobRegister, + JobID: job.ID, + AnnotatePlan: true, + } + + // Process the evaluation + err := h.Process(NewSystemScheduler, eval) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Ensure a single plan + if len(h.Plans) != 1 { + t.Fatalf("bad: %#v", h.Plans) + } + plan := h.Plans[0] + + // Ensure the plan allocated + var planned []*structs.Allocation + for _, allocList := range plan.NodeAllocation { + planned = append(planned, allocList...) + } + if len(planned) != 10 { + t.Fatalf("bad: %#v", plan) + } + + // Lookup the allocations by JobID + out, err := h.State.AllocsByJob(job.ID) + noErr(t, err) + + // Ensure all allocations placed + if len(out) != 10 { + t.Fatalf("bad: %#v", out) + } + + // Check the available nodes + if count, ok := out[0].Metrics.NodesAvailable["dc1"]; !ok || count != 10 { + t.Fatalf("bad: %#v", out[0].Metrics) + } + + h.AssertEvalStatus(t, structs.EvalStatusComplete) + + // Ensure the plan had annotations. + if plan.Annotations == nil { + t.Fatalf("expected annotations") + } + + desiredTGs := plan.Annotations.DesiredTGUpdates + if l := len(desiredTGs); l != 1 { + t.Fatalf("incorrect number of task groups; got %v; want %v", l, 1) + } + + desiredChanges, ok := desiredTGs["web"] + if !ok { + t.Fatalf("expected task group web to have desired changes") + } + + expected := &structs.DesiredUpdates{Place: 10} + if !reflect.DeepEqual(desiredChanges, expected) { + t.Fatalf("Unexpected desired updates; got %#v; want %#v", desiredChanges, expected) + } +} + func TestSystemSched_JobRegister_AddNode(t *testing.T) { h := NewHarness(t) diff --git a/scheduler/util.go b/scheduler/util.go index 794fca3ba..15c38c479 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -460,3 +460,79 @@ func taskGroupConstraints(tg *structs.TaskGroup) tgConstrainTuple { return c } + +// desiredUpdates takes the diffResult as well as the set of inplace and +// destructive updates and returns a map of task groups to their set of desired +// updates. +func desiredUpdates(diff *diffResult, inplaceUpdates, + destructiveUpdates []allocTuple) map[string]*structs.DesiredUpdates { + desiredTgs := make(map[string]*structs.DesiredUpdates) + + for _, tuple := range diff.place { + name := tuple.TaskGroup.Name + des, ok := desiredTgs[name] + if !ok { + des = &structs.DesiredUpdates{} + desiredTgs[name] = des + } + + des.Place++ + } + + for _, tuple := range diff.stop { + name := tuple.Alloc.TaskGroup + des, ok := desiredTgs[name] + if !ok { + des = &structs.DesiredUpdates{} + desiredTgs[name] = des + } + + des.Stop++ + } + + for _, tuple := range diff.ignore { + name := tuple.TaskGroup.Name + des, ok := desiredTgs[name] + if !ok { + des = &structs.DesiredUpdates{} + desiredTgs[name] = des + } + + des.Ignore++ + } + + for _, tuple := range diff.migrate { + name := tuple.TaskGroup.Name + des, ok := desiredTgs[name] + if !ok { + des = &structs.DesiredUpdates{} + desiredTgs[name] = des + } + + des.Migrate++ + } + + for _, tuple := range inplaceUpdates { + name := tuple.TaskGroup.Name + des, ok := desiredTgs[name] + if !ok { + des = &structs.DesiredUpdates{} + desiredTgs[name] = des + } + + des.InPlaceUpdate++ + } + + for _, tuple := range destructiveUpdates { + name := tuple.TaskGroup.Name + des, ok := desiredTgs[name] + if !ok { + des = &structs.DesiredUpdates{} + desiredTgs[name] = des + } + + des.DestructiveUpdate++ + } + + return desiredTgs +} diff --git a/scheduler/util_test.go b/scheduler/util_test.go index c4fa2fa30..4d63ff195 100644 --- a/scheduler/util_test.go +++ b/scheduler/util_test.go @@ -761,3 +761,60 @@ func TestProgressMade(t *testing.T) { t.Fatal("bad") } } + +func TestDesiredUpdates(t *testing.T) { + tg1 := &structs.TaskGroup{Name: "foo"} + tg2 := &structs.TaskGroup{Name: "bar"} + + place := []allocTuple{ + allocTuple{TaskGroup: tg1}, + allocTuple{TaskGroup: tg1}, + allocTuple{TaskGroup: tg1}, + allocTuple{TaskGroup: tg2}, + } + stop := []allocTuple{ + allocTuple{TaskGroup: tg2}, + allocTuple{TaskGroup: tg2}, + } + ignore := []allocTuple{ + allocTuple{TaskGroup: tg1}, + } + migrate := []allocTuple{ + allocTuple{TaskGroup: tg2}, + } + inplace := []allocTuple{ + allocTuple{TaskGroup: tg1}, + allocTuple{TaskGroup: tg1}, + } + destructive := []allocTuple{ + allocTuple{TaskGroup: tg1}, + allocTuple{TaskGroup: tg2}, + allocTuple{TaskGroup: tg2}, + } + diff := &diffResult{ + place: place, + stop: stop, + ignore: ignore, + migrate: migrate, + } + + expected := map[string]*structs.DesiredUpdates{ + "foo": { + Place: 3, + Ignore: 1, + InPlaceUpdate: 2, + DestructiveUpdate: 1, + }, + "bar": { + Place: 1, + Stop: 2, + Migrate: 1, + DestructiveUpdate: 2, + }, + } + + desired := desiredUpdates(diff, inplace, destructive) + if !reflect.DeepEqual(desired, expected) { + t.Fatalf("desiredUpdates() returned %#v; want %#v", desired, expected) + } +} From 2f74bd9a74679166a8853e0703eb7f0e57fbfee7 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 11 May 2016 11:27:00 -0700 Subject: [PATCH 08/17] remove old diff --- nomad/structs/diff/structs.go | 25 - nomad/structs/job_diff.go | 1148 ----------------------------- nomad/structs/job_diff_test.go | 673 ----------------- nomad/structs/job_diff_visitor.go | 52 -- 4 files changed, 1898 deletions(-) delete mode 100644 nomad/structs/diff/structs.go delete mode 100644 nomad/structs/job_diff.go delete mode 100644 nomad/structs/job_diff_test.go delete mode 100644 nomad/structs/job_diff_visitor.go diff --git a/nomad/structs/diff/structs.go b/nomad/structs/diff/structs.go deleted file mode 100644 index e92465dfb..000000000 --- a/nomad/structs/diff/structs.go +++ /dev/null @@ -1,25 +0,0 @@ -package diff - -import "github.com/hashicorp/nomad/nomad/structs" - -// JobPlanResponse is used to respond to a job plan request. Must be in this -// package to avoid a cyclic dependency. -type JobPlanResponse struct { - // Plan holds the decisions the scheduler made. - Plan *structs.Plan - - // The Cas value can be used when running `nomad run` to ensure that the Job - // wasn’t modified since the last plan. If the job is being created, the - // value is zero. - Cas uint64 - - // CreatedEvals is the set of evaluations created by the scheduler. The - // reasons for this can be rolling-updates or blocked evals. - CreatedEvals []*structs.Evaluation - - // Diff contains the diff of the job and annotations on whether the change - // causes an in-place update or create/destroy - Diff *JobDiff - - structs.QueryMeta -} diff --git a/nomad/structs/job_diff.go b/nomad/structs/job_diff.go deleted file mode 100644 index a6bf98bba..000000000 --- a/nomad/structs/job_diff.go +++ /dev/null @@ -1,1148 +0,0 @@ -package structs - -import ( - "fmt" - "reflect" - - "github.com/hashicorp/nomad/helper/flatmap" - "github.com/mitchellh/hashstructure" -) - -// The below are the set of primitive fields that can be diff'd automatically -// using FieldDiff. -var ( - jobPrimitiveFields = []string{ - "Region", - "ID", - "ParentID", - "Name", - "Type", - "Priority", - "AllAtOnce", - } - - constraintFields = []string{ - "LTarget", - "RTarget", - "Operand", - } - - updateStrategyFields = []string{ - "Stagger", - "MaxParallel", - } - - periodicConfigFields = []string{ - "Enabled", - "Spec", - "SpecType", - "ProhibitOverlap", - } - - taskGroupPrimitiveFields = []string{ - "Name", - "Count", - } - - restartPolicyFields = []string{ - "Attempts", - "Interval", - "Delay", - "Mode", - } - - taskPrimitiveFields = []string{ - "Name", - "Driver", - "User", - "KillTimeout", - } - - logConfigFields = []string{ - "MaxFiles", - "MaxFileSizeMB", - } - - servicePrimitiveFields = []string{ - "Name", - "PortLabel", - } - - serviceCheckPrimitiveFields = []string{ - "Name", - "Type", - "Command", - "Path", - "Protocol", - "Interval", - "Timeout", - } - - taskArtifactPrimitiveFields = []string{ - "GetterSource", - "RelativeDest", - } - - resourcesPrimitiveFields = []string{ - "CPU", - "MemoryMB", - "DiskMB", - "IOPS", - } - - networkResourcePrimitiveFields = []string{ - "Device", - "CIDR", - "IP", - "MBits", - } - - portFields = []string{ - "Label", - "Value", - } -) - -// DiffType is the high-level type of the diff. -type DiffType string - -const ( - DiffTypeNone DiffType = "Equal" - DiffTypeAdded = "Added" - DiffTypeDeleted = "Deleted" - DiffTypeEdited = "Edited" -) - -// DiffEntry contains information about a diff. -type DiffEntry struct { - Type DiffType - Annotations []string -} - -// SetDiffType sets the diff type. The inputs must be a pointer. -func (d *DiffEntry) SetDiffType(old, new interface{}) { - if reflect.DeepEqual(old, new) { - d.Type = DiffTypeNone - return - } - - oldV := reflect.ValueOf(old) - newV := reflect.ValueOf(new) - - if oldV.IsNil() { - d.Type = DiffTypeAdded - } else if newV.IsNil() { - d.Type = DiffTypeDeleted - } else { - d.Type = DiffTypeEdited - } -} - -// JobDiff contains the set of changes betwen two Jobs. -type JobDiff struct { - PrimitiveStructDiff - Constraints []*PrimitiveStructDiff - Datacenters *StringSetDiff - Update *PrimitiveStructDiff - Periodic *PrimitiveStructDiff - Meta *StringMapDiff - TaskGroups *TaskGroupsDiff -} - -// TaskGroupsDiff contains the set of Task Groups that were changed. -type TaskGroupsDiff struct { - DiffEntry - Added, Deleted []*TaskGroup - Edited []*TaskGroupDiff -} - -// TaskGroupsDiff contains the set of changes between two Task Groups. -type TaskGroupDiff struct { - PrimitiveStructDiff - Constraints []*PrimitiveStructDiff - RestartPolicy *PrimitiveStructDiff - Meta *StringMapDiff - Tasks *TasksDiff -} - -// TasksDiff contains the set of Tasks that were changed. -type TasksDiff struct { - DiffEntry - Added, Deleted []*Task - Edited []*TaskDiff -} - -// TaskDiff contains the changes between two Tasks. -type TaskDiff struct { - PrimitiveStructDiff - Constraints []*PrimitiveStructDiff - LogConfig *PrimitiveStructDiff - Env *StringMapDiff - Meta *StringMapDiff - Services *ServicesDiff - Artifacts *TaskArtifactsDiff - Resources *ResourcesDiff - Config *StringMapDiff -} - -// ServicesDiff contains the set of Services that were changed. -type ServicesDiff struct { - DiffEntry - Added, Deleted []*Service - Edited []*ServiceDiff -} - -// ServiceDiff contains the changes between two Services. -type ServiceDiff struct { - PrimitiveStructDiff - Tags *StringSetDiff - Checks *ServiceChecksDiff -} - -// ServiceChecksDiff contains the set of Service Checks that were changed. -type ServiceChecksDiff struct { - DiffEntry - Added, Deleted []*ServiceCheck - Edited []*ServiceCheckDiff -} - -// ServiceCheckDiff contains the changes between two Service Checks. -type ServiceCheckDiff struct { - PrimitiveStructDiff - Args *StringSetDiff -} - -// TaskArtifactsDiff contains the set of Task Artifacts that were changed. -type TaskArtifactsDiff struct { - DiffEntry - Added, Deleted []*TaskArtifact - Edited []*TaskArtifactDiff -} - -// TaskArtifactDiff contains the diff between two Task Artifacts. -type TaskArtifactDiff struct { - PrimitiveStructDiff - GetterOptions *StringMapDiff -} - -// ResourcesDiff contains the diff between two Resources. -type ResourcesDiff struct { - PrimitiveStructDiff - Networks *NetworkResourcesDiff -} - -// NetworkResourcesDiff contains the set of Network Resources that were changed. -type NetworkResourcesDiff struct { - DiffEntry - Added, Deleted []*NetworkResourceDiff -} - -// NetworkResourceDiff contains the diff between two Network Resources. -type NetworkResourceDiff struct { - PrimitiveStructDiff - ReservedPorts *PortsDiff - DynamicPorts *PortsDiff -} - -// PortsDiff contains the difference between two sets of Ports. -type PortsDiff struct { - DiffEntry - Added, Deleted []Port - Edited []*PrimitiveStructDiff -} - -// PrimitiveStructDiff contains the diff of two structs that only contain -// primitive fields. -type PrimitiveStructDiff struct { - DiffEntry - PrimitiveFields map[string]*FieldDiff -} - -// DiffFields performs the diff of the passed fields against the old and new -// object. -func (p *PrimitiveStructDiff) DiffFields(old, new interface{}, fields []string) { - for _, field := range fields { - oldV := getField(old, field) - newV := getField(new, field) - pDiff := NewFieldDiff(field, oldV, newV) - if pDiff != nil { - if p.PrimitiveFields == nil { - p.PrimitiveFields = make(map[string]*FieldDiff) - } - - p.PrimitiveFields[field] = pDiff - } - } -} - -// FieldDiff contains the diff between an old and new version of a field. -type FieldDiff struct { - DiffEntry - Name string - OldValue interface{} - NewValue interface{} -} - -// StringSetDiff captures the changes that occured between two sets of strings -type StringSetDiff struct { - DiffEntry - Added, Deleted []string -} - -// StringMapDiff captures the changes that occured between two string maps -type StringMapDiff struct { - DiffEntry - Added, Deleted map[string]string - Edited map[string]StringValueDelta -} - -// StringValueDelta holds the old and new value of a string. -type StringValueDelta struct { - DiffEntry - Old, New string -} - -// NewJobDiff returns the diff between two jobs. If there is no difference, nil -// is returned. -func NewJobDiff(old, new *Job) *JobDiff { - diff := &JobDiff{} - diff.SetDiffType(old, new) - if diff.Type == DiffTypeNone { - return nil - } - - // Get the diffs of the primitive fields - diff.DiffFields(old, new, jobPrimitiveFields) - - // Protect accessing nil fields, this occurs after diffing the primitives so - // that we can properly detect Added/Deleted fields. - if old == nil { - old = &Job{} - } - if new == nil { - new = &Job{} - } - - // Get the diff of the datacenters - diff.Datacenters = NewStringSetDiff(old.Datacenters, new.Datacenters) - - // Get the diff of the constraints. - diff.Constraints = setDiffPrimitiveStructs( - interfaceSlice(old.Constraints), - interfaceSlice(new.Constraints), - constraintFields) - - // Get the update strategy diff - diff.Update = NewPrimitiveStructDiff(old.Update, new.Update, updateStrategyFields) - - // Get the update strategy diff - diff.Periodic = NewPrimitiveStructDiff(old.Periodic, new.Periodic, periodicConfigFields) - - // Get the meta diff - diff.Meta = NewStringMapDiff(old.Meta, new.Meta) - - // Get the task group diff - diff.TaskGroups = setDiffTaskGroups(old.TaskGroups, new.TaskGroups) - - // If there are no changes return nil - if len(diff.PrimitiveFields)+len(diff.Constraints) == 0 && - diff.Datacenters == nil && - diff.Update == nil && - diff.Periodic == nil && - diff.Meta == nil && - diff.TaskGroups == nil { - return nil - } - - return diff -} - -// NewTaskGroupDiff returns the diff between two task groups. If there is no -// difference, nil is returned. -func NewTaskGroupDiff(old, new *TaskGroup) *TaskGroupDiff { - diff := &TaskGroupDiff{} - diff.SetDiffType(old, new) - if diff.Type == DiffTypeNone { - return nil - } - - // Get the diffs of the primitive fields - diff.DiffFields(old, new, taskGroupPrimitiveFields) - - // Protect accessing nil fields, this occurs after diffing the primitives so - // that we can properly detect Added/Deleted fields. - if old == nil { - old = &TaskGroup{} - } - if new == nil { - new = &TaskGroup{} - } - - // Get the diff of the constraints. - diff.Constraints = setDiffPrimitiveStructs( - interfaceSlice(old.Constraints), - interfaceSlice(new.Constraints), - constraintFields) - - // Get the restart policy diff - diff.RestartPolicy = NewPrimitiveStructDiff(old.RestartPolicy, new.RestartPolicy, restartPolicyFields) - - // Get the meta diff - diff.Meta = NewStringMapDiff(old.Meta, new.Meta) - - // Get the task diff - diff.Tasks = setDiffTasks(old.Tasks, new.Tasks) - - // If there are no changes return nil - if len(diff.PrimitiveFields)+len(diff.Constraints) == 0 && - diff.Tasks == nil && - diff.RestartPolicy == nil && - diff.Meta == nil { - return nil - } - - return diff -} - -// NewTaskDiff returns the diff between two tasks. If there is no difference, -// nil is returned. -func NewTaskDiff(old, new *Task) *TaskDiff { - diff := &TaskDiff{} - diff.SetDiffType(old, new) - if diff.Type == DiffTypeNone { - return nil - } - - // Get the diffs of the primitive fields - diff.DiffFields(old, new, taskPrimitiveFields) - - // Protect accessing nil fields, this occurs after diffing the primitives so - // that we can properly detect Added/Deleted fields. - if old == nil { - old = &Task{} - } - if new == nil { - new = &Task{} - } - - // Get the diff of the constraints. - diff.Constraints = setDiffPrimitiveStructs( - interfaceSlice(old.Constraints), - interfaceSlice(new.Constraints), - constraintFields) - - // Get the meta and env diff - diff.Meta = NewStringMapDiff(old.Meta, new.Meta) - diff.Env = NewStringMapDiff(old.Env, new.Env) - - // Get the log config diff - diff.LogConfig = NewPrimitiveStructDiff(old.LogConfig, new.LogConfig, logConfigFields) - - // Get the services diff - diff.Services = setDiffServices(old.Services, new.Services) - - // Get the artifacts diff - diff.Artifacts = setDiffTaskArtifacts(old.Artifacts, new.Artifacts) - - // Get the resource diff - diff.Resources = NewResourcesDiff(old.Resources, new.Resources) - - // Get the task config diff - diff.Config = NewStringMapDiff(flatmap.Flatten(old.Config), flatmap.Flatten(new.Config)) - - // If there are no changes return nil - if len(diff.PrimitiveFields)+len(diff.Constraints) == 0 && - diff.Config == nil && - diff.Artifacts == nil && - diff.LogConfig == nil && - diff.Services == nil && - diff.Env == nil && - diff.Meta == nil { - return nil - } - - return diff -} - -// NewServiceDiff returns the diff between two services. If there is no -// difference, nil is returned. -func NewServiceDiff(old, new *Service) *ServiceDiff { - diff := &ServiceDiff{} - diff.SetDiffType(old, new) - if diff.Type == DiffTypeNone { - return nil - } - - // Get the diffs of the primitive fields - diff.DiffFields(old, new, servicePrimitiveFields) - - // Protect accessing nil fields, this occurs after diffing the primitives so - // that we can properly detect Added/Deleted fields. - if old == nil { - old = &Service{} - } - if new == nil { - new = &Service{} - } - - // Get the tags diff - diff.Tags = NewStringSetDiff(old.Tags, new.Tags) - - // Get the checks diff - diff.Checks = setDiffServiceChecks(old.Checks, new.Checks) - - // If there are no changes return nil - if len(diff.PrimitiveFields) == 0 && - diff.Checks == nil && - diff.Tags == nil { - return nil - } - - return diff -} - -// NewServiceCheckDiff returns the diff between two service checks. If there is -// no difference, nil is returned. -func NewServiceCheckDiff(old, new *ServiceCheck) *ServiceCheckDiff { - diff := &ServiceCheckDiff{} - diff.SetDiffType(old, new) - if diff.Type == DiffTypeNone { - return nil - } - - // Get the diffs of the primitive fields - diff.DiffFields(old, new, serviceCheckPrimitiveFields) - - // Protect accessing nil fields, this occurs after diffing the primitives so - // that we can properly detect Added/Deleted fields. - if old == nil { - old = &ServiceCheck{} - } - if new == nil { - new = &ServiceCheck{} - } - - // Get the args diff - diff.Args = NewStringSetDiff(old.Args, new.Args) - - // If there are no changes return nil - if len(diff.PrimitiveFields) == 0 && - diff.Args == nil { - return nil - } - - return diff -} - -// NewTaskArtifactDiff returns the diff between two task artifacts. If there is -// no difference, nil is returned. -func NewTaskArtifactDiff(old, new *TaskArtifact) *TaskArtifactDiff { - diff := &TaskArtifactDiff{} - diff.SetDiffType(old, new) - if diff.Type == DiffTypeNone { - return nil - } - - // Get the diffs of the primitive fields - diff.DiffFields(old, new, taskArtifactPrimitiveFields) - - // Protect accessing nil fields, this occurs after diffing the primitives so - // that we can properly detect Added/Deleted fields. - if old == nil { - old = &TaskArtifact{} - } - if new == nil { - new = &TaskArtifact{} - } - - // Get the args diff - diff.GetterOptions = NewStringMapDiff(old.GetterOptions, new.GetterOptions) - - // If there are no changes return nil - if len(diff.PrimitiveFields) == 0 && - diff.GetterOptions == nil { - return nil - } - - return diff -} - -// NewResourcesDiff returns the diff between two resources. If there is no -// difference, nil is returned. -func NewResourcesDiff(old, new *Resources) *ResourcesDiff { - diff := &ResourcesDiff{} - diff.SetDiffType(old, new) - if diff.Type == DiffTypeNone { - return nil - } - - // Get the diffs of the primitive fields - diff.DiffFields(old, new, resourcesPrimitiveFields) - - // Protect accessing nil fields, this occurs after diffing the primitives so - // that we can properly detect Added/Deleted fields. - if old == nil { - old = &Resources{} - } - if new == nil { - new = &Resources{} - } - - // Get the network resource diff - diff.Networks = setDiffNetworkResources(old.Networks, new.Networks) - - // If there are no changes return nil - if len(diff.PrimitiveFields) == 0 && - diff.Networks == nil { - return nil - } - - return diff -} - -// NewNetworkResourceDiff returns the diff between two network resources. If -// there is no difference, nil is returned. -func NewNetworkResourceDiff(old, new *NetworkResource) *NetworkResourceDiff { - diff := &NetworkResourceDiff{} - diff.SetDiffType(old, new) - if diff.Type == DiffTypeNone { - return nil - } - - // Get the diffs of the primitive fields - diff.DiffFields(old, new, networkResourcePrimitiveFields) - - // Protect accessing nil fields, this occurs after diffing the primitives so - // that we can properly detect Added/Deleted fields. - if old == nil { - old = &NetworkResource{} - } - if new == nil { - new = &NetworkResource{} - } - - // Get the port diffs - diff.ReservedPorts = setDiffPorts(old.ReservedPorts, new.ReservedPorts) - diff.DynamicPorts = setDiffPorts(old.DynamicPorts, new.DynamicPorts) - - // If there are no changes return nil - if len(diff.PrimitiveFields) == 0 && - diff.DynamicPorts == nil && - diff.ReservedPorts == nil { - return nil - } - - return diff -} - -// NewPrimitiveStructDiff returns the diff between two structs containing only -// primitive fields. The list of fields to be diffed is passed via the fields -// parameter. If there is no difference, nil is returned. -func NewPrimitiveStructDiff(old, new interface{}, fields []string) *PrimitiveStructDiff { - if reflect.DeepEqual(old, new) { - return nil - } - - // Diff the individual fields - diff := &PrimitiveStructDiff{} - diff.DiffFields(old, new, fields) - if len(diff.PrimitiveFields) == 0 { - return nil - } - - var added, deleted bool - for _, f := range diff.PrimitiveFields { - switch f.Type { - case DiffTypeEdited: - diff.Type = DiffTypeEdited - return diff - case DiffTypeAdded: - added = true - case DiffTypeDeleted: - deleted = true - } - } - - if added && deleted { - diff.Type = DiffTypeEdited - } else if added { - diff.Type = DiffTypeAdded - } else { - diff.Type = DiffTypeDeleted - } - - return diff -} - -// NewFieldDiff returns the diff between two fields. If there is no difference, -// nil is returned. -func NewFieldDiff(name string, old, new interface{}) *FieldDiff { - diff := &FieldDiff{Name: name} - if reflect.DeepEqual(old, new) { - return nil - } else if old == nil { - diff.Type = DiffTypeAdded - diff.NewValue = new - } else if new == nil { - diff.Type = DiffTypeDeleted - diff.OldValue = old - } else { - diff.Type = DiffTypeEdited - diff.OldValue = old - diff.NewValue = new - } - - return diff -} - -// NewStringSetDiff returns the diff between two sets of strings. If there is no -// difference, nil is returned. -func NewStringSetDiff(old, new []string) *StringSetDiff { - if reflect.DeepEqual(old, new) { - return nil - } - - diff := &StringSetDiff{} - makeMap := func(inputs []string) map[string]interface{} { - m := make(map[string]interface{}) - for _, in := range inputs { - m[in] = struct{}{} - } - return m - } - - added, deleted, _, _ := keyedSetDifference(makeMap(old), makeMap(new)) - for k := range added { - diff.Added = append(diff.Added, k) - } - for k := range deleted { - diff.Deleted = append(diff.Deleted, k) - } - - la, ld := len(added), len(deleted) - if la+ld == 0 { - return nil - } else if ld == 0 { - diff.Type = DiffTypeAdded - } else if la == 0 { - diff.Type = DiffTypeDeleted - } else { - diff.Type = DiffTypeEdited - } - - return diff -} - -// NewStringMapDiff returns the diff between two maps of strings. If there is no -// difference, nil is returned. -func NewStringMapDiff(old, new map[string]string) *StringMapDiff { - if reflect.DeepEqual(old, new) { - return nil - } - - diff := &StringMapDiff{} - diff.Added = make(map[string]string) - diff.Deleted = make(map[string]string) - diff.Edited = make(map[string]StringValueDelta) - - for k, v := range old { - if _, ok := new[k]; !ok { - diff.Deleted[k] = v - } - } - for k, newV := range new { - oldV, ok := old[k] - if !ok { - diff.Added[k] = newV - continue - } - - // Key is in both, check if they have been edited. - if newV != oldV { - d := StringValueDelta{Old: oldV, New: newV} - d.Type = DiffTypeEdited - diff.Edited[k] = d - } - } - - la, ld, le := len(diff.Added), len(diff.Deleted), len(diff.Edited) - if la+ld+le == 0 { - return nil - } - - if le != 0 || la > 0 && ld > 0 { - diff.Type = DiffTypeEdited - } else if ld == 0 { - diff.Type = DiffTypeAdded - } else if la == 0 { - diff.Type = DiffTypeDeleted - } - return diff -} - -// Set helpers - -// setDiffTaskGroups does a set difference of task groups using the task group -// name as a key. -func setDiffTaskGroups(old, new []*TaskGroup) *TaskGroupsDiff { - diff := &TaskGroupsDiff{} - - oldMap := make(map[string]*TaskGroup) - newMap := make(map[string]*TaskGroup) - for _, tg := range old { - oldMap[tg.Name] = tg - } - for _, tg := range new { - newMap[tg.Name] = tg - } - - for k, v := range oldMap { - if _, ok := newMap[k]; !ok { - diff.Deleted = append(diff.Deleted, v) - } - } - for k, newV := range newMap { - oldV, ok := oldMap[k] - if !ok { - diff.Added = append(diff.Added, newV) - continue - } - - // Key is in both, check if they have been edited. - if !reflect.DeepEqual(oldV, newV) { - tgdiff := NewTaskGroupDiff(oldV, newV) - diff.Edited = append(diff.Edited, tgdiff) - } - } - - if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { - return nil - } - return diff -} - -// setDiffTasks does a set difference of tasks using the task name as a key. -func setDiffTasks(old, new []*Task) *TasksDiff { - diff := &TasksDiff{} - - oldMap := make(map[string]*Task) - newMap := make(map[string]*Task) - for _, task := range old { - oldMap[task.Name] = task - } - for _, task := range new { - newMap[task.Name] = task - } - - for k, v := range oldMap { - if _, ok := newMap[k]; !ok { - diff.Deleted = append(diff.Deleted, v) - } - } - for k, newV := range newMap { - oldV, ok := oldMap[k] - if !ok { - diff.Added = append(diff.Added, newV) - continue - } - - // Key is in both, check if they have been edited. - if !reflect.DeepEqual(oldV, newV) { - tdiff := NewTaskDiff(oldV, newV) - diff.Edited = append(diff.Edited, tdiff) - } - } - - if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { - return nil - } - return diff -} - -// setDiffServices does a set difference of Services using the service name as a -// key. -func setDiffServices(old, new []*Service) *ServicesDiff { - diff := &ServicesDiff{} - - oldMap := make(map[string]*Service) - newMap := make(map[string]*Service) - for _, s := range old { - oldMap[s.Name] = s - } - for _, s := range new { - newMap[s.Name] = s - } - - for k, v := range oldMap { - if _, ok := newMap[k]; !ok { - diff.Deleted = append(diff.Deleted, v) - } - } - for k, newV := range newMap { - oldV, ok := oldMap[k] - if !ok { - diff.Added = append(diff.Added, newV) - continue - } - - // Key is in both, check if they have been edited. - if !reflect.DeepEqual(oldV, newV) { - sdiff := NewServiceDiff(oldV, newV) - diff.Edited = append(diff.Edited, sdiff) - } - } - - if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { - return nil - } - return diff -} - -// setDiffServiceChecks does a set difference of service checks using the check -// name as a key. -func setDiffServiceChecks(old, new []*ServiceCheck) *ServiceChecksDiff { - diff := &ServiceChecksDiff{} - - oldMap := make(map[string]*ServiceCheck) - newMap := make(map[string]*ServiceCheck) - for _, s := range old { - oldMap[s.Name] = s - } - for _, s := range new { - newMap[s.Name] = s - } - - for k, v := range oldMap { - if _, ok := newMap[k]; !ok { - diff.Deleted = append(diff.Deleted, v) - } - } - for k, newV := range newMap { - oldV, ok := oldMap[k] - if !ok { - diff.Added = append(diff.Added, newV) - continue - } - - // Key is in both, check if they have been edited. - if !reflect.DeepEqual(oldV, newV) { - sdiff := NewServiceCheckDiff(oldV, newV) - diff.Edited = append(diff.Edited, sdiff) - } - } - - if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { - return nil - } - return diff -} - -// setDiffTaskArtifacts does a set difference of task artifacts using the geter -// source as a key. -func setDiffTaskArtifacts(old, new []*TaskArtifact) *TaskArtifactsDiff { - diff := &TaskArtifactsDiff{} - - oldMap := make(map[string]*TaskArtifact) - newMap := make(map[string]*TaskArtifact) - for _, ta := range old { - oldMap[ta.GetterSource] = ta - } - for _, ta := range new { - newMap[ta.GetterSource] = ta - } - - for k, v := range oldMap { - if _, ok := newMap[k]; !ok { - diff.Deleted = append(diff.Deleted, v) - } - } - for k, newV := range newMap { - oldV, ok := oldMap[k] - if !ok { - diff.Added = append(diff.Added, newV) - continue - } - - // Key is in both, check if they have been edited. - if !reflect.DeepEqual(oldV, newV) { - tdiff := NewTaskArtifactDiff(oldV, newV) - diff.Edited = append(diff.Edited, tdiff) - } - } - - if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { - return nil - } - return diff -} - -// setDiffNetworkResources does a set difference of network resources. -func setDiffNetworkResources(old, new []*NetworkResource) *NetworkResourcesDiff { - diff := &NetworkResourcesDiff{} - - added, del := setDifference(interfaceSlice(old), interfaceSlice(new)) - for _, a := range added { - nDiff := NewNetworkResourceDiff(nil, a.(*NetworkResource)) - diff.Added = append(diff.Added, nDiff) - } - for _, d := range del { - nDiff := NewNetworkResourceDiff(d.(*NetworkResource), nil) - diff.Added = append(diff.Deleted, nDiff) - } - - return diff -} - -// setDiffPorts does a set difference of ports using the label as a key. -func setDiffPorts(old, new []Port) *PortsDiff { - diff := &PortsDiff{} - - oldMap := make(map[string]Port) - newMap := make(map[string]Port) - for _, p := range old { - oldMap[p.Label] = p - } - for _, p := range new { - newMap[p.Label] = p - } - - for k, v := range oldMap { - if _, ok := newMap[k]; !ok { - diff.Deleted = append(diff.Deleted, v) - } - } - for k, newV := range newMap { - oldV, ok := oldMap[k] - if !ok { - diff.Added = append(diff.Added, newV) - continue - } - - // Key is in both, check if they have been edited. - if !reflect.DeepEqual(oldV, newV) { - pdiff := NewPrimitiveStructDiff(oldV, newV, portFields) - diff.Edited = append(diff.Edited, pdiff) - } - } - - if len(diff.Added)+len(diff.Deleted)+len(diff.Edited) == 0 { - return nil - } - return diff -} - -// setDiffPrimitiveStructs does a set difference on primitive structs. The -// caller must pass the primitive structs fields. -func setDiffPrimitiveStructs(old, new []interface{}, fields []string) []*PrimitiveStructDiff { - var diffs []*PrimitiveStructDiff - - added, del := setDifference(old, new) - for _, a := range added { - pDiff := NewPrimitiveStructDiff(nil, a, fields) - diffs = append(diffs, pDiff) - } - for _, d := range del { - pDiff := NewPrimitiveStructDiff(d, nil, fields) - diffs = append(diffs, pDiff) - } - - return diffs -} - -// Reflective helpers. - -// setDifference does a set difference on two sets of interfaces and returns the -// values that were added or deleted when comparing the new to old. -func setDifference(old, new []interface{}) (added, deleted []interface{}) { - makeSet := func(objects []interface{}) map[string]interface{} { - objMap := make(map[string]interface{}, len(objects)) - for _, obj := range objects { - hash, err := hashstructure.Hash(obj, nil) - if err != nil { - panic(err) - } - objMap[fmt.Sprintf("%d", hash)] = obj - } - - return objMap - } - - addedMap, deletedMap, _, _ := keyedSetDifference(makeSet(old), makeSet(new)) - flatten := func(in map[string]interface{}) []interface{} { - out := make([]interface{}, 0, len(in)) - for _, v := range in { - out = append(out, v) - } - return out - } - - return flatten(addedMap), flatten(deletedMap) -} - -// keyedSetDifference does a set difference on keyed object and returns the -// objects that have been added, deleted, edited and unmodified when comparing -// the new to old set. -func keyedSetDifference(old, new map[string]interface{}) ( - added, deleted map[string]interface{}, edited, unmodified []string) { - - added = make(map[string]interface{}) - deleted = make(map[string]interface{}) - - for k, v := range old { - if _, ok := new[k]; !ok { - deleted[k] = v - } - } - for k, newV := range new { - oldV, ok := old[k] - if !ok { - added[k] = newV - continue - } - - // Key is in both, check if they have been edited. - if reflect.DeepEqual(oldV, newV) { - unmodified = append(unmodified, k) - } else { - edited = append(edited, k) - } - } - - return added, deleted, edited, unmodified -} - -// interfaceSlice is a helper method that takes a slice of typed elements and -// returns a slice of interface. This method will panic if given a non-slice -// input. -func interfaceSlice(slice interface{}) []interface{} { - s := reflect.ValueOf(slice) - if s.Kind() != reflect.Slice { - panic("InterfaceSlice() given a non-slice type") - } - - ret := make([]interface{}, s.Len()) - - for i := 0; i < s.Len(); i++ { - ret[i] = s.Index(i).Interface() - } - - return ret -} - -// getField is a helper that returns the passed fields value of the given -// object. This method will panic if the field does not exist in the passed -// object. -func getField(obj interface{}, field string) interface{} { - if obj == nil { - return nil - } - - r := reflect.ValueOf(obj) - r = reflect.Indirect(r) - if !r.IsValid() { - return nil - } - - f := r.FieldByName(field) - return f.Interface() -} diff --git a/nomad/structs/job_diff_test.go b/nomad/structs/job_diff_test.go deleted file mode 100644 index 1db2ebb8e..000000000 --- a/nomad/structs/job_diff_test.go +++ /dev/null @@ -1,673 +0,0 @@ -package structs - -import ( - "reflect" - "sort" - "testing" -) - -func TestNewJobDiff_Same(t *testing.T) { - job1 := TestJob() - job2 := TestJob() - job2.ID = job1.ID - - diff := NewJobDiff(job1, job2) - if diff != nil { - t.Fatalf("expected nil job diff; got %#v", diff) - } -} - -func TestNewJobDiff_NilCases(t *testing.T) { - j := TestJob() - - // Old job nil - diff := NewJobDiff(nil, j) - if diff == nil { - t.Fatalf("expected non-nil job diff") - } - if diff.Type != DiffTypeAdded { - t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeAdded) - } - - // New job nil - diff = NewJobDiff(j, nil) - if diff == nil { - t.Fatalf("expected non-nil job diff") - } - if diff.Type != DiffTypeDeleted { - t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeDeleted) - } -} - -func TestNewJobDiff_Constraints(t *testing.T) { - c1 := &Constraint{LTarget: "foo"} - c2 := &Constraint{LTarget: "bar"} - c3 := &Constraint{LTarget: "baz"} - - // Test the added case. - j1 := &Job{Constraints: []*Constraint{c1, c2}} - j2 := &Job{Constraints: []*Constraint{c1, c2, c3}} - - diff := NewJobDiff(j1, j2) - if diff == nil { - t.Fatalf("expected non-nil job diff") - } - - if diff.Type != DiffTypeEdited { - t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeEdited) - } - - if len(diff.Constraints) != 1 { - t.Fatalf("expected one constraint diff; got %v", diff.Constraints) - } - - cdiff := diff.Constraints[0] - if cdiff.Type != DiffTypeAdded { - t.Fatalf("expected constraint to be added: %#v", cdiff) - } - if len(cdiff.PrimitiveFields) != 3 { - t.Fatalf("bad: %#v", cdiff) - } - - // Test the deleted case. - j1 = &Job{Constraints: []*Constraint{c1, c2}} - j2 = &Job{Constraints: []*Constraint{c1}} - - diff = NewJobDiff(j1, j2) - if diff == nil { - t.Fatalf("expected non-nil job diff") - } - - if diff.Type != DiffTypeEdited { - t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeEdited) - } - - if len(diff.Constraints) != 1 { - t.Fatalf("expected one constraint diff; got %v", diff.Constraints) - } - - cdiff = diff.Constraints[0] - if cdiff.Type != DiffTypeDeleted { - t.Fatalf("expected constraint to be deleted: %#v", cdiff) - } - if len(cdiff.PrimitiveFields) != 3 { - t.Fatalf("bad: %#v", cdiff) - } -} - -func TestNewJobDiff_Datacenters(t *testing.T) { - j1 := &Job{Datacenters: []string{"a", "b"}} - j2 := &Job{Datacenters: []string{"b", "c"}} - - diff := NewJobDiff(j1, j2) - if diff == nil { - t.Fatalf("expected non-nil job diff") - } - - if diff.Type != DiffTypeEdited { - t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeEdited) - } - - dd := diff.Datacenters - if dd == nil { - t.Fatalf("expected datacenter diff") - } - - if !reflect.DeepEqual(dd.Added, []string{"c"}) || - !reflect.DeepEqual(dd.Deleted, []string{"a"}) { - t.Fatalf("bad: %#v", dd) - } - -} - -func TestNewJobDiff_TaskGroups(t *testing.T) { - tg1 := &TaskGroup{Name: "foo"} - tg2 := &TaskGroup{Name: "bar"} - tg2_2 := &TaskGroup{Name: "bar", Count: 2} - tg3 := &TaskGroup{Name: "baz"} - - j1 := &Job{TaskGroups: []*TaskGroup{tg1, tg2}} - j2 := &Job{TaskGroups: []*TaskGroup{tg2_2, tg3}} - - diff := NewJobDiff(j1, j2) - if diff == nil { - t.Fatalf("expected non-nil job diff") - } - - if diff.Type != DiffTypeEdited { - t.Fatalf("got diff type %v; want %v", diff.Type, DiffTypeEdited) - } - - tgd := diff.TaskGroups - if tgd == nil { - t.Fatalf("expected task group diff") - } - - if !reflect.DeepEqual(tgd.Added, []*TaskGroup{tg3}) || - !reflect.DeepEqual(tgd.Deleted, []*TaskGroup{tg1}) { - t.Fatalf("bad: %#v", tgd) - } - - if len(tgd.Edited) != 1 { - t.Fatalf("expect one edited task group: %#v", tgd) - } - if e := tgd.Edited[0]; tgd.Type != DiffTypeEdited && len(e.PrimitiveFields) != 1 { - t.Fatalf("bad: %#v", e) - } -} - -func TestNewTaskDiff_Config(t *testing.T) { - c1 := map[string]interface{}{ - "command": "/bin/date", - "args": []string{"1", "2"}, - } - - c2 := map[string]interface{}{ - "args": []string{"1", "2"}, - } - - c3 := map[string]interface{}{ - "command": "/bin/date", - "args": []string{"1", "2"}, - "nested": &Port{ - Label: "http", - Value: 80, - }, - } - - c4 := map[string]interface{}{ - "command": "/bin/bash", - "args": []string{"1", "2"}, - } - - // No old case - t1 := &Task{Config: c1} - diff := NewTaskDiff(nil, t1) - if diff == nil { - t.Fatalf("expected non-nil diff") - } - if diff.Config == nil { - t.Fatalf("expected Config diff: %#v", diff) - } - - cdiff := diff.Config - if cdiff.Type != DiffTypeAdded { - t.Fatalf("expected Config diff type %v; got %#v", DiffTypeAdded, cdiff.Type) - } - - // No new case - diff = NewTaskDiff(t1, nil) - if diff == nil { - t.Fatalf("expected non-nil diff") - } - if diff.Config == nil { - t.Fatalf("expected Config diff: %#v", diff) - } - - cdiff = diff.Config - if cdiff.Type != DiffTypeDeleted { - t.Fatalf("expected Config diff type %v; got %#v", DiffTypeDeleted, cdiff.Type) - } - - // Same case - diff = NewTaskDiff(t1, t1) - if diff != nil { - t.Fatalf("expected nil diff") - } - - // Deleted case - t2 := &Task{Config: c2} - diff = NewTaskDiff(t1, t2) - if diff == nil { - t.Fatalf("expected non-nil diff") - } - if diff.Config == nil { - t.Fatalf("expected Config diff: %#v", diff) - } - - cdiff = diff.Config - if cdiff.Type != DiffTypeDeleted { - t.Fatalf("expected Config diff type %v; got %#v", DiffTypeDeleted, cdiff.Type) - } - - if len(cdiff.Added)+len(cdiff.Edited) != 0 && len(cdiff.Deleted) != 1 { - t.Fatalf("unexpected config diffs: %#v", cdiff) - } - - if v, ok := cdiff.Deleted["command"]; !ok || v != "/bin/date" { - t.Fatalf("bad: %#v", cdiff.Deleted) - } - - // Added case - t3 := &Task{Config: c3} - diff = NewTaskDiff(t1, t3) - if diff == nil { - t.Fatalf("expected non-nil diff") - } - if diff.Config == nil { - t.Fatalf("expected Config diff: %#v", diff) - } - - cdiff = diff.Config - if cdiff.Type != DiffTypeAdded { - t.Fatalf("expected Config diff type %v; got %#v", DiffTypeAdded, cdiff.Type) - } - - if len(cdiff.Deleted)+len(cdiff.Edited) != 0 && len(cdiff.Added) != 2 { - t.Fatalf("unexpected config diffs: %#v", cdiff) - } - - if v, ok := cdiff.Added["nested.Value"]; !ok || v != "80" { - t.Fatalf("bad: %#v", cdiff.Added) - } - if v, ok := cdiff.Added["nested.Label"]; !ok || v != "http" { - t.Fatalf("bad: %#v", cdiff.Added) - } - - // Edited case - t4 := &Task{Config: c4} - diff = NewTaskDiff(t1, t4) - if diff == nil { - t.Fatalf("expected non-nil diff") - } - if diff.Config == nil { - t.Fatalf("expected Config diff: %#v", diff) - } - - cdiff = diff.Config - if cdiff.Type != DiffTypeEdited { - t.Fatalf("expected Config diff type %v; got %#v", DiffTypeEdited, cdiff.Type) - } - - if len(cdiff.Deleted)+len(cdiff.Added) != 0 && len(cdiff.Edited) != 1 { - t.Fatalf("unexpected config diffs: %#v", cdiff) - } - - exp := StringValueDelta{Old: "/bin/date", New: "/bin/bash"} - exp.Type = DiffTypeEdited - v, ok := cdiff.Edited["command"] - if !ok || !reflect.DeepEqual(v, exp) { - t.Fatalf("bad: %#v %#v %#v", cdiff.Edited, v, exp) - } -} - -func TestNewPrimitiveStructDiff(t *testing.T) { - p1 := Port{Label: "1"} - p2 := Port{Label: "2"} - p3 := Port{} - - pdiff := NewPrimitiveStructDiff(nil, nil, portFields) - if pdiff != nil { - t.Fatalf("expected no diff: %#v", pdiff) - } - - pdiff = NewPrimitiveStructDiff(p1, p1, portFields) - if pdiff != nil { - t.Fatalf("expected no diff: %#v", pdiff) - } - - pdiff = NewPrimitiveStructDiff(nil, p1, portFields) - if pdiff == nil { - t.Fatalf("expected diff") - } - - if pdiff.Type != DiffTypeAdded { - t.Fatalf("unexpected type: got %v; want %v", pdiff.Type, DiffTypeAdded) - } - - pdiff = NewPrimitiveStructDiff(p1, nil, portFields) - if pdiff == nil { - t.Fatalf("expected diff") - } - - if pdiff.Type != DiffTypeDeleted { - t.Fatalf("unexpected type: got %v; want %v", pdiff.Type, DiffTypeDeleted) - } - - pdiff = NewPrimitiveStructDiff(p1, p2, portFields) - if pdiff == nil { - t.Fatalf("expected diff") - } - - if pdiff.Type != DiffTypeEdited { - t.Fatalf("unexpected type: got %v; want %v", pdiff.Type, DiffTypeEdited) - } - - if len(pdiff.PrimitiveFields) != 1 { - t.Fatalf("unexpected number of field diffs: %#v", pdiff.PrimitiveFields) - } - - f, ok := pdiff.PrimitiveFields["Label"] - if !ok { - t.Fatalf("expected diff on field %q", "label") - } - - if f.Type != DiffTypeEdited { - t.Fatalf("unexpected type: got %v; want %v", f.Type, DiffTypeEdited) - } - if !reflect.DeepEqual(f.OldValue, "1") || !reflect.DeepEqual(f.NewValue, "2") { - t.Fatalf("bad: %#v", f) - } - - pdiff = NewPrimitiveStructDiff(p1, p3, portFields) - if pdiff == nil { - t.Fatalf("expected diff") - } - - if pdiff.Type != DiffTypeEdited { - t.Fatalf("unexpected type: got %v; want %v", pdiff.Type, DiffTypeEdited) - } - - if len(pdiff.PrimitiveFields) != 1 { - t.Fatalf("unexpected number of field diffs: %#v", pdiff.PrimitiveFields) - } - - f = pdiff.PrimitiveFields["Label"] - if !ok { - t.Fatalf("expected diff on field %q", "Label") - } - if f.Type != DiffTypeEdited { - t.Fatalf("unexpected type: got %v; want %v", f.Type, DiffTypeEdited) - } - if !reflect.DeepEqual(f.OldValue, "1") || !reflect.DeepEqual(f.NewValue, "") { - t.Fatalf("bad: %#v", f) - } -} - -func TestSetDiffPrimitiveStructs(t *testing.T) { - p1 := Port{Label: "1"} - p2 := Port{Label: "2"} - p3 := Port{Label: "3"} - p4 := Port{Label: "4"} - p5 := Port{Label: "5"} - p6 := Port{Label: "6"} - - old := []Port{p1, p2, p3, p4} - new := []Port{p3, p4, p5, p6} - - diffs := setDiffPrimitiveStructs(interfaceSlice(old), interfaceSlice(new), portFields) - if len(diffs) != 4 { - t.Fatalf("expected four diffs: %#v", diffs) - } - - var added, deleted int - for _, diff := range diffs { - switch diff.Type { - case DiffTypeAdded: - added++ - case DiffTypeDeleted: - deleted++ - default: - t.Fatalf("unexpected diff type: %#v", diff.Type) - } - } - - if added != 2 && deleted != 2 { - t.Fatalf("incorrect counts") - } -} - -func TestNewFieldDiff(t *testing.T) { - cases := []struct { - NilExpected bool - Old, New interface{} - Expected DiffType - }{ - { - NilExpected: true, - Old: 1, - New: 1, - }, - { - NilExpected: true, - Old: true, - New: true, - }, - { - NilExpected: true, - Old: "foo", - New: "foo", - }, - { - NilExpected: true, - Old: 2.23, - New: 2.23, - }, - { - Old: 1, - New: 4, - Expected: DiffTypeEdited, - }, - { - Old: true, - New: false, - Expected: DiffTypeEdited, - }, - { - Old: "foo", - New: "bar", - Expected: DiffTypeEdited, - }, - { - Old: 2.23, - New: 12.511, - Expected: DiffTypeEdited, - }, - { - Old: nil, - New: 4, - Expected: DiffTypeAdded, - }, - { - Old: nil, - New: true, - Expected: DiffTypeAdded, - }, - { - Old: nil, - New: "bar", - Expected: DiffTypeAdded, - }, - { - Old: nil, - New: 12.511, - Expected: DiffTypeAdded, - }, - { - Old: 4, - New: nil, - Expected: DiffTypeDeleted, - }, - { - Old: true, - New: nil, - Expected: DiffTypeDeleted, - }, - { - Old: "bar", - New: nil, - Expected: DiffTypeDeleted, - }, - { - Old: 12.511, - New: nil, - Expected: DiffTypeDeleted, - }, - } - - for i, c := range cases { - diff := NewFieldDiff("foo", c.Old, c.New) - if diff == nil { - if !c.NilExpected { - t.Fatalf("case %d: diff was nil and unexpected", i+1) - } - continue - } - - if diff.Type != c.Expected { - t.Fatalf("case %d: wanted type %v; got %v", i+1, diff.Type, c.Expected) - } - } -} - -func TestStringSetDiff(t *testing.T) { - values := []string{"1", "2", "3", "4", "5", "6"} - - // Edited case - setDiff := NewStringSetDiff(values[:4], values[2:]) - if setDiff.Type != DiffTypeEdited { - t.Fatalf("got type %v; want %v", setDiff.Type, DiffTypeEdited) - } - - addedExp := []string{"5", "6"} - deletedExp := []string{"1", "2"} - sort.Strings(setDiff.Added) - sort.Strings(setDiff.Deleted) - - if !reflect.DeepEqual(addedExp, setDiff.Added) || - !reflect.DeepEqual(deletedExp, setDiff.Deleted) { - t.Fatalf("bad: %#v", setDiff) - } - - // Added case - setDiff = NewStringSetDiff(nil, values) - if setDiff.Type != DiffTypeAdded { - t.Fatalf("got type %v; want %v", setDiff.Type, DiffTypeAdded) - } - - // Deleted case - setDiff = NewStringSetDiff(values, nil) - if setDiff.Type != DiffTypeDeleted { - t.Fatalf("got type %v; want %v", setDiff.Type, DiffTypeDeleted) - } -} - -func TestStringMapDiff(t *testing.T) { - m1 := map[string]string{ - "a": "aval", - "b": "bval", - } - m2 := map[string]string{ - "b": "bval2", - "c": "cval", - } - - // Edited case - expected := &StringMapDiff{ - DiffEntry: DiffEntry{ - Type: DiffTypeEdited, - }, - Added: map[string]string{"c": "cval"}, - Deleted: map[string]string{"a": "aval"}, - Edited: map[string]StringValueDelta{ - "b": StringValueDelta{Old: "bval", - DiffEntry: DiffEntry{ - Type: DiffTypeEdited, - }, - New: "bval2", - }, - }, - } - - act := NewStringMapDiff(m1, m2) - if !reflect.DeepEqual(act, expected) { - t.Fatalf("got %#v; want %#v", act, expected) - } - - // Added case - diff := NewStringMapDiff(nil, m1) - if diff.Type != DiffTypeAdded { - t.Fatalf("got type %v; want %v", diff.Type, DiffTypeAdded) - } - - // Deleted case - diff = NewStringMapDiff(m1, nil) - if diff.Type != DiffTypeDeleted { - t.Fatalf("got type %v; want %v", diff.Type, DiffTypeDeleted) - } -} - -func TestSetDifference(t *testing.T) { - old := []interface{}{1, 2} - new := []interface{}{2, 3} - added, deleted := setDifference(old, new) - - if len(added) != 1 && len(deleted) != 1 { - t.Fatalf("bad: %#v %#v", added, deleted) - } - - a, ok := added[0].(int) - if !ok || a != 3 { - t.Fatalf("bad: %v %v", a, ok) - } - - d, ok := deleted[0].(int) - if !ok || d != 1 { - t.Fatalf("bad: %v %v", a, ok) - } -} - -func TestKeyedSetDifference(t *testing.T) { - oldMap := map[string]interface{}{ - "a": 1, - "b": 2, - "c": 3, - } - newMap := map[string]interface{}{ - "b": 3, - "c": 3, - "d": 4, - } - - added, deleted, edited, unmodified := keyedSetDifference(oldMap, newMap) - - if v, ok := added["d"]; !ok || v.(int) != 4 { - t.Fatalf("bad: %#v", added) - } - if v, ok := deleted["a"]; !ok || v.(int) != 1 { - t.Fatalf("bad: %#v", deleted) - } - if l := len(edited); l != 1 || edited[0] != "b" { - t.Fatalf("bad: %#v", edited) - } - if l := len(unmodified); l != 1 || unmodified[0] != "c" { - t.Fatalf("bad: %#v", edited) - } -} - -func TestInterfaceSlice(t *testing.T) { - j1 := TestJob() - j2 := TestJob() - jobs := []*Job{j1, j2} - - slice := interfaceSlice(jobs) - if len(slice) != 2 { - t.Fatalf("bad: %#v", slice) - } - - f := slice[0] - actJob1, ok := f.(*Job) - if !ok { - t.Fatalf("unexpected type: %v", actJob1) - } - - if !reflect.DeepEqual(actJob1, j1) { - t.Fatalf("got %#v, want %#v", actJob1, j1) - } -} - -func TestGetField(t *testing.T) { - j := TestJob() - exp := "foo" - j.Type = "foo" - - i := getField(j, "Type") - act, ok := i.(string) - if !ok { - t.Fatalf("expected to get string type back") - } - - if act != exp { - t.Fatalf("got %v; want %v", act, exp) - } -} diff --git a/nomad/structs/job_diff_visitor.go b/nomad/structs/job_diff_visitor.go deleted file mode 100644 index a1747fc6f..000000000 --- a/nomad/structs/job_diff_visitor.go +++ /dev/null @@ -1,52 +0,0 @@ -package structs - -// JobVisitor is the set of types a visitor must implement to traverse a JobDiff -// structure. -type JobVisitor interface { - VisitJob(*JobDiff) - VisitTaskGroups(*TaskGroupsDiff) - VisitTaskGroup(*TaskGroupDiff) - VisitTasks(*TasksDiff) - VisitTask(*TaskDiff) - VisitServices(*ServicesDiff) - VisitService(*ServiceDiff) - VisitServiceChecks(*ServiceChecksDiff) - VisitServiceCheck(*ServiceCheckDiff) - VisitTaskArtifactsDiff(*TaskArtifactsDiff) - VisitTaskArtifactDiff(*TaskArtifactDiff) - VisitResources(*ResourcesDiff) - VisitNetworkResources(*NetworkResourcesDiff) - VisitNetworkResource(*NetworkResourceDiff) - VisitPorts(*PortsDiff) - VisitPrimitiveStruct(*PrimitiveStructDiff) - VisitField(*FieldDiff) - VisitStringSet(*StringSetDiff) - VisitStringMap(*StringMapDiff) - VisitStringValueDelta(*StringValueDelta) -} - -// JobDiffComponent is the method a diff component must implement to be visited. -type JobDiffComponent interface { - Accept(JobVisitor) -} - -func (j *JobDiff) Accept(v JobVisitor) { v.VisitJob(j) } -func (t *TaskGroupsDiff) Accept(v JobVisitor) { v.VisitTaskGroups(t) } -func (t *TaskGroupDiff) Accept(v JobVisitor) { v.VisitTaskGroup(t) } -func (t *TasksDiff) Accept(v JobVisitor) { v.VisitTasks(t) } -func (t *TaskDiff) Accept(v JobVisitor) { v.VisitTask(t) } -func (s *ServicesDiff) Accept(v JobVisitor) { v.VisitServices(s) } -func (s *ServiceDiff) Accept(v JobVisitor) { v.VisitService(s) } -func (s *ServiceChecksDiff) Accept(v JobVisitor) { v.VisitServiceChecks(s) } -func (s *ServiceCheckDiff) Accept(v JobVisitor) { v.VisitServiceCheck(s) } -func (t *TaskArtifactsDiff) Accept(v JobVisitor) { v.VisitTaskArtifactsDiff(t) } -func (t *TaskArtifactDiff) Accept(v JobVisitor) { v.VisitTaskArtifactDiff(t) } -func (r *ResourcesDiff) Accept(v JobVisitor) { v.VisitResources(r) } -func (n *NetworkResourcesDiff) Accept(v JobVisitor) { v.VisitNetworkResources(n) } -func (n *NetworkResourceDiff) Accept(v JobVisitor) { v.VisitNetworkResource(n) } -func (p *PortsDiff) Accept(v JobVisitor) { v.VisitPorts(p) } -func (p *PrimitiveStructDiff) Accept(v JobVisitor) { v.VisitPrimitiveStruct(p) } -func (f *FieldDiff) Accept(v JobVisitor) { v.VisitField(f) } -func (s *StringSetDiff) Accept(v JobVisitor) { v.VisitStringSet(s) } -func (s *StringMapDiff) Accept(v JobVisitor) { v.VisitStringMap(s) } -func (s *StringValueDelta) Accept(v JobVisitor) { v.VisitStringValueDelta(s) } From 24bfaa70acf13bb8adc51ad4b76a2e265adc8242 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 11 May 2016 15:36:28 -0700 Subject: [PATCH 09/17] Fix switching diff structures --- nomad/job_endpoint.go | 10 +- nomad/structs/diff.go | 36 ++--- scheduler/annotate.go | 143 ++++++++++++++----- scheduler/annotate_test.go | 281 ++++++++++++++++++++++--------------- scheduler/util.go | 2 +- scheduler/util_test.go | 5 +- 6 files changed, 297 insertions(+), 180 deletions(-) diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index b00ccffe5..1e567a150 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -460,8 +460,14 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) // Annotate and store the diff if args.Diff { - jobDiff := structs.NewJobDiff(oldJob, args.Job) - scheduler.Annotate(jobDiff) + jobDiff, err := oldJob.Diff(args.Job, true) + if err != nil { + return fmt.Errorf("failed to create job diff: %v", err) + } + + if err := scheduler.Annotate(jobDiff, planner.Plan); err != nil { + return fmt.Errorf("failed to annotate job diff: %v", err) + } reply.Diff = jobDiff } diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index 6cd8a5be2..c70af0f76 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -10,26 +10,6 @@ import ( "github.com/mitchellh/hashstructure" ) -const ( - // AnnotationForcesDestructiveUpdate marks a diff as causing a destructive - // update. - AnnotationForcesDestructiveUpdate = "forces create/destroy update" - - // AnnotationForcesInplaceUpdate marks a diff as causing an in-place - // update. - AnnotationForcesInplaceUpdate = "forces in-place update" -) - -// UpdateTypes denote the type of update to occur against the task group. -const ( - UpdateTypeIgnore = "ignore" - UpdateTypeCreate = "create" - UpdateTypeDestroy = "destroy" - UpdateTypeMigrate = "migrate" - UpdateTypeInplaceUpdate = "in-place update" - UpdateTypeDestructiveUpdate = "create/destroy update" -) - // DiffType denotes the type of a diff object. type DiffType string @@ -171,7 +151,7 @@ type TaskGroupDiff struct { Fields []*FieldDiff Objects []*ObjectDiff Tasks []*TaskDiff - Updates map[string]int + Updates map[string]uint64 } // Diff returns a diff of two task groups. If contextual diff is enabled, @@ -847,9 +827,10 @@ func (o ObjectDiffs) Swap(i, j int) { o[i], o[j] = o[j], o[i] } func (o ObjectDiffs) Less(i, j int) bool { return o[i].Less(o[j]) } type FieldDiff struct { - Type DiffType - Name string - Old, New string + Type DiffType + Name string + Old, New string + Annotations []string } // fieldDiff returns a FieldDiff if old and new are different otherwise, it @@ -880,7 +861,12 @@ func fieldDiff(old, new, name string, contextual bool) *FieldDiff { } func (f *FieldDiff) GoString() string { - return fmt.Sprintf("%q (%s): %q => %q", f.Name, f.Type, f.Old, f.New) + out := fmt.Sprintf("%q (%s): %q => %q", f.Name, f.Type, f.Old, f.New) + if len(f.Annotations) != 0 { + out += fmt.Sprintf(" (%s)", strings.Join(f.Annotations, ", ")) + } + + return out } func (f *FieldDiff) Less(other *FieldDiff) bool { diff --git a/scheduler/annotate.go b/scheduler/annotate.go index 00209f921..559e640b7 100644 --- a/scheduler/annotate.go +++ b/scheduler/annotate.go @@ -1,6 +1,11 @@ package scheduler -import "github.com/hashicorp/nomad/nomad/structs" +import ( + "fmt" + "strconv" + + "github.com/hashicorp/nomad/nomad/structs" +) const ( AnnotationForcesCreate = "forces create" @@ -9,95 +14,167 @@ const ( AnnotationForcesDestructiveUpdate = "forces create/destroy update" ) -// Annotate takes the diff between the old and new version of a Job and will add -// annotations to the diff to aide human understanding of the plan. +// UpdateTypes denote the type of update to occur against the task group. +const ( + UpdateTypeIgnore = "ignore" + UpdateTypeCreate = "create" + UpdateTypeDestroy = "destroy" + UpdateTypeMigrate = "migrate" + UpdateTypeInplaceUpdate = "in-place update" + UpdateTypeDestructiveUpdate = "create/destroy update" +) + +// Annotate takes the diff between the old and new version of a Job, the +// scheduler plan and will add annotations to the diff to aide human +// understanding of the plan. // // Currently the things that are annotated are: -// * Count up and count down changes +// * Task group changes will be annotated with: +// * Count up and count down changes +// * Update counts (creates, destroys, migrates, etc) // * Task changes will be annotated with: // * forces create/destroy update // * forces in-place update -func Annotate(diff *structs.JobDiff) { +func Annotate(diff *structs.JobDiff, plan *structs.Plan) error { // No annotation needed as the job was either just submitted or deleted. if diff.Type != structs.DiffTypeEdited { - return + return nil } tgDiffs := diff.TaskGroups - if tgDiffs == nil { - return + if len(tgDiffs) == 0 { + return nil } - for _, tgDiff := range tgDiffs.Edited { - annotateTaskGroup(tgDiff) + for _, tgDiff := range tgDiffs { + if err := annotateTaskGroup(tgDiff, plan); err != nil { + return err + } } + + return nil } // annotateTaskGroup takes a task group diff and annotates it. -func annotateTaskGroup(diff *structs.TaskGroupDiff) { +func annotateTaskGroup(diff *structs.TaskGroupDiff, plan *structs.Plan) error { // Don't annotate unless the task group was edited. If it was a // create/destroy it is not needed. - if diff == nil || diff.Type != structs.DiffTypeEdited { - return + if diff.Type != structs.DiffTypeEdited { + return nil + } + + // Annotate the updates + if plan != nil && plan.Annotations != nil { + tg, ok := plan.Annotations.DesiredTGUpdates[diff.Name] + if ok { + if diff.Updates == nil { + diff.Updates = make(map[string]uint64, 6) + } + + if tg.Ignore != 0 { + diff.Updates[UpdateTypeIgnore] = tg.Ignore + } + if tg.Place != 0 { + diff.Updates[UpdateTypeCreate] = tg.Place + } + if tg.Migrate != 0 { + diff.Updates[UpdateTypeMigrate] = tg.Migrate + } + if tg.Stop != 0 { + diff.Updates[UpdateTypeDestroy] = tg.Stop + } + if tg.InPlaceUpdate != 0 { + diff.Updates[UpdateTypeInplaceUpdate] = tg.InPlaceUpdate + } + if tg.DestructiveUpdate != 0 { + diff.Updates[UpdateTypeDestructiveUpdate] = tg.DestructiveUpdate + } + } } // Annotate the count - annotateCountChange(diff) + if err := annotateCountChange(diff); err != nil { + return err + } // Annotate the tasks. taskDiffs := diff.Tasks - if taskDiffs == nil { - return + if len(taskDiffs) == 0 { + return nil } - for _, taskDiff := range taskDiffs.Edited { + for _, taskDiff := range taskDiffs { annotateTask(taskDiff) } + + return nil } // annotateCountChange takes a task group diff and annotates the count // parameter. -func annotateCountChange(diff *structs.TaskGroupDiff) { +func annotateCountChange(diff *structs.TaskGroupDiff) error { // Don't annotate unless the task was edited. If it was a create/destroy it // is not needed. - if diff == nil || diff.Type != structs.DiffTypeEdited { - return + if diff.Type != structs.DiffTypeEdited { + return nil } - countDiff, ok := diff.PrimitiveFields["Count"] - if !ok { - return + var countDiff *structs.FieldDiff + for _, diff := range diff.Fields { + if diff.Name == "Count" { + countDiff = diff + break + } + } + + // Didn't find + if countDiff == nil { + fmt.Println("NO COUNT") + return nil + } + oldV, err := strconv.Atoi(countDiff.Old) + if err != nil { + return err + } + + newV, err := strconv.Atoi(countDiff.New) + if err != nil { + return err } - oldV := countDiff.OldValue.(int) - newV := countDiff.NewValue.(int) if oldV < newV { countDiff.Annotations = append(countDiff.Annotations, AnnotationForcesCreate) } else if newV < oldV { countDiff.Annotations = append(countDiff.Annotations, AnnotationForcesDestroy) } + + return nil } // annotateCountChange takes a task diff and annotates it. func annotateTask(diff *structs.TaskDiff) { // Don't annotate unless the task was edited. If it was a create/destroy it // is not needed. - if diff == nil || diff.Type != structs.DiffTypeEdited { + if diff.Type != structs.DiffTypeEdited { return } // All changes to primitive fields result in a destructive update. destructive := false - if len(diff.PrimitiveFields) != 0 { + if len(diff.Fields) != 0 { destructive = true } - if diff.Env != nil || - diff.Meta != nil || - diff.Artifacts != nil || - diff.Resources != nil || - diff.Config != nil { - destructive = true + // Changes that can be done in-place are log configs, services and + // constraints. + for _, oDiff := range diff.Objects { + switch oDiff.Name { + case "LogConfig", "Service", "Constraint": + continue + default: + destructive = true + break + } } if destructive { diff --git a/scheduler/annotate_test.go b/scheduler/annotate_test.go index b22174267..5f90ef23f 100644 --- a/scheduler/annotate_test.go +++ b/scheduler/annotate_test.go @@ -18,52 +18,40 @@ func TestAnnotateCountChange_NonEdited(t *testing.T) { func TestAnnotateCountChange(t *testing.T) { up := &structs.FieldDiff{ - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeEdited, - }, - Name: "Count", - OldValue: 1, - NewValue: 3, + Type: structs.DiffTypeEdited, + Name: "Count", + Old: "1", + New: "3", } down := &structs.FieldDiff{ - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeEdited, - }, - Name: "Count", - OldValue: 3, - NewValue: 1, + Type: structs.DiffTypeEdited, + Name: "Count", + Old: "3", + New: "1", } tgUp := &structs.TaskGroupDiff{ - PrimitiveStructDiff: structs.PrimitiveStructDiff{ - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeEdited, - }, - PrimitiveFields: map[string]*structs.FieldDiff{ - "Count": up, - }, - }, + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{up}, } tgDown := &structs.TaskGroupDiff{ - PrimitiveStructDiff: structs.PrimitiveStructDiff{ - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeEdited, - }, - PrimitiveFields: map[string]*structs.FieldDiff{ - "Count": down, - }, - }, + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{down}, } // Test the up case - annotateCountChange(tgUp) - countDiff := tgUp.PrimitiveFields["Count"] + if err := annotateCountChange(tgUp); err != nil { + t.Fatalf("annotateCountChange(%#v) failed: %v", tgUp, err) + } + countDiff := tgUp.Fields[0] if len(countDiff.Annotations) != 1 || countDiff.Annotations[0] != AnnotationForcesCreate { t.Fatalf("incorrect annotation: %#v", tgUp) } // Test the down case - annotateCountChange(tgDown) - countDiff = tgDown.PrimitiveFields["Count"] + if err := annotateCountChange(tgDown); err != nil { + t.Fatalf("annotateCountChange(%#v) failed: %v", tgDown, err) + } + countDiff = tgDown.Fields[0] if len(countDiff.Annotations) != 1 || countDiff.Annotations[0] != AnnotationForcesDestroy { t.Fatalf("incorrect annotation: %#v", tgDown) } @@ -81,93 +69,126 @@ func TestAnnotateTask_NonEdited(t *testing.T) { func TestAnnotateTask_Destructive(t *testing.T) { cases := []*structs.TaskDiff{ { - PrimitiveStructDiff: structs.PrimitiveStructDiff{ - DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{ + { Type: structs.DiffTypeEdited, - }, - PrimitiveFields: map[string]*structs.FieldDiff{ - "Driver": &structs.FieldDiff{ - Name: "Driver", - OldValue: "docker", - NewValue: "exec", - }, + Name: "Driver", + Old: "docker", + New: "exec", }, }, }, { - PrimitiveStructDiff: structs.PrimitiveStructDiff{ - DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{ + { Type: structs.DiffTypeEdited, - }, - PrimitiveFields: map[string]*structs.FieldDiff{ - "User": &structs.FieldDiff{ - Name: "User", - OldValue: "", - NewValue: "specific", - }, + Name: "User", + Old: "alice", + New: "bob", }, }, }, { - Env: &structs.StringMapDiff{ - DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{ + { Type: structs.DiffTypeAdded, - }, - Added: map[string]string{ - "foo": "bar", + Name: "Env[foo]", + Old: "foo", + New: "bar", }, }, }, { - Meta: &structs.StringMapDiff{ - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeEdited, - }, - Edited: map[string]structs.StringValueDelta{ - "foo": { - Old: "a", - New: "b", - }, - }, - }, - }, - { - Artifacts: &structs.TaskArtifactsDiff{ - DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{ + { Type: structs.DiffTypeAdded, - }, - Added: []*structs.TaskArtifact{ - { - GetterSource: "foo", - }, + Name: "Meta[foo]", + Old: "foo", + New: "bar", }, }, }, { - Resources: &structs.ResourcesDiff{ - PrimitiveStructDiff: structs.PrimitiveStructDiff{ - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeEdited, - }, - PrimitiveFields: map[string]*structs.FieldDiff{ - "CPU": &structs.FieldDiff{ - Name: "CPU", - OldValue: 500, - NewValue: 1000, + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Artifact", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "GetterOptions[bam]", + Old: "", + New: "baz", + }, + { + Type: structs.DiffTypeAdded, + Name: "GetterSource", + Old: "", + New: "bam", + }, + { + Type: structs.DiffTypeAdded, + Name: "RelativeDest", + Old: "", + New: "bam", }, }, }, }, }, { - Config: &structs.StringMapDiff{ - DiffEntry: structs.DiffEntry{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { Type: structs.DiffTypeEdited, + Name: "Resources", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeEdited, + Name: "CPU", + Old: "100", + New: "200", + }, + { + Type: structs.DiffTypeEdited, + Name: "DiskMB", + Old: "100", + New: "200", + }, + { + Type: structs.DiffTypeEdited, + Name: "IOPS", + Old: "100", + New: "200", + }, + { + Type: structs.DiffTypeEdited, + Name: "MemoryMB", + Old: "100", + New: "200", + }, + }, }, - Edited: map[string]structs.StringValueDelta{ - "command": { - Old: "/bin/date", - New: "/bin/bash", + }, + }, + { + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeEdited, + Name: "Config", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeEdited, + Name: "bam[1]", + Old: "b", + New: "c", + }, }, }, }, @@ -186,44 +207,70 @@ func TestAnnotateTask_Destructive(t *testing.T) { func TestAnnotateTask_Inplace(t *testing.T) { cases := []*structs.TaskDiff{ { - Constraints: []*structs.PrimitiveStructDiff{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ { - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeEdited, - }, - PrimitiveFields: map[string]*structs.FieldDiff{ - "RTarget": &structs.FieldDiff{ - Name: "RTarget", - OldValue: "linux", - NewValue: "windows", + Type: structs.DiffTypeAdded, + Name: "Constraint", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "LTarget", + Old: "", + New: "baz", + }, + { + Type: structs.DiffTypeAdded, + Name: "Operand", + Old: "", + New: "baz", + }, + { + Type: structs.DiffTypeAdded, + Name: "RTarget", + Old: "", + New: "baz", }, }, }, }, }, { - LogConfig: &structs.PrimitiveStructDiff{ - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeEdited, - }, - PrimitiveFields: map[string]*structs.FieldDiff{ - "MaxFileSizeMB": &structs.FieldDiff{ - Name: "MaxFileSizeMB", - OldValue: 100, - NewValue: 128, + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeAdded, + Name: "LogConfig", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "MaxFileSizeMB", + Old: "", + New: "10", + }, + { + Type: structs.DiffTypeAdded, + Name: "MaxFiles", + Old: "", + New: "1", + }, }, }, }, }, { - Services: &structs.ServicesDiff{ - DiffEntry: structs.DiffEntry{ - Type: structs.DiffTypeAdded, - }, - Added: []*structs.Service{ - { - Name: "foo", - PortLabel: "rpc", + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeEdited, + Name: "Service", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeEdited, + Name: "PortLabel", + Old: "baz", + New: "baz2", + }, }, }, }, diff --git a/scheduler/util.go b/scheduler/util.go index 15c38c479..120440918 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -480,7 +480,7 @@ func desiredUpdates(diff *diffResult, inplaceUpdates, } for _, tuple := range diff.stop { - name := tuple.Alloc.TaskGroup + name := tuple.TaskGroup.Name des, ok := desiredTgs[name] if !ok { des = &structs.DesiredUpdates{} diff --git a/scheduler/util_test.go b/scheduler/util_test.go index 4d63ff195..338f92d3c 100644 --- a/scheduler/util_test.go +++ b/scheduler/util_test.go @@ -765,6 +765,7 @@ func TestProgressMade(t *testing.T) { func TestDesiredUpdates(t *testing.T) { tg1 := &structs.TaskGroup{Name: "foo"} tg2 := &structs.TaskGroup{Name: "bar"} + a2 := &structs.Allocation{TaskGroup: "foo"} place := []allocTuple{ allocTuple{TaskGroup: tg1}, @@ -773,8 +774,8 @@ func TestDesiredUpdates(t *testing.T) { allocTuple{TaskGroup: tg2}, } stop := []allocTuple{ - allocTuple{TaskGroup: tg2}, - allocTuple{TaskGroup: tg2}, + allocTuple{TaskGroup: tg2, Alloc: a2}, + allocTuple{TaskGroup: tg2, Alloc: a2}, } ignore := []allocTuple{ allocTuple{TaskGroup: tg1}, From ab4c184916d0dda0d85dd3bab447f2212e7e127d Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 11 May 2016 15:44:27 -0700 Subject: [PATCH 10/17] Undo testing.go --- nomad/structs/structs_test.go | 78 ++++++++++++++++++++++++++++++++++- nomad/structs/testing.go | 77 ---------------------------------- 2 files changed, 76 insertions(+), 79 deletions(-) delete mode 100644 nomad/structs/testing.go diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index e1ba1d06a..01b828ae8 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -93,8 +93,82 @@ func TestJob_Validate(t *testing.T) { } } +func testJob() *Job { + return &Job{ + Region: "global", + ID: GenerateUUID(), + Name: "my-job", + Type: JobTypeService, + Priority: 50, + AllAtOnce: false, + Datacenters: []string{"dc1"}, + Constraints: []*Constraint{ + &Constraint{ + LTarget: "$attr.kernel.name", + RTarget: "linux", + Operand: "=", + }, + }, + Periodic: &PeriodicConfig{ + Enabled: false, + }, + TaskGroups: []*TaskGroup{ + &TaskGroup{ + Name: "web", + Count: 10, + RestartPolicy: &RestartPolicy{ + Attempts: 3, + Interval: 10 * time.Minute, + Delay: 1 * time.Minute, + }, + Tasks: []*Task{ + &Task{ + Name: "web", + Driver: "exec", + Config: map[string]interface{}{ + "command": "/bin/date", + }, + Env: map[string]string{ + "FOO": "bar", + }, + Artifacts: []*TaskArtifact{ + { + GetterSource: "http://foo.com", + }, + }, + Services: []*Service{ + { + Name: "${TASK}-frontend", + PortLabel: "http", + }, + }, + Resources: &Resources{ + CPU: 500, + MemoryMB: 256, + Networks: []*NetworkResource{ + &NetworkResource{ + MBits: 50, + DynamicPorts: []Port{{Label: "http"}}, + }, + }, + }, + }, + }, + Meta: map[string]string{ + "elb_check_type": "http", + "elb_check_interval": "30s", + "elb_check_min": "3", + }, + }, + }, + Meta: map[string]string{ + "owner": "armon", + }, + } +} + func TestJob_Copy(t *testing.T) { - j := TestJob() + j := testJob() c := j.Copy() if !reflect.DeepEqual(j, c) { t.Fatalf("Copy() returned an unequal Job; got %#v; want %#v", c, j) @@ -470,7 +544,7 @@ func TestEncodeDecode(t *testing.T) { } func BenchmarkEncodeDecode(b *testing.B) { - job := TestJob() + job := testJob() for i := 0; i < b.N; i++ { buf, err := Encode(1, job) diff --git a/nomad/structs/testing.go b/nomad/structs/testing.go deleted file mode 100644 index 3893e2369..000000000 --- a/nomad/structs/testing.go +++ /dev/null @@ -1,77 +0,0 @@ -package structs - -import "time" - -func TestJob() *Job { - return &Job{ - Region: "global", - ID: GenerateUUID(), - Name: "my-job", - Type: JobTypeService, - Priority: 50, - AllAtOnce: false, - Datacenters: []string{"dc1"}, - Constraints: []*Constraint{ - &Constraint{ - LTarget: "$attr.kernel.name", - RTarget: "linux", - Operand: "=", - }, - }, - Periodic: &PeriodicConfig{ - Enabled: false, - }, - TaskGroups: []*TaskGroup{ - &TaskGroup{ - Name: "web", - Count: 10, - RestartPolicy: &RestartPolicy{ - Attempts: 3, - Interval: 10 * time.Minute, - Delay: 1 * time.Minute, - }, - Tasks: []*Task{ - &Task{ - Name: "web", - Driver: "exec", - Config: map[string]interface{}{ - "command": "/bin/date", - }, - Env: map[string]string{ - "FOO": "bar", - }, - Artifacts: []*TaskArtifact{ - { - GetterSource: "http://foo.com", - }, - }, - Services: []*Service{ - { - Name: "${TASK}-frontend", - PortLabel: "http", - }, - }, - Resources: &Resources{ - CPU: 500, - MemoryMB: 256, - Networks: []*NetworkResource{ - &NetworkResource{ - MBits: 50, - DynamicPorts: []Port{{Label: "http"}}, - }, - }, - }, - }, - }, - Meta: map[string]string{ - "elb_check_type": "http", - "elb_check_interval": "30s", - "elb_check_min": "3", - }, - }, - }, - Meta: map[string]string{ - "owner": "armon", - }, - } -} From 6d69e3996674abd698fa4ecc005ab2bc340974cc Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 11 May 2016 16:31:50 -0700 Subject: [PATCH 11/17] Test task group update annotations --- scheduler/annotate.go | 2 -- scheduler/annotate_test.go | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/scheduler/annotate.go b/scheduler/annotate.go index 559e640b7..461be9906 100644 --- a/scheduler/annotate.go +++ b/scheduler/annotate.go @@ -1,7 +1,6 @@ package scheduler import ( - "fmt" "strconv" "github.com/hashicorp/nomad/nomad/structs" @@ -129,7 +128,6 @@ func annotateCountChange(diff *structs.TaskGroupDiff) error { // Didn't find if countDiff == nil { - fmt.Println("NO COUNT") return nil } oldV, err := strconv.Atoi(countDiff.Old) diff --git a/scheduler/annotate_test.go b/scheduler/annotate_test.go index 5f90ef23f..e8713d93e 100644 --- a/scheduler/annotate_test.go +++ b/scheduler/annotate_test.go @@ -7,6 +7,48 @@ import ( "github.com/hashicorp/nomad/nomad/structs" ) +func TestAnnotateTaskGroup_Updates(t *testing.T) { + plan := &structs.Plan{ + Annotations: &structs.PlanAnnotations{ + DesiredTGUpdates: map[string]*structs.DesiredUpdates{ + "foo": &structs.DesiredUpdates{ + Ignore: 1, + Place: 2, + Migrate: 3, + Stop: 4, + InPlaceUpdate: 5, + DestructiveUpdate: 6, + }, + }, + }, + } + + tgDiff := &structs.TaskGroupDiff{ + Type: structs.DiffTypeEdited, + Name: "foo", + } + expected := &structs.TaskGroupDiff{ + Type: structs.DiffTypeEdited, + Name: "foo", + Updates: map[string]uint64{ + UpdateTypeIgnore: 1, + UpdateTypeCreate: 2, + UpdateTypeMigrate: 3, + UpdateTypeDestroy: 4, + UpdateTypeInplaceUpdate: 5, + UpdateTypeDestructiveUpdate: 6, + }, + } + + if err := annotateTaskGroup(tgDiff, plan); err != nil { + t.Fatalf("annotateTaskGroup(%#v, %#v) failed: %#v", tgDiff, plan, err) + } + + if !reflect.DeepEqual(tgDiff, expected) { + t.Fatalf("got %#v, want %#v", tgDiff, expected) + } +} + func TestAnnotateCountChange_NonEdited(t *testing.T) { tg := &structs.TaskGroupDiff{} tgOrig := &structs.TaskGroupDiff{} From 2bd962ca0dbe1abae60b9715b681e077b30a835f Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 11 May 2016 17:02:14 -0700 Subject: [PATCH 12/17] add endpoint tests --- nomad/job_endpoint_test.go | 98 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index c65e81047..a9fc7128c 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -914,3 +914,101 @@ func TestJobEndpoint_Evaluations(t *testing.T) { t.Fatalf("bad: %#v", resp2.Evaluations) } } + +func TestJobEndpoint_Plan_WithDiff(t *testing.T) { + s1 := testServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + }) + defer s1.Shutdown() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + + // Create the register request + job := mock.Job() + req := &structs.JobRegisterRequest{ + Job: job, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + + // Fetch the response + var resp structs.JobRegisterResponse + if err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + if resp.Index == 0 { + t.Fatalf("bad index: %d", resp.Index) + } + + // Create a plan request + planReq := &structs.JobPlanRequest{ + Job: job, + Diff: true, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + + // Fetch the response + var planResp structs.JobPlanResponse + if err := msgpackrpc.CallWithCodec(codec, "Job.Plan", planReq, &planResp); err != nil { + t.Fatalf("err: %v", err) + } + + // Check the response + if planResp.Cas == 0 { + t.Fatalf("bad cas: %d", planResp.Cas) + } + if planResp.Plan == nil { + t.Fatalf("no plan") + } + if planResp.Diff == nil { + t.Fatalf("no diff") + } +} + +func TestJobEndpoint_Plan_NoDiff(t *testing.T) { + s1 := testServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + }) + defer s1.Shutdown() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + + // Create the register request + job := mock.Job() + req := &structs.JobRegisterRequest{ + Job: job, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + + // Fetch the response + var resp structs.JobRegisterResponse + if err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + if resp.Index == 0 { + t.Fatalf("bad index: %d", resp.Index) + } + + // Create a plan request + planReq := &structs.JobPlanRequest{ + Job: job, + Diff: false, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + + // Fetch the response + var planResp structs.JobPlanResponse + if err := msgpackrpc.CallWithCodec(codec, "Job.Plan", planReq, &planResp); err != nil { + t.Fatalf("err: %v", err) + } + + // Check the response + if planResp.Cas == 0 { + t.Fatalf("bad cas: %d", planResp.Cas) + } + if planResp.Plan == nil { + t.Fatalf("no plan") + } + if planResp.Diff != nil { + t.Fatalf("got diff") + } +} From b634486ebdcad868f57666cfc56283a19d401393 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 11 May 2016 18:51:48 -0700 Subject: [PATCH 13/17] Plan api --- api/jobs.go | 88 +++++++++++++++++++++++++++++++++++ api/jobs_test.go | 70 ++++++++++++++++++++++++++++ command/agent/job_endpoint.go | 1 + nomad/job_endpoint.go | 1 + nomad/structs/structs.go | 2 +- 5 files changed, 161 insertions(+), 1 deletion(-) diff --git a/api/jobs.go b/api/jobs.go index 9ff4d8376..eb097090a 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -1,6 +1,7 @@ package api import ( + "fmt" "sort" "time" ) @@ -116,6 +117,24 @@ func (j *Jobs) PeriodicForce(jobID string, q *WriteOptions) (string, *WriteMeta, return resp.EvalID, wm, nil } +func (j *Jobs) Plan(job *Job, diff bool, q *WriteOptions) (*JobPlanResponse, *WriteMeta, error) { + if job == nil { + return nil, nil, fmt.Errorf("must pass non-nil job") + } + + var resp JobPlanResponse + req := &JobPlanRequest{ + Job: job, + Diff: diff, + } + wm, err := j.client.write("/v1/job/"+job.ID+"/plan", req, &resp, q) + if err != nil { + return nil, nil, err + } + + return &resp, wm, nil +} + // periodicForceResponse is used to deserialize a force response type periodicForceResponse struct { EvalID string @@ -256,3 +275,72 @@ type registerJobResponse struct { type deregisterJobResponse struct { EvalID string } + +type JobPlanRequest struct { + Job *Job + Diff bool +} + +type JobPlanResponse struct { + Cas uint64 + CreatedEvals []*Evaluation + Diff *JobDiff + SchedulerOutput *SchedulerOutput `json:"Plan"` +} + +type JobDiff struct { + Type string + ID string + Fields []*FieldDiff + Objects []*ObjectDiff + TaskGroups []*TaskGroupDiff +} + +type TaskGroupDiff struct { + Type string + Name string + Fields []*FieldDiff + Objects []*ObjectDiff + Tasks []*TaskDiff + Updates map[string]uint64 +} + +type TaskDiff struct { + Type string + Name string + Fields []*FieldDiff + Objects []*ObjectDiff + Annotations []string +} + +type FieldDiff struct { + Type string + Name string + Old, New string + Annotations []string +} + +type ObjectDiff struct { + Type string + Name string + Fields []*FieldDiff + Objects []*ObjectDiff +} + +type SchedulerOutput struct { + FailedAllocs []*Allocations + Annotations *PlanAnnotations +} + +type PlanAnnotations struct { + DesiredTGUpdates map[string]*DesiredUpdates +} + +type DesiredUpdates struct { + Ignore uint64 + Place uint64 + Migrate uint64 + Stop uint64 + InPlaceUpdate uint64 + DestructiveUpdate uint64 +} diff --git a/api/jobs_test.go b/api/jobs_test.go index 45e6325df..50616da6d 100644 --- a/api/jobs_test.go +++ b/api/jobs_test.go @@ -350,6 +350,76 @@ func TestJobs_PeriodicForce(t *testing.T) { t.Fatalf("evaluation %q missing", evalID) } +func TestJobs_Plan(t *testing.T) { + c, s := makeClient(t, nil, nil) + defer s.Stop() + jobs := c.Jobs() + + // Create a job and attempt to register it + job := testJob() + eval, wm, err := jobs.Register(job, nil) + if err != nil { + t.Fatalf("err: %s", err) + } + if eval == "" { + t.Fatalf("missing eval id") + } + assertWriteMeta(t, wm) + + // Check that passing a nil job fails + if _, _, err := jobs.Plan(nil, true, nil); err == nil { + t.Fatalf("expect an error when job isn't provided") + } + + // Make a plan request + planResp, wm, err := jobs.Plan(job, true, nil) + if err != nil { + t.Fatalf("err: %s", err) + } + if planResp == nil { + t.Fatalf("nil response") + } + + if planResp.Cas == 0 { + t.Fatalf("bad Cas value: %#v", planResp) + } + if planResp.Diff == nil { + t.Fatalf("got nil diff: %#v", planResp) + } + if planResp.SchedulerOutput == nil { + t.Fatalf("got nil scheduler output: %#v", planResp) + } + // Can make this assertion because there are no clients. + if len(planResp.CreatedEvals) == 0 { + t.Fatalf("got no CreatedEvals: %#v", planResp) + } + + // Make a plan request w/o the diff + planResp, wm, err = jobs.Plan(job, false, nil) + if err != nil { + t.Fatalf("err: %s", err) + } + assertWriteMeta(t, wm) + + if planResp == nil { + t.Fatalf("nil response") + } + + if planResp.Cas == 0 { + t.Fatalf("bad Cas value: %d", planResp.Cas) + } + if planResp.Diff != nil { + t.Fatalf("got non-nil diff: %#v", planResp) + } + if planResp.SchedulerOutput == nil { + t.Fatalf("got nil scheduler output: %#v", planResp) + } + // Can make this assertion because there are no clients. + if len(planResp.CreatedEvals) == 0 { + t.Fatalf("got no CreatedEvals: %#v", planResp) + } +} + func TestJobs_NewBatchJob(t *testing.T) { job := NewBatchJob("job1", "myjob", "region1", 5) expect := &Job{ diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 74a6e9fff..12e40e542 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -99,6 +99,7 @@ func (s *HTTPServer) jobPlan(resp http.ResponseWriter, req *http.Request, if err := s.agent.RPC("Job.Plan", &args, &out); err != nil { return nil, err } + setIndex(resp, out.Index) return out, nil } diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 1e567a150..9c0aa15fc 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -474,6 +474,7 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) reply.Cas = index reply.Plan = planner.Plan reply.CreatedEvals = planner.CreatedEvals + reply.Index = index return nil } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 7355d03ee..86ab17012 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -416,7 +416,7 @@ type JobPlanResponse struct { // causes an in-place update or create/destroy Diff *JobDiff - QueryMeta + WriteMeta } // SingleAllocResponse is used to return a single allocation From 7a44ec5ccc39001d75d1b35ee549e61e28cd84f8 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 12 May 2016 11:29:38 -0700 Subject: [PATCH 14/17] Remove plan from the response --- api/jobs.go | 13 ++++--------- api/jobs_test.go | 8 ++++---- nomad/job_endpoint.go | 8 +++----- nomad/job_endpoint_test.go | 8 ++++---- nomad/structs/structs.go | 4 ++-- scheduler/annotate.go | 14 +++++++------- scheduler/annotate_test.go | 24 +++++++++++------------- 7 files changed, 35 insertions(+), 44 deletions(-) diff --git a/api/jobs.go b/api/jobs.go index eb097090a..91fc2aa9b 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -282,10 +282,10 @@ type JobPlanRequest struct { } type JobPlanResponse struct { - Cas uint64 - CreatedEvals []*Evaluation - Diff *JobDiff - SchedulerOutput *SchedulerOutput `json:"Plan"` + Cas uint64 + CreatedEvals []*Evaluation + Diff *JobDiff + Annotations *PlanAnnotations } type JobDiff struct { @@ -327,11 +327,6 @@ type ObjectDiff struct { Objects []*ObjectDiff } -type SchedulerOutput struct { - FailedAllocs []*Allocations - Annotations *PlanAnnotations -} - type PlanAnnotations struct { DesiredTGUpdates map[string]*DesiredUpdates } diff --git a/api/jobs_test.go b/api/jobs_test.go index 50616da6d..99c744082 100644 --- a/api/jobs_test.go +++ b/api/jobs_test.go @@ -386,8 +386,8 @@ func TestJobs_Plan(t *testing.T) { if planResp.Diff == nil { t.Fatalf("got nil diff: %#v", planResp) } - if planResp.SchedulerOutput == nil { - t.Fatalf("got nil scheduler output: %#v", planResp) + if planResp.Annotations == nil { + t.Fatalf("got nil annotations: %#v", planResp) } // Can make this assertion because there are no clients. if len(planResp.CreatedEvals) == 0 { @@ -411,8 +411,8 @@ func TestJobs_Plan(t *testing.T) { if planResp.Diff != nil { t.Fatalf("got non-nil diff: %#v", planResp) } - if planResp.SchedulerOutput == nil { - t.Fatalf("got nil scheduler output: %#v", planResp) + if planResp.Annotations == nil { + t.Fatalf("got nil annotations: %#v", planResp) } // Can make this assertion because there are no clients. if len(planResp.CreatedEvals) == 0 { diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 9c0aa15fc..40cec0940 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -455,24 +455,22 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) return err } - // Clear the job from the plan - planner.Plan.Job = nil - // Annotate and store the diff + annotations := planner.Plan.Annotations if args.Diff { jobDiff, err := oldJob.Diff(args.Job, true) if err != nil { return fmt.Errorf("failed to create job diff: %v", err) } - if err := scheduler.Annotate(jobDiff, planner.Plan); err != nil { + if err := scheduler.Annotate(jobDiff, annotations); err != nil { return fmt.Errorf("failed to annotate job diff: %v", err) } reply.Diff = jobDiff } reply.Cas = index - reply.Plan = planner.Plan + reply.Annotations = annotations reply.CreatedEvals = planner.CreatedEvals reply.Index = index return nil diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index a9fc7128c..8569de091 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -956,8 +956,8 @@ func TestJobEndpoint_Plan_WithDiff(t *testing.T) { if planResp.Cas == 0 { t.Fatalf("bad cas: %d", planResp.Cas) } - if planResp.Plan == nil { - t.Fatalf("no plan") + if planResp.Annotations == nil { + t.Fatalf("no annotations") } if planResp.Diff == nil { t.Fatalf("no diff") @@ -1005,8 +1005,8 @@ func TestJobEndpoint_Plan_NoDiff(t *testing.T) { if planResp.Cas == 0 { t.Fatalf("bad cas: %d", planResp.Cas) } - if planResp.Plan == nil { - t.Fatalf("no plan") + if planResp.Annotations == nil { + t.Fatalf("no annotations") } if planResp.Diff != nil { t.Fatalf("got diff") diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 86ab17012..4b1142a88 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -400,8 +400,8 @@ type JobListResponse struct { // JobPlanResponse is used to respond to a job plan request type JobPlanResponse struct { - // Plan holds the decisions the scheduler made. - Plan *Plan + // Annotations stores annotations explaining decisions the scheduler made. + Annotations *PlanAnnotations // The Cas value can be used when running `nomad run` to ensure that the Job // wasn’t modified since the last plan. If the job is being created, the diff --git a/scheduler/annotate.go b/scheduler/annotate.go index 461be9906..002164e74 100644 --- a/scheduler/annotate.go +++ b/scheduler/annotate.go @@ -24,8 +24,8 @@ const ( ) // Annotate takes the diff between the old and new version of a Job, the -// scheduler plan and will add annotations to the diff to aide human -// understanding of the plan. +// scheduler's plan annotations and will add annotations to the diff to aide +// human understanding of the plan. // // Currently the things that are annotated are: // * Task group changes will be annotated with: @@ -34,7 +34,7 @@ const ( // * Task changes will be annotated with: // * forces create/destroy update // * forces in-place update -func Annotate(diff *structs.JobDiff, plan *structs.Plan) error { +func Annotate(diff *structs.JobDiff, annotations *structs.PlanAnnotations) error { // No annotation needed as the job was either just submitted or deleted. if diff.Type != structs.DiffTypeEdited { return nil @@ -46,7 +46,7 @@ func Annotate(diff *structs.JobDiff, plan *structs.Plan) error { } for _, tgDiff := range tgDiffs { - if err := annotateTaskGroup(tgDiff, plan); err != nil { + if err := annotateTaskGroup(tgDiff, annotations); err != nil { return err } } @@ -55,7 +55,7 @@ func Annotate(diff *structs.JobDiff, plan *structs.Plan) error { } // annotateTaskGroup takes a task group diff and annotates it. -func annotateTaskGroup(diff *structs.TaskGroupDiff, plan *structs.Plan) error { +func annotateTaskGroup(diff *structs.TaskGroupDiff, annotations *structs.PlanAnnotations) error { // Don't annotate unless the task group was edited. If it was a // create/destroy it is not needed. if diff.Type != structs.DiffTypeEdited { @@ -63,8 +63,8 @@ func annotateTaskGroup(diff *structs.TaskGroupDiff, plan *structs.Plan) error { } // Annotate the updates - if plan != nil && plan.Annotations != nil { - tg, ok := plan.Annotations.DesiredTGUpdates[diff.Name] + if annotations != nil { + tg, ok := annotations.DesiredTGUpdates[diff.Name] if ok { if diff.Updates == nil { diff.Updates = make(map[string]uint64, 6) diff --git a/scheduler/annotate_test.go b/scheduler/annotate_test.go index e8713d93e..acf589b85 100644 --- a/scheduler/annotate_test.go +++ b/scheduler/annotate_test.go @@ -8,17 +8,15 @@ import ( ) func TestAnnotateTaskGroup_Updates(t *testing.T) { - plan := &structs.Plan{ - Annotations: &structs.PlanAnnotations{ - DesiredTGUpdates: map[string]*structs.DesiredUpdates{ - "foo": &structs.DesiredUpdates{ - Ignore: 1, - Place: 2, - Migrate: 3, - Stop: 4, - InPlaceUpdate: 5, - DestructiveUpdate: 6, - }, + annotations := &structs.PlanAnnotations{ + DesiredTGUpdates: map[string]*structs.DesiredUpdates{ + "foo": &structs.DesiredUpdates{ + Ignore: 1, + Place: 2, + Migrate: 3, + Stop: 4, + InPlaceUpdate: 5, + DestructiveUpdate: 6, }, }, } @@ -40,8 +38,8 @@ func TestAnnotateTaskGroup_Updates(t *testing.T) { }, } - if err := annotateTaskGroup(tgDiff, plan); err != nil { - t.Fatalf("annotateTaskGroup(%#v, %#v) failed: %#v", tgDiff, plan, err) + if err := annotateTaskGroup(tgDiff, annotations); err != nil { + t.Fatalf("annotateTaskGroup(%#v, %#v) failed: %#v", tgDiff, annotations, err) } if !reflect.DeepEqual(tgDiff, expected) { From bed4cb7a9f0767360e061e81aacddd98e5f43f26 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 13 May 2016 11:53:11 -0700 Subject: [PATCH 15/17] Fixes --- nomad/structs/diff.go | 9 +- nomad/structs/diff_test.go | 97 ++++++++ scheduler/annotate.go | 58 ++--- scheduler/annotate_test.go | 459 ++++++++++++++++++++++--------------- scheduler/util.go | 2 +- scheduler/util_test.go | 2 +- 6 files changed, 414 insertions(+), 213 deletions(-) diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index c70af0f76..2ad3e7c91 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -61,15 +61,20 @@ func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) { var oldPrimitiveFlat, newPrimitiveFlat map[string]string filter := []string{"ID", "Status", "StatusDescription", "CreateIndex", "ModifyIndex", "JobModifyIndex"} + // Have to treat this special since it is a struct literal, not a pointer + var jUpdate, otherUpdate *UpdateStrategy + if j == nil && other == nil { return diff, nil } else if j == nil { j = &Job{} + otherUpdate = &other.Update diff.Type = DiffTypeAdded newPrimitiveFlat = flatmap.Flatten(other, filter, true) diff.ID = other.ID } else if other == nil { other = &Job{} + jUpdate = &j.Update diff.Type = DiffTypeDeleted oldPrimitiveFlat = flatmap.Flatten(j, filter, true) diff.ID = j.ID @@ -82,6 +87,8 @@ 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) } + jUpdate = &j.Update + otherUpdate = &other.Update oldPrimitiveFlat = flatmap.Flatten(j, filter, true) newPrimitiveFlat = flatmap.Flatten(other, filter, true) diff.ID = other.ID @@ -114,7 +121,7 @@ func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) { diff.TaskGroups = tgs // Update diff - if uDiff := primitiveObjectDiff(j.Update, other.Update, nil, "Update", contextual); uDiff != nil { + if uDiff := primitiveObjectDiff(jUpdate, otherUpdate, nil, "Update", contextual); uDiff != nil { diff.Objects = append(diff.Objects, uDiff) } diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 45b7c3835..a851d891b 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -181,6 +181,103 @@ func TestJobDiff(t *testing.T) { New: "", }, }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeDeleted, + Name: "Update", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "MaxParallel", + Old: "0", + New: "", + }, + { + Type: DiffTypeDeleted, + Name: "Stagger", + Old: "0", + New: "", + }, + }, + }, + }, + }, + }, + { + // Primitive only added job + Old: nil, + New: &Job{ + Region: "foo", + ID: "foo", + Name: "foo", + Type: "batch", + Priority: 10, + AllAtOnce: true, + Meta: map[string]string{ + "foo": "bar", + }, + }, + Expected: &JobDiff{ + Type: DiffTypeAdded, + ID: "foo", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "AllAtOnce", + Old: "", + New: "true", + }, + { + Type: DiffTypeAdded, + Name: "Meta[foo]", + Old: "", + New: "bar", + }, + { + Type: DiffTypeAdded, + Name: "Name", + Old: "", + New: "foo", + }, + { + Type: DiffTypeAdded, + Name: "Priority", + Old: "", + New: "10", + }, + { + Type: DiffTypeAdded, + Name: "Region", + Old: "", + New: "foo", + }, + { + Type: DiffTypeAdded, + Name: "Type", + Old: "", + New: "batch", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Update", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "MaxParallel", + Old: "", + New: "0", + }, + { + Type: DiffTypeAdded, + Name: "Stagger", + Old: "", + New: "0", + }, + }, + }, + }, }, }, { diff --git a/scheduler/annotate.go b/scheduler/annotate.go index 002164e74..66c405fd6 100644 --- a/scheduler/annotate.go +++ b/scheduler/annotate.go @@ -35,11 +35,6 @@ const ( // * forces create/destroy update // * forces in-place update func Annotate(diff *structs.JobDiff, annotations *structs.PlanAnnotations) error { - // No annotation needed as the job was either just submitted or deleted. - if diff.Type != structs.DiffTypeEdited { - return nil - } - tgDiffs := diff.TaskGroups if len(tgDiffs) == 0 { return nil @@ -56,12 +51,6 @@ func Annotate(diff *structs.JobDiff, annotations *structs.PlanAnnotations) error // annotateTaskGroup takes a task group diff and annotates it. func annotateTaskGroup(diff *structs.TaskGroupDiff, annotations *structs.PlanAnnotations) error { - // Don't annotate unless the task group was edited. If it was a - // create/destroy it is not needed. - if diff.Type != structs.DiffTypeEdited { - return nil - } - // Annotate the updates if annotations != nil { tg, ok := annotations.DesiredTGUpdates[diff.Name] @@ -103,7 +92,7 @@ func annotateTaskGroup(diff *structs.TaskGroupDiff, annotations *structs.PlanAnn } for _, taskDiff := range taskDiffs { - annotateTask(taskDiff) + annotateTask(taskDiff, diff) } return nil @@ -112,12 +101,6 @@ func annotateTaskGroup(diff *structs.TaskGroupDiff, annotations *structs.PlanAnn // annotateCountChange takes a task group diff and annotates the count // parameter. func annotateCountChange(diff *structs.TaskGroupDiff) error { - // Don't annotate unless the task was edited. If it was a create/destroy it - // is not needed. - if diff.Type != structs.DiffTypeEdited { - return nil - } - var countDiff *structs.FieldDiff for _, diff := range diff.Fields { if diff.Name == "Count" { @@ -130,14 +113,24 @@ func annotateCountChange(diff *structs.TaskGroupDiff) error { if countDiff == nil { return nil } - oldV, err := strconv.Atoi(countDiff.Old) - if err != nil { - return err + var oldV, newV int + var err error + if countDiff.Old == "" { + oldV = 0 + } else { + oldV, err = strconv.Atoi(countDiff.Old) + if err != nil { + return err + } } - newV, err := strconv.Atoi(countDiff.New) - if err != nil { - return err + if countDiff.New == "" { + newV = 0 + } else { + newV, err = strconv.Atoi(countDiff.New) + if err != nil { + return err + } } if oldV < newV { @@ -150,13 +143,22 @@ func annotateCountChange(diff *structs.TaskGroupDiff) error { } // annotateCountChange takes a task diff and annotates it. -func annotateTask(diff *structs.TaskDiff) { - // Don't annotate unless the task was edited. If it was a create/destroy it - // is not needed. - if diff.Type != structs.DiffTypeEdited { +func annotateTask(diff *structs.TaskDiff, parent *structs.TaskGroupDiff) { + if diff.Type == structs.DiffTypeNone { return } + // The whole task group is changing + if parent.Type == structs.DiffTypeAdded || parent.Type == structs.DiffTypeDeleted { + if diff.Type == structs.DiffTypeAdded { + diff.Annotations = append(diff.Annotations, AnnotationForcesCreate) + return + } else if diff.Type == structs.DiffTypeDeleted { + diff.Annotations = append(diff.Annotations, AnnotationForcesDestroy) + return + } + } + // All changes to primitive fields result in a destructive update. destructive := false if len(diff.Fields) != 0 { diff --git a/scheduler/annotate_test.go b/scheduler/annotate_test.go index acf589b85..9ffcac2e8 100644 --- a/scheduler/annotate_test.go +++ b/scheduler/annotate_test.go @@ -98,230 +98,325 @@ func TestAnnotateCountChange(t *testing.T) { } func TestAnnotateTask_NonEdited(t *testing.T) { - td := &structs.TaskDiff{} - tdOrig := &structs.TaskDiff{} - annotateTask(td) + tgd := &structs.TaskGroupDiff{Type: structs.DiffTypeNone} + td := &structs.TaskDiff{Type: structs.DiffTypeNone} + tdOrig := &structs.TaskDiff{Type: structs.DiffTypeNone} + annotateTask(td, tgd) if !reflect.DeepEqual(tdOrig, td) { t.Fatalf("annotateTask(%#v) should not have caused any annotation: %#v", tdOrig, td) } } -func TestAnnotateTask_Destructive(t *testing.T) { - cases := []*structs.TaskDiff{ +func TestAnnotateTask(t *testing.T) { + cases := []struct { + Diff *structs.TaskDiff + Parent *structs.TaskGroupDiff + Desired string + }{ { - Type: structs.DiffTypeEdited, - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeEdited, - Name: "Driver", - Old: "docker", - New: "exec", + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeEdited, + Name: "Driver", + Old: "docker", + New: "exec", + }, }, }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, }, { - Type: structs.DiffTypeEdited, - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeEdited, - Name: "User", - Old: "alice", - New: "bob", + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeEdited, + Name: "User", + Old: "alice", + New: "bob", + }, }, }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, }, { - Type: structs.DiffTypeEdited, - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeAdded, - Name: "Env[foo]", - Old: "foo", - New: "bar", + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Env[foo]", + Old: "foo", + New: "bar", + }, }, }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, }, { - Type: structs.DiffTypeEdited, - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeAdded, - Name: "Meta[foo]", - Old: "foo", - New: "bar", + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Meta[foo]", + Old: "foo", + New: "bar", + }, }, }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, }, { - Type: structs.DiffTypeEdited, - Objects: []*structs.ObjectDiff{ - { - Type: structs.DiffTypeAdded, - Name: "Artifact", - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeAdded, - Name: "GetterOptions[bam]", - Old: "", - New: "baz", - }, - { - Type: structs.DiffTypeAdded, - Name: "GetterSource", - Old: "", - New: "bam", - }, - { - Type: structs.DiffTypeAdded, - Name: "RelativeDest", - Old: "", - New: "bam", + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Artifact", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "GetterOptions[bam]", + Old: "", + New: "baz", + }, + { + Type: structs.DiffTypeAdded, + Name: "GetterSource", + Old: "", + New: "bam", + }, + { + Type: structs.DiffTypeAdded, + Name: "RelativeDest", + Old: "", + New: "bam", + }, }, }, }, }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, }, { - Type: structs.DiffTypeEdited, - Objects: []*structs.ObjectDiff{ - { - Type: structs.DiffTypeEdited, - Name: "Resources", - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeEdited, - Name: "CPU", - Old: "100", - New: "200", - }, - { - Type: structs.DiffTypeEdited, - Name: "DiskMB", - Old: "100", - New: "200", - }, - { - Type: structs.DiffTypeEdited, - Name: "IOPS", - Old: "100", - New: "200", - }, - { - Type: structs.DiffTypeEdited, - Name: "MemoryMB", - Old: "100", - New: "200", + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeEdited, + Name: "Resources", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeEdited, + Name: "CPU", + Old: "100", + New: "200", + }, + { + Type: structs.DiffTypeEdited, + Name: "DiskMB", + Old: "100", + New: "200", + }, + { + Type: structs.DiffTypeEdited, + Name: "IOPS", + Old: "100", + New: "200", + }, + { + Type: structs.DiffTypeEdited, + Name: "MemoryMB", + Old: "100", + New: "200", + }, }, }, }, }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, }, { - Type: structs.DiffTypeEdited, - Objects: []*structs.ObjectDiff{ - { - Type: structs.DiffTypeEdited, - Name: "Config", - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeEdited, - Name: "bam[1]", - Old: "b", - New: "c", + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeEdited, + Name: "Config", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeEdited, + Name: "bam[1]", + Old: "b", + New: "c", + }, }, }, }, }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, + }, + { + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Constraint", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "LTarget", + Old: "", + New: "baz", + }, + { + Type: structs.DiffTypeAdded, + Name: "Operand", + Old: "", + New: "baz", + }, + { + Type: structs.DiffTypeAdded, + Name: "RTarget", + Old: "", + New: "baz", + }, + }, + }, + }, + }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesInplaceUpdate, + }, + { + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeAdded, + Name: "LogConfig", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "MaxFileSizeMB", + Old: "", + New: "10", + }, + { + Type: structs.DiffTypeAdded, + Name: "MaxFiles", + Old: "", + New: "1", + }, + }, + }, + }, + }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesInplaceUpdate, + }, + { + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeEdited, + Objects: []*structs.ObjectDiff{ + { + Type: structs.DiffTypeEdited, + Name: "Service", + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeEdited, + Name: "PortLabel", + Old: "baz", + New: "baz2", + }, + }, + }, + }, + }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesInplaceUpdate, + }, + // Task deleted new parent + { + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeDeleted, + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Driver", + Old: "", + New: "exec", + }, + }, + }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeAdded}, + Desired: AnnotationForcesDestroy, + }, + // Task Added new parent + { + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeAdded, + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Driver", + Old: "", + New: "exec", + }, + }, + }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeAdded}, + Desired: AnnotationForcesCreate, + }, + // Task deleted existing parent + { + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeDeleted, + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Driver", + Old: "", + New: "exec", + }, + }, + }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, + }, + // Task Added existing parent + { + Diff: &structs.TaskDiff{ + Type: structs.DiffTypeAdded, + Fields: []*structs.FieldDiff{ + { + Type: structs.DiffTypeAdded, + Name: "Driver", + Old: "", + New: "exec", + }, + }, + }, + Parent: &structs.TaskGroupDiff{Type: structs.DiffTypeEdited}, + Desired: AnnotationForcesDestructiveUpdate, }, } for i, c := range cases { - c.Type = structs.DiffTypeEdited - annotateTask(c) - if len(c.Annotations) != 1 || c.Annotations[0] != AnnotationForcesDestructiveUpdate { - t.Fatalf("case %d not properly annotated %#v", i+1, c) - } - } -} - -func TestAnnotateTask_Inplace(t *testing.T) { - cases := []*structs.TaskDiff{ - { - Type: structs.DiffTypeEdited, - Objects: []*structs.ObjectDiff{ - { - Type: structs.DiffTypeAdded, - Name: "Constraint", - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeAdded, - Name: "LTarget", - Old: "", - New: "baz", - }, - { - Type: structs.DiffTypeAdded, - Name: "Operand", - Old: "", - New: "baz", - }, - { - Type: structs.DiffTypeAdded, - Name: "RTarget", - Old: "", - New: "baz", - }, - }, - }, - }, - }, - { - Type: structs.DiffTypeEdited, - Objects: []*structs.ObjectDiff{ - { - Type: structs.DiffTypeAdded, - Name: "LogConfig", - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeAdded, - Name: "MaxFileSizeMB", - Old: "", - New: "10", - }, - { - Type: structs.DiffTypeAdded, - Name: "MaxFiles", - Old: "", - New: "1", - }, - }, - }, - }, - }, - { - Type: structs.DiffTypeEdited, - Objects: []*structs.ObjectDiff{ - { - Type: structs.DiffTypeEdited, - Name: "Service", - Fields: []*structs.FieldDiff{ - { - Type: structs.DiffTypeEdited, - Name: "PortLabel", - Old: "baz", - New: "baz2", - }, - }, - }, - }, - }, - } - - for i, c := range cases { - c.Type = structs.DiffTypeEdited - annotateTask(c) - if len(c.Annotations) != 1 || c.Annotations[0] != AnnotationForcesInplaceUpdate { - t.Fatalf("case %d not properly annotated %#v", i+1, c) + annotateTask(c.Diff, c.Parent) + if len(c.Diff.Annotations) != 1 || c.Diff.Annotations[0] != c.Desired { + t.Fatalf("case %d not properly annotated; got %s, want %s", i+1, c.Diff.Annotations[0], c.Desired) } } } diff --git a/scheduler/util.go b/scheduler/util.go index 76f0f578b..e6ece1417 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -520,7 +520,7 @@ func desiredUpdates(diff *diffResult, inplaceUpdates, } for _, tuple := range diff.stop { - name := tuple.TaskGroup.Name + name := tuple.Alloc.TaskGroup des, ok := desiredTgs[name] if !ok { des = &structs.DesiredUpdates{} diff --git a/scheduler/util_test.go b/scheduler/util_test.go index 61b549cbe..b2f518e57 100644 --- a/scheduler/util_test.go +++ b/scheduler/util_test.go @@ -783,7 +783,7 @@ func TestProgressMade(t *testing.T) { func TestDesiredUpdates(t *testing.T) { tg1 := &structs.TaskGroup{Name: "foo"} tg2 := &structs.TaskGroup{Name: "bar"} - a2 := &structs.Allocation{TaskGroup: "foo"} + a2 := &structs.Allocation{TaskGroup: "bar"} place := []allocTuple{ allocTuple{TaskGroup: tg1}, From 5085c25f8b0e415873fb1b74d36dc091c0df2d44 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 16 May 2016 11:48:44 -0700 Subject: [PATCH 16/17] Rename Cas to JobModifyIndex --- nomad/job_endpoint.go | 2 +- nomad/job_endpoint_test.go | 8 ++++---- nomad/structs/structs.go | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 40cec0940..ec9186f8c 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -469,7 +469,7 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) reply.Diff = jobDiff } - reply.Cas = index + reply.JobModifyIndex = index reply.Annotations = annotations reply.CreatedEvals = planner.CreatedEvals reply.Index = index diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 8569de091..47cc2e9cc 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -953,8 +953,8 @@ func TestJobEndpoint_Plan_WithDiff(t *testing.T) { } // Check the response - if planResp.Cas == 0 { - t.Fatalf("bad cas: %d", planResp.Cas) + if planResp.JobModifyIndex == 0 { + t.Fatalf("bad cas: %d", planResp.JobModifyIndex) } if planResp.Annotations == nil { t.Fatalf("no annotations") @@ -1002,8 +1002,8 @@ func TestJobEndpoint_Plan_NoDiff(t *testing.T) { } // Check the response - if planResp.Cas == 0 { - t.Fatalf("bad cas: %d", planResp.Cas) + if planResp.JobModifyIndex == 0 { + t.Fatalf("bad cas: %d", planResp.JobModifyIndex) } if planResp.Annotations == nil { t.Fatalf("no annotations") diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 4b1142a88..bfc2b221d 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -403,10 +403,10 @@ type JobPlanResponse struct { // Annotations stores annotations explaining decisions the scheduler made. Annotations *PlanAnnotations - // The Cas value can be used when running `nomad run` to ensure that the Job - // wasn’t modified since the last plan. If the job is being created, the - // value is zero. - Cas uint64 + // JobModifyIndex is the modification index of the job. The value can be + // used when running `nomad run` to ensure that the Job wasn’t modified + // since the last plan. If the job is being created, the value is zero. + JobModifyIndex uint64 // CreatedEvals is the set of evaluations created by the scheduler. The // reasons for this can be rolling-updates or blocked evals. From a231f6f9982d6b2021801c5e50807fd5927caa01 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 16 May 2016 12:49:18 -0700 Subject: [PATCH 17/17] Switch to using the harness --- api/jobs.go | 8 ++--- api/jobs_test.go | 8 ++--- command/agent/job_endpoint_test.go | 2 +- nomad/job_endpoint.go | 11 +++++-- scheduler/inmem_planner.go | 36 --------------------- scheduler/{scheduler_test.go => testing.go} | 13 ++------ scheduler/util_test.go | 7 ++++ 7 files changed, 27 insertions(+), 58 deletions(-) delete mode 100644 scheduler/inmem_planner.go rename scheduler/{scheduler_test.go => testing.go} (92%) diff --git a/api/jobs.go b/api/jobs.go index 91fc2aa9b..71c761caa 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -282,10 +282,10 @@ type JobPlanRequest struct { } type JobPlanResponse struct { - Cas uint64 - CreatedEvals []*Evaluation - Diff *JobDiff - Annotations *PlanAnnotations + JobModifyIndex uint64 + CreatedEvals []*Evaluation + Diff *JobDiff + Annotations *PlanAnnotations } type JobDiff struct { diff --git a/api/jobs_test.go b/api/jobs_test.go index 99c744082..8ca0b55fc 100644 --- a/api/jobs_test.go +++ b/api/jobs_test.go @@ -380,8 +380,8 @@ func TestJobs_Plan(t *testing.T) { t.Fatalf("nil response") } - if planResp.Cas == 0 { - t.Fatalf("bad Cas value: %#v", planResp) + if planResp.JobModifyIndex == 0 { + t.Fatalf("bad JobModifyIndex value: %#v", planResp) } if planResp.Diff == nil { t.Fatalf("got nil diff: %#v", planResp) @@ -405,8 +405,8 @@ func TestJobs_Plan(t *testing.T) { t.Fatalf("nil response") } - if planResp.Cas == 0 { - t.Fatalf("bad Cas value: %d", planResp.Cas) + if planResp.JobModifyIndex == 0 { + t.Fatalf("bad JobModifyIndex value: %d", planResp.JobModifyIndex) } if planResp.Diff != nil { t.Fatalf("got non-nil diff: %#v", planResp) diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 22c448525..c0de9ee3e 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -510,7 +510,7 @@ func TestHTTP_JobPlan(t *testing.T) { // Check the response plan := obj.(structs.JobPlanResponse) - if plan.Plan == nil { + if plan.Annotations == nil { t.Fatalf("bad: %v", plan) } diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index ec9186f8c..13a8c9e44 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -443,7 +443,9 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) // Create an in-memory Planner that returns no errors and stores the // submitted plan and created evals. - planner := scheduler.NewInMemoryPlanner() + planner := &scheduler.Harness{ + State: &snap.StateStore, + } // Create the scheduler and run it sched, err := scheduler.NewScheduler(eval.Type, j.srv.logger, snap, planner) @@ -456,7 +458,10 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) } // Annotate and store the diff - annotations := planner.Plan.Annotations + if plans := len(planner.Plans); plans != 1 { + return fmt.Errorf("scheduler resulted in an unexpected number of plans: %d", plans) + } + annotations := planner.Plans[0].Annotations if args.Diff { jobDiff, err := oldJob.Diff(args.Job, true) if err != nil { @@ -471,7 +476,7 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) reply.JobModifyIndex = index reply.Annotations = annotations - reply.CreatedEvals = planner.CreatedEvals + reply.CreatedEvals = planner.CreateEvals reply.Index = index return nil } diff --git a/scheduler/inmem_planner.go b/scheduler/inmem_planner.go deleted file mode 100644 index 1c9ccbbd6..000000000 --- a/scheduler/inmem_planner.go +++ /dev/null @@ -1,36 +0,0 @@ -package scheduler - -import "github.com/hashicorp/nomad/nomad/structs" - -// InMemPlanner is an in-memory Planner that can be used to invoke the scheduler -// without side-effects. -type InMemPlanner struct { - CreatedEvals []*structs.Evaluation - Plan *structs.Plan -} - -// NewInMemoryPlanner returns a new in-memory planner. -func NewInMemoryPlanner() *InMemPlanner { - return &InMemPlanner{} -} - -func (i *InMemPlanner) SubmitPlan(plan *structs.Plan) (*structs.PlanResult, State, error) { - i.Plan = plan - - // Create a fully committed plan result. - result := &structs.PlanResult{ - NodeUpdate: plan.NodeUpdate, - NodeAllocation: plan.NodeAllocation, - } - - return result, nil, nil -} - -func (i *InMemPlanner) UpdateEval(eval *structs.Evaluation) error { - return nil -} - -func (i *InMemPlanner) CreateEval(eval *structs.Evaluation) error { - i.CreatedEvals = append(i.CreatedEvals, eval) - return nil -} diff --git a/scheduler/scheduler_test.go b/scheduler/testing.go similarity index 92% rename from scheduler/scheduler_test.go rename to scheduler/testing.go index 526ac2d90..fbd1aeda7 100644 --- a/scheduler/scheduler_test.go +++ b/scheduler/testing.go @@ -29,9 +29,9 @@ func (r *RejectPlan) CreateEval(*structs.Evaluation) error { return nil } -// Harness is a lightweight testing harness for schedulers. -// It manages a state store copy and provides the planner -// interface. It can be extended for various testing uses. +// Harness is a lightweight testing harness for schedulers. It manages a state +// store copy and provides the planner interface. It can be extended for various +// testing uses or for invoking the scheduler without side effects. type Harness struct { State *state.StateStore @@ -178,10 +178,3 @@ func (h *Harness) AssertEvalStatus(t *testing.T, state string) { t.Fatalf("bad: %#v", update) } } - -// noErr is used to assert there are no errors -func noErr(t *testing.T, err error) { - if err != nil { - t.Fatalf("err: %v", err) - } -} diff --git a/scheduler/util_test.go b/scheduler/util_test.go index b2f518e57..0d5cc915b 100644 --- a/scheduler/util_test.go +++ b/scheduler/util_test.go @@ -12,6 +12,13 @@ import ( "github.com/hashicorp/nomad/nomad/structs" ) +// noErr is used to assert there are no errors +func noErr(t *testing.T, err error) { + if err != nil { + t.Fatalf("err: %v", err) + } +} + func TestMaterializeTaskGroups(t *testing.T) { job := mock.Job() index := materializeTaskGroups(job)