From 27f0d2e655a5c11c89103847c485fa06babc1954 Mon Sep 17 00:00:00 2001
From: Felipe Lalanne <1822826+pipex@users.noreply.github.com>
Date: Fri, 7 Apr 2023 16:05:13 -0400
Subject: [PATCH] Improve net alias comparison to prevent unwanted restarts

Network aliases are now compared checking that the target state is a
subset of the current state. This will prevent service restarts due to
additional aliases created by docker in the container.

Closes: #2134
Change-type: patch
---
 src/compose/service.ts            | 17 +++----
 test/unit/compose/service.spec.ts | 74 +++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/src/compose/service.ts b/src/compose/service.ts
index f24acb6c..58b343e8 100644
--- a/src/compose/service.ts
+++ b/src/compose/service.ts
@@ -1074,20 +1074,17 @@ export class Service {
 			if (current.aliases == null) {
 				sameNetwork = false;
 			} else {
-				// Take out the container id from both aliases, as it *will* be present
-				// in a currently running container, and can also be present in the target
-				// for example when doing a start-service
-				// Also sort the aliases, so we can do a simple comparison
 				const [currentAliases, targetAliases] = [
 					current.aliases,
 					target.aliases,
-				].map((aliases) =>
-					_.sortBy(
-						aliases.filter((a) => !_.startsWith(this.containerId || '', a)),
-					),
-				);
+				];
 
-				sameNetwork = _.isEqual(currentAliases, targetAliases);
+				// Docker may add keep old container ids as aliases for a specific service after
+				// restarts, this means that the target aliases really needs to be a subset of the
+				// current aliases to prevent service restarts when re-applying the same target state
+				sameNetwork =
+					_.intersection(currentAliases, targetAliases).length ===
+					targetAliases.length;
 			}
 		}
 		if (target.ipv4Address != null) {
diff --git a/test/unit/compose/service.spec.ts b/test/unit/compose/service.spec.ts
index 2d766d14..16392fb8 100644
--- a/test/unit/compose/service.spec.ts
+++ b/test/unit/compose/service.spec.ts
@@ -574,6 +574,80 @@ describe('compose/service: unit tests', () => {
 				expect(svc1.isEqualConfig(svc2, {})).to.be.true;
 			});
 		});
+
+		it('should accept that target network aliases are a subset of current network aliases', async () => {
+			const svc1 = await Service.fromComposeObject(
+				{
+					appId: 1,
+					serviceId: 1,
+					serviceName: 'test',
+					composition: {
+						networks: {
+							test: {
+								aliases: ['hello', 'world'],
+							},
+						},
+					},
+				},
+				{ appName: 'test' } as any,
+			);
+			const svc2 = await Service.fromComposeObject(
+				{
+					appId: 1,
+					serviceId: 1,
+					serviceName: 'test',
+					composition: {
+						networks: {
+							test: {
+								aliases: ['hello', 'sweet', 'world'],
+							},
+						},
+					},
+				},
+				{ appName: 'test' } as any,
+			);
+
+			// All aliases in target service (svc1) are contained in service 2
+			expect(svc2.isEqualConfig(svc1, {})).to.be.true;
+			// But the opposite is not true
+			expect(svc1.isEqualConfig(svc2, {})).to.be.false;
+		});
+
+		it('should accept equal lists of network aliases', async () => {
+			const svc1 = await Service.fromComposeObject(
+				{
+					appId: 1,
+					serviceId: 1,
+					serviceName: 'test',
+					composition: {
+						networks: {
+							test: {
+								aliases: ['hello', 'world'],
+							},
+						},
+					},
+				},
+				{ appName: 'test' } as any,
+			);
+			const svc2 = await Service.fromComposeObject(
+				{
+					appId: 1,
+					serviceId: 1,
+					serviceName: 'test',
+					composition: {
+						networks: {
+							test: {
+								aliases: ['hello', 'world'],
+							},
+						},
+					},
+				},
+				{ appName: 'test' } as any,
+			);
+
+			expect(svc1.isEqualConfig(svc2, {})).to.be.true;
+			expect(svc2.isEqualConfig(svc1, {})).to.be.true;
+		});
 	});
 
 	describe('Feature labels', () => {