Skip to content
This repository has been archived by the owner on Mar 31, 2024. It is now read-only.

Commit

Permalink
[dev/cli] detect worker type using env, not cluster module (elastic#8…
Browse files Browse the repository at this point in the history
…3977)

* [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 <spalger@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
3 people committed Nov 25, 2020
1 parent 9440389 commit b4dc64b
Show file tree
Hide file tree
Showing 22 changed files with 43 additions and 72 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 @@ -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,
};
}
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
8 changes: 4 additions & 4 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 @@ -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
Expand All @@ -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>({
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-legacy-logging/src/log_format_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand All @@ -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}`;
}
}
7 changes: 0 additions & 7 deletions packages/kbn-legacy-logging/src/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 { LegacyLoggingConfig } from '../schema';
Expand All @@ -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') {
Expand Down
13 changes: 2 additions & 11 deletions packages/kbn-legacy-logging/src/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 @@ -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']);
}
}
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 @@ -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'
Expand Down
4 changes: 1 addition & 3 deletions src/cli/cluster/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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 || '',
};
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('--ssl', 'Run the dev server using HTTPS')
Expand Down Expand Up @@ -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),
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
Loading

0 comments on commit b4dc64b

Please sign in to comment.