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

Raft peer removal bug #13098

Merged
merged 2 commits into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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');
});
});