Use a breadcrumb to mark that a reboot is required

As changes to config.json may restart the supervisor before it can
trigger the reboot (or something can kill the supervisor before it can run that step),
the supervisor needs a persistent signal that a reboot is required
(instead of the current transient signal).

With this commit, the supervisor will now create a breadcrumb in the
host `/tmp` folder, that will be checked as the last step of the
configuration changes.

src/device-config.ts

@@ -1,5 +1,6 @@
 import * as _ from 'lodash';
 import { inspect } from 'util';
+import { promises as fs } from 'fs';
 
 import * as config from './config';
 import * as db from './db';
@@ -15,9 +16,19 @@ import { SchemaTypeKey } from './config/schema-type';
 import { matchesAnyBootConfig } from './config/backends';
 import { ConfigBackend } from './config/backends/backend';
 import { Odmdata } from './config/backends/odmdata';
+import * as fsUtils from './lib/fs-utils';
 
 const vpnServiceName = 'openvpn';
 
+// This indicates the file on the host /tmp directory that
+// marks the need for a reboot. Since reboot is only triggered for now
+// by some config changes, we leave this here for now. There is planned
+// functionality to allow image installs to require reboots, at that moment
+// this constant can be moved somewhere else
+const REBOOT_BREADCRUMB = fsUtils.getPathOnHost(
+	'/tmp/balena-supervisor/reboot-after-apply',
+);
+
 interface ConfigOption {
 	envVarName: string;
 	varType: string;
@@ -34,7 +45,6 @@ export interface ConfigStep {
 	action: keyof DeviceActionExecutors | 'reboot' | 'noop';
 	humanReadableTarget?: Dictionary<string>;
 	target?: string | Dictionary<string>;
-	rebootRequired?: boolean;
 }
 
 interface DeviceActionExecutorOpts {
@@ -50,9 +60,9 @@ interface DeviceActionExecutors {
 	changeConfig: DeviceActionExecutorFn;
 	setVPNEnabled: DeviceActionExecutorFn;
 	setBootConfig: DeviceActionExecutorFn;
+	setRebootBreadcrumb: DeviceActionExecutorFn;
 }
 
-let rebootRequired = false;
 const actionExecutors: DeviceActionExecutors = {
 	changeConfig: async (step) => {
 		try {
@@ -70,9 +80,6 @@ const actionExecutors: DeviceActionExecutors = {
 					success: true,
 				});
 			}
-			if (step.rebootRequired) {
-				rebootRequired = true;
-			}
 		} catch (err) {
 			if (step.humanReadableTarget) {
 				logger.logConfigChange(step.humanReadableTarget, {
@@ -110,6 +117,11 @@ const actionExecutors: DeviceActionExecutors = {
 			await setBootConfig(backend, step.target as Dictionary<string>);
 		}
 	},
+	setRebootBreadcrumb: async () => {
+		// Just create the file. The last step in the target state calculation will check
+		// the file and create a reboot step
+		await fsUtils.touch(REBOOT_BREADCRUMB);
+	},
 };
 
 const configBackends: ConfigBackend[] = [];
@@ -444,11 +456,14 @@ function getConfigSteps(
 	);
 
 	if (!_.isEmpty(configChanges)) {
+		if (reboot) {
+			steps.push({ action: 'setRebootBreadcrumb' });
+		}
+
 		steps.push({
 			action: 'changeConfig',
 			target: configChanges,
 			humanReadableTarget: humanReadableConfigChanges,
-			rebootRequired: reboot,
 		});
 	}
 
@@ -523,7 +538,25 @@ async function getBackendSteps(
 		}
 	}
 
-	return steps;
+	return [
+		// All backend steps require a reboot
+		...(steps.length > 0
+			? [{ action: 'setRebootBreadcrumb' } as ConfigStep]
+			: []),
+		...steps,
+	];
+}
+
+async function isRebootRequired() {
+	const hasBreadcrumb = await fsUtils.exists(REBOOT_BREADCRUMB);
+	if (hasBreadcrumb) {
+		const stats = await fs.stat(REBOOT_BREADCRUMB);
+
+		// If the breadcrumb exists and the last modified time is greater than the
+		// boot time, that means we need to reboot
+		return stats.mtime.getTime() > fsUtils.getBootTime().getTime();
+	}
+	return false;
 }
 
 export async function getRequiredSteps(
@@ -542,19 +575,19 @@ export async function getRequiredSteps(
 	);
 
 	const configSteps = getConfigSteps(current, target);
-	const steps = ([] as ConfigStep[]).concat(
-		configSteps,
-		await getVPNSteps(current, target),
+	const steps = [
+		...configSteps,
+		...(await getVPNSteps(current, target)),
 
 		// Only apply backend steps if no more config changes are left since
 		// changing config.json may restart the supervisor
-		configSteps.length > 0 &&
-			// if any config step is a not 'noop' step, skip the backend steps
-			configSteps.filter((s) => s.action !== 'noop').length > 0
+		...(configSteps.length > 0 &&
+		// if any config step is a not 'noop' step, skip the backend steps
+		configSteps.filter((s) => s.action !== 'noop').length > 0
 			? // Set a 'noop' action so the apply function knows to retry
-			  [{ action: 'noop' }]
-			: await getBackendSteps(current, target),
-	);
+			  [{ action: 'noop' } as ConfigStep]
+			: await getBackendSteps(current, target)),
+	];
 
 	// Check if there is either no steps, or they are all
 	// noops, and we need to reboot. We want to do this
@@ -562,6 +595,7 @@ export async function getRequiredSteps(
 	// connection, the device will try to start containers
 	// before any boot config has been applied, which can
 	// cause problems
+	const rebootRequired = await isRebootRequired();
 	if (_.every(steps, { action: 'noop' }) && rebootRequired) {
 		steps.push({
 			action: 'reboot',
@@ -657,7 +691,6 @@ export async function setBootConfig(
 			{},
 			'Apply boot config success',
 		);
-		rebootRequired = true;
 		return true;
 	} catch (err) {
 		logger.logSystemMessage(

test/12-device-config.spec.ts

@@ -739,7 +739,6 @@ describe('getRequiredSteps', () => {
 				local: {
 					config: {
 						SUPERVISOR_POLL_INTERVAL: 900000,
-						SUPERVISOR_PERSISTENT_LOGGING: true,
 						HOST_CONFIG_enable_uart: true,
 					},
 				},
@@ -748,18 +747,44 @@ describe('getRequiredSteps', () => {
 				local: {
 					config: {
 						SUPERVISOR_POLL_INTERVAL: 600000,
-						SUPERVISOR_PERSISTENT_LOGGING: false,
 						HOST_CONFIG_enable_uart: false,
 					},
 				},
 			} as any,
 		);
 		expect(steps.map((s) => s.action)).to.have.members([
+			// No reboot is required by this config change
 			'changeConfig',
 			'noop', // The noop has to be here since there are also changes from config backends
 		]);
 	});
 
+	it('sets the rebooot breadcrumb for config steps that require a reboot', async () => {
+		const steps = await deviceConfig.getRequiredSteps(
+			{
+				local: {
+					config: {
+						SUPERVISOR_POLL_INTERVAL: 900000,
+						SUPERVISOR_PERSISTENT_LOGGING: false,
+					},
+				},
+			} as any,
+			{
+				local: {
+					config: {
+						SUPERVISOR_POLL_INTERVAL: 600000,
+						SUPERVISOR_PERSISTENT_LOGGING: true,
+					},
+				},
+			} as any,
+		);
+		expect(steps.map((s) => s.action)).to.have.members([
+			'setRebootBreadcrumb',
+			'changeConfig',
+			'noop',
+		]);
+	});
+
 	it('returns required steps for backends if no steps are required for config.json', async () => {
 		const steps = await deviceConfig.getRequiredSteps(
 			{
@@ -781,6 +806,9 @@ describe('getRequiredSteps', () => {
 				},
 			} as any,
 		);
-		expect(steps.map((s) => s.action)).to.have.members(['setBootConfig']);
+		expect(steps.map((s) => s.action)).to.have.members([
+			'setRebootBreadcrumb',
+			'setBootConfig',
+		]);
 	});
 });