Fix delete-then-download strategy

The strategy has been broken for a while but it was not clear how to
fix it before the changes to image management. This PR fixes application
manager to remove images before downloading the new image. This will
only have an effect on changing images.

Closes: #1233
Change-type: patch
This commit is contained in:
Felipe Lalanne 2021-11-11 16:28:02 -03:00
parent 9846fa64c7
commit 394377e0a1
2 changed files with 134 additions and 27 deletions

View File

@ -680,9 +680,9 @@ function saveAndRemoveImages(
}) ?? _.find(availableImages, { dockerImageId: svc.config.image }),
),
) as imageManager.Image[];
const targetImages = _.flatMap(target, (app) =>
_.map(app.services, imageForService),
);
const targetServices = Object.values(target).flatMap((app) => app.services);
const targetImages = targetServices.map(imageForService);
const availableAndUnused = _.filter(
availableWithoutIds,
@ -741,32 +741,35 @@ function saveAndRemoveImages(
});
}
const deltaSources = _.map(imagesToDownload, (image) => {
return bestDeltaSource(image, availableImages);
});
// Find images that will be be used as delta sources. Any existing image for the
// same app service is considered a delta source unless the target service has set
// the `delete-then-download` strategy
const deltaSources = imagesToDownload
.filter(
(img) =>
// We don't need to look for delta sources for delete-then-download
// services
!targetServices.some(
(svc) =>
imageManager.isSameImage(img, imageForService(svc)) &&
svc.config.labels['io.balena.update.strategy'] ===
'delete-then-download',
),
)
.map((img) => bestDeltaSource(img, availableImages))
.filter((img) => img != null);
const proxyvisorImages = proxyvisor.imagesInUse(current, target);
const potentialDeleteThenDownload = _(current)
.flatMap((app) => _.values(app.services))
.filter(
(svc) =>
svc.config.labels['io.balena.update.strategy'] ===
'delete-then-download' && svc.status === 'Stopped',
)
.value();
const imagesToRemove = _.filter(
availableAndUnused.concat(potentialDeleteThenDownload.map(imageForService)),
(image) => {
const notUsedForDelta = !_.includes(deltaSources, image.name);
const notUsedByProxyvisor = !_.some(proxyvisorImages, (proxyvisorImage) =>
imageManager.isSameImage(image, {
name: proxyvisorImage,
}),
);
return notUsedForDelta && notUsedByProxyvisor;
},
);
const imagesToRemove = availableAndUnused.filter((image) => {
const notUsedForDelta = !deltaSources.includes(image.name);
const notUsedByProxyvisor = !proxyvisorImages.some((proxyvisorImage) =>
imageManager.isSameImage(image, {
name: proxyvisorImage,
}),
);
return notUsedForDelta && notUsedByProxyvisor;
});
return imagesToSave
.map((image) => ({ action: 'saveImage', image } as CompositionStep))

View File

@ -389,6 +389,110 @@ describe('compose/application-manager', () => {
.that.deep.includes({ serviceName: 'main' });
});
it('infers a kill step when a service has to be updated but the strategy is delete-then-download', async () => {
const labels = {
'io.balena.update.strategy': 'delete-then-download',
};
const targetApps = createApps(
{
services: [
await createService({
image: 'image-new',
labels,
appId: 1,
commit: 'new-release',
}),
],
networks: [DEFAULT_NETWORK],
},
true,
);
const {
currentApps,
availableImages,
downloading,
containerIdsByAppId,
} = createCurrentState({
services: [
await createService({
image: 'image-old',
labels,
appId: 1,
commit: 'old-release',
}),
],
networks: [DEFAULT_NETWORK],
});
const [killStep] = await applicationManager.inferNextSteps(
currentApps,
targetApps,
{
downloading,
availableImages,
containerIdsByAppId,
},
);
expect(killStep).to.have.property('action').that.equals('kill');
expect(killStep)
.to.have.property('current')
.that.deep.includes({ serviceName: 'main' });
});
it('infers a remove step when the current service has stopped and the strategy is delete-then-download', async () => {
const labels = {
'io.balena.update.strategy': 'delete-then-download',
};
const targetApps = createApps(
{
services: [
await createService({
image: 'image-new',
labels,
appId: 1,
serviceName: 'main',
commit: 'new-release',
}),
],
},
true,
);
const {
currentApps,
availableImages,
downloading,
containerIdsByAppId,
} = createCurrentState({
services: [],
images: [
createImage({
appId: 1,
name: 'image-old',
serviceName: 'main',
dockerImageId: 'image-old-id',
}),
],
networks: [DEFAULT_NETWORK],
});
const [removeImage] = await applicationManager.inferNextSteps(
currentApps,
targetApps,
{
downloading,
availableImages,
containerIdsByAppId,
},
);
// First we should see a kill
expect(removeImage).to.have.property('action').that.equals('removeImage');
expect(removeImage)
.to.have.property('image')
.that.deep.includes({ name: 'image-old' });
});
it('does not infer to kill a service with default strategy if a dependency is not downloaded', async () => {
const targetApps = createApps(
{