Skip to content
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

Update UI to use new allocated ports fields #8631

Merged
merged 9 commits into from
Aug 20, 2020
4 changes: 4 additions & 0 deletions command/agent/alloc_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}

Expand Down
17 changes: 17 additions & 0 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 40 additions & 0 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
5 changes: 4 additions & 1 deletion ui/app/controllers/allocations/allocation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
21 changes: 0 additions & 21 deletions ui/app/controllers/allocations/allocation/task/index.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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;
Expand Down
9 changes: 9 additions & 0 deletions ui/app/models/port.js
Original file line number Diff line number Diff line change
@@ -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;
}
1 change: 1 addition & 0 deletions ui/app/models/resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ export default class Resources extends Fragment {
@attr('number') disk;
@attr('number') iops;
@fragmentArray('network', { defaultValue: () => [] }) networks;
@fragmentArray('port', { defaultValue: () => [] }) ports;
}
18 changes: 18 additions & 0 deletions ui/app/serializers/port.js
Original file line number Diff line number Diff line change
@@ -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);
}
}
5 changes: 5 additions & 0 deletions ui/app/serializers/resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
13 changes: 5 additions & 8 deletions ui/app/templates/allocations/allocation/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@
<th>Last Event</th>
<t.sort-by @prop="events.lastObject.time">Time</t.sort-by>
<th>Volumes</th>
<th>Addresses</th>
<th>CPU</th>
<th>Memory</th>
</t.head>
Expand All @@ -129,25 +128,23 @@
</div>
</div>

{{#if this.network.ports.length}}
{{#if this.ports.length}}
<div class="boxed-section" data-test-allocation-ports>
<div class="boxed-section-head">
Ports
</div>
<div class="boxed-section-body is-full-bleed">
<ListTable @source={{this.network.ports}} as |t|>
<ListTable @source={{this.ports}} as |t|>
<t.head>
<th class="is-2">Name</th>
<th class="is-1">Dynamic?</th>
<th>Name</th>
<th>Host Address</th>
<th>Mapped Port</th>
</t.head>
<t.body as |row|>
<tr data-test-allocation-port>
<td data-test-allocation-port-name>{{row.model.name}}</td>
<td data-test-allocation-port-is-dynamic>{{if row.model.isDynamic "Yes" "No"}}</td>
<td data-test-allocation-port-name>{{row.model.label}}</td>
<td data-test-allocation-port-address>
<a href="http://{{this.network.ip}}:{{row.model.port}}" target="_blank" rel="noopener noreferrer">{{this.network.ip}}:{{row.model.port}}</a>
<a href="http://{{row.model.hostIp}}:{{row.model.value}}" target="_blank" rel="noopener noreferrer">{{row.model.hostIp}}:{{row.model.value}}</a>
</td>
<td data-test-allocation-port-to>{{row.model.to}}</td>
</tr>
Expand Down
28 changes: 0 additions & 28 deletions ui/app/templates/allocations/allocation/task/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -125,34 +125,6 @@
</div>
{{/if}}

{{#if this.network.ports.length}}
<div class="boxed-section" data-test-task-addresses>
<div class="boxed-section-head">
Addresses
</div>
<div class="boxed-section-body is-full-bleed">
<ListTable @source={{this.network.ports}} as |t|>
<t.head>
<th class="is-1">Dynamic?</th>
<th class="is-2">Name</th>
<th>Address</th>
</t.head>
<t.body as |row|>
<tr data-test-task-address>
<td data-test-task-address-is-dynamic>{{if row.model.isDynamic "Yes" "No"}}</td>
<td data-test-task-address-name>{{row.model.name}}</td>
<td data-test-task-address-address>
<a href="http://{{this.network.ip}}:{{row.model.port}}" target="_blank" rel="noopener noreferrer">
{{this.network.ip}}:{{row.model.port}}
</a>
</td>
</tr>
</t.body>
</ListTable>
</div>
</div>
{{/if}}

{{#if this.model.task.volumeMounts.length}}
<div data-test-volumes class="boxed-section">
<div class="boxed-section-head">
Expand Down
12 changes: 0 additions & 12 deletions ui/app/templates/components/task-row.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,6 @@
{{/each}}
</ul>
</td>
<td data-test-ports>
<ul>
{{#let this.task.resources.networks.firstObject as |network|}}
{{#each network.ports as |port|}}
<li data-test-port>
<strong>{{port.name}}:</strong>
<a href="http://{{network.ip}}:{{port.port}}" target="_blank" rel="noopener noreferrer">{{network.ip}}:{{port.port}}</a>
</li>
{{/each}}
{{/let}}
</ul>
</td>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty unimportant but this means the corresponding tasks.ports property in tests/pages/allocations/detail can be deleted. I feel like there are probably several existing orphaned page objects properties though as we don’t have a good way to detect things we aren’t using anymore.

<td data-test-cpu class="is-1 has-text-centered">
{{#if this.task.isRunning}}
{{#if (and (not this.cpu) this.fetchStats.isRunning)}}
Expand Down
15 changes: 15 additions & 0 deletions ui/mirage/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};
}

Expand Down Expand Up @@ -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(),
}))
}
29 changes: 4 additions & 25 deletions ui/tests/acceptance/allocation-detail-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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];

Expand All @@ -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}`);
Expand Down Expand Up @@ -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);
});
});

Expand Down
Loading