From 058076afd045b5aed4f4779927656c3f466da604 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 9 Jan 2020 09:25:07 -0500 Subject: [PATCH] client: stop using alloc.TaskResources Now that alloc.Canonicalize() is called in all alloc sources in the client (i.e. on state restore and RPC fetching), we no longer need to check alloc.TaskResources. alloc.AllocatedResources is always non-nil through alloc runner. Though, early on, we check for alloc validity, so NewTaskRunner and TaskEnv must still check. `TestClient_AddAllocError` test validates that behavior. --- client/allocrunner/taskrunner/service_hook.go | 24 ++---------- client/allocrunner/taskrunner/task_runner.go | 39 +++---------------- client/taskenv/env.go | 25 ------------ client/taskenv/env_test.go | 4 ++ 4 files changed, 14 insertions(+), 78 deletions(-) diff --git a/client/allocrunner/taskrunner/service_hook.go b/client/allocrunner/taskrunner/service_hook.go index 956033f3f..02b8d75b7 100644 --- a/client/allocrunner/taskrunner/service_hook.go +++ b/client/allocrunner/taskrunner/service_hook.go @@ -63,16 +63,8 @@ func newServiceHook(c serviceHookConfig) *serviceHook { delay: c.task.ShutdownDelay, } - // COMPAT(0.11): AllocatedResources was added in 0.9 so assume its set - // in 0.11. - if c.alloc.AllocatedResources != nil { - if res := c.alloc.AllocatedResources.Tasks[c.task.Name]; res != nil { - h.networks = res.Networks - } - } else { - if res := c.alloc.TaskResources[c.task.Name]; res != nil { - h.networks = res.Networks - } + if res := c.alloc.AllocatedResources.Tasks[c.task.Name]; res != nil { + h.networks = res.Networks } if c.alloc.DeploymentStatus != nil && c.alloc.DeploymentStatus.Canary { @@ -116,17 +108,9 @@ func (h *serviceHook) Update(ctx context.Context, req *interfaces.TaskUpdateRequ canary = req.Alloc.DeploymentStatus.Canary } - // COMPAT(0.11): AllocatedResources was added in 0.9 so assume its set - // in 0.11. var networks structs.Networks - if req.Alloc.AllocatedResources != nil { - if res := req.Alloc.AllocatedResources.Tasks[h.taskName]; res != nil { - networks = res.Networks - } - } else { - if res := req.Alloc.TaskResources[h.taskName]; res != nil { - networks = res.Networks - } + if res := req.Alloc.AllocatedResources.Tasks[h.taskName]; res != nil { + networks = res.Networks } task := req.Alloc.LookupTask(h.taskName) diff --git a/client/allocrunner/taskrunner/task_runner.go b/client/allocrunner/taskrunner/task_runner.go index 9f6e5ac34..506a3aca9 100644 --- a/client/allocrunner/taskrunner/task_runner.go +++ b/client/allocrunner/taskrunner/task_runner.go @@ -301,23 +301,7 @@ func NewTaskRunner(config *Config) (*TaskRunner, error) { } tr.taskResources = tres } else { - // COMPAT(0.11): Upgrade from 0.8 resources to 0.9+ resources - // Grab the old task resources - oldTr, ok := tr.alloc.TaskResources[tr.taskName] - if !ok { - return nil, fmt.Errorf("no task resources found on allocation") - } - - // Convert the old to new - tr.taskResources = &structs.AllocatedTaskResources{ - Cpu: structs.AllocatedCpuResources{ - CpuShares: int64(oldTr.CPU), - }, - Memory: structs.AllocatedMemoryResources{ - MemoryMB: int64(oldTr.MemoryMB), - }, - Networks: oldTr.Networks, - } + return nil, fmt.Errorf("no task resources found on allocation") } // Build the restart tracker. @@ -1253,15 +1237,9 @@ func (tr *TaskRunner) UpdateStats(ru *cstructs.TaskResourceUsage) { func (tr *TaskRunner) setGaugeForMemory(ru *cstructs.TaskResourceUsage) { alloc := tr.Alloc() var allocatedMem float32 - if alloc.AllocatedResources != nil { - if taskRes := alloc.AllocatedResources.Tasks[tr.taskName]; taskRes != nil { - // Convert to bytes to match other memory metrics - allocatedMem = float32(taskRes.Memory.MemoryMB) * 1024 * 1024 - } - } else if taskRes := alloc.TaskResources[tr.taskName]; taskRes != nil { - // COMPAT(0.11) Remove in 0.11 when TaskResources is removed - allocatedMem = float32(taskRes.MemoryMB) * 1024 * 1024 - + if taskRes := alloc.AllocatedResources.Tasks[tr.taskName]; taskRes != nil { + // Convert to bytes to match other memory metrics + allocatedMem = float32(taskRes.Memory.MemoryMB) * 1024 * 1024 } if !tr.clientConfig.DisableTaggedMetrics { @@ -1303,13 +1281,8 @@ func (tr *TaskRunner) setGaugeForMemory(ru *cstructs.TaskResourceUsage) { func (tr *TaskRunner) setGaugeForCPU(ru *cstructs.TaskResourceUsage) { alloc := tr.Alloc() var allocatedCPU float32 - if alloc.AllocatedResources != nil { - if taskRes := alloc.AllocatedResources.Tasks[tr.taskName]; taskRes != nil { - allocatedCPU = float32(taskRes.Cpu.CpuShares) - } - } else if taskRes := alloc.TaskResources[tr.taskName]; taskRes != nil { - // COMPAT(0.11) Remove in 0.11 when TaskResources is removed - allocatedCPU = float32(taskRes.CPU) + if taskRes := alloc.AllocatedResources.Tasks[tr.taskName]; taskRes != nil { + allocatedCPU = float32(taskRes.Cpu.CpuShares) } if !tr.clientConfig.DisableTaggedMetrics { diff --git a/client/taskenv/env.go b/client/taskenv/env.go index 57402b442..0be24d63f 100644 --- a/client/taskenv/env.go +++ b/client/taskenv/env.go @@ -603,7 +603,6 @@ func (b *Builder) setAlloc(alloc *structs.Allocation) *Builder { tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup) - // COMPAT(0.11): Remove in 0.11 b.otherPorts = make(map[string]string, len(tg.Tasks)*2) if alloc.AllocatedResources != nil { // Populate task resources @@ -645,30 +644,6 @@ func (b *Builder) setAlloc(alloc *structs.Allocation) *Builder { addGroupPort(b.otherPorts, p) } } - } else if alloc.TaskResources != nil { - if tr, ok := alloc.TaskResources[b.taskName]; ok { - // Copy networks to prevent sharing - b.networks = make([]*structs.NetworkResource, len(tr.Networks)) - for i, n := range tr.Networks { - b.networks[i] = n.Copy() - } - - } - - for taskName, resources := range alloc.TaskResources { - // Add ports from other tasks - if taskName == b.taskName { - continue - } - for _, nw := range resources.Networks { - for _, p := range nw.ReservedPorts { - addPort(b.otherPorts, taskName, nw.IP, p.Label, p.Value) - } - for _, p := range nw.DynamicPorts { - addPort(b.otherPorts, taskName, nw.IP, p.Label, p.Value) - } - } - } } upstreams := []structs.ConsulUpstream{} diff --git a/client/taskenv/env_test.go b/client/taskenv/env_test.go index f68bb05d7..035291436 100644 --- a/client/taskenv/env_test.go +++ b/client/taskenv/env_test.go @@ -266,6 +266,10 @@ func TestEnvironment_AsList_Old(t *testing.T) { }, }, } + + // simulate canonicalization on restore or fetch + a.Canonicalize() + task := a.Job.TaskGroups[0].Tasks[0] task.Env = map[string]string{ "taskEnvKey": "taskEnvVal",