Skip to content

Commit d15158a

Browse files
cywang117cywang117
committed
Add Docker network label if custom ipam config
In a target release where the only change is the addition or removal of a custom ipam config, the Supervisor does not recreate the network due to ignoring ipam config differences when comparing current and target network (in network.isEqualConfig). This commit implements the addition of a network label if the target compose object includes a network with custom ipam. With the label, the Supervisor will detect a difference between a network with a custom ipam and a network without, without needing to compare the ipam configs themselves. This is a major change, as devices running networks with custom ipam configs will have their networks recreated to add the network label. Closes: #2251 Change-type: major See: https://balena.fibery.io/Work/Project/Fix-Supervisor-not-recreating-network-when-passed-custom-ipam-config-1127 Signed-off-by: Christina Ying Wang <[email protected]>
1 parent b6f0ecb commit d15158a

File tree

4 files changed

+219
-78
lines changed

4 files changed

+219
-78
lines changed

‎src/compose/network.ts

+9
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,15 @@ class NetworkImpl implements Network {
160160
configOnly: network.config_only || false,
161161
};
162162

163+
// Add label if there's non-default ipam config
164+
// e.g. explicitly defined subnet or gateway.
165+
// When updating between a release where the ipam config
166+
// changes, this label informs the Supervisor that
167+
// there's an ipam diff that requires recreating the network.
168+
if (net.config.ipam.config.length > 0) {
169+
net.config.labels['io.balena.private.ipam.config'] = 'true';
170+
}
171+
163172
return net;
164173
}
165174

‎test/integration/compose/network.spec.ts

+2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ describe('compose/network: integration tests', () => {
6767
Labels: {
6868
'io.balena.supervised': 'true',
6969
'io.balena.app-id': '12345',
70+
// This label should be present as we've defined a custom ipam config
71+
'io.balena.private.ipam.config': 'true',
7072
},
7173
Options: {},
7274
ConfigOnly: false,

‎test/integration/state-engine.spec.ts

+180-52
Original file line numberDiff line numberDiff line change
@@ -260,38 +260,44 @@ describe('state engine', () => {
260260
});
261261
});
262262

263-
it('updates an app with two services with a network change', async () => {
263+
it('updates an app with two services with a network change where the only change is a custom ipam config addition', async () => {
264+
const services = {
265+
'1': {
266+
image: 'alpine:latest',
267+
imageId: 11,
268+
serviceName: 'one',
269+
restart: 'unless-stopped',
270+
running: true,
271+
command: 'sleep infinity',
272+
stop_signal: 'SIGKILL',
273+
networks: ['default'],
274+
labels: {},
275+
environment: {},
276+
},
277+
'2': {
278+
image: 'alpine:latest',
279+
imageId: 12,
280+
serviceName: 'two',
281+
restart: 'unless-stopped',
282+
running: true,
283+
command: 'sleep infinity',
284+
stop_signal: 'SIGKILL',
285+
networks: ['default'],
286+
labels: {},
287+
environment: {},
288+
},
289+
};
264290
await setTargetState({
265291
config: {},
266292
apps: {
267293
'123': {
268294
name: 'test-app',
269295
commit: 'deadbeef',
270296
releaseId: 1,
271-
services: {
272-
'1': {
273-
image: 'alpine:latest',
274-
imageId: 11,
275-
serviceName: 'one',
276-
restart: 'unless-stopped',
277-
running: true,
278-
command: 'sleep infinity',
279-
stop_signal: 'SIGKILL',
280-
labels: {},
281-
environment: {},
282-
},
283-
'2': {
284-
image: 'alpine:latest',
285-
imageId: 12,
286-
serviceName: 'two',
287-
restart: 'unless-stopped',
288-
running: true,
289-
command: 'sleep infinity',
290-
labels: {},
291-
environment: {},
292-
},
297+
services,
298+
networks: {
299+
default: {},
293300
},
294-
networks: {},
295301
volumes: {},
296302
},
297303
},
@@ -311,39 +317,109 @@ describe('state engine', () => {
311317
]);
312318
const containerIds = containers.map(({ Id }) => Id);
313319

320+
// Network should not have custom ipam config
321+
const defaultNet = await docker.getNetwork('123_default').inspect();
322+
expect(defaultNet)
323+
.to.have.property('IPAM')
324+
.to.not.deep.equal({
325+
Config: [{ Gateway: '192.168.91.1', Subnet: '192.168.91.0/24' }],
326+
Driver: 'default',
327+
Options: {},
328+
});
329+
330+
// Network should not have custom ipam label
331+
expect(defaultNet)
332+
.to.have.property('Labels')
333+
.to.not.have.property('io.balena.private.ipam.config');
334+
314335
await setTargetState({
315336
config: {},
316337
apps: {
317338
'123': {
318339
name: 'test-app',
319340
commit: 'deadca1f',
320341
releaseId: 2,
321-
services: {
322-
'1': {
323-
image: 'alpine:latest',
324-
imageId: 21,
325-
serviceName: 'one',
326-
restart: 'unless-stopped',
327-
running: true,
328-
command: 'sleep infinity',
329-
stop_signal: 'SIGKILL',
330-
networks: ['default'],
331-
labels: {},
332-
environment: {},
333-
},
334-
'2': {
335-
image: 'alpine:latest',
336-
imageId: 22,
337-
serviceName: 'two',
338-
restart: 'unless-stopped',
339-
running: true,
340-
command: 'sh -c "echo two && sleep infinity"',
341-
stop_signal: 'SIGKILL',
342-
networks: ['default'],
343-
labels: {},
344-
environment: {},
342+
services,
343+
networks: {
344+
default: {
345+
driver: 'bridge',
346+
ipam: {
347+
config: [
348+
{ gateway: '192.168.91.1', subnet: '192.168.91.0/24' },
349+
],
350+
driver: 'default',
351+
},
345352
},
346353
},
354+
volumes: {},
355+
},
356+
},
357+
});
358+
359+
const updatedContainers = await docker.listContainers();
360+
expect(
361+
updatedContainers.map(({ Names, State }) => ({ Name: Names[0], State })),
362+
).to.have.deep.members([
363+
{ Name: '/one_11_2_deadca1f', State: 'running' },
364+
{ Name: '/two_12_2_deadca1f', State: 'running' },
365+
]);
366+
367+
// Container ids must have changed
368+
expect(updatedContainers.map(({ Id }) => Id)).to.not.have.members(
369+
containerIds,
370+
);
371+
372+
// Network should have custom ipam config
373+
const customNet = await docker.getNetwork('123_default').inspect();
374+
expect(customNet)
375+
.to.have.property('IPAM')
376+
.to.deep.equal({
377+
Config: [{ Gateway: '192.168.91.1', Subnet: '192.168.91.0/24' }],
378+
Driver: 'default',
379+
Options: {},
380+
});
381+
382+
// Network should have custom ipam label
383+
expect(customNet)
384+
.to.have.property('Labels')
385+
.to.have.property('io.balena.private.ipam.config');
386+
});
387+
388+
it('updates an app with two services with a network change where the only change is a custom ipam config removal', async () => {
389+
const services = {
390+
'1': {
391+
image: 'alpine:latest',
392+
imageId: 11,
393+
serviceName: 'one',
394+
restart: 'unless-stopped',
395+
running: true,
396+
command: 'sleep infinity',
397+
stop_signal: 'SIGKILL',
398+
networks: ['default'],
399+
labels: {},
400+
environment: {},
401+
},
402+
'2': {
403+
image: 'alpine:latest',
404+
imageId: 12,
405+
serviceName: 'two',
406+
restart: 'unless-stopped',
407+
running: true,
408+
command: 'sleep infinity',
409+
stop_signal: 'SIGKILL',
410+
networks: ['default'],
411+
labels: {},
412+
environment: {},
413+
},
414+
};
415+
await setTargetState({
416+
config: {},
417+
apps: {
418+
'123': {
419+
name: 'test-app',
420+
commit: 'deadbeef',
421+
releaseId: 1,
422+
services,
347423
networks: {
348424
default: {
349425
driver: 'bridge',
@@ -360,26 +436,78 @@ describe('state engine', () => {
360436
},
361437
});
362438

439+
const state = await getCurrentState();
440+
expect(
441+
state.apps['123'].services.map((s: any) => s.serviceName),
442+
).to.deep.equal(['one', 'two']);
443+
444+
// Network should have custom ipam config
445+
const customNet = await docker.getNetwork('123_default').inspect();
446+
expect(customNet)
447+
.to.have.property('IPAM')
448+
.to.deep.equal({
449+
Config: [{ Gateway: '192.168.91.1', Subnet: '192.168.91.0/24' }],
450+
Driver: 'default',
451+
Options: {},
452+
});
453+
454+
// Network should have custom ipam label
455+
expect(customNet)
456+
.to.have.property('Labels')
457+
.to.have.property('io.balena.private.ipam.config');
458+
459+
const containers = await docker.listContainers();
460+
expect(
461+
containers.map(({ Names, State }) => ({ Name: Names[0], State })),
462+
).to.have.deep.members([
463+
{ Name: '/one_11_1_deadbeef', State: 'running' },
464+
{ Name: '/two_12_1_deadbeef', State: 'running' },
465+
]);
466+
const containerIds = containers.map(({ Id }) => Id);
467+
468+
await setTargetState({
469+
config: {},
470+
apps: {
471+
'123': {
472+
name: 'test-app',
473+
commit: 'deadca1f',
474+
releaseId: 2,
475+
services,
476+
networks: {
477+
default: {},
478+
},
479+
volumes: {},
480+
},
481+
},
482+
});
483+
363484
const updatedContainers = await docker.listContainers();
364485
expect(
365486
updatedContainers.map(({ Names, State }) => ({ Name: Names[0], State })),
366487
).to.have.deep.members([
367-
{ Name: '/one_21_2_deadca1f', State: 'running' },
368-
{ Name: '/two_22_2_deadca1f', State: 'running' },
488+
{ Name: '/one_11_2_deadca1f', State: 'running' },
489+
{ Name: '/two_12_2_deadca1f', State: 'running' },
369490
]);
370491

371492
// Container ids must have changed
372493
expect(updatedContainers.map(({ Id }) => Id)).to.not.have.members(
373494
containerIds,
374495
);
375496

376-
expect(await docker.getNetwork('123_default').inspect())
497+
// Network should not have custom ipam config
498+
const defaultNet = await docker.getNetwork('123_default').inspect();
499+
expect(defaultNet)
377500
.to.have.property('IPAM')
378-
.to.deep.equal({
501+
.to.not.deep.equal({
379502
Config: [{ Gateway: '192.168.91.1', Subnet: '192.168.91.0/24' }],
380503
Driver: 'default',
381504
Options: {},
382505
});
506+
507+
// Network should not have custom ipam label
508+
expect(defaultNet)
509+
.to.have.property('Labels')
510+
.to.not.have.property('io.balena.private.ipam.config');
383511
});
384512

385513
it('updates an app with two services with a network removal', async () => {

‎test/unit/compose/network.spec.ts

+28-26
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,8 @@ describe('compose/network', () => {
183183
'io.balena.supervised': 'true',
184184
'io.balena.app-id': '12345',
185185
'com.docker.some-label': 'yes',
186+
// This label should be present as we've defined a custom ipam config
187+
'io.balena.private.ipam.config': 'true',
186188
});
187189

188190
expect(dockerConfig.Options).to.deep.equal({
@@ -344,12 +346,14 @@ describe('compose/network', () => {
344346
'io.resin.features.something': '123',
345347
'io.balena.features.dummy': 'abc',
346348
'io.balena.supervised': 'true',
349+
'io.balena.private.ipam.config': 'true',
347350
} as NetworkInspectInfo['Labels'],
348351
} as NetworkInspectInfo);
349352

350353
expect(network.config.labels).to.deep.equal({
351354
'io.balena.features.something': '123',
352355
'io.balena.features.dummy': 'abc',
356+
'io.balena.private.ipam.config': 'true',
353357
});
354358
});
355359
});
@@ -425,34 +429,32 @@ describe('compose/network', () => {
425429
});
426430

427431
describe('comparing network configurations', () => {
428-
it('ignores IPAM configuration', () => {
429-
const network = Network.fromComposeObject('default', 12345, 'deadbeef', {
430-
ipam: {
431-
driver: 'default',
432-
config: [
433-
{
434-
subnet: '172.20.0.0/16',
435-
ip_range: '172.20.10.0/24',
436-
gateway: '172.20.0.1',
437-
},
438-
],
439-
options: {},
432+
it('distinguishes a network with custom ipam config from a network without', () => {
433+
const customIpam = Network.fromComposeObject(
434+
'default',
435+
12345,
436+
'deadbeef',
437+
{
438+
ipam: {
439+
driver: 'default',
440+
config: [
441+
{
442+
subnet: '172.20.0.0/16',
443+
gateway: '172.20.0.1',
444+
},
445+
],
446+
options: {},
447+
},
440448
},
441-
});
442-
expect(
443-
network.isEqualConfig(
444-
Network.fromComposeObject('default', 12345, 'deadbeef', {}),
445-
),
446-
).to.be.true;
449+
);
450+
const noCustomIpam = Network.fromComposeObject(
451+
'default',
452+
12345,
453+
'deadbeef',
454+
{},
455+
);
447456

448-
// Only ignores ipam.config, not other ipam elements
449-
expect(
450-
network.isEqualConfig(
451-
Network.fromComposeObject('default', 12345, 'deadbeef', {
452-
ipam: { driver: 'aaa' },
453-
}),
454-
),
455-
).to.be.false;
457+
expect(customIpam.isEqualConfig(noCustomIpam)).to.be.false;
456458
});
457459

458460
it('compares configurations recursively', () => {

0 commit comments

Comments
 (0)