-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
client: handle 0.8 server network resources #5641
Conversation
Fixes #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 (21a2d93+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
MBits: 50, | ||
DynamicPorts: []structs.Port{{Label: "http", Value: 2000}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unclear how values in the tests were chosen - given that rest of tests don't reference admin
port and 2000/5000 don't appear in the final expected results. I did my best to capture what the intent of test is and made values consistent with how this and the values in TestEnvironment_AsList
so both tests get the same expected output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're moving everything to use the AllocatedResources
field, this is still needed for compatibility but I would add a comment.
nomad/nomad/structs/structs.go
Lines 7382 to 7386 in a988ae8
// COMPAT(0.11): Remove in 0.11 | |
// TaskResources is the set of resources allocated to each | |
// task. These should sum to the total Resources. Dynamic ports will be | |
// set by the scheduler. | |
TaskResources map[string]*Resources |
The if statement where this appears now is entirely about compatibility and already commented in Lines 587 to 589 in 4a16a31
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #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:
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]
nomad/client/taskenv/env.go
Lines 624 to 639 in 24e9040
[2] https://github.com/hashicorp/nomad/blob/master/client/allocrunner/taskrunner/task_runner.go#L287-L303