Only consider certain array fields without order

Various fields returned from the docker daemon don't retain order (for
example the volumes field). We now only select certain array values to
compare taking order into account.

Change-type: patch
Signed-off-by: Cameron Diver <cameron@balena.io>

src/compose/service.ts

@@ -46,8 +46,6 @@ export class Service {
 		'devices',
 		'capAdd',
 		'capDrop',
-		'dns',
-		'dnsSearch',
 		'dnsOpt',
 		'tmpfs',
 		'extraHosts',
@@ -56,6 +54,13 @@ export class Service {
 		'groupAdd',
 		'securityOpt',
 	];
+	public static orderedConfigArrayFields: ServiceConfigArrayField[] = [
+		'dns',
+		'dnsSearch',
+	];
+	public static allConfigArrayFields: ServiceConfigArrayField[] = Service.configArrayFields.concat(
+		Service.orderedConfigArrayFields,
+	);
 
 	// A list of fields to ignore when comparing container configuration
 	private static omitFields = [
@@ -69,7 +74,7 @@ export class Service {
 		// These fields are special case, due to network_mode:service:<service>
 		'networkMode',
 		'hostname',
-	].concat(Service.configArrayFields);
+	].concat(Service.allConfigArrayFields);
 
 	private constructor() {}
 
@@ -684,17 +689,23 @@ export class Service {
 		let sameConfig = _.isEqual(thisOmitted, otherOmitted);
 		const nonArrayEquals = sameConfig;
 
-		// Check for array fields which don't match
-		const differentArrayFields: string[] = [];
-		sameConfig =
-			sameConfig &&
-			_.every(Service.configArrayFields, (field: keyof ServiceConfig) => {
-				const eq = _.isEqual(this.config[field], service.config[field]);
-				if (!eq) {
-					differentArrayFields.push(field);
-				}
-				return eq;
-			});
+		// Because the service config does not have an index
+		// field, we must first convert to unknown. We know that
+		// this conversion is fine as the
+		// Service.configArrayFields and
+		// Service.orderedConfigArrayFields are defined as
+		// fields inside of Service.config
+		const arrayEq = ComposeUtils.compareArrayFields(
+			(this.config as unknown) as Dictionary<unknown>,
+			(service.config as unknown) as Dictionary<unknown>,
+			Service.configArrayFields,
+			Service.orderedConfigArrayFields,
+		);
+		sameConfig = sameConfig && arrayEq.equal;
+		let differentArrayFields: string[] = [];
+		if (!arrayEq.equal) {
+			differentArrayFields = arrayEq.difference;
+		}
 
 		if (!(sameConfig && sameNetworks)) {
 			// Add some console output for why a service is not matching

src/compose/utils.ts

@@ -499,3 +499,60 @@ export function normalizeLabels(labels: {
 	);
 	return _.assign({}, otherLabels, legacyLabels, balenaLabels);
 }
+
+function compareArrayField(
+	arr1: unknown[],
+	arr2: unknown[],
+	ordered: boolean,
+): boolean {
+	if (!ordered) {
+		arr1 = _.sortBy(arr1);
+		arr2 = _.sortBy(arr2);
+	}
+	return _.isEqual(arr1, arr2);
+}
+
+export function compareArrayFields<T extends Dictionary<unknown>>(
+	obj1: T,
+	obj2: T,
+	nonOrderedFields: Array<keyof T>,
+	orderedFields: Array<keyof T>,
+): { equal: false; difference: string[] };
+export function compareArrayFields<T extends Dictionary<unknown>>(
+	obj1: T,
+	obj2: T,
+	nonOrderedFields: Array<keyof T>,
+	orderedFields: Array<keyof T>,
+): { equal: true };
+export function compareArrayFields<T extends Dictionary<unknown>>(
+	obj1: T,
+	obj2: T,
+	nonOrderedFields: Array<keyof T>,
+	orderedFields: Array<keyof T>,
+): { equal: boolean; difference?: string[] } {
+	let equal = true;
+	const difference: string[] = [];
+	for (const { fields, ordered } of [
+		{ fields: nonOrderedFields, ordered: false },
+		{ fields: orderedFields, ordered: true },
+	]) {
+		for (const field of fields) {
+			if (
+				!compareArrayField(
+					obj1[field] as unknown[],
+					obj2[field] as unknown[],
+					ordered,
+				)
+			) {
+				equal = false;
+				difference.push(field as string);
+			}
+		}
+	}
+
+	if (equal) {
+		return { equal };
+	} else {
+		return { equal, difference };
+	}
+}

test/04-service.spec.coffee

@@ -1,4 +1,4 @@
-{ expect } = require './lib/chai-config'
+{ assert, expect } = require './lib/chai-config'
 
 _ = require 'lodash'
 
@@ -210,6 +210,101 @@ describe 'compose/service', ->
 
 		expect(service.config).to.have.property('expose').that.deep.equals(['80/tcp', '100/tcp'])
 
+	describe 'Ordered array parameters', ->
+		it 'Should correctly compare ordered array parameters', ->
+			svc1 = Service.fromComposeObject({
+				appId: 1,
+				serviceId: 1,
+				serviceName: 'test',
+				dns: [
+					'8.8.8.8',
+					'1.1.1.1',
+				]
+			}, { appName: 'test' })
+			svc2 = Service.fromComposeObject({
+				appId: 1,
+				serviceId: 1,
+				serviceName: 'test',
+				dns: [
+					'8.8.8.8',
+					'1.1.1.1',
+				]
+			}, { appName: 'test' })
+			assert(svc1.isEqualConfig(svc2))
+
+			svc2 = Service.fromComposeObject({
+				appId: 1,
+				serviceId: 1,
+				serviceName: 'test',
+				dns: [
+					'1.1.1.1',
+					'8.8.8.8',
+				]
+			}, { appName: 'test' })
+			assert(!svc1.isEqualConfig(svc2))
+
+		it 'should correctly compare non-ordered array parameters', ->
+			svc1 = Service.fromComposeObject({
+				appId: 1,
+				serviceId: 1,
+				serviceName: 'test',
+				volumes: [
+					'abcdef',
+					'ghijk',
+				]
+			}, { appName: 'test' })
+			svc2 = Service.fromComposeObject({
+				appId: 1,
+				serviceId: 1,
+				serviceName: 'test',
+				volumes: [
+					'abcdef',
+					'ghijk',
+				]
+			}, { appName: 'test' })
+			assert(svc1.isEqualConfig(svc2))
+
+			svc2 = Service.fromComposeObject({
+				appId: 1,
+				serviceId: 1,
+				serviceName: 'test',
+				volumes: [
+					'ghijk',
+					'abcdef',
+				]
+			}, { appName: 'test' })
+			assert(svc1.isEqualConfig(svc2))
+
+		it 'should correctly compare both ordered and non-ordered array parameters', ->
+			svc1 = Service.fromComposeObject({
+				appId: 1,
+				serviceId: 1,
+				serviceName: 'test',
+				volumes: [
+					'abcdef',
+					'ghijk',
+				],
+				dns: [
+					'8.8.8.8',
+					'1.1.1.1',
+				]
+			}, { appName: 'test' })
+			svc2 = Service.fromComposeObject({
+				appId: 1,
+				serviceId: 1,
+				serviceName: 'test',
+				volumes: [
+					'ghijk',
+					'abcdef',
+				],
+				dns: [
+					'8.8.8.8',
+					'1.1.1.1',
+				]
+			}, { appName: 'test' })
+			assert(svc1.isEqualConfig(svc2))
+
+
 	describe 'parseMemoryNumber()', ->
 		makeComposeServiceWithLimit = (memLimit) ->
 			Service.fromComposeObject({