Merge pull request #923 from balena-io/fix-airgapped-preload

state-engine: Add rate limited steps to device-config

src/device-config.ts

@@ -32,7 +32,7 @@ interface ConfigStep {
 	// TODO: This is a bit of a mess, the DeviceConfig class shouldn't
 	// know that the reboot action exists as it is implemented by
 	// DeviceState. Fix this weird circular dependency
-	action: keyof DeviceActionExecutors | 'reboot';
+	action: keyof DeviceActionExecutors | 'reboot' | 'noop';
 	humanReadableTarget?: Dictionary<string>;
 	target?: string | Dictionary<string>;
 	rebootRequired?: boolean;
@@ -131,6 +131,17 @@ export class DeviceConfig {
 		..._.map(DeviceConfig.configKeys, 'envVarName'),
 	];
 
+	private rateLimits: Dictionary<{
+		duration: number;
+		lastAttempt: number | null;
+	}> = {
+		setVPNEnabled: {
+			// Only try to switch the VPN once an hour
+			duration: 60 * 60 * 1000,
+			lastAttempt: null,
+		},
+	};
+
 	public constructor({ db, config, logger }: DeviceConfigConstructOpts) {
 		this.db = db;
 		this.config = config;
@@ -361,7 +372,7 @@ export class DeviceConfig {
 			{},
 		);
 
-		const steps: ConfigStep[] = [];
+		let steps: ConfigStep[] = [];
 
 		const { deviceType, unmanaged } = await this.config.getMany([
 			'deviceType',
@@ -413,6 +424,27 @@ export class DeviceConfig {
 			});
 		}
 
+		const now = Date.now();
+		steps = _.map(steps, step => {
+			const action = step.action;
+			if (action in this.rateLimits) {
+				const lastAttempt = this.rateLimits[action].lastAttempt;
+				this.rateLimits[action].lastAttempt = now;
+
+				// If this step should be rate limited, we replace it with a noop.
+				// We do this instead of removing it, as we don't actually want the
+				// state engine to think that it's successfully applied the target state,
+				// as it won't reattempt the change until the target state changes
+				if (
+					lastAttempt != null &&
+					Date.now() - lastAttempt < this.rateLimits[action].duration
+				) {
+					return { action: 'noop' } as ConfigStep;
+				}
+			}
+			return step;
+		});
+
 		// Do we need to change the boot config?
 		if (this.bootConfigChangeRequired(backend, current, target, deviceType)) {
 			steps.push({
@@ -431,7 +463,7 @@ export class DeviceConfig {
 	}
 
 	public executeStepAction(step: ConfigStep, opts: DeviceActionExecutorOpts) {
-		if (step.action !== 'reboot') {
+		if (step.action !== 'reboot' && step.action !== 'noop') {
 			return this.actionExecutors[step.action](step, opts);
 		}
 	}

src/device-state.coffee

@@ -578,8 +578,7 @@ module.exports = class DeviceState extends EventEmitter
 			console.log('Scheduling another update attempt due to failure: ', delay, err)
 			@triggerApplyTarget({ force, delay, initial })
 
-	applyTarget: ({ force = false, initial = false, intermediate = false, skipLock = false } = {}) =>
-		nextDelay = 200
+	applyTarget: ({ force = false, initial = false, intermediate = false, skipLock = false, nextDelay = 200, retryCount = 0 } = {}) =>
 		Promise.try =>
 			if !intermediate
 				@applyBlocker
@@ -594,11 +593,23 @@ module.exports = class DeviceState extends EventEmitter
 					.then ([ currentState, targetState ]) =>
 						@deviceConfig.getRequiredSteps(currentState, targetState)
 						.then (deviceConfigSteps) =>
-							if !_.isEmpty(deviceConfigSteps)
-								return deviceConfigSteps
+							noConfigSteps = _.every(deviceConfigSteps, ({ action }) -> action is 'noop')
+							if not noConfigSteps
+								return [false, deviceConfigSteps]
 							else
 								@applications.getRequiredSteps(currentState, targetState, extraState, intermediate)
-		.then (steps) =>
+								.then (appSteps) ->
+									# We need to forward to no-ops to the next step if the application state is done
+									# The true and false values below represent whether we should add an exponential
+									# backoff if the steps are all no-ops. the reason that this has to be different
+									# is that a noop step from the application manager means that we should keep running
+									# in a tight loop, waiting for an image to download (for example). The device-config
+									# steps generally have a much higher time to wait before the no-ops will stop, so we
+									# should try to reduce the effect that this will have
+									if _.isEmpty(appSteps) and noConfigSteps
+										return [true, deviceConfigSteps]
+									return [false, appSteps]
+		.then ([backoff, steps]) =>
 			if _.isEmpty(steps)
 				@emitAsync('apply-target-state-end', null)
 				if !intermediate
@@ -611,12 +622,17 @@ module.exports = class DeviceState extends EventEmitter
 			if !intermediate
 				@reportCurrentState(update_pending: true)
 			if _.every(steps, (step) -> step.action == 'noop')
+				if backoff
+					retryCount += 1
+					# Backoff to a maximum of 10 minutes
+					nextDelay = Math.min(2 ** retryCount * 1000, 60 * 10 * 1000)
+				else
 					nextDelay = 1000
 			Promise.map steps, (step) =>
 				@applyStep(step, { force, initial, intermediate, skipLock })
 			.delay(nextDelay)
 			.then =>
-				@applyTarget({ force, initial, intermediate, skipLock })
+				@applyTarget({ force, initial, intermediate, skipLock, nextDelay, retryCount })
 		.catch (err) =>
 			@applyError(err, { force, initial, intermediate })
 

src/network.ts

@@ -42,7 +42,7 @@ export function enableCheck(enable: boolean) {
 
 async function vpnStatusInotifyCallback(): Promise<void> {
 	try {
-		await fs.lstat(constants.vpnStatusPath);
+		await fs.lstat(`${constants.vpnStatusPath}/active`);
 		isConnectivityCheckPaused = true;
 	} catch {
 		isConnectivityCheckPaused = false;