Use runHealthchecks action for GET /v1/healthy

Signed-off-by: Christina Ying Wang <christina@balena.io>

src/device-api/actions.ts

@@ -0,0 +1,24 @@
+import log from '../lib/supervisor-console';
+
+/**
+ * Run an array of healthchecks, outputting whether all passed or not
+ * Used by:
+ * - GET /v1/healthy
+ */
+export const runHealthchecks = async (
+	healthchecks: Array<() => Promise<boolean>>,
+) => {
+	const HEALTHCHECK_FAILURE = 'Healthcheck failed';
+
+	try {
+		const checks = await Promise.all(healthchecks.map((fn) => fn()));
+		if (checks.some((check) => !check)) {
+			throw new Error(HEALTHCHECK_FAILURE);
+		}
+	} catch {
+		log.error(HEALTHCHECK_FAILURE);
+		return false;
+	}
+
+	return true;
+};

src/device-api/index.ts

@@ -3,6 +3,7 @@ import * as _ from 'lodash';
 
 import * as middleware from './middleware';
 import * as apiKeys from './api-keys';
+import * as actions from './actions';
 import * as eventTracker from '../event-tracker';
 import { reportCurrentState } from '../device-state';
 import proxyvisor from '../proxyvisor';
@@ -43,15 +44,10 @@ export class SupervisorAPI {
 		this.api.use(middleware.logging);
 
 		this.api.get('/v1/healthy', async (_req, res) => {
-			try {
-				const healths = await Promise.all(this.healthchecks.map((fn) => fn()));
-				if (!_.every(healths)) {
-					log.error('Healthcheck failed');
-					return res.status(500).send('Unhealthy');
-				}
+			const isHealthy = await actions.runHealthchecks(this.healthchecks);
+			if (isHealthy) {
 				return res.sendStatus(200);
-			} catch {
-				log.error('Healthcheck failed');
+			} else {
 				return res.status(500).send('Unhealthy');
 			}
 		});

test/integration/device-api/v1.spec.ts

@@ -0,0 +1,53 @@
+import * as express from 'express';
+import { SinonStub, stub } from 'sinon';
+import * as request from 'supertest';
+
+import * as config from '~/src/config';
+import * as deviceApi from '~/src/device-api';
+import * as actions from '~/src/device-api/actions';
+import * as v1 from '~/src/device-api/v1';
+
+describe('device-api/v1', () => {
+	let api: express.Application;
+
+	before(async () => {
+		await config.initialized();
+
+		// `api` is a private property on SupervisorAPI but
+		// passing it directly to supertest is easier than
+		// setting up an API listen port & timeout
+		api = new deviceApi.SupervisorAPI({
+			routers: [v1.router],
+			healthchecks: [],
+			// @ts-expect-error
+		}).api;
+	});
+
+	describe('GET /v1/healthy', () => {
+		after(() => {
+			api = new deviceApi.SupervisorAPI({
+				routers: [v1.router],
+				healthchecks: [],
+				// @ts-expect-error
+			}).api;
+		});
+
+		it('responds with 200 because all healthchecks pass', async () => {
+			api = new deviceApi.SupervisorAPI({
+				routers: [v1.router],
+				healthchecks: [stub().resolves(true), stub().resolves(true)],
+				// @ts-expect-error
+			}).api;
+			await request(api).get('/v1/healthy').expect(200);
+		});
+
+		it('responds with 500 because some healthchecks did not pass', async () => {
+			api = new deviceApi.SupervisorAPI({
+				routers: [v1.router],
+				healthchecks: [stub().resolves(false), stub().resolves(true)],
+				// @ts-expect-error
+			}).api;
+			await request(api).get('/v1/healthy').expect(500);
+		});
+	});
+});

test/legacy/41-device-api-v1.spec.ts

@@ -39,7 +39,6 @@ import App from '~/src/compose/app';
 
 describe('SupervisorAPI [V1 Endpoints]', () => {
 	let api: SupervisorAPI;
-	let healthCheckStubs: SinonStub[];
 	let targetStateCacheMock: SinonStub;
 	const request = supertest(
 		`http://127.0.0.1:${mockedAPI.mockedOptions.listenPort}`,
@@ -87,15 +86,9 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 		// Do not apply target state
 		stub(deviceState, 'applyStep').resolves();
 
-		// Stub health checks so we can modify them whenever needed
-		healthCheckStubs = [
-			stub(apiBinder, 'healthcheck'),
-			stub(deviceState, 'healthcheck'),
-		];
-
 		// The mockedAPI contains stubs that might create unexpected results
 		// See the module to know what has been stubbed
-		api = await mockedAPI.create(healthCheckStubs);
+		api = await mockedAPI.create([]);
 
 		// Start test API
 		await api.listen(
@@ -120,8 +113,6 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 			}
 		}
 		(deviceState.applyStep as SinonStub).restore();
-		// Restore healthcheck stubs
-		healthCheckStubs.forEach((hc) => hc.restore());
 		// Remove any test data generated
 		await mockedAPI.cleanUp();
 		targetStateCacheMock.restore();
@@ -179,41 +170,6 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 		});
 	});
 
-	describe('GET /v1/healthy', () => {
-		it('returns OK because all checks pass', async () => {
-			// Make all healthChecks pass
-			healthCheckStubs.forEach((hc) => hc.resolves(true));
-			await request
-				.get('/v1/healthy')
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`)
-				.expect(sampleResponses.V1.GET['/healthy'].statusCode)
-				.then((response) => {
-					expect(response.body).to.deep.equal(
-						sampleResponses.V1.GET['/healthy'].body,
-					);
-					expect(response.text).to.deep.equal(
-						sampleResponses.V1.GET['/healthy'].text,
-					);
-				});
-		});
-		it('Fails because some checks did not pass', async () => {
-			healthCheckStubs.forEach((hc) => hc.resolves(false));
-			await request
-				.get('/v1/healthy')
-				.set('Accept', 'application/json')
-				.expect(sampleResponses.V1.GET['/healthy [2]'].statusCode)
-				.then((response) => {
-					expect(response.body).to.deep.equal(
-						sampleResponses.V1.GET['/healthy [2]'].body,
-					);
-					expect(response.text).to.deep.equal(
-						sampleResponses.V1.GET['/healthy [2]'].text,
-					);
-				});
-		});
-	});
-
 	describe('GET /v1/apps/:appId', () => {
 		it('does not return information for an application when there is more than 1 container', async () => {
 			await request

test/unit/device-api/actions.spec.ts

@@ -0,0 +1,25 @@
+import { expect } from 'chai';
+
+import * as actions from '~/src/device-api/actions';
+
+describe('device-api/actions', () => {
+	describe('runs healthchecks', () => {
+		const taskTrue = () => Promise.resolve(true);
+		const taskFalse = () => Promise.resolve(false);
+		const taskError = () => {
+			throw new Error();
+		};
+
+		it('resolves true if all healthchecks pass', async () => {
+			expect(await actions.runHealthchecks([taskTrue, taskTrue])).to.be.true;
+		});
+
+		it('resolves false if any healthchecks throw an error or fail', async () => {
+			expect(await actions.runHealthchecks([taskTrue, taskFalse])).to.be.false;
+			expect(await actions.runHealthchecks([taskTrue, taskError])).to.be.false;
+			expect(await actions.runHealthchecks([taskFalse, taskError])).to.be.false;
+			expect(await actions.runHealthchecks([taskFalse, taskFalse])).to.be.false;
+			expect(await actions.runHealthchecks([taskError, taskError])).to.be.false;
+		});
+	});
+});