From a883ede16a4bc57153cd0c1c00f532cb9092a8f5 Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 20 Nov 2020 12:57:24 -0700 Subject: [PATCH 1/4] [dev/cli] detect worker type using env, not cluster module --- packages/kbn-config/src/__mocks__/env.ts | 3 +-- .../kbn-config/src/__snapshots__/env.test.ts.snap | 12 ++++++------ packages/kbn-config/src/env.test.ts | 2 +- packages/kbn-config/src/env.ts | 6 +++--- src/apm.js | 4 ---- src/cli/cluster/cluster_manager.ts | 2 -- src/cli/cluster/worker.ts | 2 +- src/cli/serve/serve.js | 6 +++--- src/core/server/bootstrap.ts | 11 +++++------ src/core/server/http/http_service.test.ts | 2 +- src/core/server/http/http_service.ts | 2 +- src/core/server/legacy/legacy_service.test.ts | 4 ++-- src/core/server/legacy/legacy_service.ts | 8 +++----- src/core/server/plugins/plugins_service.test.ts | 8 ++++---- src/core/server/plugins/plugins_service.ts | 4 ++-- src/core/server/server.test.ts | 4 ++-- src/core/server/server.ts | 2 +- src/core/test_helpers/kbn_server.ts | 2 +- src/legacy/server/kbn_server.js | 3 +-- src/legacy/server/logging/log_format_string.js | 4 ++-- src/legacy/server/logging/rotate/index.ts | 7 ------- src/legacy/server/logging/rotate/log_rotator.ts | 5 ++--- 22 files changed, 42 insertions(+), 61 deletions(-) diff --git a/packages/kbn-config/src/__mocks__/env.ts b/packages/kbn-config/src/__mocks__/env.ts index f37ac14e602358..3bb24debda7807 100644 --- a/packages/kbn-config/src/__mocks__/env.ts +++ b/packages/kbn-config/src/__mocks__/env.ts @@ -43,7 +43,6 @@ export function getEnvOptions(options: DeepPartial = {}): 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 index 005fa6e680f8b8..09cd1acbd7177a 100644 --- a/packages/kbn-config/src/__snapshots__/env.test.ts.snap +++ b/packages/kbn-config/src/__snapshots__/env.test.ts.snap @@ -23,7 +23,7 @@ Env { "/some/other/path/some-kibana.yml", ], "homeDir": "/test/kibanaRoot", - "isDevClusterMaster": false, + "isDevCliParent": false, "logDir": "/test/kibanaRoot/log", "mode": Object { "dev": true, @@ -69,7 +69,7 @@ Env { "/some/other/path/some-kibana.yml", ], "homeDir": "/test/kibanaRoot", - "isDevClusterMaster": false, + "isDevCliParent": false, "logDir": "/test/kibanaRoot/log", "mode": Object { "dev": false, @@ -114,7 +114,7 @@ Env { "/test/cwd/config/kibana.yml", ], "homeDir": "/test/kibanaRoot", - "isDevClusterMaster": true, + "isDevCliParent": true, "logDir": "/test/kibanaRoot/log", "mode": Object { "dev": true, @@ -159,7 +159,7 @@ Env { "/some/other/path/some-kibana.yml", ], "homeDir": "/test/kibanaRoot", - "isDevClusterMaster": false, + "isDevCliParent": false, "logDir": "/test/kibanaRoot/log", "mode": Object { "dev": false, @@ -204,7 +204,7 @@ Env { "/some/other/path/some-kibana.yml", ], "homeDir": "/test/kibanaRoot", - "isDevClusterMaster": false, + "isDevCliParent": false, "logDir": "/test/kibanaRoot/log", "mode": Object { "dev": false, @@ -249,7 +249,7 @@ Env { "/some/other/path/some-kibana.yml", ], "homeDir": "/some/home/dir", - "isDevClusterMaster": false, + "isDevCliParent": false, "logDir": "/some/home/dir/log", "mode": Object { "dev": false, diff --git a/packages/kbn-config/src/env.test.ts b/packages/kbn-config/src/env.test.ts index 1613a90951d403..5aee33e6fda5e6 100644 --- 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 index e7b4658262235b..bc6d2ae070acf1 100644 --- 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 */ @@ -105,7 +105,7 @@ export class Env { * Indicates that this Kibana instance is run as development Node Cluster master. * @internal */ - public readonly isDevClusterMaster: boolean; + public readonly isDevCliParent: boolean; /** * @internal @@ -123,7 +123,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({ diff --git a/src/apm.js b/src/apm.js index bde37fa006c610..4f5fe29cbb5faa 100644 --- 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 index 931650a67687cf..f8ef388a82f280 100644 --- a/src/cli/cluster/cluster_manager.ts +++ b/src/cli/cluster/cluster_manager.ts @@ -35,8 +35,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 index d28065765070b5..ad8c60dcbac80d 100644 --- a/src/cli/cluster/worker.ts +++ b/src/cli/cluster/worker.ts @@ -88,7 +88,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 index f344d3b70ed9d3..158b7ac8948434 100644 --- 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('--open', 'Open a browser window to the base url after the server is started') @@ -242,7 +242,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 index ff1a5c0340c466..6711a8b8987e58 100644 --- 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 index 11cea88fa0dd2f..3d553224612888 100644 --- 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 index 0127a6493e7fd1..171a20160d26d4 100644 --- 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 index 5cc6fcb1335072..fe19ef9d0a774e 100644 --- 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 index 3111c8daf79815..4ae6c9d4375760 100644 --- 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 index 02b82c17ed4fc0..601e0038b0cf0c 100644 --- 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>({ 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 index 5967e6d5358dec..e1622b1e192311 100644 --- 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(); - 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 index 0c7ebbcb527ec2..f377bfc3217354 100644 --- 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 index 0f7e8cced999c0..e253663d8dc8dd 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -124,7 +124,7 @@ export class Server { const legacyConfigSetup = await this.legacy.setupLegacyConfig(); // rely on dev server to validate config, don't validate in the parent process - if (!this.env.isDevClusterMaster) { + if (!this.env.isDevCliParent) { // Immediately terminate in case of invalid configuration // This needs to be done after plugin discovery await this.configService.validate(); diff --git a/src/core/test_helpers/kbn_server.ts b/src/core/test_helpers/kbn_server.ts index 93a173cdbdece4..30bb1f71a7ec0f 100644 --- a/src/core/test_helpers/kbn_server.ts +++ b/src/core/test_helpers/kbn_server.ts @@ -83,7 +83,7 @@ export function createRootWithSettings( dist: false, ...cliArgs, }, - isDevClusterMaster: false, + isDevCliParent: false, }); return new Root( diff --git a/src/legacy/server/kbn_server.js b/src/legacy/server/kbn_server.js index 013da35d2acb7e..eeef62e2bcb7dc 100644 --- a/src/legacy/server/kbn_server.js +++ b/src/legacy/server/kbn_server.js @@ -19,7 +19,6 @@ import { constant, once, compact, flatten } from 'lodash'; -import { isWorker } from 'cluster'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { fromRoot, pkg } from '../../core/server/utils'; import { Config } from './config'; @@ -121,7 +120,7 @@ export default class KbnServer { const { server, config } = this; - if (isWorker) { + if (process.env.isDevCliChild) { // help parent process know when we are ready process.send(['WORKER_LISTENING']); } diff --git a/src/legacy/server/logging/log_format_string.js b/src/legacy/server/logging/log_format_string.js index cbbf71dd894acb..f696e6f9b0002c 100644 --- a/src/legacy/server/logging/log_format_string.js +++ b/src/legacy/server/logging/log_format_string.js @@ -53,7 +53,7 @@ const type = _.memoize(function (t) { 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 default class KbnLoggerStringFormat extends LogFormat { format(data) { @@ -70,6 +70,6 @@ export default class KbnLoggerStringFormat extends LogFormat { return s + `[${color(t)(t)}]`; }, ''); - return `${workerType}${type(data.type)} [${time}] ${tags} ${msg}`; + return `${prefix}${type(data.type)} [${time}] ${tags} ${msg}`; } } diff --git a/src/legacy/server/logging/rotate/index.ts b/src/legacy/server/logging/rotate/index.ts index d6b7cfa76f9ee4..cc7186c06cf3ff 100644 --- a/src/legacy/server/logging/rotate/index.ts +++ b/src/legacy/server/logging/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 { KibanaConfig } from '../../kbn_server'; @@ -30,12 +29,6 @@ export async function setupLoggingRotate(server: Server, config: KibanaConfig) { 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.get('logging.dest') === 'stdout') { diff --git a/src/legacy/server/logging/rotate/log_rotator.ts b/src/legacy/server/logging/rotate/log_rotator.ts index c4054b2daed456..a82b7baa83a329 100644 --- a/src/legacy/server/logging/rotate/log_rotator.ts +++ b/src/legacy/server/logging/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'; @@ -348,8 +347,8 @@ export class LogRotator { } _sendReloadLogConfigSignal() { - if (isMaster) { - (process as NodeJS.EventEmitter).emit('SIGHUP'); + if (!process.env.isDevCliChild) { + process.emit('SIGHUP', 'SIGHUP'); return; } From 2d034f5c944d7b9645d9e21df8ebd5b0f828ff3b Mon Sep 17 00:00:00 2001 From: spalger Date: Fri, 20 Nov 2020 14:06:19 -0700 Subject: [PATCH 2/4] remove unused property --- src/cli/cluster/worker.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cli/cluster/worker.ts b/src/cli/cluster/worker.ts index ad8c60dcbac80d..26b2a643e53737 100644 --- 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; From 6347688544c4458a213c7bb6319fcfb555010473 Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 23 Nov 2020 11:20:37 -0700 Subject: [PATCH 3/4] assume that if process.send is undefined we are not a child --- packages/kbn-legacy-logging/src/rotate/log_rotator.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/kbn-legacy-logging/src/rotate/log_rotator.ts b/packages/kbn-legacy-logging/src/rotate/log_rotator.ts index 11d3f497916dcf..fc2c088f01ddea 100644 --- a/packages/kbn-legacy-logging/src/rotate/log_rotator.ts +++ b/packages/kbn-legacy-logging/src/rotate/log_rotator.ts @@ -350,7 +350,7 @@ export class LogRotator { } _sendReloadLogConfigSignal() { - if (!process.env.isDevCliChild) { + if (!process.env.isDevCliChild || !process.send) { process.emit('SIGHUP', 'SIGHUP'); return; } @@ -358,14 +358,6 @@ export class LogRotator { // 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']); } } From 6f72a36cc063ced04896ef609ae651ad3ee1a0fa Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 24 Nov 2020 13:20:16 -0700 Subject: [PATCH 4/4] update comment --- packages/kbn-config/src/env.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-config/src/env.ts b/packages/kbn-config/src/env.ts index d02c841e613379..4ae8d7b7f9aa51 100644 --- a/packages/kbn-config/src/env.ts +++ b/packages/kbn-config/src/env.ts @@ -101,7 +101,7 @@ 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 isDevCliParent: boolean;