Skip to content

Commit

Permalink
Merge pull request #8477 from hashicorp/b-ui/cross-region-alloc-lifec…
Browse files Browse the repository at this point in the history 
…ycle-buttons

UI: Always add the active region as a query param to API requests
  • Loading branch information
@DingoEatingFuzz
DingoEatingFuzz committed Jul 21, 2020
2 parents 133e93e + 544d941 commit 248e844
Show file tree
Hide file tree
Showing 6 changed files with 385 additions and 172 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ BUG FIXES:
* ui: Fixed missing namespace query param after changing acl tokens [[GH-8413](https://github.com/hashicorp/nomad/issues/8413)]
* ui: Fixed exec to derive group and task when possible from allocation [[GH-8463](https://github.com/hashicorp/nomad/pull/8463)]
* ui: Fixed runtime error when clicking "Run Job" while a prefix filter is set [[GH-8412](https://github.com/hashicorp/nomad/issues/8412)]
* ui: Fixed the absence of the region query parameter on various actions, such as job stop, allocation restart, node drain. [[GH-8477](https://github.com/hashicorp/nomad/issues/8477)]
* vault: Fixed a bug where vault identity policies not considered in permissions check [[GH-7732](https://github.com/hashicorp/nomad/issues/7732)]

## 0.12.0 (July 9, 2020)
Expand Down
13 changes: 10 additions & 3 deletions ui/app/adapters/application.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,18 @@ export default class ApplicationAdapter extends RESTAdapter {
});
}

ajaxOptions(url, type, options = {}) {
ajaxOptions(url, verb, options = {}) {
options.data || (options.data = {});
if (this.get('system.shouldIncludeRegion')) {
// Region should only ever be a query param. The default ajaxOptions
// behavior is to include data attributes in the requestBody for PUT
// and POST requests. This works around that.
const region = this.get('system.activeRegion');
if (region) {
options.data.region = region;
url = associateRegion(url, region);
}
}
return super.ajaxOptions(url, type, options);
return super.ajaxOptions(url, verb, options);
}

// In order to remove stale records from the store, findHasMany has to unload
Expand Down Expand Up @@ -119,3 +122,7 @@ export default class ApplicationAdapter extends RESTAdapter {
return this.urlForFindRecord(...arguments);
}
}

function associateRegion(url, region) {
return url.indexOf('?') !== -1 ? `${url}&region=${region}` : `${url}?region=${region}`;
}
139 changes: 89 additions & 50 deletions ui/tests/unit/adapters/allocation-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,69 +9,108 @@ module('Unit | Adapter | Allocation', function(hooks) {
this.store = this.owner.lookup('service:store');
this.subject = () => this.store.adapterFor('allocation');

window.localStorage.clear();

this.server = startMirage();

this.server.create('namespace');
this.server.create('node');
this.server.create('job', { createAllocations: false });
this.server.create('allocation', { id: 'alloc-1' });
this.initialize = async (allocationId, { region } = {}) => {
if (region) window.localStorage.nomadActiveRegion = region;

this.server.create('namespace');
this.server.create('region', { id: 'region-1' });
this.server.create('region', { id: 'region-2' });

this.server.create('node');
this.server.create('job', { createAllocations: false });
this.server.create('allocation', { id: 'alloc-1' });
this.system = this.owner.lookup('service:system');
await this.system.get('namespaces');
this.system.get('shouldIncludeRegion');
await this.system.get('defaultRegion');

const allocation = await this.store.findRecord('allocation', allocationId);
this.server.pretender.handledRequests.length = 0;

return allocation;
};
});

hooks.afterEach(function() {
this.server.shutdown();
});

test('`stop` makes the correct API call', async function(assert) {
const { pretender } = this.server;
const allocationId = 'alloc-1';
const testCases = [
{
variation: '',
id: 'alloc-1',
task: 'task-name',
region: null,
path: 'some/path',
ls: `GET /v1/client/fs/ls/alloc-1?path=${encodeURIComponent('some/path')}`,
stat: `GET /v1/client/fs/stat/alloc-1?path=${encodeURIComponent('some/path')}`,
stop: 'POST /v1/allocation/alloc-1/stop',
restart: 'PUT /v1/client/allocation/alloc-1/restart',
},
{
variation: 'with non-default region',
id: 'alloc-1',
task: 'task-name',
region: 'region-2',
path: 'some/path',
ls: `GET /v1/client/fs/ls/alloc-1?path=${encodeURIComponent('some/path')}&region=region-2`,
stat: `GET /v1/client/fs/stat/alloc-1?path=${encodeURIComponent(
'some/path'
)}&region=region-2`,
stop: 'POST /v1/allocation/alloc-1/stop?region=region-2',
restart: 'PUT /v1/client/allocation/alloc-1/restart?region=region-2',
},
];

const allocation = await this.store.findRecord('allocation', allocationId);
pretender.handledRequests.length = 0;
testCases.forEach(testCase => {
test(`ls makes the correct API call ${testCase.variation}`, async function(assert) {
const { pretender } = this.server;
const allocation = await this.initialize(testCase.id, { region: testCase.region });

await this.subject().stop(allocation);
const req = pretender.handledRequests[0];
assert.equal(
`${req.method} ${req.url}`,
`POST /v1/allocation/${allocationId}/stop`,
`POST /v1/allocation/${allocationId}/stop`
);
});
await this.subject().ls(allocation, testCase.path);
const req = pretender.handledRequests[0];
assert.equal(`${req.method} ${req.url}`, testCase.ls);
});

test('`restart` makes the correct API call', async function(assert) {
const { pretender } = this.server;
const allocationId = 'alloc-1';
test(`stat makes the correct API call ${testCase.variation}`, async function(assert) {
const { pretender } = this.server;
const allocation = await this.initialize(testCase.id, { region: testCase.region });

const allocation = await this.store.findRecord('allocation', allocationId);
pretender.handledRequests.length = 0;
await this.subject().stat(allocation, testCase.path);
const req = pretender.handledRequests[0];
assert.equal(`${req.method} ${req.url}`, testCase.stat);
});

await this.subject().restart(allocation);
const req = pretender.handledRequests[0];
assert.equal(
`${req.method} ${req.url}`,
`PUT /v1/client/allocation/${allocationId}/restart`,
`PUT /v1/client/allocation/${allocationId}/restart`
);
});
test(`stop makes the correct API call ${testCase.variation}`, async function(assert) {
const { pretender } = this.server;
const allocation = await this.initialize(testCase.id, { region: testCase.region });

await this.subject().stop(allocation);
const req = pretender.handledRequests[0];
assert.equal(`${req.method} ${req.url}`, testCase.stop);
});

test(`restart makes the correct API call ${testCase.variation}`, async function(assert) {
const { pretender } = this.server;
const allocation = await this.initialize(testCase.id, { region: testCase.region });

await this.subject().restart(allocation);
const req = pretender.handledRequests[0];
assert.equal(`${req.method} ${req.url}`, testCase.restart);
});

test(`restart with optional task name makes the correct API call ${testCase.variation}`, async function(assert) {
const { pretender } = this.server;
const allocation = await this.initialize(testCase.id, { region: testCase.region });

test('`restart` takes an optional task name and makes the correct API call', async function(assert) {
const { pretender } = this.server;
const allocationId = 'alloc-1';
const taskName = 'task-name';

const allocation = await this.store.findRecord('allocation', allocationId);
pretender.handledRequests.length = 0;

await this.subject().restart(allocation, taskName);
const req = pretender.handledRequests[0];
assert.equal(
`${req.method} ${req.url}`,
`PUT /v1/client/allocation/${allocationId}/restart`,
`PUT /v1/client/allocation/${allocationId}/restart`
);
assert.deepEqual(
JSON.parse(req.requestBody),
{ TaskName: taskName },
'Request body is correct'
);
await this.subject().restart(allocation, testCase.task);
const req = pretender.handledRequests[0];
assert.equal(`${req.method} ${req.url}`, testCase.restart);
assert.deepEqual(JSON.parse(req.requestBody), { TaskName: testCase.task });
});
});
});
68 changes: 68 additions & 0 deletions ui/tests/unit/adapters/deployment-test.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { module, test } from 'qunit';
import { setupTest } from 'ember-qunit';
import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage';

module('Unit | Adapter | Deployment', function(hooks) {
setupTest(hooks);

hooks.beforeEach(async function() {
this.store = this.owner.lookup('service:store');
this.system = this.owner.lookup('service:system');
this.subject = () => this.store.adapterFor('deployment');

window.localStorage.clear();

this.server = startMirage();

this.initialize = async ({ region } = {}) => {
if (region) window.localStorage.nomadActiveRegion = region;

this.server.create('region', { id: 'region-1' });
this.server.create('region', { id: 'region-2' });

this.server.create('node');
const job = this.server.create('job', { createAllocations: false });
const deploymentRecord = server.schema.deployments.where({ jobId: job.id }).models[0];

this.system.get('shouldIncludeRegion');
await this.system.get('defaultRegion');

const deployment = await this.store.findRecord('deployment', deploymentRecord.id);
this.server.pretender.handledRequests.length = 0;

return deployment;
};
});

hooks.afterEach(function() {
this.server.shutdown();
});

const testCases = [
{
variation: '',
region: null,
promote: id => `POST /v1/deployment/promote/${id}`,
},
{
variation: 'with non-default region',
region: 'region-2',
promote: id => `POST /v1/deployment/promote/${id}?region=region-2`,
},
];

testCases.forEach(testCase => {
test(`promote makes the correct API call ${testCase.variation}`, async function(assert) {
const deployment = await this.initialize({ region: testCase.region });
await this.subject().promote(deployment);

const request = this.server.pretender.handledRequests[0];

assert.equal(`${request.method} ${request.url}`, testCase.promote(deployment.id));
assert.deepEqual(JSON.parse(request.requestBody), {
DeploymentId: deployment.id,
All: true,
});
});
});
});

0 comments on commit 248e844

Please sign in to comment.