From 9846fa64c7b50301db3d62acb31b26be1f77ace9 Mon Sep 17 00:00:00 2001 From: Felipe Lalanne Date: Thu, 11 Nov 2021 17:25:51 -0300 Subject: [PATCH 1/2] Update tsconfig.json to use es2019 --- tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tsconfig.json b/tsconfig.json index 5759b187..1ec3cc9e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,7 +9,7 @@ "inlineSourceMap": true, "outDir": "./build/", "skipLibCheck": true, - "lib": ["es6"], + "lib": ["es2019"], "resolveJsonModule": true, "allowJs": true }, From 394377e0a183d5a26ae6527f5aafb1f29e7fa140 Mon Sep 17 00:00:00 2001 From: Felipe Lalanne Date: Thu, 11 Nov 2021 16:28:02 -0300 Subject: [PATCH 2/2] 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 --- src/compose/application-manager.ts | 57 +++++----- test/src/compose/application-manager.spec.ts | 104 +++++++++++++++++++ 2 files changed, 134 insertions(+), 27 deletions(-) diff --git a/src/compose/application-manager.ts b/src/compose/application-manager.ts index c6de2885..c2107e35 100644 --- a/src/compose/application-manager.ts +++ b/src/compose/application-manager.ts @@ -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)) diff --git a/test/src/compose/application-manager.spec.ts b/test/src/compose/application-manager.spec.ts index 6968a596..99166103 100644 --- a/test/src/compose/application-manager.spec.ts +++ b/test/src/compose/application-manager.spec.ts @@ -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( {