From 61c52a4a69712ab22c78c2967b0ec39141fd0dda Mon Sep 17 00:00:00 2001 From: Michael Schurter <mschurter@hashicorp.com> Date: Fri, 3 Dec 2021 17:06:20 -0800 Subject: [PATCH] wip alloc copy fixes --- nomad/structs/funcs.go | 1 + nomad/structs/network.go | 1 + nomad/structs/structs.go | 48 +++++++++++++++++++++++++++++++--------- scheduler/rank.go | 1 + scheduler/util.go | 11 +++------ 5 files changed, 44 insertions(+), 18 deletions(-) diff --git a/nomad/structs/funcs.go b/nomad/structs/funcs.go index 1820443a7b..0350ff2982 100644 --- a/nomad/structs/funcs.go +++ b/nomad/structs/funcs.go @@ -193,6 +193,7 @@ func AllocsFit(node *Node, allocs []*Allocation, netIdx *NetworkIndex, checkDevi } // Check if the network is overcommitted + //XXX Remove if netIdx.Overcommitted() { return false, "bandwidth exceeded", used, nil } diff --git a/nomad/structs/network.go b/nomad/structs/network.go index 76de50bfc6..ce6e4a3086 100644 --- a/nomad/structs/network.go +++ b/nomad/structs/network.go @@ -216,6 +216,7 @@ func (idx *NetworkIndex) AddReserved(n *NetworkResource) (collide bool) { for _, port := range ports { // Guard against invalid port if port.Value < 0 || port.Value >= MaxValidPort { + //XXX Is it safe to return early here? or should we continue so used is properly updated return true } if used.Check(uint(port.Value)) { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 50f9f640b7..44d1dc5b8f 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2529,6 +2529,12 @@ func (p AllocatedPorts) Get(label string) (AllocatedPortMapping, bool) { return AllocatedPortMapping{}, false } +func (p AllocatedPorts) Copy() AllocatedPorts { + n := make(AllocatedPorts, len(p)) + copy(n, p) + return n +} + type Port struct { // Label is the key for HCL port stanzas: port "foo" {} Label string @@ -3690,7 +3696,7 @@ func (a AllocatedSharedResources) Copy() AllocatedSharedResources { return AllocatedSharedResources{ Networks: a.Networks.Copy(), DiskMB: a.DiskMB, - Ports: a.Ports, + Ports: a.Ports.Copy(), } } @@ -9121,6 +9127,21 @@ type DesiredTransition struct { ForceReschedule *bool } +// Copy returns a copy of the DesiredTransition +func (d DesiredTransition) Copy() DesiredTransition { + cpy := DesiredTransition{} + if d.Migrate != nil { + cpy.Migrate = helper.BoolToPtr(*d.Migrate) + } + if d.Reschedule != nil { + cpy.Reschedule = helper.BoolToPtr(*d.Reschedule) + } + if d.ForceReschedule != nil { + cpy.ForceReschedule = helper.BoolToPtr(*d.ForceReschedule) + } + return cpy +} + // Merge merges the two desired transitions, preferring the values from the // passed in object. func (d *DesiredTransition) Merge(o *DesiredTransition) { @@ -9373,9 +9394,9 @@ func (a *Allocation) copyImpl(job bool) *Allocation { na.Job = na.Job.Copy() } - na.AllocatedResources = na.AllocatedResources.Copy() - na.Resources = na.Resources.Copy() - na.SharedResources = na.SharedResources.Copy() + na.AllocatedResources = a.AllocatedResources.Copy() + na.Resources = a.Resources.Copy() + na.SharedResources = a.SharedResources.Copy() if a.TaskResources != nil { tr := make(map[string]*Resources, len(na.TaskResources)) @@ -9385,18 +9406,25 @@ func (a *Allocation) copyImpl(job bool) *Allocation { na.TaskResources = tr } - na.Metrics = na.Metrics.Copy() - na.DeploymentStatus = na.DeploymentStatus.Copy() + na.Metrics = a.Metrics.Copy() + na.DesiredTransition = a.DesiredTransition.Copy() if a.TaskStates != nil { - ts := make(map[string]*TaskState, len(na.TaskStates)) - for task, state := range na.TaskStates { - ts[task] = state.Copy() + na.TaskStates = make(map[string]*TaskState, len(a.TaskStates)) + for task, state := range a.TaskStates { + na.TaskStates[task] = state.Copy() + } + } + if a.AllocStates != nil { + na.AllocStates = make([]*AllocState, len(a.AllocStates)) + for i, s := range a.AllocStates { + na.AllocStates[i] = s } - na.TaskStates = ts } + na.DeploymentStatus = a.DeploymentStatus.Copy() na.RescheduleTracker = a.RescheduleTracker.Copy() + na.NetworkStatus = a.NetworkStatus.Copy() na.PreemptedAllocations = helper.CopySliceString(a.PreemptedAllocations) return na } diff --git a/scheduler/rank.go b/scheduler/rank.go index 9bd19b9bd4..0a483e298e 100644 --- a/scheduler/rank.go +++ b/scheduler/rank.go @@ -378,6 +378,7 @@ OUTER: } // Reserve this to prevent another task from colliding + //XXX Check return value?! netIdx.AddReserved(offer) // Update the network ask to the offer diff --git a/scheduler/util.go b/scheduler/util.go index 869442b78f..0fcd2ff54c 100644 --- a/scheduler/util.go +++ b/scheduler/util.go @@ -799,9 +799,8 @@ func inplaceUpdate(ctx Context, eval *structs.Evaluation, job *structs.Job, resources.Devices = devices } - // Create a shallow copy - newAlloc := new(structs.Allocation) - *newAlloc = *update.Alloc + // Create a copy + newAlloc := update.Alloc.Copy() // Update the allocation newAlloc.EvalID = eval.ID @@ -810,11 +809,7 @@ func inplaceUpdate(ctx Context, eval *structs.Evaluation, job *structs.Job, newAlloc.AllocatedResources = &structs.AllocatedResources{ Tasks: option.TaskResources, TaskLifecycles: option.TaskLifecycles, - Shared: structs.AllocatedSharedResources{ - DiskMB: int64(update.TaskGroup.EphemeralDisk.SizeMB), - Ports: update.Alloc.AllocatedResources.Shared.Ports, - Networks: update.Alloc.AllocatedResources.Shared.Networks.Copy(), - }, + Shared: update.Alloc.AllocatedResources.Shared.Copy(), } newAlloc.Metrics = ctx.Metrics() ctx.Plan().AppendAlloc(newAlloc, nil) -- GitLab