Skip to content

Commit

Permalink
[Non-Inclusive Language] Deprecate isDevClusterMaster in favor of isD…
Browse files Browse the repository at this point in the history
…evClusterManager

As part of the meta issue opensearch-project/OpenSearch#2589 to track the plan and progress of applying inclusive naming across OpenSearch Repositories

In this change we are deprecating the terminology "isDevClusterMaster" and adding  "isDevClusterManager" for future usages.

Signed-off-by: manasvinibs <manasvis@amazon.com>
  • Loading branch information
manasvinibs committed Jun 17, 2022
1 parent dfbfec4 commit 0a6f43c
Show file tree
Hide file tree
Showing 13 changed files with 219 additions and 23 deletions.
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

0 comments on commit 0a6f43c

Please sign in to comment.