[dev/cli] detect worker type using env, not cluster module (#83977)

Spencer · spalger · kibanamachine · web-flow · commit ea8ea4e4e0c6 · 2020-11-25T08:03:47.000-07:00
* [dev/cli] detect worker type using env, not cluster module

* remove unused property

* assume that if process.send is undefined we are not a child

* update comment

Co-authored-by: spalger &lt;spalger@users.noreply.github.com&gt;
Co-authored-by: Kibana Machine &lt;42973632+kibanamachine@users.noreply.github.com&gt;
diff --git a/packages/kbn-config/src/__mocks__/env.ts b/packages/kbn-config/src/__mocks__/env.ts
@@ -42,7 +42,6 @@ export function getEnvOptions(options: DeepPartial<EnvOptions> = {}): EnvOptions
       runExamples: false,
       ...(options.cliArgs || {}),
     },
-    isDevClusterMaster:
-      options.isDevClusterMaster !== undefined ? options.isDevClusterMaster : false,
+    isDevCliParent: options.isDevCliParent !== undefined ? options.isDevCliParent : false,
   };
 }
diff --git a/packages/kbn-config/src/__snapshots__/env.test.ts.snap b/packages/kbn-config/src/__snapshots__/env.test.ts.snap
diff --git a/packages/kbn-config/src/env.test.ts b/packages/kbn-config/src/env.test.ts
@@ -47,7 +47,7 @@ test('correctly creates default environment in dev mode.', () => {
     REPO_ROOT,
     getEnvOptions({
       configs: ['/test/cwd/config/kibana.yml'],
-      isDevClusterMaster: true,
+      isDevCliParent: true,
     })
   );
 
diff --git a/packages/kbn-config/src/env.ts b/packages/kbn-config/src/env.ts
@@ -26,7 +26,7 @@ import { PackageInfo, EnvironmentMode } from './types';
 export interface EnvOptions {
   configs: string[];
   cliArgs: CliArgs;
-  isDevClusterMaster: boolean;
+  isDevCliParent: boolean;
 }
 
 /** @internal */
@@ -101,10 +101,10 @@ export class Env {
   public readonly configs: readonly string[];
 
   /**
-   * Indicates that this Kibana instance is run as development Node Cluster master.
+   * Indicates that this Kibana instance is running in the parent process of the dev cli.
    * @internal
    */
-  public readonly isDevClusterMaster: boolean;
+  public readonly isDevCliParent: boolean;
 
   /**
    * @internal
@@ -122,7 +122,7 @@ export class Env {
 
     this.cliArgs = Object.freeze(options.cliArgs);
     this.configs = Object.freeze(options.configs);
-    this.isDevClusterMaster = options.isDevClusterMaster;
+    this.isDevCliParent = options.isDevCliParent;
 
     const isDevMode = this.cliArgs.dev || this.cliArgs.envName === 'development';
     this.mode = Object.freeze<EnvironmentMode>({
diff --git a/packages/kbn-legacy-logging/src/log_format_string.ts b/packages/kbn-legacy-logging/src/log_format_string.ts
@@ -54,7 +54,7 @@ const type = _.memoize((t: string) => {
   return color(t)(_.pad(t, 7).slice(0, 7));
 });
 
-const workerType = process.env.kbnWorkerType ? `${type(process.env.kbnWorkerType)} ` : '';
+const prefix = process.env.isDevCliChild ? `${type('server')} ` : '';
 
 export class KbnLoggerStringFormat extends BaseLogFormat {
   format(data: Record<string, any>) {
@@ -71,6 +71,6 @@ export class KbnLoggerStringFormat extends BaseLogFormat {
         return s + `[${color(t)(t)}]`;
       }, '');
 
-    return `${workerType}${type(data.type)} [${time}] ${tags} ${msg}`;
+    return `${prefix}${type(data.type)} [${time}] ${tags} ${msg}`;
   }
 }
diff --git a/packages/kbn-legacy-logging/src/rotate/index.ts b/packages/kbn-legacy-logging/src/rotate/index.ts
@@ -17,7 +17,6 @@
  * under the License.
  */
 
-import { isMaster, isWorker } from 'cluster';
 import { Server } from '@hapi/hapi';
 import { LogRotator } from './log_rotator';
 import { LegacyLoggingConfig } from '../schema';
@@ -30,12 +29,6 @@ export async function setupLoggingRotate(server: Server, config: LegacyLoggingCo
     return;
   }
 
-  // We just want to start the logging rotate service once
-  // and we choose to use the master (prod) or the worker server (dev)
-  if (!isMaster && isWorker && process.env.kbnWorkerType !== 'server') {
-    return;
-  }
-
   // We don't want to run logging rotate server if
   // we are not logging to a file
   if (config.dest === 'stdout') {
diff --git a/packages/kbn-legacy-logging/src/rotate/log_rotator.ts b/packages/kbn-legacy-logging/src/rotate/log_rotator.ts
@@ -18,7 +18,6 @@
  */
 
 import * as chokidar from 'chokidar';
-import { isMaster } from 'cluster';
 import fs from 'fs';
 import { Server } from '@hapi/hapi';
 import { throttle } from 'lodash';
@@ -351,22 +350,14 @@ export class LogRotator {
   }
 
   _sendReloadLogConfigSignal() {
-    if (isMaster) {
-      (process as NodeJS.EventEmitter).emit('SIGHUP');
+    if (!process.env.isDevCliChild || !process.send) {
+      process.emit('SIGHUP', 'SIGHUP');
       return;
     }
 
     // Send a special message to the cluster manager
     // so it can forward it correctly
     // It will only run when we are under cluster mode (not under a production environment)
-    if (!process.send) {
-      this.log(
-        ['error', 'logging:rotate'],
-        'For some unknown reason process.send is not defined, the rotation was not successful'
-      );
-      return;
-    }
-
     process.send(['RELOAD_LOGGING_CONFIG_FROM_SERVER_WORKER']);
   }
 }
diff --git a/src/apm.js b/src/apm.js
@@ -30,10 +30,6 @@ let apmConfig;
 const isKibanaDistributable = Boolean(build && build.distributable === true);
 
 module.exports = function (serviceName = name) {
-  if (process.env.kbnWorkerType === 'optmzr') {
-    return;
-  }
-
   apmConfig = loadConfiguration(process.argv, ROOT_DIR, isKibanaDistributable);
   const conf = apmConfig.getConfig(serviceName);
   const apm = require('elastic-apm-node');
diff --git a/src/cli/cluster/cluster_manager.ts b/src/cli/cluster/cluster_manager.ts
@@ -33,8 +33,6 @@ import { BasePathProxyServer } from '../../core/server/http';
 import { Log } from './log';
 import { Worker } from './worker';
 
-process.env.kbnWorkerType = 'managr';
-
 export type SomeCliArgs = Pick<
   CliArgs,
   | 'quiet'
diff --git a/src/cli/cluster/worker.ts b/src/cli/cluster/worker.ts
@@ -56,7 +56,6 @@ export class Worker extends EventEmitter {
   private readonly clusterBinder: BinderFor;
   private readonly processBinder: BinderFor;
 
-  private type: string;
   private title: string;
   private log: any;
   private forkBinder: BinderFor | null = null;
@@ -76,7 +75,6 @@ export class Worker extends EventEmitter {
     super();
 
     this.log = opts.log;
-    this.type = opts.type;
     this.title = opts.title || opts.type;
     this.watch = opts.watch !== false;
     this.startCount = 0;
@@ -88,7 +86,7 @@ export class Worker extends EventEmitter {
 
     this.env = {
       NODE_OPTIONS: process.env.NODE_OPTIONS || '',
-      kbnWorkerType: this.type,
+      isDevCliChild: 'true',
       kbnWorkerArgv: JSON.stringify([...(opts.baseArgv || baseArgv), ...(opts.argv || [])]),
       ELASTIC_APM_SERVICE_NAME: opts.apmServiceName || '',
     };
diff --git a/src/cli/serve/serve.js b/src/cli/serve/serve.js
@@ -43,7 +43,7 @@ function canRequire(path) {
 }
 
 const CLUSTER_MANAGER_PATH = resolve(__dirname, '../cluster/cluster_manager');
-const CAN_CLUSTER = canRequire(CLUSTER_MANAGER_PATH);
+const DEV_MODE_SUPPORTED = canRequire(CLUSTER_MANAGER_PATH);
 
 const REPL_PATH = resolve(__dirname, '../repl');
 const CAN_REPL = canRequire(REPL_PATH);
@@ -189,7 +189,7 @@ export default function (program) {
       );
   }
 
-  if (CAN_CLUSTER) {
+  if (DEV_MODE_SUPPORTED) {
     command
       .option('--dev', 'Run the server with development mode defaults')
       .option('--ssl', 'Run the dev server using HTTPS')
@@ -240,7 +240,7 @@ export default function (program) {
         dist: !!opts.dist,
       },
       features: {
-        isClusterModeSupported: CAN_CLUSTER,
+        isCliDevModeSupported: DEV_MODE_SUPPORTED,
         isReplModeSupported: CAN_REPL,
       },
       applyConfigOverrides: (rawConfig) => applyConfigOverrides(rawConfig, opts, unknownOptions),
diff --git a/src/core/server/bootstrap.ts b/src/core/server/bootstrap.ts
@@ -18,16 +18,15 @@
  */
 
 import chalk from 'chalk';
-import { isMaster } from 'cluster';
 import { CliArgs, Env, RawConfigService } from './config';
 import { Root } from './root';
 import { CriticalError } from './errors';
 
 interface KibanaFeatures {
-  // Indicates whether we can run Kibana in a so called cluster mode in which
-  // Kibana is run as a "worker" process together with optimizer "worker" process
-  // that are orchestrated by the "master" process (dev mode only feature).
-  isClusterModeSupported: boolean;
+  // Indicates whether we can run Kibana in dev mode in which Kibana is run as
+  // a child process together with optimizer "worker" processes that are
+  // orchestrated by a parent process (dev mode only feature).
+  isCliDevModeSupported: boolean;
 
   // Indicates whether we can run Kibana in REPL mode (dev mode only feature).
   isReplModeSupported: boolean;
@@ -71,7 +70,7 @@ export async function bootstrap({
   const env = Env.createDefault(REPO_ROOT, {
     configs,
     cliArgs,
-    isDevClusterMaster: isMaster && cliArgs.dev && features.isClusterModeSupported,
+    isDevCliParent: cliArgs.dev && features.isCliDevModeSupported && !process.env.isDevCliChild,
   });
 
   const rawConfigService = new RawConfigService(env.configs, applyConfigOverrides);
diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts
@@ -264,7 +264,7 @@ test('does not start http server if process is dev cluster master', async () =>
   const service = new HttpService({
     coreId,
     configService,
-    env: Env.createDefault(REPO_ROOT, getEnvOptions({ isDevClusterMaster: true })),
+    env: Env.createDefault(REPO_ROOT, getEnvOptions({ isDevCliParent: true })),
     logger,
   });
 
diff --git a/src/core/server/http/http_service.ts b/src/core/server/http/http_service.ts
@@ -158,7 +158,7 @@ export class HttpService
    * @internal
    * */
   private shouldListen(config: HttpConfig) {
-    return !this.coreContext.env.isDevClusterMaster && config.autoListen;
+    return !this.coreContext.env.isDevCliParent && config.autoListen;
   }
 
   public async stop() {
diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts
@@ -362,7 +362,7 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => {
         REPO_ROOT,
         getEnvOptions({
           cliArgs: { silent: true, basePath: false },
-          isDevClusterMaster: true,
+          isDevCliParent: true,
         })
       ),
       logger,
@@ -391,7 +391,7 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => {
         REPO_ROOT,
         getEnvOptions({
           cliArgs: { quiet: true, basePath: true },
-          isDevClusterMaster: true,
+          isDevCliParent: true,
         })
       ),
       logger,
diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts
@@ -144,7 +144,7 @@ export class LegacyService implements CoreService {
     this.log.debug('starting legacy service');
 
     // Receive initial config and create kbnServer/ClusterManager.
-    if (this.coreContext.env.isDevClusterMaster) {
+    if (this.coreContext.env.isDevCliParent) {
       await this.createClusterManager(this.legacyRawConfig!);
     } else {
       this.kbnServer = await this.createKbnServer(
@@ -310,10 +310,8 @@ export class LegacyService implements CoreService {
       logger: this.coreContext.logger,
     });
 
-    // The kbnWorkerType check is necessary to prevent the repl
-    // from being started multiple times in different processes.
-    // We only want one REPL.
-    if (this.coreContext.env.cliArgs.repl && process.env.kbnWorkerType === 'server') {
+    // Prevent the repl from being started multiple times in different processes.
+    if (this.coreContext.env.cliArgs.repl && process.env.isDevCliChild) {
       // eslint-disable-next-line @typescript-eslint/no-var-requires
       require('./cli').startRepl(kbnServer);
     }
diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts
@@ -102,7 +102,7 @@ const createPlugin = (
   });
 };
 
-async function testSetup(options: { isDevClusterMaster?: boolean } = {}) {
+async function testSetup(options: { isDevCliParent?: boolean } = {}) {
   mockPackage.raw = {
     branch: 'feature-v1',
     version: 'v1',
@@ -116,7 +116,7 @@ async function testSetup(options: { isDevClusterMaster?: boolean } = {}) {
   coreId = Symbol('core');
   env = Env.createDefault(REPO_ROOT, {
     ...getEnvOptions(),
-    isDevClusterMaster: options.isDevClusterMaster ?? false,
+    isDevCliParent: options.isDevCliParent ?? false,
   });
 
   config$ = new BehaviorSubject<Record<string, any>>({ plugins: { initialize: true } });
@@ -638,10 +638,10 @@ describe('PluginsService', () => {
   });
 });
 
-describe('PluginService when isDevClusterMaster is true', () => {
+describe('PluginService when isDevCliParent is true', () => {
   beforeEach(async () => {
     await testSetup({
-      isDevClusterMaster: true,
+      isDevCliParent: true,
     });
   });
 
diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts
@@ -90,7 +90,7 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
 
   constructor(private readonly coreContext: CoreContext) {
     this.log = coreContext.logger.get('plugins-service');
-    this.discoveryDisabled = coreContext.env.isDevClusterMaster;
+    this.discoveryDisabled = coreContext.env.isDevCliParent;
     this.pluginsSystem = new PluginsSystem(coreContext);
     this.configService = coreContext.configService;
     this.config$ = coreContext.configService
@@ -133,7 +133,7 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
     const config = await this.config$.pipe(first()).toPromise();
 
     let contracts = new Map<PluginName, unknown>();
-    const initialize = config.initialize && !this.coreContext.env.isDevClusterMaster;
+    const initialize = config.initialize && !this.coreContext.env.isDevCliParent;
     if (initialize) {
       contracts = await this.pluginsSystem.setupPlugins(deps);
       this.registerPluginStaticDirs(deps);
diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts
@@ -216,10 +216,10 @@ test(`doesn't setup core services if legacy config validation fails`, async () =
   expect(mockI18nService.setup).not.toHaveBeenCalled();
 });
 
-test(`doesn't validate config if env.isDevClusterMaster is true`, async () => {
+test(`doesn't validate config if env.isDevCliParent is true`, async () => {
   const devParentEnv = Env.createDefault(REPO_ROOT, {
     ...getEnvOptions(),
-    isDevClusterMaster: true,
+    isDevCliParent: true,
   });
 
   const server = new Server(rawConfigService, devParentEnv, logger);
diff --git a/src/core/server/server.ts b/src/core/server/server.ts
diff --git a/src/core/test_helpers/kbn_server.ts b/src/core/test_helpers/kbn_server.ts
diff --git a/src/legacy/server/kbn_server.js b/src/legacy/server/kbn_server.js

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,6 @@ export function getEnvOptions(options: DeepPartial<EnvOptions> = {}): EnvOptions`
`42`	`42`	`runExamples: false,`
`43`	`43`	`...(options.cliArgs \|\| {}),`
`44`	`44`	`},`
`45`		`- isDevClusterMaster:`
`46`		`- options.isDevClusterMaster !== undefined ? options.isDevClusterMaster : false,`
	`45`	`+ isDevCliParent: options.isDevCliParent !== undefined ? options.isDevCliParent : false,`
`47`	`46`	`};`
`48`	`47`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ const type = _.memoize((t: string) => {`
`54`	`54`	`return color(t)(_.pad(t, 7).slice(0, 7));`
`55`	`55`	`});`
`56`	`56`
`57`		-const workerType = process.env.kbnWorkerType ? `${type(process.env.kbnWorkerType)} ` : '';
	`57`	+const prefix = process.env.isDevCliChild ? `${type('server')} ` : '';
`58`	`58`
`59`	`59`	`export class KbnLoggerStringFormat extends BaseLogFormat {`
`60`	`60`	`format(data: Record<string, any>) {`
`@@ -71,6 +71,6 @@ export class KbnLoggerStringFormat extends BaseLogFormat {`
`71`	`71`	return s + `[${color(t)(t)}]`;
`72`	`72`	`}, '');`
`73`	`73`
`74`		- return `${workerType}${type(data.type)} [${time}] ${tags} ${msg}`;
	`74`	+ return `${prefix}${type(data.type)} [${time}] ${tags} ${msg}`;
`75`	`75`	`}`
`76`	`76`	`}`