Skip to content

Commit 07eca00

Browse files
spalgerspalger
authored
[savedObjects] wait for Kibana index on every write (#14202)
* [savedObjects] wait for Kibana index on every write * [es/waitUntilReady] make test failure less likely by waiting for green * [es/healthCheck] assert that plugin.status.once was called * [savedObjectsClient] avoid importing noop * [savedObjects] use milliseconds for indexCheckTimeout * [savedObjectsClient/onBeforeWrite] don't 404 when kibana index has unassigned shards * [savedObjectsClient/create] cast 404 caused by index missing to 503
1 parent 7e32c50 commit 07eca00

13 files changed

Lines changed: 235 additions & 62 deletions

File tree

‎src/core_plugins/elasticsearch/lib/__tests__/health_check.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('plugins/elasticsearch', () => {
4242
status: {
4343
red: sinon.stub(),
4444
green: sinon.stub(),
45-
yellow: sinon.stub()
45+
yellow: sinon.stub(),
4646
}
4747
};
4848

@@ -207,13 +207,19 @@ describe('plugins/elasticsearch', () => {
207207
});
208208

209209
describe('#waitUntilReady', function () {
210-
it('polls health until index is ready', function () {
210+
it('polls health until index is ready, then waits for green status', function () {
211211
const clusterHealth = cluster.callWithInternalUser.withArgs('cluster.health', sinon.match.any);
212212
clusterHealth.onCall(0).returns(Promise.resolve({ timed_out: true }));
213213
clusterHealth.onCall(1).returns(Promise.resolve({ status: 'red' }));
214214
clusterHealth.onCall(2).returns(Promise.resolve({ status: 'green' }));
215215

216+
plugin.status.once = sinon.spy(function (event, handler) {
217+
expect(event).to.be('green');
218+
setImmediate(handler);
219+
});
220+
216221
return health.waitUntilReady().then(function () {
222+
sinon.assert.calledOnce(plugin.status.once);
217223
sinon.assert.calledThrice(clusterHealth);
218224
});
219225
});

‎src/core_plugins/elasticsearch/lib/health_check.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ export default function (plugin, server) {
6363
if (health !== READY) {
6464
return Promise.delay(REQUEST_DELAY).then(waitUntilReady);
6565
}
66+
67+
return new Promise((resolve) => {
68+
plugin.status.once('green', resolve);
69+
});
6670
});
6771
}
6872

‎src/server/config/schema.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,11 @@ export default () => Joi.object({
197197
}))
198198
}))
199199
}).default(),
200+
201+
savedObjects: Joi.object({
202+
indexCheckTimeout: Joi.number().default(5000)
203+
}).default(),
204+
200205
uiSettings: Joi.object({
201206
// this is used to prevent the uiSettings from initializing. Since they
202207
// require the elasticsearch plugin in order to function we need to turn

‎src/server/saved_objects/client/__tests__/saved_objects_client.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import expect from 'expect.js';
22
import sinon from 'sinon';
3+
import { delay } from 'bluebird';
34
import { SavedObjectsClient } from '../saved_objects_client';
45
import * as getSearchDslNS from '../lib/search_dsl/search_dsl';
56
import { getSearchDsl } from '../lib';
7+
import elasticsearch from 'elasticsearch';
68

79
describe('SavedObjectsClient', () => {
810
const sandbox = sinon.sandbox.create();
911

1012
let callAdminCluster;
13+
let onBeforeWrite;
1114
let savedObjectsClient;
1215
const mockTimestamp = '2017-08-14T15:49:14.886Z';
1316
const mockTimestampFields = { updated_at: mockTimestamp };
@@ -75,11 +78,13 @@ describe('SavedObjectsClient', () => {
7578

7679
beforeEach(() => {
7780
callAdminCluster = sandbox.stub();
81+
onBeforeWrite = sandbox.stub();
7882

7983
savedObjectsClient = new SavedObjectsClient({
8084
index: '.kibana-test',
8185
mappings,
8286
callCluster: callAdminCluster,
87+
onBeforeWrite
8388
});
8489

8590
sandbox.stub(savedObjectsClient, '_getCurrentTime').returns(mockTimestamp);
@@ -123,6 +128,7 @@ describe('SavedObjectsClient', () => {
123128
});
124129

125130
expect(callAdminCluster.calledOnce).to.be(true);
131+
sinon.assert.calledOnce(onBeforeWrite);
126132

127133
const args = callAdminCluster.getCall(0).args;
128134
expect(args[0]).to.be('index');
@@ -136,6 +142,7 @@ describe('SavedObjectsClient', () => {
136142
});
137143

138144
expect(callAdminCluster.calledOnce).to.be(true);
145+
sinon.assert.calledOnce(onBeforeWrite);
139146

140147
const args = callAdminCluster.getCall(0).args;
141148
expect(args[0]).to.be('create');
@@ -147,6 +154,7 @@ describe('SavedObjectsClient', () => {
147154
}, { id: 'logstash-*' });
148155

149156
expect(callAdminCluster.calledOnce).to.be(true);
157+
sinon.assert.calledOnce(onBeforeWrite);
150158

151159
const args = callAdminCluster.getCall(0).args;
152160
expect(args[1].id).to.be('index-pattern:logstash-*');
@@ -158,6 +166,7 @@ describe('SavedObjectsClient', () => {
158166
});
159167

160168
expect(callAdminCluster.calledOnce).to.be(true);
169+
sinon.assert.calledOnce(onBeforeWrite);
161170

162171
const args = callAdminCluster.getCall(0).args;
163172
expect(args[1].id).to.match(/index-pattern:[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}/);
@@ -174,6 +183,7 @@ describe('SavedObjectsClient', () => {
174183
]);
175184

176185
expect(callAdminCluster.calledOnce).to.be(true);
186+
sinon.assert.calledOnce(onBeforeWrite);
177187

178188
const args = callAdminCluster.getCall(0).args;
179189

@@ -191,6 +201,7 @@ describe('SavedObjectsClient', () => {
191201

192202
await savedObjectsClient.bulkCreate([{ type: 'foo', id: 'bar', attributes: {} }], { overwrite: false });
193203
sinon.assert.calledOnce(callAdminCluster);
204+
sinon.assert.calledOnce(onBeforeWrite);
194205
sinon.assert.calledWithExactly(callAdminCluster, 'bulk', sinon.match({
195206
body: [
196207
// uses create because overwriting is not allowed
@@ -200,9 +211,11 @@ describe('SavedObjectsClient', () => {
200211
}));
201212

202213
callAdminCluster.reset();
214+
onBeforeWrite.reset();
203215

204216
await savedObjectsClient.bulkCreate([{ type: 'foo', id: 'bar', attributes: {} }], { overwrite: true });
205217
sinon.assert.calledOnce(callAdminCluster);
218+
sinon.assert.calledOnce(onBeforeWrite);
206219
sinon.assert.calledWithExactly(callAdminCluster, 'bulk', sinon.match({
207220
body: [
208221
// uses index because overwriting is allowed
@@ -301,6 +314,7 @@ describe('SavedObjectsClient', () => {
301314

302315
try {
303316
await savedObjectsClient.delete('index-pattern', 'logstash-*');
317+
sinon.assert.calledOnce(onBeforeWrite);
304318
expect().fail('should throw error');
305319
} catch(e) {
306320
expect(e.output.statusCode).to.eql(404);
@@ -313,6 +327,7 @@ describe('SavedObjectsClient', () => {
313327
await savedObjectsClient.delete('index-pattern', 'logstash-*');
314328

315329
expect(callAdminCluster.calledOnce).to.be(true);
330+
sinon.assert.calledOnce(onBeforeWrite);
316331

317332
const [method, args] = callAdminCluster.getCall(0).args;
318333
expect(method).to.be('delete');
@@ -335,6 +350,8 @@ describe('SavedObjectsClient', () => {
335350
await savedObjectsClient.find({ searchFields: 'string' });
336351
throw new Error('expected find() to reject');
337352
} catch (error) {
353+
sinon.assert.notCalled(callAdminCluster);
354+
sinon.assert.notCalled(onBeforeWrite);
338355
expect(error).to.have.property('message').contain('must be an array');
339356
}
340357
});
@@ -344,6 +361,8 @@ describe('SavedObjectsClient', () => {
344361
await savedObjectsClient.find({ fields: 'string' });
345362
throw new Error('expected find() to reject');
346363
} catch (error) {
364+
sinon.assert.notCalled(callAdminCluster);
365+
sinon.assert.notCalled(onBeforeWrite);
347366
expect(error).to.have.property('message').contain('must be an array');
348367
}
349368
});
@@ -366,6 +385,7 @@ describe('SavedObjectsClient', () => {
366385
getSearchDsl.returns({ query: 1, aggregations: 2 });
367386
await savedObjectsClient.find();
368387
sinon.assert.calledOnce(callAdminCluster);
388+
sinon.assert.notCalled(onBeforeWrite);
369389
sinon.assert.calledWithExactly(callAdminCluster, 'search', sinon.match({
370390
body: sinon.match({
371391
query: 1,
@@ -397,6 +417,7 @@ describe('SavedObjectsClient', () => {
397417
await savedObjectsClient.find({ perPage: 10, page: 6 });
398418

399419
expect(callAdminCluster.calledOnce).to.be(true);
420+
sinon.assert.notCalled(onBeforeWrite);
400421

401422
const options = callAdminCluster.getCall(0).args[1];
402423
expect(options.size).to.be(10);
@@ -407,6 +428,7 @@ describe('SavedObjectsClient', () => {
407428
await savedObjectsClient.find({ fields: ['title'] });
408429

409430
expect(callAdminCluster.calledOnce).to.be(true);
431+
sinon.assert.notCalled(onBeforeWrite);
410432

411433
const options = callAdminCluster.getCall(0).args[1];
412434
expect(options._source).to.eql([
@@ -433,6 +455,7 @@ describe('SavedObjectsClient', () => {
433455

434456
it('formats Elasticsearch response', async () => {
435457
const response = await savedObjectsClient.get('index-pattern', 'logstash-*');
458+
sinon.assert.notCalled(onBeforeWrite);
436459
expect(response).to.eql({
437460
id: 'logstash-*',
438461
type: 'index-pattern',
@@ -447,6 +470,7 @@ describe('SavedObjectsClient', () => {
447470
it('prepends type to the id', async () => {
448471
await savedObjectsClient.get('index-pattern', 'logstash-*');
449472

473+
sinon.assert.notCalled(onBeforeWrite);
450474
const [, args] = callAdminCluster.getCall(0).args;
451475
expect(args.id).to.eql('index-pattern:logstash-*');
452476
expect(args.type).to.eql('doc');
@@ -463,6 +487,7 @@ describe('SavedObjectsClient', () => {
463487
]);
464488

465489
expect(callAdminCluster.calledOnce).to.be(true);
490+
sinon.assert.notCalled(onBeforeWrite);
466491

467492
const options = callAdminCluster.getCall(0).args[1];
468493
expect(options.body.docs).to.eql([
@@ -478,6 +503,7 @@ describe('SavedObjectsClient', () => {
478503

479504
expect(response.saved_objects).to.have.length(0);
480505
expect(callAdminCluster.notCalled).to.be(true);
506+
sinon.assert.notCalled(onBeforeWrite);
481507
});
482508

483509
it('reports error on missed objects', async () => {
@@ -499,6 +525,9 @@ describe('SavedObjectsClient', () => {
499525
[{ id: 'good', type: 'config' }, { id: 'bad', type: 'config' }]
500526
);
501527

528+
sinon.assert.notCalled(onBeforeWrite);
529+
sinon.assert.calledOnce(callAdminCluster);
530+
502531
expect(savedObjects).to.have.length(2);
503532
expect(savedObjects[0]).to.eql({
504533
id: 'good',
@@ -557,6 +586,7 @@ describe('SavedObjectsClient', () => {
557586
await savedObjectsClient.update('index-pattern', 'logstash-*', { title: 'Testing' });
558587

559588
expect(callAdminCluster.calledOnce).to.be(true);
589+
sinon.assert.calledOnce(onBeforeWrite);
560590

561591
const args = callAdminCluster.getCall(0).args;
562592

@@ -573,4 +603,34 @@ describe('SavedObjectsClient', () => {
573603
});
574604
});
575605
});
606+
607+
describe('onBeforeWrite', () => {
608+
it('blocks calls to callCluster of requests', async () => {
609+
onBeforeWrite.returns(delay(500));
610+
callAdminCluster.returns({ result: 'deleted', found: true });
611+
612+
const deletePromise = savedObjectsClient.delete('type', 'id');
613+
await delay(100);
614+
sinon.assert.calledOnce(onBeforeWrite);
615+
sinon.assert.notCalled(callAdminCluster);
616+
await deletePromise;
617+
sinon.assert.calledOnce(onBeforeWrite);
618+
sinon.assert.calledOnce(callAdminCluster);
619+
});
620+
621+
it('can throw es errors and have them decorated as SavedObjectsClient errors', async () => {
622+
const es401 = new elasticsearch.errors[401];
623+
expect(SavedObjectsClient.errors.isNotAuthorizedError(es401)).to.be(false);
624+
onBeforeWrite.throws(es401);
625+
626+
try {
627+
await savedObjectsClient.delete('type', 'id');
628+
throw new Error('expected savedObjectsClient.delete() to reject');
629+
} catch (error) {
630+
sinon.assert.calledOnce(onBeforeWrite);
631+
expect(error).to.be(es401);
632+
expect(SavedObjectsClient.errors.isNotAuthorizedError(error)).to.be(true);
633+
}
634+
});
635+
});
576636
});

‎src/server/saved_objects/client/lib/errors.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import Boom from 'boom';
33
const code = Symbol('SavedObjectsClientErrorCode');
44

55
function decorate(error, errorCode, statusCode, message) {
6+
if (isSavedObjectsClientError(error)) {
7+
return error;
8+
}
9+
610
const boom = Boom.boomify(error, {
711
statusCode,
812
message,
@@ -14,6 +18,10 @@ function decorate(error, errorCode, statusCode, message) {
1418
return boom;
1519
}
1620

21+
export function isSavedObjectsClientError(error) {
22+
return error && !!error[code];
23+
}
24+
1725
// 400 - badRequest
1826
const CODE_BAD_REQUEST = 'SavedObjectsClient/badRequest';
1927
export function decorateBadRequestError(error, reason) {

0 commit comments

Comments
 (0)