Skip to content

Commit

Permalink
settings: do not query ES for settings in non-green status (elastic#9308
Browse files Browse the repository at this point in the history
)

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.
  • Loading branch information
epixa authored Dec 1, 2016
1 parent 4ef3b68 commit a15d1c9
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
9 changes: 8 additions & 1 deletion src/core_plugins/elasticsearch/lib/health_check.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
13 changes: 11 additions & 2 deletions src/ui/settings/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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()
Expand All @@ -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: {
Expand Down
7 changes: 7 additions & 0 deletions src/ui/settings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down

0 comments on commit a15d1c9

Please sign in to comment.