Skip to content

Commit

Permalink
Raft peer removal bug (#13098)
Browse files Browse the repository at this point in the history
* fixes issue removing raft peer via cli not reflected in UI until refresh

* adds changelog entry
  • Loading branch information
zofskeez committed Nov 18, 2021
1 parent 0079130 commit 4fb699f
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 2 deletions.
3 changes: 3 additions & 0 deletions changelog/13098.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fixes issue removing raft storage peer via cli not reflected in UI until refresh
```
6 changes: 5 additions & 1 deletion ui/app/adapters/server.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import ApplicationAdapter from './application';
const fetchUrl = '/v1/sys/storage/raft/configuration';

export default ApplicationAdapter.extend({
urlForFindAll() {
return '/v1/sys/storage/raft/configuration';
return fetchUrl;
},
urlForQuery() {
return fetchUrl;
},
urlForDeleteRecord() {
return '/v1/sys/storage/raft/remove-peer';
Expand Down
5 changes: 4 additions & 1 deletion ui/app/routes/vault/cluster/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import ClusterRoute from 'vault/mixins/cluster-route';

export default Route.extend(ClusterRoute, {
model() {
return this.store.findAll('server');
// findAll method will return all records in store as well as response from server
// when removing a peer via the cli, stale records would continue to appear until refresh
// query method will only return records from response
return this.store.query('server', {});
},

actions: {
Expand Down
27 changes: 27 additions & 0 deletions ui/mirage/factories/configuration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Factory, trait } from 'ember-cli-mirage';
import faker from 'faker';

export default Factory.extend({
auth: null,
data: null, // populated via traits
lease_duration: 0,
lease_id: '',
renewable: () => faker.datatype.boolean(),
request_id: () => faker.datatype.uuid(),
warnings: null,
wrap_info: null,

// add servers to test raft storage configuration
withRaft: trait({
afterCreate(config, server) {
if (!config.data) {
config.data = {
config: {
index: 0,
servers: server.serializerOrRegistry.serialize(server.createList('server', 2)),
},
};
}
},
}),
});
10 changes: 10 additions & 0 deletions ui/mirage/factories/server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Factory } from 'ember-cli-mirage';
import faker from 'faker';

export default Factory.extend({
address: () => faker.internet.ip(),
node_id: i => `raft_node_${i}`,
protocol_version: '3',
voter: () => faker.datatype.boolean(),
leader: () => faker.datatype.boolean(),
});
73 changes: 73 additions & 0 deletions ui/tests/acceptance/raft-storage-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { module, test } from 'qunit';
import { setupApplicationTest } from 'ember-qunit';
import { setupMirage } from 'ember-cli-mirage/test-support';
import { click, visit } from '@ember/test-helpers';
import authPage from 'vault/tests/pages/auth';
import logout from 'vault/tests/pages/logout';

module('Acceptance | raft storage', function(hooks) {
setupApplicationTest(hooks);
setupMirage(hooks);

hooks.beforeEach(async function() {
this.config = this.server.create('configuration', 'withRaft');
this.server.get('/sys/internal/ui/resultant-acl', () =>
this.server.create('configuration', { data: { root: true } })
);
this.server.get('/sys/license/features', () => ({}));
await authPage.login();
});
hooks.afterEach(function() {
return logout.visit();
});

test('it should render correct number of raft peers', async function(assert) {
assert.expect(3);

let didRemovePeer = false;
this.server.get('/sys/storage/raft/configuration', () => {
if (didRemovePeer) {
this.config.data.config.servers.pop();
} else {
// consider peer removed by external means (cli) after initial request
didRemovePeer = true;
}
return this.config;
});

await visit('/vault/storage/raft');
assert.dom('[data-raft-row]').exists({ count: 2 }, '2 raft peers render in table');
// leave route and return to trigger config fetch
await visit('/vault/secrets');
await visit('/vault/storage/raft');
const store = this.owner.lookup('service:store');
assert.equal(
store.peekAll('server').length,
2,
'Store contains 2 server records since remove peer was triggered externally'
);
assert.dom('[data-raft-row]').exists({ count: 1 }, 'Only raft nodes from response are rendered');
});

test('it should remove raft peer', async function(assert) {
assert.expect(3);

this.server.get('/sys/storage/raft/configuration', () => this.config);
this.server.post('/sys/storage/raft/remove-peer', (schema, req) => {
const body = JSON.parse(req.requestBody);
assert.equal(
body.server_id,
this.config.data.config.servers[1].node_id,
'Remove peer request made with node id'
);
return {};
});

await visit('/vault/storage/raft');
assert.dom('[data-raft-row]').exists({ count: 2 }, '2 raft peers render in table');
await click('[data-raft-row]:nth-child(2) [data-test-popup-menu-trigger]');
await click('[data-test-confirm-action-trigger]');
await click('[data-test-confirm-button');
assert.dom('[data-raft-row]').exists({ count: 1 }, 'Raft peer successfully removed');
});
});

0 comments on commit 4fb699f

Please sign in to comment.