Use write + sync when writing configs to /mnt/boot

This commit updates all backends that write to /mnt/boot to do it
through a new `lib/host-utils` module. Writes are now done using write +
sync as rename is not an atomic operation in vfat.

The change also applies for writes through the `/v1/host-config`
endpoint.

Finally this change includes some improvements on tests.

Change-type: patch
This commit is contained in:
Felipe Lalanne 2022-01-26 18:56:08 -03:00
parent 5686cc363d
commit c04955354a
18 changed files with 646 additions and 538 deletions

View File

@ -1,25 +1,9 @@
import * as _ from 'lodash';
import * as constants from '../../lib/constants';
import { writeFileAtomic, exec } from '../../lib/fs-utils';
export interface ConfigOptions {
[key: string]: string | string[];
}
export const bootMountPoint = `${constants.rootMountPoint}${constants.bootMountPoint}`;
export async function remountAndWriteAtomic(
file: string,
data: string | Buffer,
): Promise<void> {
// Here's the dangerous part:
await exec(
`mount -t vfat -o remount,rw ${constants.bootBlockDevice} ${bootMountPoint}`,
);
await writeFileAtomic(file, data);
}
export abstract class ConfigBackend {
// Does this config backend support the given device type?
public abstract matches(

View File

@ -2,13 +2,9 @@ import * as _ from 'lodash';
import { promises as fs } from 'fs';
import * as path from 'path';
import {
ConfigOptions,
ConfigBackend,
bootMountPoint,
remountAndWriteAtomic,
} from './backend';
import { ConfigOptions, ConfigBackend } from './backend';
import { exec, exists } from '../../lib/fs-utils';
import * as hostUtils from '../../lib/host-utils';
import * as constants from '../../lib/constants';
import * as logger from '../../logger';
import log from '../../lib/supervisor-console';
@ -27,7 +23,7 @@ export class ConfigFs extends ConfigBackend {
constants.rootMountPoint,
'boot/acpi-tables',
);
private readonly ConfigFilePath = path.join(bootMountPoint, 'configfs.json'); // use constant for mount path, rename to ssdt.txt
private readonly ConfigFilePath = hostUtils.pathOnBoot('configfs.json');
private readonly ConfigfsMountPoint = path.join(
constants.rootMountPoint,
'sys/kernel/config',
@ -116,7 +112,7 @@ export class ConfigFs extends ConfigBackend {
}
private async writeConfigJSON(config: ConfigfsConfig): Promise<void> {
await remountAndWriteAtomic(this.ConfigFilePath, JSON.stringify(config));
await hostUtils.writeToBoot(this.ConfigFilePath, JSON.stringify(config));
}
private async loadConfiguredSsdt(config: ConfigfsConfig): Promise<void> {

View File

@ -1,15 +1,11 @@
import * as _ from 'lodash';
import { promises as fs } from 'fs';
import {
ConfigOptions,
ConfigBackend,
bootMountPoint,
remountAndWriteAtomic,
} from './backend';
import { ConfigOptions, ConfigBackend } from './backend';
import * as constants from '../../lib/constants';
import log from '../../lib/supervisor-console';
import { exists } from '../../lib/fs-utils';
import * as hostUtils from '../../lib/host-utils';
/**
* A backend to handle Raspberry Pi host configuration
@ -24,7 +20,7 @@ import { exists } from '../../lib/fs-utils';
export class ConfigTxt extends ConfigBackend {
private static bootConfigVarPrefix = `${constants.hostConfigVarPrefix}CONFIG_`;
private static bootConfigPath = `${bootMountPoint}/config.txt`;
private static bootConfigPath = hostUtils.pathOnBoot(`config.txt`);
public static bootConfigVarRegex = new RegExp(
'(?:' + _.escapeRegExp(ConfigTxt.bootConfigVarPrefix) + ')(.+)',
@ -70,7 +66,7 @@ export class ConfigTxt extends ConfigBackend {
if (await exists(ConfigTxt.bootConfigPath)) {
configContents = await fs.readFile(ConfigTxt.bootConfigPath, 'utf-8');
} else {
await fs.writeFile(ConfigTxt.bootConfigPath, '');
await hostUtils.writeToBoot(ConfigTxt.bootConfigPath, '');
}
const conf: ConfigOptions = {};
@ -126,7 +122,7 @@ export class ConfigTxt extends ConfigBackend {
}
});
const confStr = `${confStatements.join('\n')}\n`;
await remountAndWriteAtomic(ConfigTxt.bootConfigPath, confStr);
await hostUtils.writeToBoot(ConfigTxt.bootConfigPath, confStr);
}
public isSupportedConfig(configName: string): boolean {

View File

@ -2,12 +2,7 @@ import * as _ from 'lodash';
import { promises as fs } from 'fs';
import * as semver from 'semver';
import {
ConfigOptions,
ConfigBackend,
bootMountPoint,
remountAndWriteAtomic,
} from './backend';
import { ConfigOptions, ConfigBackend } from './backend';
import {
ExtlinuxFile,
Directive,
@ -17,6 +12,7 @@ import {
import * as constants from '../../lib/constants';
import log from '../../lib/supervisor-console';
import { ExtLinuxEnvError, ExtLinuxParseError } from '../../lib/errors';
import * as hostUtils from '../../lib/host-utils';
// The OS version when extlinux moved to READ ONLY partition
const EXTLINUX_READONLY = '2.47.0';
@ -31,7 +27,9 @@ const EXTLINUX_READONLY = '2.47.0';
export class Extlinux extends ConfigBackend {
private static bootConfigVarPrefix = `${constants.hostConfigVarPrefix}EXTLINUX_`;
private static bootConfigPath = `${bootMountPoint}/extlinux/extlinux.conf`;
private static bootConfigPath = hostUtils.pathOnBoot(
`extlinux/extlinux.conf`,
);
private static supportedConfigValues = ['isolcpus', 'fdt'];
private static supportedDirectives = ['APPEND', 'FDT'];
@ -136,7 +134,7 @@ export class Extlinux extends ConfigBackend {
);
// Write new extlinux configuration
return await remountAndWriteAtomic(
return await hostUtils.writeToBoot(
Extlinux.bootConfigPath,
Extlinux.extlinuxFileToString(parsedBootFile),
);

View File

@ -1,16 +1,12 @@
import * as _ from 'lodash';
import { promises as fs } from 'fs';
import {
ConfigOptions,
ConfigBackend,
bootMountPoint,
remountAndWriteAtomic,
} from './backend';
import { ConfigOptions, ConfigBackend } from './backend';
import * as constants from '../../lib/constants';
import log from '../../lib/supervisor-console';
import { ExtraUEnvError } from '../../lib/errors';
import { exists } from '../../lib/fs-utils';
import * as hostUtils from '../../lib/host-utils';
/**
* Entry describes the configurable items in an extra_uEnv file
@ -43,7 +39,7 @@ const OPTION_REGEX = /^\s*(\w+)=(.*)$/;
export class ExtraUEnv extends ConfigBackend {
private static bootConfigVarPrefix = `${constants.hostConfigVarPrefix}EXTLINUX_`;
private static bootConfigPath = `${bootMountPoint}/extra_uEnv.txt`;
private static bootConfigPath = hostUtils.pathOnBoot(`extra_uEnv.txt`);
private static entries: Record<EntryKey, Entry> = {
custom_fdt_file: { key: 'custom_fdt_file', collection: false },
@ -94,7 +90,7 @@ export class ExtraUEnv extends ConfigBackend {
});
// Write new extra_uEnv configuration
return await remountAndWriteAtomic(
return await hostUtils.writeToBoot(
ExtraUEnv.bootConfigPath,
ExtraUEnv.configToString(supportedOptions),
);

View File

@ -5,16 +5,12 @@ import * as path from 'path';
import * as constants from '../../lib/constants';
import { exists } from '../../lib/fs-utils';
import * as hostUtils from '../../lib/host-utils';
import log from '../../lib/supervisor-console';
import {
bootMountPoint,
ConfigBackend,
ConfigOptions,
remountAndWriteAtomic,
} from './backend';
import { ConfigBackend, ConfigOptions } from './backend';
export class SplashImage extends ConfigBackend {
private static readonly BASEPATH = path.join(bootMountPoint, 'splash');
private static readonly BASEPATH = hostUtils.pathOnBoot('splash');
private static readonly DEFAULT = path.join(
SplashImage.BASEPATH,
'balena-logo-default.png',
@ -86,7 +82,7 @@ export class SplashImage extends ConfigBackend {
const buffer = Buffer.from(image, 'base64');
if (this.isPng(buffer)) {
// Write the buffer to the given location
await remountAndWriteAtomic(where, buffer);
await hostUtils.writeToBoot(where, buffer);
} else {
throw new Error('Splash image should be a base64 encoded PNG image');
}

View File

@ -8,7 +8,8 @@ import * as config from './config';
import * as constants from './lib/constants';
import * as dbus from './lib/dbus';
import { ENOENT, InternalInconsistencyError } from './lib/errors';
import { writeFileAtomic, mkdirp, unlinkAll } from './lib/fs-utils';
import { mkdirp, unlinkAll } from './lib/fs-utils';
import { writeToBoot } from './lib/host-utils';
import * as updateLock from './lib/update-lock';
const redsocksHeader = stripIndent`
@ -133,7 +134,7 @@ async function setProxy(maybeConf: ProxyConfig | null): Promise<void> {
const conf = maybeConf as ProxyConfig;
await mkdirp(proxyBasePath);
if (_.isArray(conf.noProxy)) {
await writeFileAtomic(noProxyPath, conf.noProxy.join('\n'));
await writeToBoot(noProxyPath, conf.noProxy.join('\n'));
}
let currentConf: ProxyConfig | undefined;
@ -150,7 +151,7 @@ async function setProxy(maybeConf: ProxyConfig | null): Promise<void> {
${generateRedsocksConfEntries({ ...currentConf, ...conf })}
${redsocksFooter}
`;
await writeFileAtomic(redsocksConfPath, redsocksConf);
await writeToBoot(redsocksConfPath, redsocksConf);
}
// restart balena-proxy-config if it is loaded and NOT PartOf redsocks-conf.target

23
src/lib/host-utils.ts Normal file
View File

@ -0,0 +1,23 @@
import * as path from 'path';
import * as constants from './constants';
import * as fsUtils from './fs-utils';
// Returns an absolute path starting from the hostOS root partition
// This path is accessible from within the Supervisor container
export function pathOnRoot(relPath: string) {
return path.join(constants.rootMountPoint, relPath);
}
// Returns an absolute path starting from the hostOS boot partition
// This path is accessible from within the Supervisor container
export function pathOnBoot(relPath: string) {
return pathOnRoot(path.join(constants.bootMountPoint, relPath));
}
// Receives an absolute path for a file under the boot partition (e.g. `/mnt/root/mnt/boot/config.txt`)
// and writes the given data. This function uses the best effort to write a file trying to minimize corruption
// due to a power cut. Given that the boot partition is a vfat filesystem, this means
// using write + sync
export async function writeToBoot(file: string, data: string | Buffer) {
return await fsUtils.writeAndSyncFile(file, data);
}

View File

@ -124,8 +124,10 @@ describe('Config', () => {
});
describe('Config data sources', () => {
after(() => {
afterEach(() => {
// Clean up memoized values
fnSchema.deviceArch.clear();
fnSchema.deviceType.clear();
});
it('should obtain deviceArch from device-type.json', async () => {
@ -177,27 +179,28 @@ describe('Config', () => {
}),
);
// Make a first call to get the value to be memoized
await conf.get('deviceType');
await conf.get('deviceArch');
expect(fs.readFile).to.be.called;
(fs.readFile as SinonStub).resetHistory();
const deviceArch = await conf.get('deviceArch');
expect(deviceArch).to.equal(arch);
// The result should still be memoized from the
// call on the previous test
// The result should still be memoized from the previous call
expect(fs.readFile).to.not.be.called;
const deviceType = await conf.get('deviceType');
expect(deviceType).to.equal(slug);
// The result should still be memoized from the
// call on the previous test
// The result should still be memoized from the previous call
expect(fs.readFile).to.not.be.called;
(fs.readFile as SinonStub).restore();
});
it('should not memoize errors when reading deviceArch', (done) => {
// Clean up memoized value
fnSchema.deviceArch.clear();
// File not found
stub(fs, 'readFile').throws('File not found');
@ -225,9 +228,6 @@ describe('Config', () => {
});
it('should not memoize errors when reading deviceType', (done) => {
// Clean up memoized value
fnSchema.deviceType.clear();
// File not found
stub(fs, 'readFile').throws('File not found');

File diff suppressed because it is too large Load Diff

View File

@ -181,16 +181,15 @@ describe('Extlinux Configuration', () => {
});
it('sets new config values', async () => {
stub(fsUtils, 'writeFileAtomic').resolves();
stub(fsUtils, 'exec').resolves();
stub(fsUtils, 'writeAndSyncFile').resolves();
await backend.setBootConfig({
fdt: '/boot/mycustomdtb.dtb',
isolcpus: '2',
});
expect(fsUtils.writeFileAtomic).to.be.calledWith(
'./test/data/mnt/boot/extlinux/extlinux.conf',
expect(fsUtils.writeAndSyncFile).to.be.calledWith(
'test/data/mnt/boot/extlinux/extlinux.conf',
stripIndent`
DEFAULT primary
TIMEOUT 30
@ -204,8 +203,7 @@ describe('Extlinux Configuration', () => {
);
// Restore stubs
(fsUtils.writeFileAtomic as SinonStub).restore();
(fsUtils.exec as SinonStub).restore();
(fsUtils.writeAndSyncFile as SinonStub).restore();
});
it('only allows supported configuration options', () => {

View File

@ -108,8 +108,7 @@ describe('extra_uEnv Configuration', () => {
});
it('sets new config values', async () => {
stub(fsUtils, 'writeFileAtomic').resolves();
stub(fsUtils, 'exec').resolves();
stub(fsUtils, 'writeAndSyncFile').resolves();
const logWarningStub = spy(Log, 'warn');
// This config contains a value set from something else
@ -127,8 +126,8 @@ describe('extra_uEnv Configuration', () => {
console: 'tty0', // not supported so won't be set
});
expect(fsUtils.writeFileAtomic).to.be.calledWith(
'./test/data/mnt/boot/extra_uEnv.txt',
expect(fsUtils.writeAndSyncFile).to.be.calledWith(
'test/data/mnt/boot/extra_uEnv.txt',
'custom_fdt_file=/boot/mycustomdtb.dtb\nextra_os_cmdline=isolcpus=2\n',
);
@ -137,14 +136,12 @@ describe('extra_uEnv Configuration', () => {
);
// Restore stubs
(fsUtils.writeFileAtomic as SinonStub).restore();
(fsUtils.exec as SinonStub).restore();
(fsUtils.writeAndSyncFile as SinonStub).restore();
logWarningStub.restore();
});
it('sets new config values containing collections', async () => {
stub(fsUtils, 'writeFileAtomic').resolves();
stub(fsUtils, 'exec').resolves();
stub(fsUtils, 'writeAndSyncFile').resolves();
const logWarningStub = spy(Log, 'warn');
// @ts-ignore accessing private value
@ -166,14 +163,13 @@ describe('extra_uEnv Configuration', () => {
splash: '', // collection entry so should be concatted to other collections of this entry
});
expect(fsUtils.writeFileAtomic).to.be.calledWith(
'./test/data/mnt/boot/extra_uEnv.txt',
expect(fsUtils.writeAndSyncFile).to.be.calledWith(
'test/data/mnt/boot/extra_uEnv.txt',
'custom_fdt_file=/boot/mycustomdtb.dtb\nextra_os_cmdline=isolcpus=2 console=tty0 splash\n',
);
// Restore stubs
(fsUtils.writeFileAtomic as SinonStub).restore();
(fsUtils.exec as SinonStub).restore();
(fsUtils.writeAndSyncFile as SinonStub).restore();
logWarningStub.restore();
// @ts-ignore accessing private value
ExtraUEnv.supportedConfigs = previousSupportedConfigs;

View File

@ -44,9 +44,11 @@ const req = {
};
describe('Target state', () => {
before(() => {
before(async () => {
// maxPollTime starts as undefined
deviceState.__set__('maxPollTime', 60000);
stub(deviceState, 'applyStep').resolves();
});
beforeEach(() => {
@ -59,6 +61,10 @@ describe('Target state', () => {
(request.getRequestInstance as SinonStub).restore();
});
after(async () => {
(deviceState.applyStep as SinonStub).restore();
});
describe('update', () => {
it('should throw if a 304 is received but no local cache exists', async () => {
// new request returns 304

View File

@ -83,6 +83,9 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
await deviceState.initialized;
await targetStateCache.initialized;
// Do not apply target state
stub(deviceState, 'applyStep').resolves();
// Stub health checks so we can modify them whenever needed
healthCheckStubs = [
stub(apiBinder, 'healthcheck'),
@ -91,7 +94,7 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
// The mockedAPI contains stubs that might create unexpected results
// See the module to know what has been stubbed
api = await mockedAPI.create();
api = await mockedAPI.create(healthCheckStubs);
// Start test API
await api.listen(
@ -119,6 +122,7 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
throw e;
}
}
(deviceState.applyStep as SinonStub).restore();
// Restore healthcheck stubs
healthCheckStubs.forEach((hc) => hc.restore());
// Remove any test data generated

View File

@ -343,7 +343,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
lockMock.restore();
});
it.skip('should return 200 for an existing service', async () => {
it('should return 200 for an existing service', async () => {
await mockedDockerode.testWithData(
{ containers: mockContainers, images: mockImages },
async () => {

View File

@ -15,12 +15,11 @@ describe('Splash image configuration', () => {
const uri = `data:image/png;base64,${logo}`;
let readDirStub: SinonStub;
let readFileStub: SinonStub;
let writeFileAtomicStub: SinonStub;
let writeAndSyncFileStub: SinonStub;
beforeEach(() => {
// Setup stubs
writeFileAtomicStub = stub(fsUtils, 'writeFileAtomic').resolves();
stub(fsUtils, 'exec').resolves();
writeAndSyncFileStub = stub(fsUtils, 'writeAndSyncFile').resolves();
readFileStub = stub(fs, 'readFile').resolves(
Buffer.from(logo, 'base64') as any,
);
@ -32,8 +31,7 @@ describe('Splash image configuration', () => {
afterEach(() => {
// Restore stubs
writeFileAtomicStub.restore();
(fsUtils.exec as SinonStub).restore();
writeAndSyncFileStub.restore();
readFileStub.restore();
readDirStub.restore();
});
@ -50,7 +48,7 @@ describe('Splash image configuration', () => {
);
// Should make a copy
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo-default.png',
Buffer.from(logo, 'base64'),
);
@ -178,7 +176,7 @@ describe('Splash image configuration', () => {
await backend.setBootConfig({ image: uri });
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/resin-logo.png',
Buffer.from(logo, 'base64'),
);
@ -189,7 +187,7 @@ describe('Splash image configuration', () => {
await backend.setBootConfig({ image: uri });
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo.png',
Buffer.from(logo, 'base64'),
);
@ -200,7 +198,7 @@ describe('Splash image configuration', () => {
await backend.setBootConfig({ image: uri });
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo.png',
Buffer.from(logo, 'base64'),
);
@ -211,7 +209,7 @@ describe('Splash image configuration', () => {
await backend.setBootConfig({ image: logo });
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo.png',
Buffer.from(logo, 'base64'),
);
@ -224,7 +222,7 @@ describe('Splash image configuration', () => {
expect(readFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo-default.png',
);
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo.png',
Buffer.from(defaultLogo, 'base64'),
);
@ -237,7 +235,7 @@ describe('Splash image configuration', () => {
expect(readFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo-default.png',
);
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/resin-logo.png',
Buffer.from(defaultLogo, 'base64'),
);
@ -250,7 +248,7 @@ describe('Splash image configuration', () => {
expect(readFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo-default.png',
);
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo.png',
Buffer.from(defaultLogo, 'base64'),
);
@ -263,7 +261,7 @@ describe('Splash image configuration', () => {
expect(readFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/balena-logo-default.png',
);
expect(writeFileAtomicStub).to.be.calledOnceWith(
expect(writeAndSyncFileStub).to.be.calledOnceWith(
'test/data/mnt/boot/splash/resin-logo.png',
Buffer.from(defaultLogo, 'base64'),
);
@ -271,12 +269,12 @@ describe('Splash image configuration', () => {
it('should throw if arg is not a valid base64 string', async () => {
expect(backend.setBootConfig({ image: 'somestring' })).to.be.rejected;
expect(writeFileAtomicStub).to.not.be.called;
expect(writeAndSyncFileStub).to.not.be.called;
});
it('should throw if image is not a valid PNG file', async () => {
expect(backend.setBootConfig({ image: 'aGVsbG8=' })).to.be.rejected;
expect(writeFileAtomicStub).to.not.be.called;
expect(writeAndSyncFileStub).to.not.be.called;
});
});

View File

@ -1 +1 @@
{"applicationId":78373,"deviceType":"raspberrypi3","appUpdatePollInterval":3000,"listenPort":2345,"vpnPort":443,"apiEndpoint":"http://0.0.0.0:3000","registryEndpoint":"registry2.resin.io","deltaEndpoint":"https://delta.resin.io","mixpanelToken":"baz","apiKey":"boo","version":"2.0.6+rev3.prod"}
{"applicationId":78373,"deviceType":"raspberrypi4-64","appUpdatePollInterval":3000,"listenPort":2345,"vpnPort":443,"apiEndpoint":"http://0.0.0.0:3000","registryEndpoint":"registry2.resin.io","deltaEndpoint":"https://delta.resin.io","mixpanelToken":"baz","apiKey":"boo","version":"2.0.6+rev3.prod"}

View File

@ -125,7 +125,9 @@ const mockedOptions = {
*
*/
async function create(): Promise<SupervisorAPI> {
async function create(
healthchecks = [deviceState.healthcheck, apiBinder.healthcheck],
): Promise<SupervisorAPI> {
// Get SupervisorAPI construct options
await createAPIOpts();
@ -135,7 +137,7 @@ async function create(): Promise<SupervisorAPI> {
// Create SupervisorAPI
const api = new SupervisorAPI({
routers: [deviceState.router, buildRoutes()],
healthchecks: [deviceState.healthcheck, apiBinder.healthcheck],
healthchecks,
});
const macAddress = await config.get('macAddress');