From 4a16a317ee5f33d823c518d807279bb2a932540c Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 2 May 2019 11:58:04 -0400 Subject: [PATCH] client: handle 0.8 server network resources Fixes https://github.com/hashicorp/nomad/issues/5587 When a nomad 0.9 client is handling an alloc generated by a nomad 0.8 server, we should check the alloc.TaskResources for networking details rather than task.Resources. We check alloc.TaskResources for networking for other tasks in the task group [1], so it's a bit odd that we used the task.Resources struct here. TaskRunner also uses `alloc.TaskResources`[2]. The task.Resources struct in 0.8 was sparsly populated, resulting to storing of 0 in port mapping env vars: ``` vagrant@nomad-server-01:~$ nomad version Nomad v0.8.7 (21a2d93eecf018ad2209a5eab6aae6c359267933+CHANGES) vagrant@nomad-server-01:~$ nomad server members Name Address Port Status Leader Protocol Build Datacenter Region nomad-server-01.global 10.199.0.11 4648 alive true 2 0.8.7 dc1 global vagrant@nomad-server-01:~$ nomad alloc status -json 5b34649b | jq '.Job.TaskGroups[0].Tasks[0].Resources.Networks' [ { "CIDR": "", "Device": "", "DynamicPorts": [ { "Label": "db", "Value": 0 } ], "IP": "", "MBits": 10, "ReservedPorts": null } ] vagrant@nomad-server-01:~$ nomad alloc status -json 5b34649b | jq '.TaskResources' { "redis": { "CPU": 500, "DiskMB": 0, "IOPS": 0, "MemoryMB": 256, "Networks": [ { "CIDR": "", "Device": "eth1", "DynamicPorts": [ { "Label": "db", "Value": 21722 } ], "IP": "10.199.0.21", "MBits": 10, "ReservedPorts": null } ] } } ``` Also, updated the test values to mimic how Nomad 0.8 structs are represented, and made its result match the non compact values in `TestEnvironment_AsList`. [1] https://github.com/hashicorp/nomad/blob/24e9040b18a4f893e2f353288948a0f7cd9d82e4/client/taskenv/env.go#L624-L639 [2] https://github.com/hashicorp/nomad/blob/master/client/allocrunner/taskrunner/task_runner.go#L287-L303 --- client/taskenv/env.go | 15 +++++++++------ client/taskenv/env_test.go | 16 ++++++++-------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/client/taskenv/env.go b/client/taskenv/env.go index 78eecf4d5d0..5b42a17a753 100644 --- a/client/taskenv/env.go +++ b/client/taskenv/env.go @@ -544,15 +544,9 @@ func (b *Builder) setTask(task *structs.Task) *Builder { if task.Resources == nil { b.memLimit = 0 b.cpuLimit = 0 - b.networks = []*structs.NetworkResource{} } else { b.memLimit = int64(task.Resources.MemoryMB) b.cpuLimit = int64(task.Resources.CPU) - // Copy networks to prevent sharing - b.networks = make([]*structs.NetworkResource, len(task.Resources.Networks)) - for i, n := range task.Resources.Networks { - b.networks[i] = n.Copy() - } } return b } @@ -622,6 +616,15 @@ func (b *Builder) setAlloc(alloc *structs.Allocation) *Builder { } } } 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 { diff --git a/client/taskenv/env_test.go b/client/taskenv/env_test.go index 457befbf367..531d8f16840 100644 --- a/client/taskenv/env_test.go +++ b/client/taskenv/env_test.go @@ -228,12 +228,11 @@ func TestEnvironment_AsList_Old(t *testing.T) { Device: "eth0", IP: "192.168.0.100", ReservedPorts: []structs.Port{ - {Label: "admin", Value: 5000}, {Label: "ssh", Value: 22}, {Label: "other", Value: 1234}, }, MBits: 50, - DynamicPorts: []structs.Port{{Label: "http"}}, + DynamicPorts: []structs.Port{{Label: "http", Value: 2000}}, }, }, } @@ -244,10 +243,10 @@ func TestEnvironment_AsList_Old(t *testing.T) { Networks: []*structs.NetworkResource{ { Device: "eth0", - IP: "192.168.0.100", - ReservedPorts: []structs.Port{{Label: "admin", Value: 5000}}, + IP: "127.0.0.1", + ReservedPorts: []structs.Port{{Label: "https", Value: 8080}}, MBits: 50, - DynamicPorts: []structs.Port{{Label: "http", Value: 2000}}, + DynamicPorts: []structs.Port{{Label: "http", Value: 80}}, }, }, }, @@ -270,10 +269,11 @@ func TestEnvironment_AsList_Old(t *testing.T) { "taskEnvKey": "taskEnvVal", } task.Resources.Networks = []*structs.NetworkResource{ + // Nomad 0.8 didn't fully populate the fields in task Resource Networks { - IP: "127.0.0.1", - ReservedPorts: []structs.Port{{Label: "http", Value: 80}}, - DynamicPorts: []structs.Port{{Label: "https", Value: 8080}}, + IP: "", + ReservedPorts: []structs.Port{{Label: "https"}}, + DynamicPorts: []structs.Port{{Label: "http"}}, }, } env := NewBuilder(n, a, task, "global").SetDriverNetwork(