Refactoring and PR feedback

Author: rudolf

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';

src/core/server/elasticsearch/elasticsearch_service.ts

@@ -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)) },
 

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,

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
   );
 };

src/core/server/elasticsearch/version_check/is_es_compatible_with_kibana.test.ts renamed to 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);
       });
     });
   });

src/core/server/elasticsearch/version_check/is_es_compatible_with_kibana.ts renamed to 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;
+}

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,