Skip to content

Commit

Permalink
migrate pid file to core (#77037)
Browse files Browse the repository at this point in the history
* migrate pid file to core

* use correct log level + add comment

* move `unhandledRejection` to service's setup

* update comment
  • Loading branch information
pgayvallet authored Sep 15, 2020
1 parent d190a2a commit cdea019
Show file tree
Hide file tree
Showing 11 changed files with 278 additions and 86 deletions.
26 changes: 24 additions & 2 deletions src/core/server/environment/environment_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { BehaviorSubject } from 'rxjs';
import { EnvironmentService } from './environment_service';
import { resolveInstanceUuid } from './resolve_uuid';
import { createDataFolder } from './create_data_folder';
import { writePidFile } from './write_pid_file';
import { CoreContext } from '../core_context';

import { configServiceMock } from '../config/config_service.mock';
Expand All @@ -35,12 +36,20 @@ jest.mock('./create_data_folder', () => ({
createDataFolder: jest.fn(),
}));

jest.mock('./write_pid_file', () => ({
writePidFile: jest.fn(),
}));

const pathConfig = {
data: 'data-folder',
};
const serverConfig = {
uuid: 'SOME_UUID',
};
const pidConfig = {
file: '/pid/file',
exclusive: 'false',
};

const getConfigService = () => {
const configService = configServiceMock.create();
Expand All @@ -51,6 +60,9 @@ const getConfigService = () => {
if (path === 'server') {
return new BehaviorSubject(serverConfig);
}
if (path === 'pid') {
return new BehaviorSubject(pidConfig);
}
return new BehaviorSubject({});
});
return configService;
Expand All @@ -76,7 +88,7 @@ describe('UuidService', () => {
expect(resolveInstanceUuid).toHaveBeenCalledWith({
pathConfig,
serverConfig,
logger: logger.get('uuid'),
logger: logger.get('environment'),
});
});

Expand All @@ -86,7 +98,17 @@ describe('UuidService', () => {
expect(createDataFolder).toHaveBeenCalledTimes(1);
expect(createDataFolder).toHaveBeenCalledWith({
pathConfig,
logger: logger.get('uuid'),
logger: logger.get('environment'),
});
});

it('calls writePidFile with correct parameters', async () => {
const service = new EnvironmentService(coreContext);
await service.setup();
expect(writePidFile).toHaveBeenCalledTimes(1);
expect(writePidFile).toHaveBeenCalledWith({
pidConfig,
logger: logger.get('environment'),
});
});

Expand Down
13 changes: 11 additions & 2 deletions src/core/server/environment/environment_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import { Logger } from '../logging';
import { IConfigService } from '../config';
import { PathConfigType, config as pathConfigDef } from '../path';
import { HttpConfigType, config as httpConfigDef } from '../http';
import { PidConfigType, config as pidConfigDef } from './pid_config';
import { resolveInstanceUuid } from './resolve_uuid';
import { createDataFolder } from './create_data_folder';
import { writePidFile } from './write_pid_file';

/**
* @internal
Expand All @@ -43,17 +45,24 @@ export class EnvironmentService {
private uuid: string = '';

constructor(core: CoreContext) {
this.log = core.logger.get('uuid');
this.log = core.logger.get('environment');
this.configService = core.configService;
}

public async setup() {
const [pathConfig, serverConfig] = await Promise.all([
const [pathConfig, serverConfig, pidConfig] = await Promise.all([
this.configService.atPath<PathConfigType>(pathConfigDef.path).pipe(take(1)).toPromise(),
this.configService.atPath<HttpConfigType>(httpConfigDef.path).pipe(take(1)).toPromise(),
this.configService.atPath<PidConfigType>(pidConfigDef.path).pipe(take(1)).toPromise(),
]);

// was present in the legacy `pid` file.
process.on('unhandledRejection', (reason) => {
this.log.warn(`Detected an unhandled Promise rejection.\n${reason}`);
});

await createDataFolder({ pathConfig, logger: this.log });
await writePidFile({ pidConfig, logger: this.log });

this.uuid = await resolveInstanceUuid({
pathConfig,
Expand Down
1 change: 1 addition & 0 deletions src/core/server/environment/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ import { promisify } from 'util';
export const readFile = promisify(Fs.readFile);
export const writeFile = promisify(Fs.writeFile);
export const mkdir = promisify(Fs.mkdir);
export const exists = promisify(Fs.exists);
1 change: 1 addition & 0 deletions src/core/server/environment/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
*/

export { EnvironmentService, InternalEnvironmentServiceSetup } from './environment_service';
export { config, PidConfigType } from './pid_config';
30 changes: 30 additions & 0 deletions src/core/server/environment/pid_config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { TypeOf, schema } from '@kbn/config-schema';

export const config = {
path: 'pid',
schema: schema.object({
file: schema.maybe(schema.string()),
exclusive: schema.boolean({ defaultValue: false }),
}),
};

export type PidConfigType = TypeOf<typeof config.schema>;
144 changes: 144 additions & 0 deletions src/core/server/environment/write_pid_file.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { writeFile, exists } from './fs';
import { writePidFile } from './write_pid_file';
import { loggingSystemMock } from '../logging/logging_system.mock';

jest.mock('./fs', () => ({
writeFile: jest.fn(),
exists: jest.fn(),
}));

const writeFileMock = writeFile as jest.MockedFunction<typeof writeFile>;
const existsMock = exists as jest.MockedFunction<typeof exists>;

const pid = String(process.pid);

describe('writePidFile', () => {
let logger: ReturnType<typeof loggingSystemMock.createLogger>;

beforeEach(() => {
logger = loggingSystemMock.createLogger();
jest.spyOn(process, 'once');

writeFileMock.mockImplementation(() => Promise.resolve());
existsMock.mockImplementation(() => Promise.resolve(false));
});

afterEach(() => {
jest.clearAllMocks();
});

const allLogs = () =>
Object.entries(loggingSystemMock.collect(logger)).reduce((messages, [key, value]) => {
return [...messages, ...(key === 'log' ? [] : (value as any[]).map(([msg]) => [key, msg]))];
}, [] as any[]);

it('does nothing if `pid.file` is not set', async () => {
await writePidFile({
pidConfig: {
file: undefined,
exclusive: false,
},
logger,
});
expect(writeFile).not.toHaveBeenCalled();
expect(process.once).not.toHaveBeenCalled();
expect(allLogs()).toMatchInlineSnapshot(`Array []`);
});

it('writes the pid file to `pid.file`', async () => {
existsMock.mockResolvedValue(false);

await writePidFile({
pidConfig: {
file: '/pid-file',
exclusive: false,
},
logger,
});

expect(writeFile).toHaveBeenCalledTimes(1);
expect(writeFile).toHaveBeenCalledWith('/pid-file', pid);

expect(process.once).toHaveBeenCalledTimes(2);
expect(process.once).toHaveBeenCalledWith('exit', expect.any(Function));
expect(process.once).toHaveBeenCalledWith('SIGINT', expect.any(Function));

expect(allLogs()).toMatchInlineSnapshot(`
Array [
Array [
"debug",
"wrote pid file to /pid-file",
],
]
`);
});

it('throws an error if the file exists and `pid.exclusive is true`', async () => {
existsMock.mockResolvedValue(true);

await expect(
writePidFile({
pidConfig: {
file: '/pid-file',
exclusive: true,
},
logger,
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"pid file already exists at /pid-file"`);

expect(writeFile).not.toHaveBeenCalled();
expect(process.once).not.toHaveBeenCalled();
expect(allLogs()).toMatchInlineSnapshot(`Array []`);
});

it('logs a warning if the file exists and `pid.exclusive` is false', async () => {
existsMock.mockResolvedValue(true);

await writePidFile({
pidConfig: {
file: '/pid-file',
exclusive: false,
},
logger,
});

expect(writeFile).toHaveBeenCalledTimes(1);
expect(writeFile).toHaveBeenCalledWith('/pid-file', pid);

expect(process.once).toHaveBeenCalledTimes(2);
expect(process.once).toHaveBeenCalledWith('exit', expect.any(Function));
expect(process.once).toHaveBeenCalledWith('SIGINT', expect.any(Function));

expect(allLogs()).toMatchInlineSnapshot(`
Array [
Array [
"debug",
"wrote pid file to /pid-file",
],
Array [
"warn",
"pid file already exists at /pid-file",
],
]
`);
});
});
64 changes: 64 additions & 0 deletions src/core/server/environment/write_pid_file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { unlinkSync as unlink } from 'fs';
import once from 'lodash/once';
import { Logger } from '../logging';
import { writeFile, exists } from './fs';
import { PidConfigType } from './pid_config';

export const writePidFile = async ({
pidConfig,
logger,
}: {
pidConfig: PidConfigType;
logger: Logger;
}) => {
const path = pidConfig.file;
if (!path) {
return;
}

const pid = String(process.pid);

if (await exists(path)) {
const message = `pid file already exists at ${path}`;
if (pidConfig.exclusive) {
throw new Error(message);
} else {
logger.warn(message, { path, pid });
}
}

await writeFile(path, pid);

logger.debug(`wrote pid file to ${path}`, { path, pid });

const clean = once(() => {
unlink(path);
});

process.once('exit', clean); // for "natural" exits
process.once('SIGINT', () => {
// for Ctrl-C exits
clean();
// resend SIGINT
process.kill(process.pid, 'SIGINT');
});
};
3 changes: 2 additions & 1 deletion src/core/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { PluginsService, config as pluginsConfig } from './plugins';
import { SavedObjectsService } from '../server/saved_objects';
import { MetricsService, opsConfig } from './metrics';
import { CapabilitiesService } from './capabilities';
import { EnvironmentService } from './environment';
import { EnvironmentService, config as pidConfig } from './environment';
import { StatusService } from './status/status_service';

import { config as cspConfig } from './csp';
Expand Down Expand Up @@ -310,6 +310,7 @@ export class Server {
uiSettingsConfig,
opsConfig,
statusConfig,
pidConfig,
];

this.configService.addDeprecationProvider(rootConfigPath, coreDeprecationProvider);
Expand Down
5 changes: 1 addition & 4 deletions src/legacy/server/config/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ export default () =>
basePathProxyTarget: Joi.number().default(5603),
}).default(),

pid: Joi.object({
file: Joi.string(),
exclusive: Joi.boolean().default(false),
}).default(),
pid: HANDLED_IN_NEW_PLATFORM,

csp: HANDLED_IN_NEW_PLATFORM,

Expand Down
Loading

0 comments on commit cdea019

Please sign in to comment.