From cdea019dfc6b95498e3f2f72ac457ec5e3c1c45f Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Tue, 15 Sep 2020 15:16:23 +0200 Subject: [PATCH] migrate pid file to core (#77037) * migrate pid file to core * use correct log level + add comment * move `unhandledRejection` to service's setup * update comment --- .../environment/environment_service.test.ts | 26 +++- .../server/environment/environment_service.ts | 13 +- src/core/server/environment/fs.ts | 1 + src/core/server/environment/index.ts | 1 + src/core/server/environment/pid_config.ts | 30 ++++ .../server/environment/write_pid_file.test.ts | 144 ++++++++++++++++++ src/core/server/environment/write_pid_file.ts | 64 ++++++++ src/core/server/server.ts | 3 +- src/legacy/server/config/schema.js | 5 +- src/legacy/server/kbn_server.js | 4 - src/legacy/server/pid/index.js | 73 --------- 11 files changed, 278 insertions(+), 86 deletions(-) create mode 100644 src/core/server/environment/pid_config.ts create mode 100644 src/core/server/environment/write_pid_file.test.ts create mode 100644 src/core/server/environment/write_pid_file.ts delete mode 100644 src/legacy/server/pid/index.js diff --git a/src/core/server/environment/environment_service.test.ts b/src/core/server/environment/environment_service.test.ts index 06fd250ebe4f9..f6cffb5e26a9e 100644 --- a/src/core/server/environment/environment_service.test.ts +++ b/src/core/server/environment/environment_service.test.ts @@ -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'; @@ -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(); @@ -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; @@ -76,7 +88,7 @@ describe('UuidService', () => { expect(resolveInstanceUuid).toHaveBeenCalledWith({ pathConfig, serverConfig, - logger: logger.get('uuid'), + logger: logger.get('environment'), }); }); @@ -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'), }); }); diff --git a/src/core/server/environment/environment_service.ts b/src/core/server/environment/environment_service.ts index 6a0b1122c7053..caa4f34bcfaa7 100644 --- a/src/core/server/environment/environment_service.ts +++ b/src/core/server/environment/environment_service.ts @@ -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 @@ -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(pathConfigDef.path).pipe(take(1)).toPromise(), this.configService.atPath(httpConfigDef.path).pipe(take(1)).toPromise(), + this.configService.atPath(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, diff --git a/src/core/server/environment/fs.ts b/src/core/server/environment/fs.ts index dc040ccb73615..b79c70dbee280 100644 --- a/src/core/server/environment/fs.ts +++ b/src/core/server/environment/fs.ts @@ -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); diff --git a/src/core/server/environment/index.ts b/src/core/server/environment/index.ts index 57a26d5ea3c79..92b57c6af2fff 100644 --- a/src/core/server/environment/index.ts +++ b/src/core/server/environment/index.ts @@ -18,3 +18,4 @@ */ export { EnvironmentService, InternalEnvironmentServiceSetup } from './environment_service'; +export { config, PidConfigType } from './pid_config'; diff --git a/src/core/server/environment/pid_config.ts b/src/core/server/environment/pid_config.ts new file mode 100644 index 0000000000000..ee9963016717e --- /dev/null +++ b/src/core/server/environment/pid_config.ts @@ -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; diff --git a/src/core/server/environment/write_pid_file.test.ts b/src/core/server/environment/write_pid_file.test.ts new file mode 100644 index 0000000000000..f9eb78a486970 --- /dev/null +++ b/src/core/server/environment/write_pid_file.test.ts @@ -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; +const existsMock = exists as jest.MockedFunction; + +const pid = String(process.pid); + +describe('writePidFile', () => { + let logger: ReturnType; + + 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", + ], + ] + `); + }); +}); diff --git a/src/core/server/environment/write_pid_file.ts b/src/core/server/environment/write_pid_file.ts new file mode 100644 index 0000000000000..6ee20af02d7b0 --- /dev/null +++ b/src/core/server/environment/write_pid_file.ts @@ -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'); + }); +}; diff --git a/src/core/server/server.ts b/src/core/server/server.ts index a02b0f51b559f..609a7fc83baf9 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -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'; @@ -310,6 +310,7 @@ export class Server { uiSettingsConfig, opsConfig, statusConfig, + pidConfig, ]; this.configService.addDeprecationProvider(rootConfigPath, coreDeprecationProvider); diff --git a/src/legacy/server/config/schema.js b/src/legacy/server/config/schema.js index ce7a500a00dc8..f8736fb30f90e 100644 --- a/src/legacy/server/config/schema.js +++ b/src/legacy/server/config/schema.js @@ -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, diff --git a/src/legacy/server/kbn_server.js b/src/legacy/server/kbn_server.js index a5eefd140c8fa..24d00abb99a05 100644 --- a/src/legacy/server/kbn_server.js +++ b/src/legacy/server/kbn_server.js @@ -29,7 +29,6 @@ import { coreMixin } from './core'; import { loggingMixin } from './logging'; import warningsMixin from './warnings'; import { statusMixin } from './status'; -import pidMixin from './pid'; import configCompleteMixin from './config/complete'; import { optimizeMixin } from '../../optimize'; import * as Plugins from './plugins'; @@ -93,9 +92,6 @@ export default class KbnServer { warningsMixin, statusMixin, - // writes pid file - pidMixin, - // scan translations dirs, register locale files and initialize i18n engine. i18nMixin, diff --git a/src/legacy/server/pid/index.js b/src/legacy/server/pid/index.js deleted file mode 100644 index d7b9da1292252..0000000000000 --- a/src/legacy/server/pid/index.js +++ /dev/null @@ -1,73 +0,0 @@ -/* - * 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 _ from 'lodash'; -import Boom from 'boom'; -import Bluebird from 'bluebird'; -import { unlinkSync as unlink } from 'fs'; -const writeFile = Bluebird.promisify(require('fs').writeFile); - -export default Bluebird.method(function (kbnServer, server, config) { - const path = config.get('pid.file'); - if (!path) return; - - const pid = String(process.pid); - - return writeFile(path, pid, { flag: 'wx' }) - .catch(function (err) { - if (err.code !== 'EEXIST') throw err; - - const message = `pid file already exists at ${path}`; - const metadata = { - path: path, - pid: pid, - }; - - if (config.get('pid.exclusive')) { - throw Boom.internal(message, { message, ...metadata }); - } else { - server.log(['pid', 'warning'], message, metadata); - } - - return writeFile(path, pid); - }) - .then(function () { - server.logWithMetadata(['pid', 'debug'], `wrote pid file to ${path}`, { - path: path, - pid: pid, - }); - - const clean = _.once(function () { - unlink(path); - }); - - process.once('exit', clean); // for "natural" exits - process.once('SIGINT', function () { - // for Ctrl-C exits - clean(); - - // resend SIGINT - process.kill(process.pid, 'SIGINT'); - }); - - process.on('unhandledRejection', function (reason) { - server.log(['warning'], `Detected an unhandled Promise rejection.\n${reason}`); - }); - }); -});