From 1570fd424b8c192844ce308761ac8f329dac48f5 Mon Sep 17 00:00:00 2001 From: Theodor Gherzan Date: Sat, 16 Nov 2019 18:43:23 +0000 Subject: [PATCH 1/3] Create config.txt if there isn't one already Change-type: patch Signed-off-by: Theodor Gherzan --- src/config/backend.ts | 83 +++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/src/config/backend.ts b/src/config/backend.ts index fa13c910..a0220009 100644 --- a/src/config/backend.ts +++ b/src/config/backend.ts @@ -106,51 +106,58 @@ export class RPiConfigBackend extends DeviceConfigBackend { return _.startsWith(deviceType, 'raspberry') || deviceType === 'fincm3'; } - public getBootConfig(): Promise { - return Promise.resolve( - fs.readFile(RPiConfigBackend.bootConfigPath, 'utf-8'), - ).then(confStr => { - const conf: ConfigOptions = {}; - const configStatements = confStr.split(/\r?\n/); + public async getBootConfig(): Promise { + let configContents = ''; - for (const configStr of configStatements) { - // Don't show warnings for comments and empty lines - const trimmed = _.trimStart(configStr); - if (_.startsWith(trimmed, '#') || trimmed === '') { - continue; - } - let keyValue = /^([^=]+)=(.*)$/.exec(configStr); - if (keyValue != null) { - const [, key, value] = keyValue; - if (!_.includes(RPiConfigBackend.arrayConfigKeys, key)) { - conf[key] = value; - } else { - if (conf[key] == null) { - conf[key] = []; - } - const confArr = conf[key]; - if (!_.isArray(confArr)) { - throw new Error( - `Expected '${key}' to have a config array but got ${typeof confArr}`, - ); - } - confArr.push(value); - } - continue; - } + if (await fs.exists(RPiConfigBackend.bootConfigPath)) { + configContents = await fs.readFile( + RPiConfigBackend.bootConfigPath, + 'utf-8', + ); + } else { + await fs.writeFile(RPiConfigBackend.bootConfigPath, ''); + } - // Try the next regex instead - keyValue = /^(initramfs) (.+)/.exec(configStr); - if (keyValue != null) { - const [, key, value] = keyValue; + const conf: ConfigOptions = {}; + const configStatements = configContents.split(/\r?\n/); + + for (const configStr of configStatements) { + // Don't show warnings for comments and empty lines + const trimmed = _.trimStart(configStr); + if (_.startsWith(trimmed, '#') || trimmed === '') { + continue; + } + let keyValue = /^([^=]+)=(.*)$/.exec(configStr); + if (keyValue != null) { + const [, key, value] = keyValue; + if (!_.includes(RPiConfigBackend.arrayConfigKeys, key)) { conf[key] = value; } else { - log.warn(`Could not parse config.txt entry: ${configStr}. Ignoring.`); + if (conf[key] == null) { + conf[key] = []; + } + const confArr = conf[key]; + if (!_.isArray(confArr)) { + throw new Error( + `Expected '${key}' to have a config array but got ${typeof confArr}`, + ); + } + confArr.push(value); } + continue; } - return conf; - }); + // Try the next regex instead + keyValue = /^(initramfs) (.+)/.exec(configStr); + if (keyValue != null) { + const [, key, value] = keyValue; + conf[key] = value; + } else { + log.warn(`Could not parse config.txt entry: ${configStr}. Ignoring.`); + } + } + + return conf; } public setBootConfig(opts: ConfigOptions): Promise { From 8589dbf3d1409942905182e9f5b1cf8a125a89ea Mon Sep 17 00:00:00 2001 From: Theodor Gherzan Date: Sat, 16 Nov 2019 19:22:55 +0000 Subject: [PATCH 2/3] Refactor code to use async/await syntax Signed-off-by: Theodor Gherzan --- src/config/backend.ts | 235 ++++++++++++++++-------------- test/13-device-config.spec.coffee | 14 +- 2 files changed, 133 insertions(+), 116 deletions(-) diff --git a/src/config/backend.ts b/src/config/backend.ts index a0220009..e47003aa 100644 --- a/src/config/backend.ts +++ b/src/config/backend.ts @@ -1,12 +1,8 @@ -import * as Promise from 'bluebird'; -import * as childProcessSync from 'child_process'; import * as _ from 'lodash'; -import { fs } from 'mz'; +import { child_process, fs } from 'mz'; import * as constants from '../lib/constants'; -import * as fsUtils from '../lib/fs-utils'; - -const childProcess: any = Promise.promisifyAll(childProcessSync); +import { writeFileAtomic } from '../lib/fs-utils'; import log from '../lib/supervisor-console'; @@ -25,18 +21,15 @@ interface ExtlinuxFile { const bootMountPoint = `${constants.rootMountPoint}${constants.bootMountPoint}`; -function remountAndWriteAtomic(file: string, data: string): Promise { - // TODO: Find out why the below Promise.resolve() is required +async function remountAndWriteAtomic( + file: string, + data: string, +): Promise { // Here's the dangerous part: - return Promise.resolve( - childProcess.execAsync( - `mount -t vfat -o remount,rw ${constants.bootBlockDevice} ${bootMountPoint}`, - ), - ) - .then(() => { - return fsUtils.writeFileAtomic(file, data); - }) - .return(); + await child_process.exec( + `mount -t vfat -o remount,rw ${constants.bootBlockDevice} ${bootMountPoint}`, + ); + await writeFileAtomic(file, data); } export abstract class DeviceConfigBackend { @@ -160,7 +153,7 @@ export class RPiConfigBackend extends DeviceConfigBackend { return conf; } - public setBootConfig(opts: ConfigOptions): Promise { + public async setBootConfig(opts: ConfigOptions): Promise { let confStatements: string[] = []; _.each(opts, (value, key) => { @@ -177,7 +170,7 @@ export class RPiConfigBackend extends DeviceConfigBackend { const confStr = `${confStatements.join('\n')}\n`; - return remountAndWriteAtomic(RPiConfigBackend.bootConfigPath, confStr); + await remountAndWriteAtomic(RPiConfigBackend.bootConfigPath, confStr); } public isSupportedConfig(configName: string): boolean { @@ -222,107 +215,131 @@ export class ExtlinuxConfigBackend extends DeviceConfigBackend { return _.startsWith(deviceType, 'jetson-tx'); } - public getBootConfig(): Promise { - return Promise.resolve( - fs.readFile(ExtlinuxConfigBackend.bootConfigPath, 'utf-8'), - ).then(confStr => { - const parsedBootFile = ExtlinuxConfigBackend.parseExtlinuxFile(confStr); + public async getBootConfig(): Promise { + let confContents: string; - // First find the default label name - const defaultLabel = _.find(parsedBootFile.globals, (_v, l) => { - if (l === 'DEFAULT') { - return true; - } - return false; - }); + try { + confContents = await fs.readFile( + ExtlinuxConfigBackend.bootConfigPath, + 'utf-8', + ); + } catch { + // In the rare case where the user might have deleted extlinux conf file between linux boot and supervisor boot + // We do not have any backup to fallback too; warn the user of a possible brick + throw new Error( + 'Could not find extlinux file. Device is possibly bricked', + ); + } - if (defaultLabel == null) { - throw new Error('Could not find default entry for extlinux.conf file'); + const parsedBootFile = ExtlinuxConfigBackend.parseExtlinuxFile( + confContents, + ); + + // First find the default label name + const defaultLabel = _.find(parsedBootFile.globals, (_v, l) => { + if (l === 'DEFAULT') { + return true; } - - const labelEntry = parsedBootFile.labels[defaultLabel]; - - if (labelEntry == null) { - throw new Error( - `Cannot find default label entry (label: ${defaultLabel}) for extlinux.conf file`, - ); - } - - // All configuration options come from the `APPEND` directive in the default label entry - const appendEntry = labelEntry.APPEND; - - if (appendEntry == null) { - throw new Error( - 'Could not find APPEND directive in default extlinux.conf boot entry', - ); - } - - const conf: ConfigOptions = {}; - const values = appendEntry.split(' '); - for (const value of values) { - const parts = value.split('='); - if (this.isSupportedConfig(parts[0])) { - if (parts.length !== 2) { - throw new Error( - `Could not parse extlinux configuration entry: ${values} [value with error: ${value}]`, - ); - } - conf[parts[0]] = parts[1]; - } - } - - return conf; + return false; }); + + if (defaultLabel == null) { + throw new Error('Could not find default entry for extlinux.conf file'); + } + + const labelEntry = parsedBootFile.labels[defaultLabel]; + + if (labelEntry == null) { + throw new Error( + `Cannot find default label entry (label: ${defaultLabel}) for extlinux.conf file`, + ); + } + + // All configuration options come from the `APPEND` directive in the default label entry + const appendEntry = labelEntry.APPEND; + + if (appendEntry == null) { + throw new Error( + 'Could not find APPEND directive in default extlinux.conf boot entry', + ); + } + + const conf: ConfigOptions = {}; + const values = appendEntry.split(' '); + for (const value of values) { + const parts = value.split('='); + if (this.isSupportedConfig(parts[0])) { + if (parts.length !== 2) { + throw new Error( + `Could not parse extlinux configuration entry: ${values} [value with error: ${value}]`, + ); + } + conf[parts[0]] = parts[1]; + } + } + + return conf; } - public setBootConfig(opts: ConfigOptions): Promise { + public async setBootConfig(opts: ConfigOptions): Promise { // First get a representation of the configuration file, with all balena-supported configuration removed - return Promise.resolve( - fs.readFile(ExtlinuxConfigBackend.bootConfigPath), - ).then(data => { - const extlinuxFile = ExtlinuxConfigBackend.parseExtlinuxFile( - data.toString(), - ); - const defaultLabel = extlinuxFile.globals.DEFAULT; - if (defaultLabel == null) { - throw new Error( - 'Could not find DEFAULT directive entry in extlinux.conf', - ); - } - const defaultEntry = extlinuxFile.labels[defaultLabel]; - if (defaultEntry == null) { - throw new Error( - `Could not find default extlinux.conf entry: ${defaultLabel}`, - ); - } + let confContents: string; - if (defaultEntry.APPEND == null) { - throw new Error( - `extlinux.conf APPEND directive not found for default entry: ${defaultLabel}, not sure how to proceed!`, - ); - } - - const appendLine = _.filter(defaultEntry.APPEND.split(' '), entry => { - const lhs = entry.split('='); - return !this.isSupportedConfig(lhs[0]); - }); - - // Apply the new configuration to the "plain" append line above - - _.each(opts, (value, key) => { - appendLine.push(`${key}=${value}`); - }); - - defaultEntry.APPEND = appendLine.join(' '); - const extlinuxString = ExtlinuxConfigBackend.extlinuxFileToString( - extlinuxFile, - ); - - return remountAndWriteAtomic( + try { + confContents = await fs.readFile( ExtlinuxConfigBackend.bootConfigPath, - extlinuxString, + 'utf-8', ); + } catch { + // In the rare case where the user might have deleted extlinux conf file between linux boot and supervisor boot + // We do not have any backup to fallback too; warn the user of a possible brick + throw new Error( + 'Could not find extlinux file. Device is possibly bricked', + ); + } + + const extlinuxFile = ExtlinuxConfigBackend.parseExtlinuxFile( + confContents.toString(), + ); + const defaultLabel = extlinuxFile.globals.DEFAULT; + if (defaultLabel == null) { + throw new Error( + 'Could not find DEFAULT directive entry in extlinux.conf', + ); + } + const defaultEntry = extlinuxFile.labels[defaultLabel]; + if (defaultEntry == null) { + throw new Error( + `Could not find default extlinux.conf entry: ${defaultLabel}`, + ); + } + + if (defaultEntry.APPEND == null) { + throw new Error( + `extlinux.conf APPEND directive not found for default entry: ${defaultLabel}, not sure how to proceed!`, + ); + } + + const appendLine = _.filter(defaultEntry.APPEND.split(' '), entry => { + const lhs = entry.split('='); + return !this.isSupportedConfig(lhs[0]); }); + + // Apply the new configuration to the "plain" append line above + + _.each(opts, (value, key) => { + appendLine.push(`${key}=${value}`); + }); + + defaultEntry.APPEND = appendLine.join(' '); + const extlinuxString = ExtlinuxConfigBackend.extlinuxFileToString( + extlinuxFile, + ); + + await remountAndWriteAtomic( + ExtlinuxConfigBackend.bootConfigPath, + extlinuxString, + ); } public isSupportedConfig(configName: string): boolean { diff --git a/test/13-device-config.spec.coffee b/test/13-device-config.spec.coffee index 7403330d..19ff0b6c 100644 --- a/test/13-device-config.spec.coffee +++ b/test/13-device-config.spec.coffee @@ -13,7 +13,7 @@ fsUtils = require '../src/lib/fs-utils' extlinuxBackend = new ExtlinuxConfigBackend() rpiConfigBackend = new RPiConfigBackend() -childProcess = require 'child_process' +{ child_process } = require 'mz' describe 'DeviceConfig', -> before -> @@ -111,7 +111,7 @@ describe 'DeviceConfig', -> it 'writes the target config.txt', -> stub(fsUtils, 'writeFileAtomic').resolves() - stub(childProcess, 'execAsync').resolves() + stub(child_process, 'exec').resolves() current = { HOST_CONFIG_initramfs: 'initramf.gz 0x00800000' HOST_CONFIG_dtparam: '"i2c=on","audio=on"' @@ -131,7 +131,7 @@ describe 'DeviceConfig', -> promise.then => @deviceConfig.setBootConfig(rpiConfigBackend, target) .then => - expect(childProcess.execAsync).to.be.calledOnce + expect(child_process.exec).to.be.calledOnce expect(@fakeLogger.logSystemMessage).to.be.calledTwice expect(@fakeLogger.logSystemMessage.getCall(1).args[2]).to.equal('Apply boot config success') expect(fsUtils.writeFileAtomic).to.be.calledWith('./test/data/mnt/boot/config.txt', '\ @@ -143,7 +143,7 @@ describe 'DeviceConfig', -> foobaz=bar\n\ ') fsUtils.writeFileAtomic.restore() - childProcess.execAsync.restore() + child_process.exec.restore() @fakeLogger.logSystemMessage.resetHistory() it 'accepts RESIN_ and BALENA_ variables', -> @@ -186,7 +186,7 @@ describe 'DeviceConfig', -> it 'should correctly write to extlinux.conf files', -> stub(fsUtils, 'writeFileAtomic').resolves() - stub(childProcess, 'execAsync').resolves() + stub(child_process, 'exec').resolves() current = { } @@ -200,7 +200,7 @@ describe 'DeviceConfig', -> promise.then => @deviceConfig.setBootConfig(extlinuxBackend, target) .then => - expect(childProcess.execAsync).to.be.calledOnce + expect(child_process.exec).to.be.calledOnce expect(@fakeLogger.logSystemMessage).to.be.calledTwice expect(@fakeLogger.logSystemMessage.getCall(1).args[2]).to.equal('Apply boot config success') expect(fsUtils.writeFileAtomic).to.be.calledWith('./test/data/mnt/boot/extlinux/extlinux.conf', '\ @@ -213,7 +213,7 @@ describe 'DeviceConfig', -> APPEND ${cbootargs} ${resin_kernel_root} ro rootwait isolcpus=2\n\ ') fsUtils.writeFileAtomic.restore() - childProcess.execAsync.restore() + child_process.exec.restore() @fakeLogger.logSystemMessage.resetHistory() describe 'Balena fin', -> From d6adfa189f858085d3d83286552156c5ebcd116f Mon Sep 17 00:00:00 2001 From: Theodor Gherzan Date: Sun, 17 Nov 2019 01:15:12 +0000 Subject: [PATCH 3/3] Do not polute test output with unncessarry stack trace Signed-off-by: Theodor Gherzan --- test/23-local-mode.ts | 3 ++- test/lib/errors.ts | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 test/lib/errors.ts diff --git a/test/23-local-mode.ts b/test/23-local-mode.ts index bf17ed8d..488cc13f 100644 --- a/test/23-local-mode.ts +++ b/test/23-local-mode.ts @@ -12,6 +12,7 @@ import LocalModeManager, { EngineSnapshotRecord, } from '../src/local-mode'; import Logger from '../src/logger'; +import ShortStackError from './lib/errors'; describe('LocalModeManager', () => { let dbFile: tmp.FileResult; @@ -188,7 +189,7 @@ describe('LocalModeManager', () => { ) => { const res = sinon.createStubInstance(c); if (removeThrows) { - res.remove.rejects(`test error removing ${type}`); + res.remove.rejects(new ShortStackError(`error removing ${type}`)); } else { res.remove.resolves(); } diff --git a/test/lib/errors.ts b/test/lib/errors.ts new file mode 100644 index 00000000..7d88d5eb --- /dev/null +++ b/test/lib/errors.ts @@ -0,0 +1,8 @@ +import TypedError = require('typed-error'); + +export default class ShortStackError extends TypedError { + constructor(err: Error | string = '') { + Error.stackTraceLimit = 1; + super(err); + } +}