Skip to content

Commit

Permalink
[dev/cli] detect worker type using env, not cluster module
Browse files Browse the repository at this point in the history
  • Loading branch information
spalger committed Nov 20, 2020
1 parent d31ee21 commit a883ede
Show file tree
Hide file tree
Showing 22 changed files with 42 additions and 61 deletions.
3 changes: 1 addition & 2 deletions packages/kbn-config/src/__mocks__/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,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,
};
}
12 changes: 6 additions & 6 deletions packages/kbn-config/src/__snapshots__/env.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/kbn-config/src/env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
);

Expand Down
6 changes: 3 additions & 3 deletions packages/kbn-config/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { PackageInfo, EnvironmentMode } from './types';
export interface EnvOptions {
configs: string[];
cliArgs: CliArgs;
isDevClusterMaster: boolean;
isDevCliParent: boolean;
}

/** @internal */
Expand Down Expand Up @@ -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
Expand All @@ -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<EnvironmentMode>({
Expand Down
4 changes: 0 additions & 4 deletions src/apm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 0 additions & 2 deletions src/cli/cluster/cluster_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion src/cli/cluster/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 || '',
};
Expand Down
6 changes: 3 additions & 3 deletions src/cli/serve/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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),
Expand Down
11 changes: 5 additions & 6 deletions src/core/server/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/http/http_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});

Expand Down
2 changes: 1 addition & 1 deletion src/core/server/http/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/legacy/legacy_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 3 additions & 5 deletions src/core/server/legacy/legacy_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 4 additions & 4 deletions src/core/server/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 } });
Expand Down Expand Up @@ -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,
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/core/server/plugins/plugins_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/core/test_helpers/kbn_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export function createRootWithSettings(
dist: false,
...cliArgs,
},
isDevClusterMaster: false,
isDevCliParent: false,
});

return new Root(
Expand Down
3 changes: 1 addition & 2 deletions src/legacy/server/kbn_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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']);
}
Expand Down
4 changes: 2 additions & 2 deletions src/legacy/server/logging/log_format_string.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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}`;
}
}
7 changes: 0 additions & 7 deletions src/legacy/server/logging/rotate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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') {
Expand Down
5 changes: 2 additions & 3 deletions src/legacy/server/logging/rotate/log_rotator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit a883ede

Please sign in to comment.