From f790c7ce77d0bf8d214d4e9cf951fb789c05d40e Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Tue, 19 Nov 2024 11:55:59 +0100 Subject: [PATCH 1/7] feat: health check --- src/health.ts | 57 ++++++++++++++++++++ src/health_test.ts | 127 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 src/health.ts create mode 100644 src/health_test.ts diff --git a/src/health.ts b/src/health.ts new file mode 100644 index 0000000000..69269b54a9 --- /dev/null +++ b/src/health.ts @@ -0,0 +1,57 @@ +import fetch from 'node-fetch'; +import { AbortSignal } from 'node-fetch/externals'; +import { URL } from 'url'; +import { KubeConfig } from './config'; +import { RequestOptions } from 'https'; + +export class Health { + public config: KubeConfig; + + public constructor(config: KubeConfig) { + this.config = config; + } + + public async readyz(opts: RequestOptions): Promise { + return this.check('/readyz', opts); + } + + public async livez(opts: RequestOptions): Promise { + return this.check('/livez', opts); + } + + private async healthz(opts: RequestOptions): Promise { + return this.check('/healthz', opts); + } + + private async check(path: string, opts: RequestOptions): Promise { + const cluster = this.config.getCurrentCluster(); + if (!cluster) { + throw new Error('No currently active cluster'); + } + + const requestURL = new URL(cluster.server + path); + const requestInit = await this.config.applyToFetchOptions(opts); + const controller = new AbortController(); + requestInit.signal = controller.signal as AbortSignal; + requestInit.method = 'GET'; + + return await fetch(requestURL.toString(), requestInit) + .then((response) => { + const status = response.status; + if (status === 200) { + return true; + } else if (status === 404) { + if (path === '/healthz') { + // /livez/readyz return 404 and healthz also returns 404, let's consider it is live + return true; + } + return this.healthz(opts); + } else { + return false; + } + }) + .catch((err) => { + return false; + }); + } +} diff --git a/src/health_test.ts b/src/health_test.ts new file mode 100644 index 0000000000..8106c7403d --- /dev/null +++ b/src/health_test.ts @@ -0,0 +1,127 @@ +import { expect } from 'chai'; +import nock from 'nock'; + +import { KubeConfig } from './config'; +import { Health } from './health'; +import { Cluster, User } from './config_types'; + +describe('Health', () => { + describe('livez', () => { + it('should throw an error if no current active cluster', async () => { + const kc = new KubeConfig(); + const health = new Health(kc); + await expect(health.livez({})).to.be.rejectedWith('No currently active cluster'); + }); + + it('should return true if /livez returns with status 200', async () => { + const kc = new KubeConfig(); + const cluster = { + name: 'foo', + server: 'https://server.com', + } as Cluster; + + const user = { + name: 'my-user', + password: 'some-password', + } as User; + kc.loadFromClusterAndUser(cluster, user); + + const scope = nock('https://server.com').get('/livez').reply(200); + const health = new Health(kc); + + const r = await health.livez({}); + expect(r).to.be.true; + scope.done(); + }); + + it('should return false if /livez returns with status 500', async () => { + const kc = new KubeConfig(); + const cluster = { + name: 'foo', + server: 'https://server.com', + } as Cluster; + + const user = { + name: 'my-user', + password: 'some-password', + } as User; + kc.loadFromClusterAndUser(cluster, user); + + const scope = nock('https://server.com').get('/livez').reply(500); + const health = new Health(kc); + + const r = await health.livez({}); + expect(r).to.be.false; + scope.done(); + }); + + it('should return true if /livez returns status 404 and /healthz returns status 200', async () => { + const kc = new KubeConfig(); + const cluster = { + name: 'foo', + server: 'https://server.com', + } as Cluster; + + const user = { + name: 'my-user', + password: 'some-password', + } as User; + kc.loadFromClusterAndUser(cluster, user); + + const scope = nock('https://server.com'); + scope.get('/livez').reply(404); + scope.get('/healthz').reply(200); + const health = new Health(kc); + + const r = await health.livez({}); + expect(r).to.be.true; + scope.done(); + }); + + it('should return false if /livez returns status 404 and /healthz returns status 500', async () => { + const kc = new KubeConfig(); + const cluster = { + name: 'foo', + server: 'https://server.com', + } as Cluster; + + const user = { + name: 'my-user', + password: 'some-password', + } as User; + kc.loadFromClusterAndUser(cluster, user); + + const scope = nock('https://server.com'); + scope.get('/livez').reply(404); + scope.get('/healthz').reply(500); + const health = new Health(kc); + + const r = await health.livez({}); + expect(r).to.be.false; + scope.done(); + }); + + it('should return true if both /livez and /healthz return status 404', async () => { + const kc = new KubeConfig(); + const cluster = { + name: 'foo', + server: 'https://server.com', + } as Cluster; + + const user = { + name: 'my-user', + password: 'some-password', + } as User; + kc.loadFromClusterAndUser(cluster, user); + + const scope = nock('https://server.com'); + scope.get('/livez').reply(404); + scope.get('/healthz').reply(200); + const health = new Health(kc); + + const r = await health.livez({}); + expect(r).to.be.true; + scope.done(); + }); + }); +}); From eb5c536397ec78f69e1bf1604a34719e83a60e08 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Tue, 19 Nov 2024 16:07:55 +0100 Subject: [PATCH 2/7] fix: review --- src/health.ts | 31 +++++++++++++++---------------- src/health_test.ts | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/health.ts b/src/health.ts index 69269b54a9..1697e12fc1 100644 --- a/src/health.ts +++ b/src/health.ts @@ -2,7 +2,7 @@ import fetch from 'node-fetch'; import { AbortSignal } from 'node-fetch/externals'; import { URL } from 'url'; import { KubeConfig } from './config'; -import { RequestOptions } from 'https'; +import { RequestOptions } from 'node:https'; export class Health { public config: KubeConfig; @@ -35,23 +35,22 @@ export class Health { requestInit.signal = controller.signal as AbortSignal; requestInit.method = 'GET'; - return await fetch(requestURL.toString(), requestInit) - .then((response) => { - const status = response.status; - if (status === 200) { + try { + const response = await fetch(requestURL.toString(), requestInit); + const status = response.status; + if (status === 200) { + return true; + } else if (status === 404) { + if (path === '/healthz') { + // /livez/readyz return 404 and healthz also returns 404, let's consider it is live return true; - } else if (status === 404) { - if (path === '/healthz') { - // /livez/readyz return 404 and healthz also returns 404, let's consider it is live - return true; - } - return this.healthz(opts); - } else { - return false; } - }) - .catch((err) => { + return this.healthz(opts); + } else { return false; - }); + } + } catch { + return false; + } } } diff --git a/src/health_test.ts b/src/health_test.ts index 8106c7403d..af482cdd6e 100644 --- a/src/health_test.ts +++ b/src/health_test.ts @@ -123,5 +123,27 @@ describe('Health', () => { expect(r).to.be.true; scope.done(); }); + + it('should return false when fetch throws an error', async () => { + const kc = new KubeConfig(); + const cluster = { + name: 'foo', + server: 'https://server.com', + } as Cluster; + + const user = { + name: 'my-user', + password: 'some-password', + } as User; + kc.loadFromClusterAndUser(cluster, user); + + const scope = nock('https://server.com'); + scope.get('/livez').replyWithError(new Error('an error')); + const health = new Health(kc); + + const r = await health.livez({}); + expect(r).to.be.false; + scope.done(); + }); }); }); From b6b89e0c2d823994577e16a50f8292ff88dd4d68 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Tue, 19 Nov 2024 16:25:38 +0100 Subject: [PATCH 3/7] fix: do not import URL --- src/health.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/health.ts b/src/health.ts index 1697e12fc1..c1b746e39c 100644 --- a/src/health.ts +++ b/src/health.ts @@ -1,6 +1,5 @@ import fetch from 'node-fetch'; import { AbortSignal } from 'node-fetch/externals'; -import { URL } from 'url'; import { KubeConfig } from './config'; import { RequestOptions } from 'node:https'; From 7804b2295e037f8e2998044d12b59ca5e9d4017f Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Tue, 19 Nov 2024 21:25:13 +0100 Subject: [PATCH 4/7] reject when fetch fails --- src/health.ts | 8 ++++---- src/health_test.ts | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/health.ts b/src/health.ts index c1b746e39c..cd49595930 100644 --- a/src/health.ts +++ b/src/health.ts @@ -39,17 +39,17 @@ export class Health { const status = response.status; if (status === 200) { return true; - } else if (status === 404) { + } + if (status === 404) { if (path === '/healthz') { // /livez/readyz return 404 and healthz also returns 404, let's consider it is live return true; } return this.healthz(opts); - } else { - return false; } - } catch { return false; + } catch { + throw new Error('Error occurred in health request'); } } } diff --git a/src/health_test.ts b/src/health_test.ts index af482cdd6e..f79220fd9a 100644 --- a/src/health_test.ts +++ b/src/health_test.ts @@ -124,7 +124,7 @@ describe('Health', () => { scope.done(); }); - it('should return false when fetch throws an error', async () => { + it('should throw an error when fetch throws an error', async () => { const kc = new KubeConfig(); const cluster = { name: 'foo', @@ -141,8 +141,7 @@ describe('Health', () => { scope.get('/livez').replyWithError(new Error('an error')); const health = new Health(kc); - const r = await health.livez({}); - expect(r).to.be.false; + await expect(health.livez({})).to.be.rejectedWith('Error occurred in health request'); scope.done(); }); }); From 7a9c3386f51451f64bf1f387d294d9c96bfaadf3 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 20 Nov 2024 10:36:08 +0100 Subject: [PATCH 5/7] export Health --- src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/index.ts b/src/index.ts index 09fc375e10..cca1256c31 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ export * from './cp'; export * from './patch'; export * from './metrics'; export * from './object'; +export * from './health'; export { ConfigOptions, User, Cluster, Context } from './config_types'; // Export AbortError and FetchError so that instanceof checks in user code will definitely use the same instances From 15d6e5214d7e325806e44ea019f13b1987f67b15 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 20 Nov 2024 12:36:51 +0100 Subject: [PATCH 6/7] fix: get signal from options --- src/health.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/health.ts b/src/health.ts index cd49595930..6df7193906 100644 --- a/src/health.ts +++ b/src/health.ts @@ -1,5 +1,4 @@ import fetch from 'node-fetch'; -import { AbortSignal } from 'node-fetch/externals'; import { KubeConfig } from './config'; import { RequestOptions } from 'node:https'; @@ -30,8 +29,9 @@ export class Health { const requestURL = new URL(cluster.server + path); const requestInit = await this.config.applyToFetchOptions(opts); - const controller = new AbortController(); - requestInit.signal = controller.signal as AbortSignal; + if (opts.signal) { + requestInit.signal = opts.signal; + } requestInit.method = 'GET'; try { From c234aa59abb1d64600150113f4e774610ccca0e2 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 20 Nov 2024 15:26:56 +0100 Subject: [PATCH 7/7] pass AbortError to caller --- src/health.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/health.ts b/src/health.ts index 6df7193906..065b874875 100644 --- a/src/health.ts +++ b/src/health.ts @@ -1,4 +1,4 @@ -import fetch from 'node-fetch'; +import fetch, { AbortError } from 'node-fetch'; import { KubeConfig } from './config'; import { RequestOptions } from 'node:https'; @@ -48,7 +48,10 @@ export class Health { return this.healthz(opts); } return false; - } catch { + } catch (err: unknown) { + if (err instanceof Error && err.name === 'AbortError') { + throw err; + } throw new Error('Error occurred in health request'); } }