Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix container name creation to generate valid DNS hostnames #2078

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ If you have upgraded system packages and find that your tests are failing to ini

### DBus

When developing on macOS you may need to install DBus on the development host.
When developing on macOS you may need to install DBus on the development host if you get errors like `Package dbus-1 was not found in the pkg-config search path.`.

1. `brew install dbus`
1. `brew install dbus` or `sudo port install dbus`
2. `npm ci`

On Debian-based systems, `sudo apt install libdbus-1-dev` would be the equivalent.
Expand Down
15 changes: 13 additions & 2 deletions src/compose/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ export class Service {
const nameMatch = container.Name.match(/.*_(\d+)_(\d+)(?:_(.*?))?$/);
if (nameMatch == null) {
throw new InternalInconsistencyError(
`Expected supervised container to have name '<serviceName>_<imageId>_<releaseId>_<commit>', got: ${container.Name}`,
`Expected supervised container to have name '<serviceName>_<imageId>_<releaseId>(_<commit>)', got: ${container.Name}`,
);
}

Expand Down Expand Up @@ -704,8 +704,19 @@ export class Service {
}
this.config.networkMode = `container:${containerId}`;
}

// Format container name to keep it under 64 characters to make it a legal DNS hostname
const imageIdPart = ('' + this.imageId).substring(0, 10);
const releaseIdPart = ('' + this.releaseId).substring(0, 10);
// Shorten the service name so that it leaves room for the required imageId and releaseId parts ( see fromDockerContainer )
const serviceNamePart = this.serviceName?.substring(0, 28);
// Use a commit prefix so that all containers will get the same commit suffix in normal cases where the name length is <= 63
const commitPrefix = this.commit?.substring(0, 12);
return {
name: `${this.serviceName}_${this.imageId}_${this.releaseId}_${this.commit}`,
name: `${serviceNamePart}_${imageIdPart}_${releaseIdPart}_${commitPrefix}`.substring(
0,
63,
),
Tty: this.config.tty,
Cmd: this.config.command,
Volumes: volumes,
Expand Down
57 changes: 57 additions & 0 deletions test/unit/compose/service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Service from '~/src/compose/service';
import Volume from '~/src/compose/volume';
import * as ServiceT from '~/src/compose/types/service';
import * as constants from '~/lib/constants';
import Dockerode = require('dockerode');

const configs = {
simple: {
Expand Down Expand Up @@ -464,6 +465,62 @@ describe('compose/service: unit tests', () => {
});
});
});

describe('Container naming', () => {
function validateExpectationsOnName(
containerCreateOptions: Dockerode.ContainerCreateOptions,
) {
console.log(containerCreateOptions.name);
const MAX_DNS_HOSTNAME_LENGTH = 63;
expect(containerCreateOptions.name?.length).to.be.lessThanOrEqual(
MAX_DNS_HOSTNAME_LENGTH,
);
const nameMatch = containerCreateOptions.name?.match(
/.*_(\d+)_(\d+)(?:_(.*?))?$/,
);
expect(nameMatch).to.be.not.null;
}

it('should create valid DNS names with long commits', async () => {
const service = await Service.fromComposeObject(
{
appId: 1234567,
appUuid: 'ae8c6ddc272547a49531149bd2dd187f',
serviceId: 1234568,
serviceName: 'test_broker_client',
imageId: '5826138',
releaseId: '2403300',
commit: '72b853b44d0246fd789c89269db742d04a2e5ef9',
},
{ appName: 'test' } as any,
);

const containerCreateOptions = service.toDockerContainer({
deviceName: 'foo',
} as any);
validateExpectationsOnName(containerCreateOptions);
});

it('should create valid DNS names with long service names', async () => {
const service = await Service.fromComposeObject(
{
appId: 1234567,
appUuid: 'ae8c6ddc272547a49531149bd2dd187f',
serviceId: 1234568,
serviceName: 'X'.repeat(64),
imageId: '5826138',
releaseId: '2403300',
commit: '72b853b44d0246fd789c89269db742d04a2e5ef9',
},
{ appName: 'test' } as any,
);

const containerCreateOptions = service.toDockerContainer({
deviceName: 'foo',
} as any);
validateExpectationsOnName(containerCreateOptions);
});
});
});

describe('Comparing services', () => {
Expand Down