Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] [Non-Inclusive Language] Replace isDevClusterMaster with isDevClusterManager #1762

Merged
merged 1 commit into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/osd-config/src/__mocks__/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,7 @@ export function getEnvOptions(options: DeepPartial<EnvOptions> = {}): EnvOptions
},
isDevClusterMaster:
options.isDevClusterMaster !== undefined ? options.isDevClusterMaster : false,
isDevClusterManager:
options.isDevClusterManager !== undefined ? options.isDevClusterManager : false,
};
}
98 changes: 91 additions & 7 deletions packages/osd-config/src/__snapshots__/env.test.ts.snap

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

42 changes: 41 additions & 1 deletion packages/osd-config/src/env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ beforeEach(() => {
mockPackage.raw = {};
});

test('correctly creates default environment in dev mode.', () => {
test('correctly creates default environment in dev mode when isDevClusterMaster (deprecated) is true', () => {
mockPackage.raw = {
branch: 'some-branch',
version: 'some-version',
Expand All @@ -59,10 +59,50 @@ test('correctly creates default environment in dev mode.', () => {
getEnvOptions({
configs: ['/test/cwd/config/opensearch_dashboards.yml'],
isDevClusterMaster: true,
isDevClusterManager: false,
})
);

expect(defaultEnv).toMatchSnapshot('env properties');
expect(defaultEnv.isDevClusterManager).toBeTruthy();
});

test('correctly creates default environment in dev mode when isDevClusterManager is true', () => {
mockPackage.raw = {
branch: 'some-branch',
version: 'some-version',
};

const defaultEnv = Env.createDefault(
REPO_ROOT,
getEnvOptions({
configs: ['/test/cwd/config/opensearch_dashboards.yml'],
isDevClusterMaster: false,
isDevClusterManager: true,
})
);

expect(defaultEnv).toMatchSnapshot('env properties');
expect(defaultEnv.isDevClusterManager).toBeTruthy();
});

test('correctly creates default environment in dev mode when isDevClusterManager and isDevClusterMaster both are true', () => {
mockPackage.raw = {
branch: 'some-branch',
version: 'some-version',
};

const defaultEnv = Env.createDefault(
REPO_ROOT,
getEnvOptions({
configs: ['/test/cwd/config/opensearch_dashboards.yml'],
isDevClusterMaster: true,
isDevClusterManager: true,
})
);

expect(defaultEnv).toMatchSnapshot('env properties');
expect(defaultEnv.isDevClusterManager).toBeTruthy();
});

test('correctly creates default environment in prod distributable mode.', () => {
Expand Down
8 changes: 5 additions & 3 deletions packages/osd-config/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ import { PackageInfo, EnvironmentMode } from './types';
export interface EnvOptions {
configs: string[];
cliArgs: CliArgs;
/** @deprecated use isDevClusterManager */
isDevClusterMaster: boolean;
isDevClusterManager: boolean;
}

/** @internal */
Expand Down Expand Up @@ -110,10 +112,10 @@ export class Env {
public readonly configs: readonly string[];

/**
* Indicates that this OpenSearch Dashboards instance is run as development Node Cluster master.
* Indicates that this OpenSearch Dashboards instance is run as development Node Cluster manager.
* @internal
*/
public readonly isDevClusterMaster: boolean;
public readonly isDevClusterManager: boolean;

/**
* @internal
Expand All @@ -137,7 +139,7 @@ export class Env {

this.cliArgs = Object.freeze(options.cliArgs);
this.configs = Object.freeze(options.configs);
this.isDevClusterMaster = options.isDevClusterMaster;
this.isDevClusterManager = options.isDevClusterManager || options.isDevClusterMaster;

const isDevMode = this.cliArgs.dev || this.cliArgs.envName === 'development';
this.mode = Object.freeze<EnvironmentMode>({
Expand Down
5 changes: 3 additions & 2 deletions src/core/server/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
*/

import chalk from 'chalk';
import { isMaster } from 'cluster';
import { isMaster as isClusterManager } from 'cluster';
import { CliArgs, Env, RawConfigService } from './config';
import { Root } from './root';
import { CriticalError } from './errors';
Expand Down Expand Up @@ -82,7 +82,8 @@ export async function bootstrap({
const env = Env.createDefault(REPO_ROOT, {
configs,
cliArgs,
isDevClusterMaster: isMaster && cliArgs.dev && features.isClusterModeSupported,
isDevClusterMaster: isClusterManager && cliArgs.dev && features.isClusterModeSupported,
isDevClusterManager: isClusterManager && cliArgs.dev && features.isClusterModeSupported,
});

const rawConfigService = new RawConfigService(env.configs, applyConfigOverrides);
Expand Down
68 changes: 66 additions & 2 deletions src/core/server/http/http_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ test('returns http server contract on setup', async () => {
});
});

test('does not start http server if process is dev cluster master', async () => {
test('does not start http server if process is dev cluster master (deprecated) or dev cluster manager', async () => {
const configService = createConfigService();
const httpServer = {
isListening: () => false,
Expand All @@ -275,7 +275,71 @@ 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({
isDevClusterManager: true,
isDevClusterMaster: true,
})
),
logger,
});

await service.setup(setupDeps);
await service.start();

expect(httpServer.start).not.toHaveBeenCalled();
});

test('does not start http server if process is dev cluster manager', async () => {
const configService = createConfigService();
const httpServer = {
isListening: () => false,
setup: jest.fn().mockReturnValue({}),
start: jest.fn(),
stop: noop,
};
mockHttpServer.mockImplementation(() => httpServer);

const service = new HttpService({
coreId,
configService,
env: Env.createDefault(
REPO_ROOT,
getEnvOptions({
isDevClusterManager: true,
isDevClusterMaster: false,
})
),
logger,
});

await service.setup(setupDeps);
await service.start();

expect(httpServer.start).not.toHaveBeenCalled();
});

test('does not start http server if process is dev cluster master (deprecated)', async () => {
const configService = createConfigService();
const httpServer = {
isListening: () => false,
setup: jest.fn().mockReturnValue({}),
start: jest.fn(),
stop: noop,
};
mockHttpServer.mockImplementation(() => httpServer);

const service = new HttpService({
coreId,
configService,
env: Env.createDefault(
REPO_ROOT,
getEnvOptions({
isDevClusterManager: false,
isDevClusterMaster: 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 @@ -169,7 +169,7 @@ export class HttpService
* @internal
* */
private shouldListen(config: HttpConfig) {
return !this.coreContext.env.isDevClusterMaster && config.autoListen;
return !this.coreContext.env.isDevClusterManager && config.autoListen;
}

public async stop() {
Expand Down
4 changes: 3 additions & 1 deletion src/core/server/legacy/legacy_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => {
REPO_ROOT,
getEnvOptions({
cliArgs: { silent: true, basePath: false },
isDevClusterManager: false,
isDevClusterMaster: true,
})
),
Expand Down Expand Up @@ -402,7 +403,8 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => {
REPO_ROOT,
getEnvOptions({
cliArgs: { quiet: true, basePath: true },
isDevClusterMaster: true,
isDevClusterManager: true,
isDevClusterMaster: false,
})
),
logger,
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/legacy/legacy_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export class LegacyService implements CoreService {
this.log.debug('starting legacy service');

// Receive initial config and create osdServer/ClusterManager.
if (this.coreContext.env.isDevClusterMaster) {
if (this.coreContext.env.isDevClusterManager) {
await this.createClusterManager(this.legacyRawConfig!);
} else {
this.osdServer = await this.createOsdServer(
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/plugins/plugins_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,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.isDevClusterManager;
if (initialize) {
contracts = await this.pluginsSystem.setupPlugins(deps);
this.registerPluginStaticDirs(deps);
Expand Down
Loading