Merge pull request #2064 from balena-os/create-default-net-as-config-only-if-host-networking

Create default network as config-only when services have host networking
This commit is contained in:
bulldozer-balena[bot] 2022-11-16 18:53:40 +00:00 committed by GitHub
commit 725a6b5274
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 129 additions and 10 deletions

View File

@ -77,12 +77,20 @@ export class App {
this.isHost = !!opts.isHost;
if (this.networks.default == null && isTargetState) {
const allHostNetworking = this.services.every(
(svc) => svc.config.networkMode === 'host',
);
// We always want a default network
this.networks.default = Network.fromComposeObject(
'default',
opts.appId,
opts.appUuid!, // app uuid always exists on the target state
{},
// app uuid always exists on the target state
opts.appUuid!,
// We don't want the default bridge to have actual addresses at all if services
// aren't using it, to minimize chances of host-Docker address conflicts.
// If config_only is specified, the network is created and available in Docker
// by name and config only, and no actual networking setup occurs.
{ config_only: allHostNetworking },
);
}
}

View File

@ -7,7 +7,11 @@ import * as logger from '../logger';
import log from '../lib/supervisor-console';
import * as ComposeUtils from './utils';
import { ComposeNetworkConfig, NetworkConfig } from './types/network';
import {
ComposeNetworkConfig,
NetworkConfig,
NetworkInspectInfo,
} from './types/network';
import { InvalidNetworkNameError } from './errors';
import { InternalInconsistencyError } from '../lib/errors';
@ -48,9 +52,7 @@ export class Network {
};
}
public static fromDockerNetwork(
network: dockerode.NetworkInspectInfo,
): Network {
public static fromDockerNetwork(network: NetworkInspectInfo): Network {
const ret = new Network();
// Detect the name and appId from the inspect data
@ -91,6 +93,7 @@ export class Network {
'io.balena.supervised',
]),
options: network.Options ?? {},
configOnly: network.ConfigOnly,
};
return ret;
@ -138,6 +141,7 @@ export class Network {
...ComposeUtils.normalizeLabels(network.labels || {}),
},
options: network.driver_opts || {},
configOnly: network.config_only || false,
};
return net;
@ -151,6 +155,7 @@ export class Network {
internal: this.config.internal,
ipam: this.config.ipam,
labels: this.config.labels,
config_only: this.config.configOnly,
};
}
@ -162,7 +167,9 @@ export class Network {
await docker.createNetwork(this.toDockerConfig());
}
public toDockerConfig(): dockerode.NetworkCreateOptions {
public toDockerConfig(): dockerode.NetworkCreateOptions & {
ConfigOnly: boolean;
} {
return {
Name: Network.generateDockerName(this.appUuid!, this.name),
Driver: this.config.driver,
@ -189,6 +196,7 @@ export class Network {
},
this.config.labels,
),
ConfigOnly: this.config.configOnly,
};
}
@ -230,7 +238,7 @@ export class Network {
}
public isEqualConfig(network: Network): boolean {
// don't compare the ipam.config if it's not present
// Don't compare the ipam.config if it's not present
// in the target state (as it will be present in the
// current state, due to docker populating it with
// default or generated values)
@ -240,6 +248,15 @@ export class Network {
configToCompare.ipam.config = [];
}
// If configOnly is true, driver will always be null even if
// it's specified as bridge in the target state, so don't compare drivers.
// Any ipam config will be included in the network, but not applied
// in the host's networking layer.
if (network.config.configOnly) {
configToCompare = _.cloneDeep(this.config);
configToCompare.driver = network.config.driver;
}
return _.isEqual(configToCompare, network.config);
}

View File

@ -1,3 +1,11 @@
import { NetworkInspectInfo as DockerNetworkInspectInfo } from 'dockerode';
// TODO: ConfigOnly is part of @types/dockerode@v3.2.0, but that version isn't
// compatible with `resin-docker-build` which is used for `npm run sync`.
export interface NetworkInspectInfo extends DockerNetworkInspectInfo {
ConfigOnly: boolean;
}
export interface ComposeNetworkConfig {
driver: string;
driver_opts: Dictionary<string>;
@ -16,6 +24,7 @@ export interface ComposeNetworkConfig {
enable_ipv6: boolean;
internal: boolean;
labels: Dictionary<string>;
config_only: boolean;
}
export interface NetworkConfig {
driver: string;
@ -33,4 +42,5 @@ export interface NetworkConfig {
internal: boolean;
labels: { [labelName: string]: string };
options: { [optName: string]: string };
configOnly: boolean;
}

View File

@ -69,6 +69,7 @@ describe('compose/network: integration tests', () => {
'io.balena.app-id': '12345',
},
Options: {},
ConfigOnly: false,
});
// Test network removal

View File

@ -78,6 +78,7 @@ export function createNetwork(network: PartialNetworkInspectInfo) {
Containers: {},
Options: {},
Labels: {},
ConfigOnly: false,
// Add defaults
...networkInspect,

View File

@ -602,6 +602,78 @@ describe('compose/app', () => {
// The network should not be created again
expectNoStep('createNetwork', steps);
});
it('should create a config-only network if network_mode is host for all services', async () => {
const svcOne = await createService({
appId: 1,
serviceName: 'one',
composition: { network_mode: 'host' },
});
const svcTwo = await createService({
appId: 1,
serviceName: 'two',
composition: { network_mode: 'host' },
});
const current = createApp({
services: [svcOne, svcTwo],
networks: [],
});
const target = createApp({
services: [svcOne, svcTwo],
networks: [],
isTarget: true,
});
const steps = current.nextStepsForAppUpdate(defaultContext, target);
const [createNetworkStep] = expectSteps('createNetwork', steps);
expect(createNetworkStep)
.to.have.property('target')
.that.has.property('config')
.that.deep.includes({ configOnly: true });
});
it('should not create a config-only network if network_mode: host is not specified for any service', async () => {
const svcOne = await createService({
appId: 1,
serviceName: 'one',
composition: { network_mode: 'host' },
});
const svcTwo = await createService({
appId: 1,
serviceName: 'two',
});
const current = createApp({
services: [svcOne, svcTwo],
networks: [],
});
const target = createApp({
services: [svcOne, svcTwo],
networks: [],
isTarget: true,
});
const steps = current.nextStepsForAppUpdate(defaultContext, target);
const [createNetworkStep] = expectSteps('createNetwork', steps);
expect(createNetworkStep)
.to.have.property('target')
.that.has.property('config')
.that.deep.includes({ configOnly: false });
});
it('should create a config-only network if there are no services in the app', async () => {
const current = createApp({});
const target = createApp({ isTarget: true });
const steps = current.nextStepsForAppUpdate(defaultContext, target);
const [createNetworkStep] = expectSteps('createNetwork', steps);
expect(createNetworkStep)
.to.have.property('target')
.that.has.property('config')
.that.deep.includes({ configOnly: true });
});
});
describe('service state behavior', () => {

View File

@ -2,7 +2,7 @@ import { expect } from 'chai';
import * as sinon from 'sinon';
import { Network } from '~/src/compose/network';
import { NetworkInspectInfo } from 'dockerode';
import { NetworkInspectInfo } from '~/src/compose/types/network';
import { log } from '~/lib/supervisor-console';
@ -32,6 +32,7 @@ describe('compose/network', () => {
'io.balena.app-id': '12345',
});
expect(network.config.options).to.deep.equal({});
expect(network.config.configOnly).to.equal(false);
});
it('normalizes legacy labels', () => {
@ -152,6 +153,7 @@ describe('compose/network', () => {
labels: {
'com.docker.some-label': 'yes',
},
config_only: true,
});
const dockerConfig = network1.toDockerConfig();
@ -174,7 +176,9 @@ describe('compose/network', () => {
},
],
});
// If ConfigOnly is true, Subnet & Gateway remain in config
// but no actual networks are created.
expect(dockerConfig.ConfigOnly).to.equal(true);
expect(dockerConfig.Labels).to.deep.equal({
'io.balena.supervised': 'true',
'io.balena.app-id': '12345',
@ -251,6 +255,7 @@ describe('compose/network', () => {
'io.balena.supervised': 'true',
'io.balena.features.something': '123',
} as NetworkInspectInfo['Labels'],
ConfigOnly: false,
} as NetworkInspectInfo);
expect(network.appId).to.equal(1234);
@ -271,6 +276,7 @@ describe('compose/network', () => {
expect(network.config.labels).to.deep.equal({
'io.balena.features.something': '123',
});
expect(network.config.configOnly).to.equal(false);
});
it('creates a network object from a docker network configuration', () => {
@ -299,6 +305,7 @@ describe('compose/network', () => {
'io.balena.features.something': '123',
'io.balena.app-id': '1234',
} as NetworkInspectInfo['Labels'],
ConfigOnly: false,
} as NetworkInspectInfo);
expect(network.appId).to.equal(1234);
@ -321,6 +328,7 @@ describe('compose/network', () => {
'io.balena.features.something': '123',
'io.balena.app-id': '1234',
});
expect(network.config.configOnly).to.equal(false);
});
it('normalizes legacy label names and excludes supervised label', () => {
@ -372,6 +380,7 @@ describe('compose/network', () => {
'io.balena.features.something': '123',
'io.balena.app-id': '12345',
} as NetworkInspectInfo['Labels'],
ConfigOnly: true,
} as NetworkInspectInfo);
expect(network.appId).to.equal(12345);
@ -399,6 +408,7 @@ describe('compose/network', () => {
'io.balena.features.something': '123',
'io.balena.app-id': '12345',
});
expect(compose.config_only).to.equal(true);
});
});