Skip to content

Commit 363b027

Browse files
authored
fix: Enable services for podman executable (#1301)
1 parent 2493c9d commit 363b027

File tree

2 files changed

+70
-29
lines changed

2 files changed

+70
-29
lines changed

src/job.ts

+35-21
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,14 @@ export class Job {
511511
}
512512

513513
if (this.services?.length) {
514+
// `host` and `none` networks do not work with services because aliases only work for
515+
// user defined networks.
516+
for (const network of this.argv.network) {
517+
if (["host", "none"].includes(network)) {
518+
throw new AssertionError({message: `Cannot add service network alias with network mode '${network}'`});
519+
}
520+
}
521+
514522
await this.createDockerNetwork(`gitlab-ci-local-${this.jobId}`);
515523

516524
await Promise.all(
@@ -720,13 +728,25 @@ export class Job {
720728
dockerCmd += `--cpus=${cpuConfig} `;
721729
}
722730

723-
// host and none networks have to be specified using --network,
724-
// since they cannot be used with `docker network connect`.
731+
// host and none networks have to be specified using --network, since they cannot be used with
732+
// `docker network connect`.
725733
for (const network of this.argv.network) {
726734
if (["host", "none"].includes(network)) {
727735
dockerCmd += `--network ${network} `;
728736
}
729737
}
738+
// The default podman network mode is not `bridge`, which means a `podman network connect` call will fail
739+
// when connecting user defined networks. The workaround is to use a user defined network on container
740+
// creation.
741+
//
742+
// See https://github.com/containers/podman/issues/19577
743+
//
744+
// This should not clash with the `host` and `none` networks above, since service creation should have
745+
// failed when using `host` or `none` networks.
746+
if (this._serviceNetworkId) {
747+
// `build` alias: https://gitlab.com/gitlab-org/gitlab-runner/-/issues/27060
748+
dockerCmd += `--network ${this._serviceNetworkId} --network-alias build `;
749+
}
730750

731751
dockerCmd += `--volume ${buildVolumeName}:/gcl-builds `;
732752
dockerCmd += `--volume ${tmpVolumeName}:${this.fileVariablesDir} `;
@@ -784,9 +804,6 @@ export class Job {
784804

785805
const {stdout: containerId} = await Utils.bash(dockerCmd, cwd);
786806

787-
if (this.services?.length) {
788-
await Utils.spawn([this.argv.containerExecutable, "network", "connect", "--alias", "build", `gitlab-ci-local-${this.jobId}`, `${containerId}`]);
789-
}
790807
for (const network of this.argv.network) {
791808
// Special network names that do not work with `docker network connect`
792809
if (["host", "none"].includes(network)) {
@@ -1218,16 +1235,19 @@ export class Job {
12181235
}
12191236
dockerCmd += `--volume ${this.buildVolumeName}:/gcl-builds `;
12201237
dockerCmd += `--volume ${this.tmpVolumeName}:${this.fileVariablesDir} `;
1221-
dockerCmd += `${serviceName} `;
12221238

1223-
// host and none networks have to be specified using --network,
1224-
// since they cannot be used with `docker network connect`.
1225-
for (const network of this.argv.network) {
1226-
if (["host", "none"].includes(network)) {
1227-
dockerCmd += `--network ${network} `;
1228-
}
1239+
// The default podman network mode is not `bridge`, which means a `podman network connect` call will fail
1240+
// when connecting user defined networks. The workaround is to use a user defined network on container
1241+
// creation.
1242+
//
1243+
// See https://github.com/containers/podman/issues/19577
1244+
dockerCmd += `--network ${this._serviceNetworkId} `;
1245+
for (const alias of aliases) {
1246+
dockerCmd += `--network-alias ${alias} `;
12291247
}
12301248

1249+
dockerCmd += `${serviceName} `;
1250+
12311251
if (serviceEntrypoint?.length ?? 0 > 1) {
12321252
serviceEntrypoint?.slice(1).forEach((e) => {
12331253
dockerCmd += `"${e}" `;
@@ -1240,13 +1260,7 @@ export class Job {
12401260
const {stdout: containerId} = await Utils.bash(dockerCmd, cwd);
12411261
this._containersToClean.push(containerId);
12421262

1243-
const aliasArgs = Array.from(aliases.values()).flatMap((alias) => ["--alias", alias]);
1244-
await Utils.spawn([this.argv.containerExecutable, "network", "connect", ...aliasArgs, `gitlab-ci-local-${this.jobId}`, `${containerId}`]);
12451263
for (const network of this.argv.network) {
1246-
// Special network names that do not work with `docker network connect`.
1247-
if (["host", "none"].includes(network)) {
1248-
continue;
1249-
}
12501264
await Utils.spawn([this.argv.containerExecutable, "network", "connect", network, `${containerId}`]);
12511265
}
12521266

@@ -1258,7 +1272,7 @@ export class Job {
12581272
return containerId;
12591273
}
12601274

1261-
private async serviceHealthCheck (writeStreams: WriteStreams, service: Service, serviceIndex: number, serviceContanerLogFile: string) {
1275+
private async serviceHealthCheck (writeStreams: WriteStreams, service: Service, serviceIndex: number, serviceContainerLogFile: string) {
12621276
const serviceAlias = service.alias;
12631277
const serviceName = service.name;
12641278

@@ -1284,7 +1298,7 @@ export class Job {
12841298
await Promise.any(Object.keys(imageInspect[0].Config.ExposedPorts).map((port) => {
12851299
if (!port.endsWith("/tcp")) return;
12861300
const portNum = parseInt(port.replace("/tcp", ""));
1287-
const spawnCmd = [this.argv.containerExecutable, "run", "--rm", `--name=gcl-wait-for-it-${this.jobId}-${serviceIndex}-${portNum}`, "--network", `gitlab-ci-local-${this.jobId}`, "docker.io/sumina46/wait-for-it", `${uniqueAlias}:${portNum}`, "-t", "30"];
1301+
const spawnCmd = [this.argv.containerExecutable, "run", "--rm", `--name=gcl-wait-for-it-${this.jobId}-${serviceIndex}-${portNum}`, "--network", `${this._serviceNetworkId}`, "docker.io/sumina46/wait-for-it", `${uniqueAlias}:${portNum}`, "-t", "30"];
12881302
return Utils.spawn(spawnCmd);
12891303
}));
12901304
const endTime = process.hrtime(time);
@@ -1296,7 +1310,7 @@ export class Job {
12961310
singleError.message.split(/\r?\n/g).forEach((line: string) => {
12971311
writeStreams.stdout(chalk`${this.formattedJobName} {redBright ${line}}\n`);
12981312
});
1299-
writeStreams.stdout(chalk`${this.formattedJobName} {redBright also see (${serviceContanerLogFile})}\n`);
1313+
writeStreams.stdout(chalk`${this.formattedJobName} {redBright also see (${serviceContainerLogFile})}\n`);
13001314
});
13011315
} finally {
13021316
// Kill all wait-for-it containers, when one have been successful

tests/test-cases/network-arg/integration.network-arg.test.ts

+35-8
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ import {handler} from "../../../src/handler";
33
import chalk from "chalk";
44
import {initSpawnSpy, initBashSpy} from "../../mocks/utils.mock";
55
import {WhenStatics} from "../../mocks/when-statics";
6+
import assert from "assert";
7+
import {AssertionError} from "assert";
68

79
beforeAll(() => {
810
initSpawnSpy(WhenStatics.all);
911
});
1012

1113

12-
test("network-host", async () => {
14+
test("network-host <test-job>", async () => {
1315
const bashSpy = initBashSpy([]);
1416

1517
const writeStreams = new WriteStreamsMock();
@@ -27,7 +29,21 @@ test("network-host", async () => {
2729
expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining(expected));
2830
});
2931

30-
test("network-none", async () => {
32+
test("network-host <service-job>", async () => {
33+
try {
34+
const writeStreams = new WriteStreamsMock();
35+
await handler({
36+
cwd: "tests/test-cases/network-arg",
37+
job: ["service-job"],
38+
network: ["host"],
39+
}, writeStreams);
40+
} catch (e) {
41+
assert(e instanceof AssertionError, "e is not instanceof AssertionError");
42+
expect(e.message).toBe(chalk`Cannot add service network alias with network mode 'host'`);
43+
}
44+
});
45+
46+
test("network-none <test-job>", async () => {
3147
const bashSpy = initBashSpy([]);
3248

3349
const writeStreams = new WriteStreamsMock();
@@ -45,7 +61,21 @@ test("network-none", async () => {
4561
expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining(expected));
4662
});
4763

48-
test("network-custom", async () => {
64+
test("network-none <service-job>", async () => {
65+
try {
66+
const writeStreams = new WriteStreamsMock();
67+
await handler({
68+
cwd: "tests/test-cases/network-arg",
69+
job: ["service-job"],
70+
network: ["none"],
71+
}, writeStreams);
72+
} catch (e) {
73+
assert(e instanceof AssertionError, "e is not instanceof AssertionError");
74+
expect(e.message).toBe(chalk`Cannot add service network alias with network mode 'none'`);
75+
}
76+
});
77+
78+
test("network-custom <test-job>", async () => {
4979
const bashSpy = initBashSpy([]);
5080
const networkSpy = initSpawnSpy([{
5181
cmdArgs: expect.arrayContaining(["docker", "network", "connect"]),
@@ -65,9 +95,7 @@ test("network-custom", async () => {
6595
expect(networkSpy).toHaveBeenCalledWith(expect.arrayContaining(["docker", "network", "connect", "custom-network2"]));
6696
});
6797

68-
test("network-custom-with-service", async () => {
69-
const bashSpy = initBashSpy([]);
70-
98+
test("network-custom <service-job>", async () => {
7199
const networkSpy = initSpawnSpy([{
72100
cmdArgs: expect.arrayContaining(["docker", "network", "connect"]),
73101
returnValue: {stdout: "", stderr: "", exitCode: 0},
@@ -78,10 +106,9 @@ test("network-custom-with-service", async () => {
78106
await handler({
79107
cwd: "tests/test-cases/network-arg",
80108
job: ["service-job"],
81-
network: ["host", "custom-network1", "custom-network2"],
109+
network: ["custom-network1", "custom-network2"],
82110
}, writeStreams);
83111

84-
expect(bashSpy).toHaveBeenCalledWith(expect.stringMatching(/--network host/), expect.any(String));
85112
expect(networkSpy).toHaveBeenCalledWith(expect.arrayContaining(["docker", "network", "connect", "custom-network1"]));
86113
expect(networkSpy).toHaveBeenCalledWith(expect.arrayContaining(["docker", "network", "connect", "custom-network2"]));
87114
});

0 commit comments

Comments
 (0)