From 76de276b924edbbeb30cdad41ed491ad20129492 Mon Sep 17 00:00:00 2001 From: Cameron Diver Date: Mon, 19 Aug 2019 11:47:51 +0100 Subject: [PATCH] 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 --- src/compose/service.ts | 39 +++++++++------ src/compose/utils.ts | 57 ++++++++++++++++++++++ test/04-service.spec.coffee | 97 ++++++++++++++++++++++++++++++++++++- 3 files changed, 178 insertions(+), 15 deletions(-) diff --git a/src/compose/service.ts b/src/compose/service.ts index 2b84a5ba..2fce4385 100644 --- a/src/compose/service.ts +++ b/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: '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, + (service.config as unknown) as Dictionary, + 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 diff --git a/src/compose/utils.ts b/src/compose/utils.ts index 511c5f1d..04defa9b 100644 --- a/src/compose/utils.ts +++ b/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>( + obj1: T, + obj2: T, + nonOrderedFields: Array, + orderedFields: Array, +): { equal: false; difference: string[] }; +export function compareArrayFields>( + obj1: T, + obj2: T, + nonOrderedFields: Array, + orderedFields: Array, +): { equal: true }; +export function compareArrayFields>( + obj1: T, + obj2: T, + nonOrderedFields: Array, + orderedFields: Array, +): { 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 }; + } +} diff --git a/test/04-service.spec.coffee b/test/04-service.spec.coffee index 997a6446..0b4f764d 100644 --- a/test/04-service.spec.coffee +++ b/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({