From 68c33870e20d97ca941ba869f2f29517cba018df Mon Sep 17 00:00:00 2001 From: jasper Date: Thu, 1 Dec 2016 13:05:28 -0500 Subject: [PATCH] settings: do not query ES for settings in non-green status (#9311) Backports PR #9308 **Commit 1:** settings: do not query ES for settings in non-green status If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. * Original sha: 43742b2cd415c5a428c6277fe357a7aadb4d44eb * Authored by Court Ewing on 2016-12-01T17:02:19Z --- src/core_plugins/elasticsearch/lib/health_check.js | 9 ++++++++- src/ui/settings/__tests__/index.js | 13 +++++++++++-- src/ui/settings/index.js | 7 +++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/core_plugins/elasticsearch/lib/health_check.js b/src/core_plugins/elasticsearch/lib/health_check.js index 4f1555893d871..00ab4e1686730 100644 --- a/src/core_plugins/elasticsearch/lib/health_check.js +++ b/src/core_plugins/elasticsearch/lib/health_check.js @@ -81,13 +81,20 @@ module.exports = function (plugin, server) { }); } + function waitForEsVersion() { + return checkEsVersion(server, kibanaVersion.get()).catch(err => { + plugin.status.red(err); + return Promise.delay(REQUEST_DELAY).then(waitForEsVersion); + }); + } + function setGreenStatus() { return plugin.status.green('Kibana index ready'); } function check() { return waitForPong() - .then(() => checkEsVersion(server, kibanaVersion.get())) + .then(waitForEsVersion) .then(waitForShards) .then(setGreenStatus) .then(_.partial(migrateConfig, server)) diff --git a/src/ui/settings/__tests__/index.js b/src/ui/settings/__tests__/index.js index 344552abdce81..d2ffc25764641 100644 --- a/src/ui/settings/__tests__/index.js +++ b/src/ui/settings/__tests__/index.js @@ -158,6 +158,14 @@ describe('ui settings', function () { })).to.equal(true); }); + it('returns an empty object when status is not green', async function () { + const { uiSettings, req } = instantiate({ + settingsStatusOverrides: { state: 'yellow' } + }); + + expect(await uiSettings.getUserProvided(req)).to.eql({}); + }); + it('returns an empty object on 404 responses', async function () { const { uiSettings, req } = instantiate({ async callWithRequest() { @@ -361,7 +369,7 @@ function expectElasticsearchUpdateQuery(server, req, configGet, doc) { }); } -function instantiate({ getResult, callWithRequest } = {}) { +function instantiate({ getResult, callWithRequest, settingsStatusOverrides } = {}) { const esStatus = { state: 'green', on: sinon.spy() @@ -370,7 +378,8 @@ function instantiate({ getResult, callWithRequest } = {}) { state: 'green', red: sinon.spy(), yellow: sinon.spy(), - green: sinon.spy() + green: sinon.spy(), + ...settingsStatusOverrides }; const kbnServer = { status: { diff --git a/src/ui/settings/index.js b/src/ui/settings/index.js index 29f5959d67038..986f2f8428fd9 100644 --- a/src/ui/settings/index.js +++ b/src/ui/settings/index.js @@ -68,6 +68,13 @@ export default function setupSettings(kbnServer, server, config) { assertRequest(req); const { callWithRequest, errors } = server.plugins.elasticsearch; + // If the ui settings status isn't green, we shouldn't be attempting to get + // user settings, since we can't be sure that all the necessary conditions + // (e.g. elasticsearch being available) are met. + if (status.state !== 'green') { + return hydrateUserSettings({}); + } + const params = getClientSettings(config); const allowedErrors = [errors[404], errors[403], errors.NoConnections]; if (ignore401Errors) allowedErrors.push(errors[401]);