ExternalVendorCode/balena-supervisor
1
0
Fork 0
mirror of https://github.com/balena-os/balena-supervisor.git synced 2024-12-19 05:37:53 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Remove imageId from container name

Browse Source 
The image id is no longer necessary to report the current state since
moving to v3 of the state endpoint and it is only kept for backwards
compatibility for some supervisor API endpoings.

Until now, `imageId` was part of the container name leading to longer
names than desired. This change removes the value from the container
name an modifies queries to look for the value in the image database.

This also removes the imageId from the log stream which should result in
reduced bandwidth usage.

Change-type: minor
This commit is contained in:
Felipe Lalanne 2023-03-07 18:15:41 -03:00
parent 3385bfa180
commit f37ee1b890
9 changed files with 72 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/compose/application-manager.ts
View File

@ -351,6 +351,7 @@ export async function getCurrentApps(): Promise<InstancedAppState> {
s.imageName = imageForService?.name ?? s.imageName;
s.releaseId = imageForService?.releaseId ?? s.releaseId;
s.imageId = imageForService?.imageId ?? s.imageId;
return s;
});
@ -697,8 +698,8 @@ function saveAndRemoveImages(
(image) =>
!_.some(currentImages.concat(targetImages), (imageInUse) => {
return _.isEqual(
_.omit(image, ['releaseId']),
_.omit(imageInUse, ['dockerImageId', 'id', 'releaseId']),
_.omit(image, ['releaseId', 'imageId']),
_.omit(imageInUse, ['dockerImageId', 'id', 'releaseId', 'imageId']),
);
}),
);
@ -833,8 +834,16 @@ export async function getLegacyState() {
// We iterate over the current running services and add them to the current state
// of the app they belong to.
for (const service of services) {
const { appId, imageId } = service;
if (!appId) {
const { appId } = service;
// We get the image for the service so we can get service metadata not
// in the containers
const imageForService = images.find(
(img) =>
img.serviceName === service.serviceName &&
img.commit === service.commit,
);
if (!appId || !imageForService) {
continue;
}
if (apps[appId] == null) {
@ -845,12 +854,10 @@ export async function getLegacyState() {
apps[appId].services = {};
}
// Get releaseId from the list of images if it can be found
service.releaseId = images.find(
(img) =>
img.serviceName === service.serviceName &&
img.commit === service.commit,
)?.releaseId;
const { releaseId: rId, imageId } = imageForService;
// Replace the service releaseId with the one from the image table above
service.releaseId = rId;
// We only send commit if all services have the same release, and it matches the target release
if (releaseId == null) {
@ -858,11 +865,7 @@ export async function getLegacyState() {
} else if (service.releaseId != null && releaseId !== service.releaseId) {
releaseId = false;
}
if (imageId == null) {
throw new InternalInconsistencyError(
`imageId not defined in ApplicationManager.getLegacyApplicationsState: ${service}`,
);
}
if (apps[appId].services[imageId] == null) {
apps[appId].services[imageId] = _.pick(service, ['status', 'releaseId']);
creationTimesAndReleases[appId][imageId] = _.pick(service, [
@ -881,20 +884,16 @@ export async function getLegacyState() {
}
for (const image of images) {
const { appId } = image;
const { appId, imageId } = image;
if (apps[appId] == null) {
apps[appId] = {};
}
if (apps[appId].services == null) {
apps[appId].services = {};
}
if (apps[appId].services[image.imageId] == null) {
apps[appId].services[image.imageId] = _.pick(image, [
'status',
'releaseId',
]);
apps[appId].services[image.imageId].download_progress =
image.downloadProgress;
if (apps[appId].services[imageId] == null) {
apps[appId].services[imageId] = _.pick(image, ['status', 'releaseId']);
apps[appId].services[imageId].download_progress = image.downloadProgress;
}
}

2
src/compose/images.ts
View File

@ -179,6 +179,8 @@ export function imageFromService(service: ServiceInfo): Image {
appUuid: service.appUuid!,
serviceId: service.serviceId!,
serviceName: service.serviceName!,
// Image id is optional in service, but it is always available
// on the target state
imageId: service.imageId!,
// Release id is optional in service, but it is always available
// on the target state

42
src/compose/service-manager.ts
View File

@ -123,7 +123,6 @@ export async function getState() {
status[service.containerId] = _.pick(service, [
'appId',
'appUuid',
'imageId',
'status',
'commit',
'createdAt',
@ -158,7 +157,7 @@ export async function updateMetadata(service: Service, target: Service) {
try {
await docker.getContainer(svc.containerId).rename({
name: `${service.serviceName}_${target.imageId}_${target.commit}`,
name: `${service.serviceName}_${target.commit}`,
});
} catch (e) {
if (isNotFoundError(e)) {
@ -351,14 +350,13 @@ export async function start(service: Service) {
}
const serviceId = service.serviceId;
const imageId = service.imageId;
if (serviceId == null || imageId == null) {
if (serviceId == null) {
throw new InternalInconsistencyError(
`serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.start`,
`serviceId not defined for service: ${service.serviceName} in ServiceManager.start`,
);
}
logger.attach(container.id, { serviceId, imageId });
logger.attach(container.id, { serviceId });
if (!alreadyStarted) {
logger.logSystemEvent(LogTypes.startServiceSuccess, { service });
@ -414,15 +412,13 @@ export function listenToEvents() {
});
const serviceId = service.serviceId;
const imageId = service.imageId;
if (serviceId == null || imageId == null) {
if (serviceId == null) {
throw new InternalInconsistencyError(
`serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.listenToEvents`,
`serviceId not defined for service: ${service.serviceName} in ServiceManager.listenToEvents`,
);
}
logger.attach(data.id, {
serviceId,
imageId,
});
} else if (status === 'destroy') {
await logMonitor.detach(data.id);
@ -466,10 +462,9 @@ export async function attachToRunning() {
for (const service of services) {
if (service.status === 'Running') {
const serviceId = service.serviceId;
const imageId = service.imageId;
if (serviceId == null || imageId == null) {
if (serviceId == null) {
throw new InternalInconsistencyError(
`serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.start`,
`serviceId not defined for service: ${service.serviceName} in ServiceManager.start`,
);
}
@ -480,7 +475,6 @@ export async function attachToRunning() {
}
logger.attach(service.containerId, {
serviceId,
imageId,
});
}
}
@ -521,15 +515,7 @@ function reportNewStatus(
containerId,
_.merge(
{ status },
_.pick(service, [
'imageId',
'appId',
'appUuid',
'serviceName',
'releaseId',
'createdAt',
'commit',
]),
_.pick(service, ['appUuid', 'serviceName', 'createdAt', 'commit']),
),
);
}
@ -545,9 +531,7 @@ async function killContainer(
// TODO: Remove the need for the wait flag
logger.logSystemEvent(LogTypes.stopService, { service });
if (service.imageId != null) {
reportNewStatus(containerId, service, 'Stopping');
}
reportNewStatus(containerId, service, 'Stopping');
const containerObj = docker.getContainer(containerId);
const killPromise = containerObj
@ -593,9 +577,7 @@ async function killContainer(
});
})
.finally(() => {
if (service.imageId != null) {
reportChange(containerId);
}
reportChange(containerId);
});
if (wait) {
@ -632,7 +614,7 @@ async function prepareForHandover(service: Service) {
const container = docker.getContainer(svc.containerId);
await container.update({ RestartPolicy: {} });
return await container.rename({
name: `old_${service.serviceName}_${service.imageId}_${service.commit}`,
name: `old_${service.serviceName}_${service.commit}`,
});
}

18
src/compose/service.ts
View File

@ -52,7 +52,12 @@ export type ServiceStatus =
export class Service {
public appId: number;
public appUuid?: string;
public imageId: number;
/**
* We make this available as it still needed by application manager to
* compare images, but it is only available in the target state.
* @deprecated
*/
public imageId?: number;
public config: ServiceConfig;
public serviceName: string;
public commit: string;
@ -622,13 +627,16 @@ export class Service {
'Attempt to build Service class from container with malformed labels',
);
}
const nameMatch = container.Name.match(/.*_(\d+)(?:_(\d+))?(?:_(.*?))?$/);
const nameMatch = container.Name.match(
/.*?(?:_(\d+))?(?:_(\d+))?(?:_([a-f0-9]*))?$/,
);
if (nameMatch == null) {
throw new InternalInconsistencyError(
`Expected supervised container to have name '<serviceName>_<imageId>_<commit>', got: ${container.Name}`,
`Expected supervised container to have name '<serviceName>_<commit>', got: ${container.Name}`,
);
}
// If we have not renamed the service yet we can still use the image id
svc.imageId = parseInt(nameMatch[1], 10);
svc.releaseId = parseInt(nameMatch[2], 10);
svc.commit = nameMatch[3];
@ -679,7 +687,7 @@ export class Service {
this.config.networkMode = `container:${containerId}`;
}
return {
name: `${this.serviceName}_${this.imageId}_${this.commit}`,
name: `${this.serviceName}_${this.commit}`,
Tty: this.config.tty,
Cmd: this.config.command,
Volumes: volumes,
@ -1139,7 +1147,7 @@ export class Service {
log.warn(
`Ignoring invalid compose volume definition ${
isString ? volume : JSON.stringify(volume)
}`,
} `,
);
}
}

4
src/device-api/actions.ts
View File

@ -274,9 +274,7 @@ export const executeServiceAction = async ({
);
}
const targetService = targetApp.services.find(
(s) =>
s.imageId === currentService.imageId ||
s.serviceName === currentService.serviceName,
(s) => s.serviceName === currentService.serviceName,
);
if (targetService == null) {
throw new NotFoundError(messages.targetServiceNotFound);

13
src/device-api/v2.ts
View File

@ -424,25 +424,32 @@ router.get('/v2/containerId', async (req: AuthorizedRequest, res) => {
router.get('/v2/state/status', async (req: AuthorizedRequest, res) => {
const appIds: number[] = [];
const pending = deviceState.isApplyInProgress();
const allImages = await images.getState();
const containerStates = (await serviceManager.getAll())
.filter((service) => req.auth.isScoped({ apps: [service.appId] }))
.map((svc) => {
appIds.push(svc.appId);
return _.pick(
const s = _.pick(
svc,
'status',
'serviceName',
'appId',
'imageId',
'serviceId',
'imageId',
'containerId',
'createdAt',
);
s.imageId = allImages.find(
(img) => img.commit === svc.commit && img.serviceName === s.serviceName,
)?.imageId;
return s;
});
let downloadProgressTotal = 0;
let downloads = 0;
const imagesStates = (await images.getState())
const imagesStates = allImages
.filter((img) => req.auth.isScoped({ apps: [img.appId] }))
.map((img) => {
appIds.push(img.appId);

5
src/logger.ts
View File

@ -137,10 +137,10 @@ export function lock(containerId: string): Bluebird.Disposer<() => void> {
});
}
type ServiceInfo = { serviceId: number; imageId: number };
type ServiceInfo = { serviceId: number };
export function attach(
containerId: string,
{ serviceId, imageId }: ServiceInfo,
{ serviceId }: ServiceInfo,
): Bluebird<void> {
// First detect if we already have an attached log stream
// for this container
@ -153,7 +153,6 @@ export function attach(
containerId,
(message: Parameters<MonitorHook>[0] & Partial<ServiceInfo>) => {
message.serviceId = serviceId;
message.imageId = imageId;
log(message);
},
);

2
test/integration/device-api/actions.spec.ts
View File

@ -128,7 +128,7 @@ describe('manages application lifecycle', () => {
return {
[appId]: {
name: 'localapp',
commit: 'localcommit',
commit: '10ca12e1ea5e',
releaseId: '1',
services,
volumes: {

18
test/integration/state-engine.spec.ts
View File

@ -134,8 +134,8 @@ describe('state engine', () => {
expect(
containers.map(({ Names, State }) => ({ Name: Names[0], State })),
).to.have.deep.members([
{ Name: '/one_11_deadbeef', State: 'running' },
{ Name: '/two_12_deadbeef', State: 'running' },
{ Name: '/one_deadbeef', State: 'running' },
{ Name: '/two_deadbeef', State: 'running' },
]);
// Test that the service is running and accesssible via port 8080
@ -181,7 +181,7 @@ describe('state engine', () => {
const containers = await docker.listContainers();
expect(
containers.map(({ Names, State }) => ({ Name: Names[0], State })),
).to.have.deep.members([{ Name: '/one_21_deadca1f', State: 'running' }]);
).to.have.deep.members([{ Name: '/one_deadca1f', State: 'running' }]);
const containerIds = containers.map(({ Id }) => Id);
await setTargetState({
@ -237,8 +237,8 @@ describe('state engine', () => {
expect(
updatedContainers.map(({ Names, State }) => ({ Name: Names[0], State })),
).to.have.deep.members([
{ Name: '/one_21_deadca1f', State: 'running' },
{ Name: '/two_22_deadca1f', State: 'running' },
{ Name: '/one_deadca1f', State: 'running' },
{ Name: '/two_deadca1f', State: 'running' },
]);
// Container ids must have changed
@ -301,8 +301,8 @@ describe('state engine', () => {
expect(
containers.map(({ Names, State }) => ({ Name: Names[0], State })),
).to.have.deep.members([
{ Name: '/one_11_deadbeef', State: 'running' },
{ Name: '/two_12_deadbeef', State: 'running' },
{ Name: '/one_deadbeef', State: 'running' },
{ Name: '/two_deadbeef', State: 'running' },
]);
const containerIds = containers.map(({ Id }) => Id);
@ -359,8 +359,8 @@ describe('state engine', () => {
expect(
updatedContainers.map(({ Names, State }) => ({ Name: Names[0], State })),
).to.have.deep.members([
{ Name: '/one_21_deadca1f', State: 'running' },
{ Name: '/two_22_deadca1f', State: 'running' },
{ Name: '/one_deadca1f', State: 'running' },
{ Name: '/two_deadca1f', State: 'running' },
]);
// Container ids must have changed