Skip to content

Commit

Permalink
Refactoring and PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
rudolf committed Jan 29, 2020
1 parent 61e8986 commit 134f221
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 47 deletions.
2 changes: 1 addition & 1 deletion src/core/server/elasticsearch/elasticsearch_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ const configSchema = schema.object({
ignoreVersionMismatch: schema.conditional(
schema.contextRef('dev'),
false,
schema.any({
schema.boolean({
validate: rawValue => {
if (rawValue === true) {
return '"ignoreVersionMismatch" can only be set to true in development mode';
Expand Down
7 changes: 7 additions & 0 deletions src/core/server/elasticsearch/elasticsearch_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,13 @@ export class ElasticsearchService implements CoreService<InternalElasticsearchSe
unknown
>).connect();

// TODO: Move to Status Service https://github.com/elastic/kibana/issues/41983
esNodesCompatibility$.subscribe(({ isCompatible, message }) => {
if (!isCompatible && message) {
this.log.error(message);
}
});

return {
legacy: { config$: clients$.pipe(map(clients => clients.config)) },

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ describe('mapNodesVersionCompatibility', () => {
describe('pollEsNodesVersion', () => {
const callWithInternalUser = jest.fn();
it('keeps polling when a poll request throws', done => {
expect.assertions(2);
expect.assertions(3);
callWithInternalUser.mockResolvedValueOnce(createNodes('5.1.0', '5.2.0', '5.0.0'));
callWithInternalUser.mockRejectedValueOnce(new Error('mock request error'));
callWithInternalUser.mockResolvedValueOnce(createNodes('5.1.0', '5.2.0', '5.1.1-Beta1'));
Expand All @@ -122,7 +122,7 @@ describe('pollEsNodesVersion', () => {
kibanaVersion: KIBANA_VERSION,
log: mockLogger,
})
.pipe(take(2))
.pipe(take(3))
.subscribe({
next: result => expect(result.isCompatible).toBeDefined(),
complete: done,
Expand Down
69 changes: 39 additions & 30 deletions src/core/server/elasticsearch/version_check/ensure_es_version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
* that defined in Kibana's package.json.
*/

import { coerce } from 'semver';
import { interval } from 'rxjs';
import { map, switchMap, catchError, distinctUntilChanged } from 'rxjs/operators';
import { isEsCompatibleWithKibana } from './is_es_compatible_with_kibana';
import { interval, from } from 'rxjs';
import { map, switchMap, distinctUntilChanged, catchError, startWith } from 'rxjs/operators';
import {
esVersionCompatibleWithKibana,
esVersionEqualsKibana,
} from './es_kibana_version_compatability';
import { Logger } from '../../logging';
import { APICaller } from '..';

Expand All @@ -37,12 +39,6 @@ export interface PollEsNodesVersionOptions {
esVersionCheckInterval: number;
}

export interface NodesInfo {
nodes: {
[key: string]: NodeInfo;
};
}

interface NodeInfo {
version: string;
ip: string;
Expand All @@ -52,6 +48,12 @@ interface NodeInfo {
name: string;
}

export interface NodesInfo {
nodes: {
[key: string]: NodeInfo;
};
}

export interface NodesVersionCompatibility {
isCompatible: boolean;
message?: string;
Expand All @@ -77,18 +79,14 @@ export function mapNodesVersionCompatibility(

// Aggregate incompatible ES nodes.
const incompatibleNodes = nodes.filter(
node => !isEsCompatibleWithKibana(node.version, kibanaVersion)
node => !esVersionCompatibleWithKibana(node.version, kibanaVersion)
);

// Aggregate ES nodes which should prompt a Kibana upgrade. It's acceptable
// if ES and Kibana versions are not the same so long as they are not
// if ES and Kibana versions are not the same as long as they are not
// incompatible, but we should warn about it.
// Ignore version qualifiers https://github.com/elastic/elasticsearch/issues/36859
const warningNodes = nodes.filter(node => {
const nodeSemVer = coerce(node.version);
const kibanaSemver = coerce(kibanaVersion);
return nodeSemVer && kibanaSemver && nodeSemVer.version !== kibanaSemver.version;
});
const warningNodes = nodes.filter(node => !esVersionEqualsKibana(node.version, kibanaVersion));

let message;
if (incompatibleNodes.length > 0) {
Expand All @@ -115,6 +113,18 @@ export function mapNodesVersionCompatibility(
};
}

// Returns true if two NodesVersionCompatibility entries match
function compareNodes(prev: NodesVersionCompatibility, curr: NodesVersionCompatibility) {
const nodesEqual = (n: NodeInfo, m: NodeInfo) => n.ip === m.ip && n.version === m.version;
return (
curr.isCompatible === prev.isCompatible &&
curr.incompatibleNodes.length === prev.incompatibleNodes.length &&
curr.warningNodes.length === prev.warningNodes.length &&
curr.incompatibleNodes.every((node, i) => nodesEqual(node, prev.incompatibleNodes[i])) &&
curr.warningNodes.every((node, i) => nodesEqual(node, prev.warningNodes[i]))
);
}

export const pollEsNodesVersion = ({
callWithInternalUser,
log,
Expand All @@ -130,22 +140,21 @@ export const pollEsNodesVersion = ({
filterPath: ['nodes.*.version', 'nodes.*.http.publish_address', 'nodes.*.ip'],
});
}),
// Log, but otherwise ignore 'nodes.info' request errors
catchError((err, caught$) => {
log.error('Unable to retrieve version information from Elasticsearch nodes.', err);
return caught$;
}),
map((nodesInfo: NodesInfo) =>
mapNodesVersionCompatibility(nodesInfo, kibanaVersion, ignoreVersionMismatch)
),
// Only emit if the IP or version numbers of the nodes changed from the
// previous result.
distinctUntilChanged((prev, curr) => {
const nodesEqual = (n: NodeInfo, m: NodeInfo) => n.ip === m.ip && n.version === m.version;
return (
curr.incompatibleNodes.every((node, i) => nodesEqual(node, prev.incompatibleNodes[i])) &&
curr.warningNodes.every((node, i) => nodesEqual(node, prev.warningNodes[i]))
catchError((_err, caught$) => {
// Return `isCompatible=false` when there's a 'nodes.info' request error
return caught$.pipe(
startWith({
isCompatible: false,
message: 'Unable to retrieve version information from Elasticsearch nodes.',
incompatibleNodes: [],
warningNodes: [],
kibanaVersion,
})
);
})
}),
distinctUntilChanged(compareNodes) // Only emit if there are new nodes or versions
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,39 @@
* under the License.
*/

import { isEsCompatibleWithKibana } from './is_es_compatible_with_kibana';
import { esVersionCompatibleWithKibana } from './es_kibana_version_compatability';

describe('plugins/elasticsearch', () => {
describe('lib/is_es_compatible_with_kibana', () => {
describe('returns false', () => {
it('when ES major is greater than Kibana major', () => {
expect(isEsCompatibleWithKibana('1.0.0', '0.0.0')).toBe(false);
expect(esVersionCompatibleWithKibana('1.0.0', '0.0.0')).toBe(false);
});

it('when ES major is less than Kibana major', () => {
expect(isEsCompatibleWithKibana('0.0.0', '1.0.0')).toBe(false);
expect(esVersionCompatibleWithKibana('0.0.0', '1.0.0')).toBe(false);
});

it('when majors are equal, but ES minor is less than Kibana minor', () => {
expect(isEsCompatibleWithKibana('1.0.0', '1.1.0')).toBe(false);
expect(esVersionCompatibleWithKibana('1.0.0', '1.1.0')).toBe(false);
});
});

describe('returns true', () => {
it('when version numbers are the same', () => {
expect(isEsCompatibleWithKibana('1.1.1', '1.1.1')).toBe(true);
expect(esVersionCompatibleWithKibana('1.1.1', '1.1.1')).toBe(true);
});

it('when majors are equal, and ES minor is greater than Kibana minor', () => {
expect(isEsCompatibleWithKibana('1.1.0', '1.0.0')).toBe(true);
expect(esVersionCompatibleWithKibana('1.1.0', '1.0.0')).toBe(true);
});

it('when majors and minors are equal, and ES patch is greater than Kibana patch', () => {
expect(isEsCompatibleWithKibana('1.1.1', '1.1.0')).toBe(true);
expect(esVersionCompatibleWithKibana('1.1.1', '1.1.0')).toBe(true);
});

it('when majors and minors are equal, but ES patch is less than Kibana patch', () => {
expect(isEsCompatibleWithKibana('1.1.0', '1.1.1')).toBe(true);
expect(esVersionCompatibleWithKibana('1.1.0', '1.1.1')).toBe(true);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@
* under the License.
*/

import semver, { coerce } from 'semver';

/**
* Let's weed out the ES versions that won't work with a given Kibana version.
* Checks for the compatibilitiy between Elasticsearch and Kibana versions
* 1. Major version differences will never work together.
* 2. Older versions of ES won't work with newer versions of Kibana.
*/

import semver from 'semver';

export function isEsCompatibleWithKibana(esVersion: string, kibanaVersion: string) {
export function esVersionCompatibleWithKibana(esVersion: string, kibanaVersion: string) {
const esVersionNumbers = {
major: semver.major(esVersion),
minor: semver.minor(esVersion),
Expand All @@ -50,3 +49,9 @@ export function isEsCompatibleWithKibana(esVersion: string, kibanaVersion: strin

return true;
}

export function esVersionEqualsKibana(nodeVersion: string, kibanaVersion: string) {
const nodeSemVer = coerce(nodeVersion);
const kibanaSemver = coerce(kibanaVersion);
return nodeSemVer && kibanaSemver && nodeSemVer.version === kibanaSemver.version;
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const versionHealthCheck = (esPlugin, logWithMetadata, esNodesCompatibili
if (!isCompatible) {
esPlugin.status.red(message);
} else {
if (message && message.length > 0) {
if (message) {
logWithMetadata(['warning'], message, {
kibanaVersion,
nodes: warningNodes,
Expand Down

0 comments on commit 134f221

Please sign in to comment.