Wait for images to be ready before moving between releases

For download-then-kill strategy, this waits for all changing images on the target
release to be available on device before killing the old services. This
will prevent that multicontainer applications get to a state where some
services of the new release start runnning much before others have been
downloaded.

When adding new services to a multicontainer app, the supervisor will
now wait for other changing services to be downloaded before starting
the new service.

Closes: #1812
Change-type: patch
This commit is contained in:
Felipe Lalanne 2021-11-10 20:22:40 +00:00
parent 43e26e5984
commit 7aedc97ee1
2 changed files with 172 additions and 27 deletions

View File

@ -488,7 +488,9 @@ export class App {
// Either this is a new service, or the current one has already been killed
return this.generateFetchOrStartStep(
target!,
context.targetApp,
needsDownload,
context.availableImages,
context.networkPairs,
context.volumePairs,
context.servicePairs,
@ -520,12 +522,13 @@ export class App {
const dependenciesMetForStart = this.dependenciesMetForServiceStart(
target,
context.targetApp,
context.availableImages,
context.networkPairs,
context.volumePairs,
context.servicePairs,
);
const dependenciesMetForKill = this.dependenciesMetForServiceKill(
target,
context.targetApp,
context.availableImages,
context.localMode,
@ -591,7 +594,9 @@ export class App {
private generateFetchOrStartStep(
target: Service,
targetApp: App,
needsDownload: boolean,
availableImages: Image[],
networkPairs: Array<ChangingPair<Network>>,
volumePairs: Array<ChangingPair<Volume>>,
servicePairs: Array<ChangingPair<Service>>,
@ -605,6 +610,8 @@ export class App {
} else if (
this.dependenciesMetForServiceStart(
target,
targetApp,
availableImages,
networkPairs,
volumePairs,
servicePairs,
@ -618,6 +625,8 @@ export class App {
// TODO: support networks instead of only network mode
private dependenciesMetForServiceStart(
target: Service,
targetApp: App,
availableImages: Image[],
networkPairs: Array<ChangingPair<Network>>,
volumePairs: Array<ChangingPair<Volume>>,
servicePairs: Array<ChangingPair<Service>>,
@ -626,7 +635,7 @@ export class App {
// different to a dependency which is in the servicePairs below, as these
// are services which are changing). We could have a dependency which is
// starting up, but is not yet running.
const depInstallingButNotRunning = _.some(this.services, (svc) => {
const depInstallingButNotRunning = _.some(targetApp.services, (svc) => {
if (target.dependsOn?.includes(svc.serviceName!)) {
if (!svc.config.running) {
return true;
@ -664,16 +673,15 @@ export class App {
return false;
}
// everything is ready for the service to start...
return true;
// do not start until all images have been downloaded
return this.targetImagesReady(targetApp, availableImages);
}
// Unless the update strategy requires an early kill (i.e kill-then-download,
// delete-then-download), we only want to kill a service once the images for the
// services it depends on have been downloaded, so as to minimize downtime (but not
// block the killing too much, potentially causing a daedlock)
// block the killing too much, potentially causing a deadlock)
private dependenciesMetForServiceKill(
target: Service,
targetApp: App,
availableImages: Image[],
localMode: boolean,
@ -685,26 +693,18 @@ export class App {
return true;
}
if (target.dependsOn != null) {
for (const dependency of target.dependsOn) {
const dependencyService = _.find(targetApp.services, {
serviceName: dependency,
});
if (
!_.some(
availableImages,
(image) =>
image.dockerImageId === dependencyService?.config.image ||
imageManager.isSameImage(image, {
name: dependencyService?.imageName!,
}),
)
) {
return false;
}
}
}
return true;
// Don't kill any services before all images have been downloaded
return this.targetImagesReady(targetApp, availableImages);
}
private targetImagesReady(targetApp: App, availableImages: Image[]) {
return targetApp.services.every((service) =>
availableImages.some(
(image) =>
image.dockerImageId === service.config.image ||
imageManager.isSameImage(image, { name: service.imageName! }),
),
);
}
public static async fromTargetState(

View File

@ -1147,7 +1147,7 @@ describe('compose/app', () => {
.that.deep.includes({ serviceName: 'test' });
});
it('should not create a service when a network it depends on is not ready', async () => {
it('should not start a service when a network it depends on is not ready', async () => {
const current = createApp({ networks: [defaultNetwork] });
const target = createApp({
services: [await createService({ networks: ['test'], appId: 1 })],
@ -1199,6 +1199,151 @@ describe('compose/app', () => {
const steps = current.nextStepsForAppUpdate(defaultContext, target);
expectSteps('kill', steps, 2);
});
it('should not infer a kill step with the default strategy before all target images have been downloaded', async () => {
const contextWithImages = {
...defaultContext,
...{
downloading: ['other-image-2'], // One of the images is being downloaded
availableImages: [
createImage({ appId: 1, name: 'main-image', serviceName: 'main' }),
createImage({
appId: 1,
name: 'other-image',
serviceName: 'other',
}),
createImage({
appId: 1,
name: 'main-image-2',
serviceName: 'main',
}),
],
},
};
const current = createApp({
services: [
await createService({
image: 'main-image',
appId: 1,
serviceName: 'main',
commit: 'old-release',
}),
await createService({
image: 'other-image',
appId: 1,
serviceName: 'other',
commit: 'old-release',
}),
],
networks: [defaultNetwork],
});
const target = createApp({
services: [
await createService({
image: 'main-image-2',
appId: 1,
serviceName: 'main',
commit: 'new-release',
}),
await createService({
image: 'other-image-2',
appId: 1,
serviceName: 'other',
commit: 'new-release',
}),
],
networks: [defaultNetwork],
isTarget: true,
});
// No kill steps should be generated
const steps = current.nextStepsForAppUpdate(contextWithImages, target);
expectNoStep('kill', steps);
});
it('should not infer a start step before all target images have been downloaded', async () => {
const contextWithImages = {
...defaultContext,
...{
downloading: ['other-image'], // One of the images is being downloaded
availableImages: [
createImage({ appId: 1, name: 'main-image', serviceName: 'main' }),
],
},
};
const current = createApp({
services: [],
networks: [defaultNetwork],
});
const target = createApp({
services: [
await createService({
image: 'main-image',
appId: 1,
serviceName: 'main',
commit: 'new-release',
}),
await createService({
image: 'other-image',
appId: 1,
serviceName: 'other',
commit: 'new-release',
}),
],
networks: [defaultNetwork],
isTarget: true,
});
// No kill steps should be generated
const steps = current.nextStepsForAppUpdate(contextWithImages, target);
expectNoStep('start', steps);
});
it('should infer a start step only when target images have been downloaded', async () => {
const contextWithImages = {
...defaultContext,
...{
downloading: [], // One of the images is being downloaded
availableImages: [
createImage({ appId: 1, name: 'main-image', serviceName: 'main' }),
createImage({
appId: 1,
name: 'other-image',
serviceName: 'other',
}),
],
},
};
const current = createApp({
services: [],
networks: [defaultNetwork],
});
const target = createApp({
services: [
await createService({
image: 'main-image',
appId: 1,
serviceName: 'main',
commit: 'new-release',
}),
await createService({
image: 'other-image',
appId: 1,
serviceName: 'other',
commit: 'new-release',
}),
],
networks: [defaultNetwork],
isTarget: true,
});
// No kill steps should be generated
const steps = current.nextStepsForAppUpdate(contextWithImages, target);
expectSteps('start', steps, 2);
});
});
describe('image state behavior', () => {