diff --git a/command/agent/alloc_endpoint.go b/command/agent/alloc_endpoint.go index 42d2d761d..5988ae69d 100644 --- a/command/agent/alloc_endpoint.go +++ b/command/agent/alloc_endpoint.go @@ -94,6 +94,7 @@ func (s *HTTPServer) allocGet(allocID string, resp http.ResponseWriter, req *htt } // Decode the payload if there is any + alloc := out.Alloc if alloc.Job != nil && len(alloc.Job.Payload) != 0 { decoded, err := snappy.Decode(nil, alloc.Job.Payload) @@ -105,6 +106,9 @@ func (s *HTTPServer) allocGet(allocID string, resp http.ResponseWriter, req *htt } alloc.SetEventDisplayMessages() + // Handle 0.12 ports upgrade path + alloc.AllocatedResources.Shared.Canonicalize() + return alloc, nil } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 3a77d15dc..0b87f77ea 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -3486,6 +3486,23 @@ func (a *AllocatedSharedResources) Subtract(delta *AllocatedSharedResources) { a.DiskMB -= delta.DiskMB } +func (a *AllocatedSharedResources) Canonicalize() { + if len(a.Networks) > 0 { + if len(a.Networks[0].DynamicPorts)+len(a.Networks[0].ReservedPorts) > 0 && len(a.Ports) == 0 { + for _, ports := range [][]Port{a.Networks[0].DynamicPorts, a.Networks[0].ReservedPorts} { + for _, p := range ports { + a.Ports = append(a.Ports, AllocatedPortMapping{ + Label: p.Label, + Value: p.Value, + To: p.To, + HostIP: a.Networks[0].IP, + }) + } + } + } + } +} + // AllocatedCpuResources captures the allocated CPU resources. type AllocatedCpuResources struct { CpuShares int64 diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 94bbe1cb6..b48a5aeb3 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -5508,3 +5508,43 @@ func TestNodeResources_Merge(t *testing.T) { }, }, res) } + +func TestAllocatedSharedResources_Canonicalize(t *testing.T) { + a := &AllocatedSharedResources{ + Networks: []*NetworkResource{ + { + IP: "127.0.0.1", + DynamicPorts: []Port{ + { + Label: "http", + Value: 22222, + To: 8080, + }, + }, + ReservedPorts: []Port{ + { + Label: "redis", + Value: 6783, + To: 6783, + }, + }, + }, + }, + } + + a.Canonicalize() + require.Exactly(t, AllocatedPorts{ + { + Label: "http", + Value: 22222, + To: 8080, + HostIP: "127.0.0.1", + }, + { + Label: "redis", + Value: 6783, + To: 6783, + HostIP: "127.0.0.1", + }, + }, a.Ports) +} diff --git a/ui/app/controllers/allocations/allocation/index.js b/ui/app/controllers/allocations/allocation/index.js index e167cdbe6..d70f311a7 100644 --- a/ui/app/controllers/allocations/allocation/index.js +++ b/ui/app/controllers/allocations/allocation/index.js @@ -39,7 +39,10 @@ export default class IndexController extends Controller.extend(Sortable) { }) error; - @alias('model.allocatedResources.networks.firstObject') network; + @computed('model.allocatedResources.ports.@each.label') + get ports() { + return (this.get('model.allocatedResources.ports') || []).sortBy('label'); + } @computed('model.taskGroup.services.@each.name') get services() { diff --git a/ui/app/controllers/allocations/allocation/task/index.js b/ui/app/controllers/allocations/allocation/task/index.js index 994f0319f..6af893dff 100644 --- a/ui/app/controllers/allocations/allocation/task/index.js +++ b/ui/app/controllers/allocations/allocation/task/index.js @@ -1,7 +1,6 @@ import Controller from '@ember/controller'; import { computed } from '@ember/object'; import { computed as overridable } from 'ember-overridable-computed'; -import { alias } from '@ember/object/computed'; import { task } from 'ember-concurrency'; import classic from 'ember-classic-decorator'; @@ -18,26 +17,6 @@ export default class IndexController extends Controller { return this.otherTaskStates.filterBy('task.lifecycle'); } - @alias('model.resources.networks.firstObject') network; - - @computed('network.{reservedPorts.[],dynamicPorts.[]}') - get ports() { - return (this.get('network.reservedPorts') || []) - .map(port => ({ - name: port.Label, - port: port.Value, - isDynamic: false, - })) - .concat( - (this.get('network.dynamicPorts') || []).map(port => ({ - name: port.Label, - port: port.Value, - isDynamic: true, - })) - ) - .sortBy('name'); - } - @overridable(() => { // { title, description } return null; diff --git a/ui/app/models/port.js b/ui/app/models/port.js new file mode 100644 index 000000000..09888274e --- /dev/null +++ b/ui/app/models/port.js @@ -0,0 +1,9 @@ +import attr from 'ember-data/attr'; +import Fragment from 'ember-data-model-fragments/fragment'; + +export default class Port extends Fragment { + @attr('string') hostIp; + @attr('string') label; + @attr('number') to; + @attr('number') value; +} diff --git a/ui/app/models/resources.js b/ui/app/models/resources.js index 9d6082009..0cf455549 100644 --- a/ui/app/models/resources.js +++ b/ui/app/models/resources.js @@ -8,4 +8,5 @@ export default class Resources extends Fragment { @attr('number') disk; @attr('number') iops; @fragmentArray('network', { defaultValue: () => [] }) networks; + @fragmentArray('port', { defaultValue: () => [] }) ports; } diff --git a/ui/app/serializers/port.js b/ui/app/serializers/port.js new file mode 100644 index 000000000..fd3681fa9 --- /dev/null +++ b/ui/app/serializers/port.js @@ -0,0 +1,18 @@ +import ApplicationSerializer from './application'; +import isIp from 'is-ip'; + +export default class PortSerializer extends ApplicationSerializer { + attrs = { + hostIp: 'HostIP', + }; + + normalize(typeHash, hash) { + const ip = hash.HostIP; + + if (isIp.v6(ip)) { + hash.HostIP = `[${ip}]`; + } + + return super.normalize(...arguments); + } +} diff --git a/ui/app/serializers/resources.js b/ui/app/serializers/resources.js index 43c9e23b7..4239a354b 100644 --- a/ui/app/serializers/resources.js +++ b/ui/app/serializers/resources.js @@ -7,4 +7,9 @@ export default class ResourcesSerializer extends ApplicationSerializer { disk: 'DiskMB', iops: 'IOPS', }; + + normalize(typeHash, hash) { + hash.Ports = hash.Ports || []; + return super.normalize(typeHash, hash); + } } diff --git a/ui/app/templates/allocations/allocation/index.hbs b/ui/app/templates/allocations/allocation/index.hbs index 11b9df8a9..f88059deb 100644 --- a/ui/app/templates/allocations/allocation/index.hbs +++ b/ui/app/templates/allocations/allocation/index.hbs @@ -109,7 +109,6 @@ Last Event Time Volumes - Addresses CPU Memory @@ -129,25 +128,23 @@ - {{#if this.network.ports.length}} + {{#if this.ports.length}}
Ports
- + - Name - Dynamic? + Name Host Address Mapped Port - {{row.model.name}} - {{if row.model.isDynamic "Yes" "No"}} + {{row.model.label}} - {{this.network.ip}}:{{row.model.port}} + {{row.model.hostIp}}:{{row.model.value}} {{row.model.to}} diff --git a/ui/app/templates/allocations/allocation/task/index.hbs b/ui/app/templates/allocations/allocation/task/index.hbs index f274e1312..4b36e9147 100644 --- a/ui/app/templates/allocations/allocation/task/index.hbs +++ b/ui/app/templates/allocations/allocation/task/index.hbs @@ -125,34 +125,6 @@
{{/if}} - {{#if this.network.ports.length}} -
-
- Addresses -
-
- - - Dynamic? - Name - Address - - - - {{if row.model.isDynamic "Yes" "No"}} - {{row.model.name}} - - - {{this.network.ip}}:{{row.model.port}} - - - - - -
-
- {{/if}} - {{#if this.model.task.volumeMounts.length}}
diff --git a/ui/app/templates/components/task-row.hbs b/ui/app/templates/components/task-row.hbs index c5038cd2d..54fdfdd88 100644 --- a/ui/app/templates/components/task-row.hbs +++ b/ui/app/templates/components/task-row.hbs @@ -38,18 +38,6 @@ {{/each}} - -
    - {{#let this.task.resources.networks.firstObject as |network|}} - {{#each network.ports as |port|}} -
  • - {{port.name}}: - {{network.ip}}:{{port.port}} -
    • - {{/each}} - {{/let}} -
- {{#if this.task.isRunning}} {{#if (and (not this.cpu) this.fetchStats.isRunning)}} diff --git a/ui/mirage/common.js b/ui/mirage/common.js index a7eaae454..45508c5c9 100644 --- a/ui/mirage/common.js +++ b/ui/mirage/common.js @@ -32,6 +32,7 @@ export function generateResources(options = {}) { DiskMB: faker.helpers.randomize(DISK_RESERVATIONS), IOPS: faker.helpers.randomize(IOPS_RESERVATIONS), Networks: generateNetworks(options.networks), + Ports: generatePorts(options.networks), }; } @@ -70,3 +71,17 @@ export function generateNetworks(options = {}) { })), })); } + +export function generatePorts(options = {}) { + return Array(faker.random.number({ + min: options.minPorts != null ? options.minPorts : 0, + max: options.maxPorts != null ? options.maxPorts : 2 + })) + .fill(null) + .map(() => ({ + Label: faker.hacker.noun(), + Value: faker.random.number({ min: 5000, max: 60000 }), + To: faker.random.number({ min: 5000, max: 60000 }), + HostIP: faker.random.boolean() ? faker.internet.ip() : faker.internet.ipv6(), + })) +} \ No newline at end of file diff --git a/ui/tests/acceptance/allocation-detail-test.js b/ui/tests/acceptance/allocation-detail-test.js index 8fc11906d..f6b0ee422 100644 --- a/ui/tests/acceptance/allocation-detail-test.js +++ b/ui/tests/acceptance/allocation-detail-test.js @@ -7,6 +7,7 @@ import { setupMirage } from 'ember-cli-mirage/test-support'; import a11yAudit from 'nomad-ui/tests/helpers/a11y-audit'; import Allocation from 'nomad-ui/tests/pages/allocations/detail'; import moment from 'moment'; +import isIp from 'is-ip'; let job; let node; @@ -195,11 +196,6 @@ module('Acceptance | allocation detail', function(hooks) { test('each task row should list high-level information for the task', async function(assert) { const task = server.db.taskStates.where({ allocationId: allocation.id }).sortBy('name')[0]; - const taskResources = allocation.taskResourceIds - .map(id => server.db.taskResources.find(id)) - .sortBy('name')[0]; - const reservedPorts = taskResources.resources.Networks[0].ReservedPorts; - const dynamicPorts = taskResources.resources.Networks[0].DynamicPorts; const events = server.db.taskEvents.where({ taskStateId: task.id }); const event = events[events.length - 1]; @@ -224,19 +220,6 @@ module('Acceptance | allocation detail', function(hooks) { 'Event Time' ); - assert.ok(reservedPorts.length, 'The task has reserved ports'); - assert.ok(dynamicPorts.length, 'The task has dynamic ports'); - - const addressesText = taskRow.ports; - reservedPorts.forEach(port => { - assert.ok(addressesText.includes(port.Label), `Found label ${port.Label}`); - assert.ok(addressesText.includes(port.Value), `Found value ${port.Value}`); - }); - dynamicPorts.forEach(port => { - assert.ok(addressesText.includes(port.Label), `Found label ${port.Label}`); - assert.ok(addressesText.includes(port.Value), `Found value ${port.Value}`); - }); - const volumesText = taskRow.volumes; volumes.forEach(volume => { assert.ok(volumesText.includes(volume.name), `Found label ${volume.name}`); @@ -306,19 +289,15 @@ module('Acceptance | allocation detail', function(hooks) { }); test('ports are listed', async function(assert) { - const serverNetwork = allocation.allocatedResources.Shared.Networks[0]; - const allServerPorts = serverNetwork.ReservedPorts.concat(serverNetwork.DynamicPorts); + const allServerPorts = allocation.allocatedResources.Shared.Ports; allServerPorts.sortBy('Label').forEach((serverPort, index) => { const renderedPort = Allocation.ports[index]; - assert.equal( - renderedPort.dynamic, - serverNetwork.ReservedPorts.includes(serverPort) ? 'No' : 'Yes' - ); assert.equal(renderedPort.name, serverPort.Label); - assert.equal(renderedPort.address, `${serverNetwork.IP}:${serverPort.Value}`); assert.equal(renderedPort.to, serverPort.To); + const expectedAddr = isIp.v6(serverPort.HostIP) ? `[${serverPort.HostIP}]:${serverPort.Value}` : `${serverPort.HostIP}:${serverPort.Value}`; + assert.equal(renderedPort.address, expectedAddr); }); }); diff --git a/ui/tests/acceptance/task-detail-test.js b/ui/tests/acceptance/task-detail-test.js index 585f03ff9..aeaa8ff00 100644 --- a/ui/tests/acceptance/task-detail-test.js +++ b/ui/tests/acceptance/task-detail-test.js @@ -182,36 +182,6 @@ module('Acceptance | task detail', function(hooks) { assert.notOk(Task.hasPrestartTasks); }); - test('the addresses table lists all reserved and dynamic ports', async function(assert) { - const taskResources = allocation.taskResourceIds - .map(id => server.db.taskResources.find(id)) - .find(resources => resources.name === task.name); - const reservedPorts = taskResources.resources.Networks[0].ReservedPorts; - const dynamicPorts = taskResources.resources.Networks[0].DynamicPorts; - const addresses = reservedPorts.concat(dynamicPorts); - - assert.equal(Task.addresses.length, addresses.length, 'All addresses are listed'); - }); - - test('each address row shows the label and value of the address', async function(assert) { - const taskResources = allocation.taskResourceIds - .map(id => server.db.taskResources.find(id)) - .findBy('name', task.name); - const networkAddress = taskResources.resources.Networks[0].IP; - const reservedPorts = taskResources.resources.Networks[0].ReservedPorts; - const dynamicPorts = taskResources.resources.Networks[0].DynamicPorts; - const address = reservedPorts.concat(dynamicPorts).sortBy('Label')[0]; - - const addressRow = Task.addresses.objectAt(0); - assert.equal( - addressRow.isDynamic, - reservedPorts.includes(address) ? 'No' : 'Yes', - 'Dynamic port is denoted as such' - ); - assert.equal(addressRow.name, address.Label, 'Label'); - assert.equal(addressRow.address, `${networkAddress}:${address.Value}`, 'Value'); - }); - test('the events table lists all recent events', async function(assert) { const events = server.db.taskEvents.where({ taskStateId: task.id }); @@ -361,10 +331,6 @@ module('Acceptance | task detail (no addresses)', function(hooks) { await Task.visit({ id: allocation.id, name: task.name }); }); - - test('when the task has no addresses, the addresses table is not shown', async function(assert) { - assert.notOk(Task.hasAddresses, 'No addresses table'); - }); }); module('Acceptance | task detail (different namespace)', function(hooks) { diff --git a/ui/tests/pages/allocations/detail.js b/ui/tests/pages/allocations/detail.js index 47cc98225..de10a1088 100644 --- a/ui/tests/pages/allocations/detail.js +++ b/ui/tests/pages/allocations/detail.js @@ -48,7 +48,6 @@ export default create({ state: text('[data-test-state]'), message: text('[data-test-message]'), time: text('[data-test-time]'), - ports: text('[data-test-ports]'), volumes: text('[data-test-volumes]'), hasUnhealthyDriver: isPresent('[data-test-icon="unhealthy-driver"]'), @@ -85,7 +84,6 @@ export default create({ ...allocations('[data-test-preemptions] [data-test-allocation]', 'preemptions'), ports: collection('[data-test-allocation-port]', { - dynamic: text('[data-test-allocation-port-is-dynamic]'), name: text('[data-test-allocation-port-name]'), address: text('[data-test-allocation-port-address]'), to: text('[data-test-allocation-port-to]'), diff --git a/ui/tests/pages/allocations/task/detail.js b/ui/tests/pages/allocations/task/detail.js index 27f4e3c76..18383a5b5 100644 --- a/ui/tests/pages/allocations/task/detail.js +++ b/ui/tests/pages/allocations/task/detail.js @@ -57,13 +57,6 @@ export default create({ isBlocking: isPresent('.icon-is-warning'), }), - hasAddresses: isPresent('[data-test-task-addresses]'), - addresses: collection('[data-test-task-address]', { - name: text('[data-test-task-address-name]'), - isDynamic: text('[data-test-task-address-is-dynamic]'), - address: text('[data-test-task-address-address]'), - }), - hasVolumes: isPresent('[data-test-volumes]'), volumes: collection('[data-test-volume]', { name: text('[data-test-volume-name]'),