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

HTTP-Server: Graceful shutdown #97223

Merged
merged 19 commits into from
Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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/kbn-cli-dev-mode/src/base_path_proxy_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { Server } from '@hapi/hapi';
import { EMPTY } from 'rxjs';
import moment from 'moment';
import supertest from 'supertest';
import {
getServerOptions,
Expand Down Expand Up @@ -35,6 +36,7 @@ describe('BasePathProxyServer', () => {
config = {
host: '127.0.0.1',
port: 10012,
gracefulShutdownTimeout: moment.duration(30, 'seconds'),
keepaliveTimeout: 1000,
socketTimeout: 1000,
cors: {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-cli-dev-mode/src/cli_dev_mode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Rx.merge(
.subscribe(exitSignal$);

// timeout where the server is allowed to exit gracefully
const GRACEFUL_TIMEOUT = 5000;
const GRACEFUL_TIMEOUT = 30000;

export type SomeCliArgs = Pick<
CliArgs,
Expand Down
4 changes: 4 additions & 0 deletions packages/kbn-cli-dev-mode/src/config/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { ByteSizeValue, schema, TypeOf } from '@kbn/config-schema';
import { ICorsConfig, IHttpConfig, ISslConfig, SslConfig, sslSchema } from '@kbn/server-http-tools';
import { Duration } from 'moment';

export const httpConfigSchema = schema.object(
{
Expand All @@ -22,6 +23,7 @@ export const httpConfigSchema = schema.object(
maxPayload: schema.byteSize({
defaultValue: '1048576b',
}),
gracefulShutdownTimeout: schema.duration({ defaultValue: '30s' }),
keepaliveTimeout: schema.number({
defaultValue: 120000,
}),
Expand All @@ -47,6 +49,7 @@ export class HttpConfig implements IHttpConfig {
host: string;
port: number;
maxPayload: ByteSizeValue;
gracefulShutdownTimeout: Duration;
keepaliveTimeout: number;
socketTimeout: number;
cors: ICorsConfig;
Expand All @@ -57,6 +60,7 @@ export class HttpConfig implements IHttpConfig {
this.host = rawConfig.host;
this.port = rawConfig.port;
this.maxPayload = rawConfig.maxPayload;
this.gracefulShutdownTimeout = rawConfig.gracefulShutdownTimeout;
this.keepaliveTimeout = rawConfig.keepaliveTimeout;
this.socketTimeout = rawConfig.socketTimeout;
this.cors = rawConfig.cors;
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-cli-dev-mode/src/dev_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ export class DevServer {
// handle graceful shutdown requests
const triggerGracefulShutdown$ = gracefulShutdown$.pipe(
mergeMap(() => {
// signal to the process that it should exit
proc.kill('SIGINT');
// no need to signal to the process that it should exit. execa already does that for us.
// proc.kill('SIGINT');

// if the timer fires before exit$ we will send SIGINT
return Rx.timer(this.gracefulTimeout).pipe(
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-server-http-tools/src/get_server_options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import moment from 'moment';
import { ByteSizeValue } from '@kbn/config-schema';
import { getServerOptions } from './get_server_options';
import { IHttpConfig } from './types';
Expand All @@ -24,6 +25,7 @@ const createConfig = (parts: Partial<IHttpConfig>): IHttpConfig => ({
port: 5601,
socketTimeout: 120000,
keepaliveTimeout: 120000,
gracefulShutdownTimeout: moment.duration(30, 'seconds'),
maxPayload: ByteSizeValue.parse('1048576b'),
...parts,
cors: {
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-server-http-tools/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { ByteSizeValue } from '@kbn/config-schema';
import type { Duration } from 'moment';

export interface IHttpConfig {
host: string;
Expand All @@ -16,6 +17,7 @@ export interface IHttpConfig {
socketTimeout: number;
cors: ICorsConfig;
ssl: ISslConfig;
gracefulShutdownTimeout: Duration;
}

export interface ICorsConfig {
Expand Down
6 changes: 6 additions & 0 deletions scripts/kibana.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@
* Side Public License, v 1.
*/

// Kibana already handles these events internally for graceful shutdowns.
// `src/cli/dev` is spawning Kibana in a child process to apply the preserve-symlinks options.
// We need to catch these events here to avoid killing the parent process before Kibana (the child) is fully finished.
process.on('SIGINT', function () {});
process.on('SIGTERM', function () {});

require('../src/cli/dev');

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

4 changes: 4 additions & 0 deletions src/core/server/http/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { IHttpConfig, SslConfig, sslSchema } from '@kbn/server-http-tools';
import { hostname } from 'os';
import url from 'url';

import type { Duration } from 'moment';
import { ServiceConfigDescriptor } from '../internal_types';
import { CspConfigType, CspConfig, ICspConfig } from '../csp';
import { ExternalUrlConfig, IExternalUrlConfig } from '../external_url';
Expand All @@ -31,6 +32,7 @@ const configSchema = schema.object(
validate: match(validBasePathRegex, "must start with a slash, don't end with one"),
})
),
gracefulShutdownTimeout: schema.duration({ defaultValue: '30s' }),
cors: schema.object(
{
enabled: schema.boolean({ defaultValue: false }),
Expand Down Expand Up @@ -182,6 +184,7 @@ export class HttpConfig implements IHttpConfig {
public externalUrl: IExternalUrlConfig;
public xsrf: { disableProtection: boolean; allowlist: string[] };
public requestId: { allowFromAnyIp: boolean; ipAllowlist: string[] };
public gracefulShutdownTimeout: Duration;

/**
* @internal
Expand Down Expand Up @@ -217,6 +220,7 @@ export class HttpConfig implements IHttpConfig {
this.externalUrl = rawExternalUrlConfig;
this.xsrf = rawHttpConfig.xsrf;
this.requestId = rawHttpConfig.requestId;
this.gracefulShutdownTimeout = rawHttpConfig.gracefulShutdownTimeout;
}
}

Expand Down
73 changes: 72 additions & 1 deletion src/core/server/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import { HttpServer } from './http_server';
import { Readable } from 'stream';
import { RequestHandlerContext } from 'kibana/server';
import { KBN_CERT_PATH, KBN_KEY_PATH } from '@kbn/dev-utils';
import { of } from 'rxjs';
import moment from 'moment';

const cookieOptions = {
name: 'sid',
Expand Down Expand Up @@ -65,6 +67,7 @@ beforeEach(() => {
cors: {
enabled: false,
},
gracefulShutdownTimeout: moment.duration(100, 'ms'),
} as any;

configWithSSL = {
Expand All @@ -79,7 +82,7 @@ beforeEach(() => {
},
} as HttpConfig;

server = new HttpServer(loggingService, 'tests');
server = new HttpServer(loggingService, 'tests', of(config.gracefulShutdownTimeout));
});

afterEach(async () => {
Expand Down Expand Up @@ -1431,3 +1434,71 @@ describe('setup contract', () => {
});
});
});

describe('Graceful shutdown', () => {
test('it should wait until the ongoing requests are resolved and reject any new ones', async () => {
const gracefulShutdownTimeout = config.gracefulShutdownTimeout.asMilliseconds();

const { registerRouter, server: innerServer } = await server.setup(config);

const router = new Router('', logger, enhanceWithContext);
router.post(
{
path: '/',
validate: { body: schema.object({ test: schema.number() }) },
options: { body: { accepts: 'application/json' } },
},
async (context, req, res) => {
// It takes to resolve the same period of the gracefulShutdownTimeout.
// Since we'll trigger the stop a few ms after, it should have time to finish
await new Promise((resolve) => setTimeout(resolve, gracefulShutdownTimeout));
return res.ok({ body: req.route });
}
);
registerRouter(router);

await server.start();

const makeRequest = () => supertest(innerServer.listener).post('/').send({ test: 1 });

const [firstResponse, , secondResponse] = await Promise.all([
// Trigger a request that should succeed
makeRequest().expect(200, {
method: 'post',
path: '/',
options: {
authRequired: true,
xsrfRequired: true,
tags: [],
timeout: {
payload: 10000,
},
body: {
parse: true, // hapi populates the default
maxBytes: 1024, // hapi populates the default
accepts: ['application/json'],
output: 'data',
},
},
}),
// Stop the server while the request is in progress
(async () => {
await new Promise((resolve) => setTimeout(resolve, gracefulShutdownTimeout / 3));
return server.stop();
})(),
// Trigger a new request while shutting down (should be rejected)
(async () => {
await new Promise((resolve) => setTimeout(resolve, (2 * gracefulShutdownTimeout) / 3));
return makeRequest().expect(503, {
statusCode: 503,
error: 'Service Unavailable',
message: 'Kibana is shutting down and not accepting new incoming requests',
});
})(),
]);

// hapi automatically sends the header `connection: 'close'` when shutting down
expect(firstResponse.header.connection).toBe('close');
expect(secondResponse.header.connection).toBe('close');
});
});
30 changes: 28 additions & 2 deletions src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import {
getRequestId,
} from '@kbn/server-http-tools';

import type { Duration } from 'moment';
import type { Observable } from 'rxjs';
import { take } from 'rxjs/operators';
import { Logger, LoggerFactory } from '../logging';
import { HttpConfig } from './http_config';
import { adoptToHapiAuthFormat, AuthenticationHandler } from './lifecycle/auth';
Expand Down Expand Up @@ -87,7 +90,11 @@ export class HttpServer {
private readonly authRequestHeaders: AuthHeadersStorage;
private readonly authResponseHeaders: AuthHeadersStorage;

constructor(private readonly logger: LoggerFactory, private readonly name: string) {
constructor(
private readonly logger: LoggerFactory,
private readonly name: string,
private readonly gracefulShutdownTimeout$: Observable<Duration>
) {
this.authState = new AuthStateStorage(() => this.authRegistered);
this.authRequestHeaders = new AuthHeadersStorage();
this.authResponseHeaders = new AuthHeadersStorage();
Expand Down Expand Up @@ -118,6 +125,7 @@ export class HttpServer {
this.setupConditionalCompression(config);
this.setupResponseLogging();
this.setupRequestStateAssignment(config);
this.setupGracefulShutdownHandlers();

return {
registerRouter: this.registerRouter.bind(this),
Expand Down Expand Up @@ -221,10 +229,16 @@ export class HttpServer {
const hasStarted = this.server.info.started > 0;
if (hasStarted) {
this.log.debug('stopping http server');

const gracefulShutdownTimeout = await this.gracefulShutdownTimeout$.pipe(take(1)).toPromise();
await this.server.stop({ timeout: gracefulShutdownTimeout.asMilliseconds() });

this.log.debug(`http server stopped`);

// Removing the listener after stopping so we don't leave any pending requests unhandled
if (this.handleServerResponseEvent) {
this.server.events.removeListener('response', this.handleServerResponseEvent);
}
await this.server.stop();
}
}

Expand All @@ -246,6 +260,18 @@ export class HttpServer {
}
}

private setupGracefulShutdownHandlers() {
this.registerOnPreRouting((request, response, toolkit) => {
if (this.stopped) {
return response.customError({
statusCode: 503,
body: { message: 'Kibana is shutting down and not accepting new incoming requests' },
});
}
return toolkit.next();
});
}

private setupBasePathRewrite(config: HttpConfig, basePathService: BasePath) {
if (config.basePath === undefined || !config.rewriteBasePath) {
return;
Expand Down
11 changes: 7 additions & 4 deletions src/core/server/http/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { Observable, Subscription, combineLatest } from 'rxjs';
import { Observable, Subscription, combineLatest, of } from 'rxjs';
import { first, map } from 'rxjs/operators';
import { Server } from '@hapi/hapi';
import { pick } from '@kbn/std';
Expand Down Expand Up @@ -69,7 +69,10 @@ export class HttpService
configService.atPath<CspConfigType>(cspConfig.path),
configService.atPath<ExternalUrlConfigType>(externalUrlConfig.path),
]).pipe(map(([http, csp, externalUrl]) => new HttpConfig(http, csp, externalUrl)));
this.httpServer = new HttpServer(logger, 'Kibana');
const gracefulShutdownTimeout$ = this.config$.pipe(
map(({ gracefulShutdownTimeout }) => gracefulShutdownTimeout)
);
this.httpServer = new HttpServer(logger, 'Kibana', gracefulShutdownTimeout$);
this.httpsRedirectServer = new HttpsRedirectServer(logger.get('http', 'redirect', 'server'));
}

Expand Down Expand Up @@ -167,7 +170,7 @@ export class HttpService
return;
}

this.configSubscription.unsubscribe();
this.configSubscription?.unsubscribe();
this.configSubscription = undefined;

if (this.notReadyServer) {
Expand All @@ -179,7 +182,7 @@ export class HttpService

private async runNotReadyServer(config: HttpConfig) {
this.log.debug('starting NotReady server');
const httpServer = new HttpServer(this.logger, 'NotReady');
const httpServer = new HttpServer(this.logger, 'NotReady', of(config.gracefulShutdownTimeout));
const { server } = await httpServer.setup(config);
this.notReadyServer = server;
// use hapi server while KibanaResponseFactory doesn't allow specifying custom headers
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/http/test_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { BehaviorSubject } from 'rxjs';
import moment from 'moment';
import { REPO_ROOT } from '@kbn/dev-utils';
import { ByteSizeValue } from '@kbn/config-schema';
import { Env } from '../config';
Expand Down Expand Up @@ -43,6 +44,7 @@ configService.atPath.mockImplementation((path) => {
allowFromAnyIp: true,
ipAllowlist: [],
},
gracefulShutdownTimeout: moment.duration(30, 'seconds'),
keepaliveTimeout: 120_000,
socketTimeout: 120_000,
} as any);
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,10 @@ export class Server {
this.log.debug('stopping server');

await this.legacy.stop();
await this.http.stop(); // HTTP server has to stop before savedObjects and ES clients are closed to be able to gracefully attempt to resolve any pending requests
await this.plugins.stop();
await this.savedObjects.stop();
await this.elasticsearch.stop();
await this.http.stop();
await this.uiSettings.stop();
await this.rendering.stop();
await this.metrics.stop();
Expand Down