From 134f221106d34c8927824949dcdef687ab5b9301 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Wed, 29 Jan 2020 13:14:36 +0100 Subject: [PATCH] Refactoring and PR feedback --- .../elasticsearch/elasticsearch_config.ts | 2 +- .../elasticsearch/elasticsearch_service.ts | 7 ++ .../version_check/ensure_es_version.test.ts | 4 +- .../version_check/ensure_es_version.ts | 69 +++++++++++-------- ...> es_kibana_version_compatability.test.ts} | 16 ++--- ....ts => es_kibana_version_compatability.ts} | 15 ++-- .../elasticsearch/lib/version_health_check.js | 2 +- 7 files changed, 68 insertions(+), 47 deletions(-) rename src/core/server/elasticsearch/version_check/{is_es_compatible_with_kibana.test.ts => es_kibana_version_compatability.test.ts} (72%) rename src/core/server/elasticsearch/version_check/{is_es_compatible_with_kibana.ts => es_kibana_version_compatability.ts} (76%) diff --git a/src/core/server/elasticsearch/elasticsearch_config.ts b/src/core/server/elasticsearch/elasticsearch_config.ts index c0fe3fd172b202..50866e5550d8ec 100644 --- a/src/core/server/elasticsearch/elasticsearch_config.ts +++ b/src/core/server/elasticsearch/elasticsearch_config.ts @@ -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'; diff --git a/src/core/server/elasticsearch/elasticsearch_service.ts b/src/core/server/elasticsearch/elasticsearch_service.ts index 44d261a03b2c91..4d897dbd8f7aa3 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.ts @@ -170,6 +170,13 @@ export class ElasticsearchService implements CoreService).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)) }, diff --git a/src/core/server/elasticsearch/version_check/ensure_es_version.test.ts b/src/core/server/elasticsearch/version_check/ensure_es_version.test.ts index 4c87773d2fcf8b..addcca3133f127 100644 --- a/src/core/server/elasticsearch/version_check/ensure_es_version.test.ts +++ b/src/core/server/elasticsearch/version_check/ensure_es_version.test.ts @@ -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')); @@ -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, diff --git a/src/core/server/elasticsearch/version_check/ensure_es_version.ts b/src/core/server/elasticsearch/version_check/ensure_es_version.ts index 238daa1760abcb..0203df124ba14e 100644 --- a/src/core/server/elasticsearch/version_check/ensure_es_version.ts +++ b/src/core/server/elasticsearch/version_check/ensure_es_version.ts @@ -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 '..'; @@ -37,12 +39,6 @@ export interface PollEsNodesVersionOptions { esVersionCheckInterval: number; } -export interface NodesInfo { - nodes: { - [key: string]: NodeInfo; - }; -} - interface NodeInfo { version: string; ip: string; @@ -52,6 +48,12 @@ interface NodeInfo { name: string; } +export interface NodesInfo { + nodes: { + [key: string]: NodeInfo; + }; +} + export interface NodesVersionCompatibility { isCompatible: boolean; message?: string; @@ -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) { @@ -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, @@ -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 ); }; diff --git a/src/core/server/elasticsearch/version_check/is_es_compatible_with_kibana.test.ts b/src/core/server/elasticsearch/version_check/es_kibana_version_compatability.test.ts similarity index 72% rename from src/core/server/elasticsearch/version_check/is_es_compatible_with_kibana.test.ts rename to src/core/server/elasticsearch/version_check/es_kibana_version_compatability.test.ts index 374ec240b2f49d..152f25c8138819 100644 --- a/src/core/server/elasticsearch/version_check/is_es_compatible_with_kibana.test.ts +++ b/src/core/server/elasticsearch/version_check/es_kibana_version_compatability.test.ts @@ -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); }); }); }); diff --git a/src/core/server/elasticsearch/version_check/is_es_compatible_with_kibana.ts b/src/core/server/elasticsearch/version_check/es_kibana_version_compatability.ts similarity index 76% rename from src/core/server/elasticsearch/version_check/is_es_compatible_with_kibana.ts rename to src/core/server/elasticsearch/version_check/es_kibana_version_compatability.ts index fd0bdd29ff8374..28b9c0a23e672d 100644 --- a/src/core/server/elasticsearch/version_check/is_es_compatible_with_kibana.ts +++ b/src/core/server/elasticsearch/version_check/es_kibana_version_compatability.ts @@ -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), @@ -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; +} diff --git a/src/legacy/core_plugins/elasticsearch/lib/version_health_check.js b/src/legacy/core_plugins/elasticsearch/lib/version_health_check.js index 5eaafccd7b8d94..4ee8307f490eb0 100644 --- a/src/legacy/core_plugins/elasticsearch/lib/version_health_check.js +++ b/src/legacy/core_plugins/elasticsearch/lib/version_health_check.js @@ -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,