Skip restarting services if they are part of conf targets

Some recent changes to the OS allowed some services to restart
automatically when the associated config files are changed.

In these cases we want to avoid restarting the same services
manually from the supervisor.

Change-type: patch
Signed-off-by: Kyle Harding <kyle@balena.io>
This commit is contained in:
Kyle Harding 2021-08-20 10:10:24 -04:00
parent 7e2ce7fc56
commit 669866b4c2
No known key found for this signature in database
GPG Key ID: 2AD73EC1FB4865E3
3 changed files with 180 additions and 16 deletions

View File

@ -152,8 +152,28 @@ async function setProxy(maybeConf: ProxyConfig | null): Promise<void> {
await writeFileAtomic(redsocksConfPath, redsocksConf);
}
await dbus.restartService('resin-proxy-config');
await dbus.restartService('redsocks');
// restart balena-proxy-config if it is loaded and NOT PartOf redsocks-conf.target
if (
(
await Bluebird.any([
dbus.servicePartOf('balena-proxy-config'),
dbus.servicePartOf('resin-proxy-config'),
])
).includes('redsocks-conf.target') === false
) {
await Bluebird.any([
dbus.restartService('balena-proxy-config'),
dbus.restartService('resin-proxy-config'),
]);
}
// restart redsocks if it is loaded and NOT PartOf redsocks-conf.target
if (
(await dbus.servicePartOf('redsocks')).includes('redsocks-conf.target') ===
false
) {
await dbus.restartService('redsocks');
}
}
const hostnamePath = path.join(constants.rootMountPoint, '/etc/hostname');
@ -164,7 +184,21 @@ async function readHostname() {
async function setHostname(val: string) {
await config.set({ hostname: val });
await dbus.restartService('resin-hostname');
// restart balena-hostname if it is loaded and NOT PartOf config-json.target
if (
(
await Bluebird.any([
dbus.servicePartOf('balena-hostname'),
dbus.servicePartOf('resin-hostname'),
])
).includes('config-json.target') === false
) {
await Bluebird.any([
dbus.restartService('balena-hostname'),
dbus.restartService('resin-hostname'),
]);
}
}
// Don't use async/await here to maintain the bluebird

View File

@ -37,6 +37,7 @@ export async function getLoginManagerInterface() {
async function startUnit(unitName: string) {
const systemd = await getSystemdInterface();
log.debug(`Starting systemd unit: ${unitName}`);
try {
systemd.StartUnit(unitName, 'fail');
} catch (e) {
@ -46,6 +47,7 @@ async function startUnit(unitName: string) {
export async function restartService(serviceName: string) {
const systemd = await getSystemdInterface();
log.debug(`Restarting systemd service: ${serviceName}`);
try {
systemd.RestartUnit(`${serviceName}.service`, 'fail');
} catch (e) {
@ -63,6 +65,7 @@ export async function startSocket(socketName: string) {
async function stopUnit(unitName: string) {
const systemd = await getSystemdInterface();
log.debug(`Stopping systemd unit: ${unitName}`);
try {
systemd.StopUnit(unitName, 'fail');
} catch (e) {
@ -116,7 +119,10 @@ export const shutdown = async () =>
}
}, 1000);
async function getUnitProperty(unitName: string, property: string) {
async function getUnitProperty(
unitName: string,
property: string,
): Promise<string> {
const systemd = await getSystemdInterface();
return new Promise((resolve, reject) => {
systemd.GetUnit(unitName, async (err: Error, unitPath: string) => {
@ -132,7 +138,7 @@ async function getUnitProperty(unitName: string, property: string) {
iface.Get(
'org.freedesktop.systemd1.Unit',
property,
(e: Error, value: unknown) => {
(e: Error, value: string) => {
if (e) {
return reject(new DbusError(e));
}
@ -146,3 +152,7 @@ async function getUnitProperty(unitName: string, property: string) {
export function serviceActiveState(serviceName: string) {
return getUnitProperty(`${serviceName}.service`, 'ActiveState');
}
export function servicePartOf(serviceName: string) {
return getUnitProperty(`${serviceName}.service`, 'PartOf');
}

View File

@ -24,7 +24,7 @@ import SupervisorAPI from '../src/supervisor-api';
import * as apiBinder from '../src/api-binder';
import * as deviceState from '../src/device-state';
import * as apiKeys from '../src/lib/api-keys';
import * as dbus from '../src//lib/dbus';
import * as dbus from '../src/lib/dbus';
import * as updateLock from '../src/lib/update-lock';
import * as TargetState from '../src/device-state/target-state';
import * as targetStateCache from '../src/device-state/target-state-cache';
@ -901,6 +901,15 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
});
it('updates the hostname with provided string if string is not empty', async () => {
// stub servicePartOf to throw exceptions for the new service names
stub(dbus, 'servicePartOf').callsFake(
async (serviceName: string): Promise<string> => {
if (serviceName === 'balena-hostname') {
throw new Error('Unit not loaded.');
}
return '';
},
);
await unlinkAll(redsocksPath, noProxyPath);
const patchBody = { network: { hostname: 'newdevice' } };
@ -915,9 +924,12 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
validatePatchResponse(response);
});
// Should restart hostname service on successful change
expect(restartServiceSpy.callCount).to.equal(1);
expect(restartServiceSpy).to.have.been.calledWith('resin-hostname');
// should restart services
expect(restartServiceSpy.callCount).to.equal(2);
expect(restartServiceSpy.args).to.deep.equal([
['balena-hostname'],
['resin-hostname'],
]);
await request
.get('/v1/device/host-config')
@ -926,6 +938,44 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
.then((response) => {
expect(response.body).to.deep.equal(patchBody);
});
(dbus.servicePartOf as SinonStub).restore();
});
it('skips restarting hostname services if they are part of config-json.target', async () => {
// stub servicePartOf to return the config-json.target we are looking for
stub(dbus, 'servicePartOf').callsFake(
async (): Promise<string> => {
return 'config-json.target';
},
);
await unlinkAll(redsocksPath, noProxyPath);
const patchBody = { network: { hostname: 'newdevice' } };
await request
.patch('/v1/device/host-config')
.send(patchBody)
.set('Accept', 'application/json')
.set('Authorization', `Bearer ${apiKeys.cloudApiKey}`)
.expect(sampleResponses.V1.PATCH['/host/device-config'].statusCode)
.then((response) => {
validatePatchResponse(response);
});
// skips restarting hostname services if they are part of config-json.target
expect(restartServiceSpy.callCount).to.equal(0);
await request
.get('/v1/device/host-config')
.set('Accept', 'application/json')
.set('Authorization', `Bearer ${apiKeys.cloudApiKey}`)
.then((response) => {
expect(response.body).to.deep.equal(patchBody);
});
(dbus.servicePartOf as SinonStub).restore();
});
it('updates hostname to first 7 digits of device uuid when sent invalid hostname', async () => {
@ -940,9 +990,12 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
validatePatchResponse(response);
});
// Should restart hostname service on successful change
expect(restartServiceSpy.callCount).to.equal(1);
expect(restartServiceSpy).to.have.been.calledWith('resin-hostname');
// should restart services
expect(restartServiceSpy.callCount).to.equal(2);
expect(restartServiceSpy.args).to.deep.equal([
['balena-hostname'],
['resin-hostname'],
]);
await request
.get('/v1/device/host-config')
@ -973,8 +1026,10 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
expect(await exists(noProxyPath)).to.be.false;
});
expect(restartServiceSpy.callCount).to.equal(2);
// should restart services
expect(restartServiceSpy.callCount).to.equal(3);
expect(restartServiceSpy.args).to.deep.equal([
['balena-proxy-config'],
['resin-proxy-config'],
['redsocks'],
]);
@ -990,6 +1045,15 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
});
it('updates proxy type when provided valid values', async () => {
// stub servicePartOf to throw exceptions for the new service names
stub(dbus, 'servicePartOf').callsFake(
async (serviceName: string): Promise<string> => {
if (serviceName === 'balena-proxy-config') {
throw new Error('Unit not loaded.');
}
return '';
},
);
// Test each proxy patch sequentially to prevent conflicts when writing to fs
let restartCallCount = 0;
for (const key of Object.keys(validProxyReqs)) {
@ -1008,8 +1072,9 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
validatePatchResponse(response);
});
// should restart services
expect(restartServiceSpy.callCount).to.equal(
++restartCallCount * 2,
++restartCallCount * 3,
);
await request
@ -1032,6 +1097,57 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
} // end for (const value of patchBodyValuesforKey)
await restoreConfFileTemplates();
} // end for (const key in validProxyReqs)
(dbus.servicePartOf as SinonStub).restore();
});
it('skips restarting proxy services when part of redsocks-conf.target', async () => {
// stub servicePartOf to return the redsocks-conf.target we are looking for
stub(dbus, 'servicePartOf').callsFake(
async (): Promise<string> => {
return 'redsocks-conf.target';
},
);
// Test each proxy patch sequentially to prevent conflicts when writing to fs
for (const key of Object.keys(validProxyReqs)) {
const patchBodyValuesforKey: string[] | number[] =
validProxyReqs[key];
for (const value of patchBodyValuesforKey) {
await request
.patch('/v1/device/host-config')
.send({ network: { proxy: { [key]: value } } })
.set('Accept', 'application/json')
.set('Authorization', `Bearer ${apiKeys.cloudApiKey}`)
.expect(
sampleResponses.V1.PATCH['/host/device-config'].statusCode,
)
.then((response) => {
validatePatchResponse(response);
});
// skips restarting proxy services when part of redsocks-conf.target
expect(restartServiceSpy.callCount).to.equal(0);
await request
.get('/v1/device/host-config')
.set('Accept', 'application/json')
.set('Authorization', `Bearer ${apiKeys.cloudApiKey}`)
.expect(hostnameProxyRes.statusCode)
.then((response) => {
expect(response.body).to.deep.equal({
network: {
hostname: hostnameProxyRes.body.network.hostname,
// All other proxy configs should be unchanged except for any values sent in patch
proxy: {
...hostnameProxyRes.body.network.proxy,
[key]: value,
},
},
});
});
} // end for (const value of patchBodyValuesforKey)
await restoreConfFileTemplates();
} // end for (const key in validProxyReqs)
(dbus.servicePartOf as SinonStub).restore();
});
it('warns on the supervisor console when provided disallowed proxy fields', async () => {
@ -1081,8 +1197,10 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
validatePatchResponse(response);
});
expect(restartServiceSpy.callCount).to.equal(2);
// should restart services
expect(restartServiceSpy.callCount).to.equal(3);
expect(restartServiceSpy.args).to.deep.equal([
['balena-proxy-config'],
['resin-proxy-config'],
['redsocks'],
]);
@ -1117,8 +1235,10 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
validatePatchResponse(response);
});
expect(restartServiceSpy.callCount).to.equal(2);
// should restart services
expect(restartServiceSpy.callCount).to.equal(3);
expect(restartServiceSpy.args).to.deep.equal([
['balena-proxy-config'],
['resin-proxy-config'],
['redsocks'],
]);