Skip to content

Commit 7cb6c58

Browse files
committed
Merge branch 'fix/compose-v1-compat' into 'main'
fix(cli): support docker-compose v1 with modern Docker engines See merge request postgres-ai/postgresai!222
2 parents 876c22f + 8987606 commit 7cb6c58

2 files changed

Lines changed: 129 additions & 2 deletions

File tree

cli/bin/postgres-ai.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2071,13 +2071,16 @@ function isDockerRunning(): boolean {
20712071
}
20722072

20732073
/**
2074-
* Get docker compose command
2074+
* Get docker compose command.
2075+
* Prefer "docker compose" (V2 plugin) over legacy "docker-compose" (V1 standalone)
2076+
* because docker-compose V1 (<=1.29) is incompatible with modern Docker engines
2077+
* (KeyError: 'ContainerConfig' on container recreation).
20752078
*/
20762079
function getComposeCmd(): string[] | null {
20772080
const tryCmd = (cmd: string, args: string[]): boolean =>
20782081
spawnSync(cmd, args, { stdio: "ignore", timeout: 5000 } as Parameters<typeof spawnSync>[2]).status === 0;
2079-
if (tryCmd("docker-compose", ["version"])) return ["docker-compose"];
20802082
if (tryCmd("docker", ["compose", "version"])) return ["docker", "compose"];
2083+
if (tryCmd("docker-compose", ["version"])) return ["docker-compose"];
20812084
return null;
20822085
}
20832086

@@ -2621,6 +2624,10 @@ mon
26212624
}
26222625

26232626
// Step 5: Start services
2627+
// Remove stopped containers left by "run --rm" dependencies (e.g. config-init)
2628+
// to avoid docker-compose v1 'ContainerConfig' error on recreation.
2629+
// Best-effort: ignore exit code — container may not exist, failure here is non-fatal.
2630+
await runCompose(["rm", "-f", "-s", "config-init"]);
26242631
console.log("Step 5: Starting monitoring services...");
26252632
const code2 = await runCompose(["up", "-d", "--force-recreate"], grafanaPassword);
26262633
if (code2 !== 0) {

cli/test/compose-cmd.test.ts

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import { describe, test, expect } from "bun:test";
2+
3+
/**
4+
* Test getComposeCmd() selection logic.
5+
* WARNING: This replicates the logic from postgres-ai.ts. If the production
6+
* function changes, this replica must be updated to match.
7+
* Since the function is internal to postgres-ai.ts, we replicate its logic
8+
* with an injectable command checker (same pattern as monitoring.test.ts).
9+
*/
10+
function getComposeCmd(
11+
tryCmd: (cmd: string, args: string[]) => boolean,
12+
): string[] | null {
13+
if (tryCmd("docker", ["compose", "version"])) return ["docker", "compose"];
14+
if (tryCmd("docker-compose", ["version"])) return ["docker-compose"];
15+
return null;
16+
}
17+
18+
describe("getComposeCmd", () => {
19+
test("prefers docker compose V2 when both are available", () => {
20+
const result = getComposeCmd(() => true);
21+
expect(result).toEqual(["docker", "compose"]);
22+
});
23+
24+
test("falls back to docker-compose V1 when V2 is unavailable", () => {
25+
const result = getComposeCmd((cmd, args) => {
26+
// V2 plugin fails, V1 standalone succeeds
27+
if (cmd === "docker" && args[0] === "compose") return false;
28+
if (cmd === "docker-compose") return true;
29+
return false;
30+
});
31+
expect(result).toEqual(["docker-compose"]);
32+
});
33+
34+
test("returns null when neither is available", () => {
35+
const result = getComposeCmd(() => false);
36+
expect(result).toBeNull();
37+
});
38+
39+
test("does not check V1 when V2 succeeds", () => {
40+
const calls: Array<{ cmd: string; args: string[] }> = [];
41+
getComposeCmd((cmd, args) => {
42+
calls.push({ cmd, args });
43+
return cmd === "docker" && args[0] === "compose";
44+
});
45+
expect(calls).toHaveLength(1);
46+
expect(calls[0]).toEqual({ cmd: "docker", args: ["compose", "version"] });
47+
});
48+
49+
test("checks V2 first, then V1", () => {
50+
const calls: Array<{ cmd: string; args: string[] }> = [];
51+
getComposeCmd((cmd, args) => {
52+
calls.push({ cmd, args });
53+
return false;
54+
});
55+
expect(calls).toHaveLength(2);
56+
expect(calls[0]).toEqual({ cmd: "docker", args: ["compose", "version"] });
57+
expect(calls[1]).toEqual({ cmd: "docker-compose", args: ["version"] });
58+
});
59+
});
60+
61+
/**
62+
* Test the monitoring startup sequence's container cleanup logic.
63+
* Before "up --force-recreate", stopped containers from "run --rm" dependencies
64+
* (e.g. config-init) must be removed to avoid docker-compose v1's
65+
* KeyError: 'ContainerConfig' bug.
66+
*
67+
* We replicate the relevant sequence from the monitoring start command
68+
* with an injectable runCompose to verify ordering and error tolerance.
69+
*/
70+
async function monitoringStartSequence(
71+
runCompose: (args: string[]) => Promise<number>,
72+
): Promise<number> {
73+
// Best-effort: remove stopped containers left by "run --rm" dependencies
74+
await runCompose(["rm", "-f", "-s", "config-init"]);
75+
// Start services
76+
const code = await runCompose(["up", "-d", "--force-recreate"]);
77+
return code;
78+
}
79+
80+
describe("monitoring start: config-init cleanup", () => {
81+
test("calls rm before up", async () => {
82+
const calls: string[][] = [];
83+
await monitoringStartSequence(async (args) => {
84+
calls.push(args);
85+
return 0;
86+
});
87+
expect(calls).toHaveLength(2);
88+
expect(calls[0]).toEqual(["rm", "-f", "-s", "config-init"]);
89+
expect(calls[1]).toEqual(["up", "-d", "--force-recreate"]);
90+
});
91+
92+
test("continues to up even when rm fails", async () => {
93+
const calls: string[][] = [];
94+
await monitoringStartSequence(async (args) => {
95+
calls.push(args);
96+
// rm returns non-zero (container doesn't exist)
97+
if (args[0] === "rm") return 1;
98+
return 0;
99+
});
100+
expect(calls).toHaveLength(2);
101+
expect(calls[0][0]).toBe("rm");
102+
expect(calls[1][0]).toBe("up");
103+
});
104+
105+
test("returns up exit code, not rm exit code", async () => {
106+
// rm fails but up succeeds → overall success
107+
const result1 = await monitoringStartSequence(async (args) => {
108+
if (args[0] === "rm") return 1;
109+
return 0;
110+
});
111+
expect(result1).toBe(0);
112+
113+
// rm succeeds but up fails → overall failure
114+
const result2 = await monitoringStartSequence(async (args) => {
115+
if (args[0] === "up") return 2;
116+
return 0;
117+
});
118+
expect(result2).toBe(2);
119+
});
120+
});

0 commit comments

Comments
 (0)