Allow target fetch before current kill if target service is new

Signed-off-by: Christina Ying Wang <[email protected]>

src/compose/app.ts

@@ -660,7 +660,10 @@ export class App {
 		volumePairs: Array<ChangingPair<Volume>>,
 		servicePairs: Array<ChangingPair<Service>>,
 	): CompositionStep | undefined {
-		if (needsDownload && this.dependenciesMetForServiceFetch(servicePairs)) {
+		if (
+			needsDownload &&
+			this.dependenciesMetForServiceFetch(target, servicePairs)
+		) {
 			// We know the service name exists as it always does for targets
 			return generateStep('fetch', {
 				image: imageManager.imageFromService(target),
@@ -681,19 +684,29 @@ export class App {
 	}
 
 	private dependenciesMetForServiceFetch(
+		target: Service,
 		servicePairs: Array<ChangingPair<Service>>,
 	) {
-		// Current services should be killed before target
-		// service fetch depending on update strategy
-		const servicePairsWithCurrent = servicePairs.filter(
+		const [servicePairsWithCurrent, servicePairsWithoutCurrent] = _.partition(
+			servicePairs,
 			(pair) => pair.current != null,
 		);
-		for (const { current, target } of servicePairsWithCurrent) {
+
+		// Target services not in current can be safely fetched
+		for (const pair of servicePairsWithoutCurrent) {
+			if (target.serviceName === pair.target!.serviceName) {
+				return true;
+			}
+		}
+
+		// Current services should be killed before target
+		// service fetch depending on update strategy
+		for (const pair of servicePairsWithCurrent) {
 			// Prefer target's update strategy if target service exists
-			const strategy = getStrategyFromService(target ?? current!);
+			const strategy = getStrategyFromService(pair.target ?? pair.current!);
 			if (
 				['kill-then-download', 'delete-then-download'].includes(strategy) &&
-				current!.config.running
+				pair.current!.config.running
 			) {
 				return false;
 			}
@@ -754,7 +767,7 @@ export class App {
 		}
 
 		// do not start until all images have been downloaded
-		return this.targetImagesReady(targetApp, availableImages);
+		return this.targetImagesReady(targetApp.services, availableImages);
 	}
 
 	// Unless the update strategy requires an early kill (i.e kill-then-download,
@@ -765,12 +778,14 @@ export class App {
 		targetApp: App,
 		availableImages: Image[],
 	) {
-		// Don't kill any services before all images have been downloaded
-		return this.targetImagesReady(targetApp, availableImages);
+		return this.targetImagesReady(targetApp.services, availableImages);
 	}
 
-	private targetImagesReady(targetApp: App, availableImages: Image[]) {
-		return targetApp.services.every((service) =>
+	private targetImagesReady(
+		targetServices: Service[],
+		availableImages: Image[],
+	) {
+		return targetServices.every((service) =>
 			availableImages.some(
 				(image) =>
 					image.dockerImageId === service.config.image ||

src/compose/update-strategies.ts

@@ -28,7 +28,7 @@ export function getStepsFromStrategy(
 				// We only kill when dependencies are already met, so that we minimize downtime
 				return generateStep('kill', { current: context.current });
 			} else {
-				return { action: 'noop' };
+				return generateStep('noop', {});
 			}
 		case 'kill-then-download':
 		case 'delete-then-download':
@@ -47,7 +47,7 @@ export function getStepsFromStrategy(
 					target: context.target,
 				});
 			} else {
-				return { action: 'noop' };
+				return generateStep('noop', {});
 			}
 		default:
 			throw new InternalInconsistencyError(