Fix config checks for ConfigFS backend

When trying to apply SSDT overlays in Up Board, the supervisor currently
gets stuck in a loop trying to apply target state. See #1465

This was due to a bug in parsing the configuration, which lead to
the method bootConfigChangeRequired returning true when no change was
needed.

Change-type: patch
Signed-off-by: Felipe Lalanne <felipe@balena.io>
Connects-to: #1465
This commit is contained in:
Felipe Lalanne 2020-09-22 16:36:33 -03:00
parent 1ac71ea552
commit a5f3002e70
3 changed files with 131 additions and 84 deletions

2
.gitignore vendored
View File

@ -10,7 +10,7 @@ tools/dind/config/
tools/dind/config.json* tools/dind/config.json*
tools/dind/apps.json tools/dind/apps.json
tools/dind/backup.tgz tools/dind/backup.tgz
test/data/config*.json test/data/**/config*.json
test/data/*.sqlite test/data/*.sqlite
test/data/led_file test/data/led_file
/coverage/ /coverage/

View File

@ -94,7 +94,7 @@ export class ConfigFs extends ConfigBackend {
`AML: ${oemId.trim()} ${oemTableId.trim()} (Rev ${oemRevision.trim()})`, `AML: ${oemId.trim()} ${oemTableId.trim()} (Rev ${oemRevision.trim()})`,
); );
} catch (e) { } catch (e) {
log.error('Issue while loading AML ${aml}', e); log.error(`Issue while loading AML ${aml}`, e);
} }
return true; return true;
} }
@ -214,7 +214,7 @@ export class ConfigFs extends ConfigBackend {
return [value]; return [value];
} else { } else {
// or, it could be parsable as the content of a JSON array; "value" | "value1","value2" // or, it could be parsable as the content of a JSON array; "value" | "value1","value2"
return value.split(',').map((v) => v.replace('"', '').trim()); return value.split(',').map((v) => v.replace(/"/g, '').trim());
} }
default: default:
return value; return value;

View File

@ -9,12 +9,15 @@ import * as logger from '../src/logger';
import { Extlinux } from '../src/config/backends/extlinux'; import { Extlinux } from '../src/config/backends/extlinux';
import { ConfigTxt } from '../src/config/backends/config-txt'; import { ConfigTxt } from '../src/config/backends/config-txt';
import { Odmdata } from '../src/config/backends/odmdata'; import { Odmdata } from '../src/config/backends/odmdata';
import { ConfigFs } from '../src/config/backends/config-fs';
import * as constants from '../src/lib/constants';
import prepare = require('./lib/prepare'); import prepare = require('./lib/prepare');
const extlinuxBackend = new Extlinux(); const extlinuxBackend = new Extlinux();
const configTxtBackend = new ConfigTxt(); const configTxtBackend = new ConfigTxt();
const odmdataBackend = new Odmdata(); const odmdataBackend = new Odmdata();
const configFsBackend = new ConfigFs();
describe('Device Backend Config', () => { describe('Device Backend Config', () => {
let logSpy: SinonSpy; let logSpy: SinonSpy;
@ -336,94 +339,138 @@ describe('Device Backend Config', () => {
}); });
}); });
// describe('ConfigFS', () => { describe('ConfigFS files', () => {
// const upboardConfig = new DeviceConfig(); it('should correctly write to configfs.json files', async () => {
// let upboardConfigBackend: ConfigBackend | null; stub(fsUtils, 'writeFileAtomic').resolves();
stub(child_process, 'exec').resolves();
// before(async () => { const current = {};
// stub(child_process, 'exec').resolves(); const target = {
// stub(fs, 'exists').resolves(true); HOST_CONFIGFS_ssdt: 'spidev1.0',
// stub(fs, 'mkdir').resolves(); };
// stub(fs, 'readdir').resolves([]);
// stub(fsUtils, 'writeFileAtomic').resolves();
// stub(fs, 'readFile').callsFake(file => { expect(
// if (file === 'test/data/mnt/boot/configfs.json') { // @ts-ignore accessing private value
// return Promise.resolve( deviceConfig.bootConfigChangeRequired(
// JSON.stringify({ configFsBackend,
// ssdt: ['spidev1,1'], current,
// }), target,
// ); 'up-board',
// } ),
// return Promise.resolve(''); ).to.equal(true);
// });
// stub(config, 'get').callsFake(key => { // @ts-ignore accessing private value
// return Promise.try(() => { await deviceConfig.setBootConfig(configFsBackend, target);
// if (key === 'deviceType') { expect(child_process.exec).to.be.calledOnce;
// return 'up-board'; expect(logSpy).to.be.calledTwice;
// } expect(logSpy.getCall(1).args[2]).to.equal('Apply boot config success');
// throw new Error('Unknown fake config key'); expect(fsUtils.writeFileAtomic).to.be.calledWith(
// }); 'test/data/mnt/boot/configfs.json',
// }); '{"ssdt":["spidev1.0"]}',
);
// // @ts-ignore accessing private value // Restore stubs
// upboardConfigBackend = await upboardConfig.getConfigBackend(); (fsUtils.writeFileAtomic as SinonStub).restore();
// expect(upboardConfigBackend).is.not.null; (child_process.exec as SinonStub).restore();
// expect((child_process.exec as SinonSpy).callCount).to.equal( });
// 3,
// 'exec not called enough times', it('should correctly load the configfs.json file', async () => {
// ); stub(child_process, 'exec').resolves();
// }); stub(fsUtils, 'writeFileAtomic').resolves();
stub(fs, 'exists').resolves(true);
// after(() => { stub(fs, 'mkdir').resolves();
// (child_process.exec as SinonStub).restore(); stub(fs, 'readdir').resolves([]);
// (fs.exists as SinonStub).restore(); stub(fs, 'readFile').callsFake((file) => {
// (fs.mkdir as SinonStub).restore(); if (file === 'test/data/mnt/boot/configfs.json') {
// (fs.readdir as SinonStub).restore(); return Promise.resolve(
// (fs.readFile as SinonStub).restore(); JSON.stringify({
// (fsUtils.writeFileAtomic as SinonStub).restore(); ssdt: ['spidev1.1'],
// (config.get as SinonStub).restore(); }),
// }); );
}
// it('should correctly load the configfs.json file', () => { return Promise.resolve('');
// expect(child_process.exec).to.be.calledWith('modprobe acpi_configfs'); });
// expect(child_process.exec).to.be.calledWith(
// 'cat test/data/boot/acpi-tables/spidev1,1.aml > test/data/sys/kernel/config/acpi/table/spidev1,1/aml', await configFsBackend.initialise();
// ); expect(child_process.exec).to.be.calledWith('modprobe acpi_configfs');
// expect((fs.exists as SinonSpy).callCount).to.equal(2); expect(child_process.exec).to.be.calledWith(
// expect((fs.readFile as SinonSpy).callCount).to.equal(4); `mount -t vfat -o remount,rw ${constants.bootBlockDevice} ./test/data/mnt/boot`,
// }); );
expect(child_process.exec).to.be.calledWith(
// it('should correctly write the configfs.json file', async () => { 'cat test/data/boot/acpi-tables/spidev1.1.aml > test/data/sys/kernel/config/acpi/table/spidev1.1/aml',
// const current = {}; );
// const target = { expect((fs.exists as SinonSpy).callCount).to.equal(2);
// HOST_CONFIGFS_ssdt: 'spidev1,1', expect((fs.readFile as SinonSpy).callCount).to.equal(4);
// };
// Restore stubs
// (child_process.exec as SinonSpy).resetHistory(); (fsUtils.writeFileAtomic as SinonStub).restore();
// (fs.exists as SinonSpy).resetHistory(); (child_process.exec as SinonStub).restore();
// (fs.mkdir as SinonSpy).resetHistory(); (fs.exists as SinonStub).restore();
// (fs.readdir as SinonSpy).resetHistory(); (fs.mkdir as SinonStub).restore();
// (fs.readFile as SinonSpy).resetHistory(); (fs.readdir as SinonStub).restore();
(fs.readFile as SinonStub).restore();
// // @ts-ignore accessing private value });
// upboardConfig.bootConfigChangeRequired(upboardConfigBackend, current, target);
// // @ts-ignore accessing private value it('requires change when target is different', () => {
// await upboardConfig.setBootConfig(upboardConfigBackend, target); expect(
deviceConfig.bootConfigChangeRequired(
// expect(child_process.exec).to.be.calledOnce; configFsBackend,
// expect(fsUtils.writeFileAtomic).to.be.calledWith( { HOST_CONFIGFS_ssdt: '' },
// 'test/data/mnt/boot/configfs.json', { HOST_CONFIGFS_ssdt: 'spidev1.0' },
// JSON.stringify({ 'up-board',
// ssdt: ['spidev1,1'], ),
// }), ).to.equal(true);
// ); expect(
// expect(logSpy).to.be.calledTwice; deviceConfig.bootConfigChangeRequired(
// expect(logSpy.getCall(1).args[2]).to.equal('Apply boot config success'); configFsBackend,
// }); { HOST_CONFIGFS_ssdt: '' },
// }); { HOST_CONFIGFS_ssdt: '"spidev1.0"' },
'up-board',
// // This will require stubbing device.reboot, gosuper.post, config.get/set ),
// it('applies the target state'); ).to.equal(true);
expect(
deviceConfig.bootConfigChangeRequired(
configFsBackend,
{ HOST_CONFIGFS_ssdt: '"spidev1.0"' },
{ HOST_CONFIGFS_ssdt: '"spidev1.0","spidev1.1"' },
'up-board',
),
).to.equal(true);
});
it('should not report change when target is equal to current', () => {
expect(
deviceConfig.bootConfigChangeRequired(
configFsBackend,
{ HOST_CONFIGFS_ssdt: '' },
{ HOST_CONFIGFS_ssdt: '' },
'up-board',
),
).to.equal(false);
expect(
deviceConfig.bootConfigChangeRequired(
configFsBackend,
{ HOST_CONFIGFS_ssdt: 'spidev1.0' },
{ HOST_CONFIGFS_ssdt: 'spidev1.0' },
'up-board',
),
).to.equal(false);
expect(
deviceConfig.bootConfigChangeRequired(
configFsBackend,
{ HOST_CONFIGFS_ssdt: 'spidev1.0' },
{ HOST_CONFIGFS_ssdt: '"spidev1.0"' },
'up-board',
),
).to.equal(false);
expect(
deviceConfig.bootConfigChangeRequired(
configFsBackend,
{ HOST_CONFIGFS_ssdt: '"spidev1.0"' },
{ HOST_CONFIGFS_ssdt: 'spidev1.0' },
'up-board',
),
).to.equal(false);
});
});
}); });