Skip to content

Commit 422ae22

Browse files
emily-shenemily-shen
authored
check for config (dockerfile) changes (#10248)
1 parent a63b379 commit 422ae22

File tree

5 files changed

+156
-136
lines changed

5 files changed

+156
-136
lines changed

‎.changeset/heavy-phones-clap.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
fix: re-push container images on deploy even if the only change was to the Dockerfile

‎packages/wrangler/src/__tests__/cloudchamber/build.test.ts‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
dockerLoginManagedRegistry,
66
getCloudflareContainerRegistry,
77
runDockerCmd,
8+
runDockerCmdWithOutput,
89
} from "@cloudflare/containers-shared";
910
import { UserError } from "../../errors";
1011
import { mockAccountId, mockApiToken } from "../helpers/mock-account-id";
@@ -17,6 +18,7 @@ vi.mock("@cloudflare/containers-shared", async (importOriginal) => {
1718
return Object.assign({}, actual, {
1819
dockerLoginManagedRegistry: vi.fn(),
1920
runDockerCmd: vi.fn(),
21+
runDockerCmdWithOutput: vi.fn(),
2022
dockerBuild: vi.fn(() => ({ abort: () => {}, ready: Promise.resolve() })),
2123
dockerImageInspect: vi.fn(),
2224
});
@@ -37,6 +39,9 @@ describe("buildAndMaybePush", () => {
3739
.mockResolvedValueOnce("[]")
3840
// return image size and number of layers
3941
.mockResolvedValueOnce("53387881 2");
42+
vi.mocked(runDockerCmdWithOutput).mockReturnValueOnce(
43+
'{"Descriptor":{"digest":"config-sha"}}'
44+
);
4045
mkdirSync("./container-context");
4146

4247
writeFileSync("./container-context/Dockerfile", dockerfile);
@@ -71,7 +76,7 @@ describe("buildAndMaybePush", () => {
7176
"/custom/docker/path",
7277
{
7378
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
74-
formatString: "{{json .RepoDigests}}",
79+
formatString: "{{ json .RepoDigests }} {{ .Id }}",
7580
}
7681
);
7782
expect(dockerImageInspect).toHaveBeenNthCalledWith(
@@ -130,7 +135,7 @@ describe("buildAndMaybePush", () => {
130135
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
131136
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
132137
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
133-
formatString: "{{json .RepoDigests}}",
138+
formatString: "{{ json .RepoDigests }} {{ .Id }}",
134139
});
135140
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
136141
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
@@ -147,7 +152,7 @@ describe("buildAndMaybePush", () => {
147152
vi.mocked(dockerImageInspect).mockReset();
148153
vi.mocked(dockerImageInspect)
149154
.mockResolvedValueOnce(
150-
'["registry.cloudflare.com/test-app@sha256:three"]'
155+
'["registry.cloudflare.com/test-app@sha256:three"] config-sha'
151156
)
152157
.mockResolvedValueOnce("53387881 2");
153158
await runWrangler(
@@ -167,26 +172,23 @@ describe("buildAndMaybePush", () => {
167172
],
168173
dockerfile,
169174
});
170-
expect(runDockerCmd).toHaveBeenCalledTimes(2);
171-
expect(runDockerCmd).toHaveBeenNthCalledWith(
172-
1,
173-
"docker",
174-
[
175-
"manifest",
176-
"inspect",
177-
`${getCloudflareContainerRegistry()}/some-account-id/test-app@sha256:three`,
178-
],
179-
"ignore"
180-
);
181-
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
175+
expect(runDockerCmdWithOutput).toHaveBeenCalledOnce();
176+
expect(runDockerCmdWithOutput).toHaveBeenCalledWith("docker", [
177+
"manifest",
178+
"inspect",
179+
"-v",
180+
`${getCloudflareContainerRegistry()}/some-account-id/test-app@sha256:three`,
181+
]);
182+
expect(runDockerCmd).toHaveBeenCalledTimes(1);
183+
expect(runDockerCmd).toHaveBeenCalledWith("docker", [
182184
"image",
183185
"rm",
184186
`${getCloudflareContainerRegistry()}/test-app:tag`,
185187
]);
186188
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
187189
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
188190
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
189-
formatString: "{{json .RepoDigests}}",
191+
formatString: "{{ json .RepoDigests }} {{ .Id }}",
190192
});
191193
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
192194
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
@@ -212,7 +214,7 @@ describe("buildAndMaybePush", () => {
212214
],
213215
dockerfile,
214216
});
215-
expect(dockerImageInspect).toHaveBeenCalledOnce();
217+
expect(dockerImageInspect).not.toHaveBeenCalledOnce();
216218
expect(dockerLoginManagedRegistry).not.toHaveBeenCalled();
217219
});
218220

‎packages/wrangler/src/__tests__/containers/deploy.test.ts‎

Lines changed: 48 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { spawn } from "node:child_process";
1+
import { execFileSync, spawn } from "node:child_process";
22
import * as fs from "node:fs";
33
import { PassThrough, Writable } from "node:stream";
44
import {
@@ -111,19 +111,19 @@ describe("wrangler deploy with containers", () => {
111111
await runWrangler("deploy index.js");
112112

113113
expect(std.out).toMatchInlineSnapshot(`
114-
"Total Upload: xx KiB / gzip: xx KiB
115-
Worker Startup Time: 100 ms
116-
Your Worker has access to the following bindings:
117-
Binding Resource
118-
env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object
114+
"Total Upload: xx KiB / gzip: xx KiB
115+
Worker Startup Time: 100 ms
116+
Your Worker has access to the following bindings:
117+
Binding Resource
118+
env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object
119119
120-
Uploaded test-name (TIMINGS)
121-
Building image my-container:Galaxy
122-
Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy
123-
Deployed test-name triggers (TIMINGS)
124-
https://test-name.test-sub-domain.workers.dev
125-
Current Version ID: Galaxy-Class"
126-
`);
120+
Uploaded test-name (TIMINGS)
121+
Building image my-container:Galaxy
122+
Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy
123+
Deployed test-name triggers (TIMINGS)
124+
https://test-name.test-sub-domain.workers.dev
125+
Current Version ID: Galaxy-Class"
126+
`);
127127
expect(std.err).toMatchInlineSnapshot(`""`);
128128
expect(std.warn).toMatchInlineSnapshot(`""`);
129129
expect(cliStd.stdout).toMatchInlineSnapshot(`
@@ -448,19 +448,19 @@ describe("wrangler deploy with containers", () => {
448448
await runWrangler("deploy --cwd src");
449449

450450
expect(std.out).toMatchInlineSnapshot(`
451-
"Total Upload: xx KiB / gzip: xx KiB
452-
Worker Startup Time: 100 ms
453-
Your Worker has access to the following bindings:
454-
Binding Resource
455-
env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object
456-
457-
Uploaded test-name (TIMINGS)
458-
Building image my-container:Galaxy
459-
Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy
460-
Deployed test-name triggers (TIMINGS)
461-
https://test-name.test-sub-domain.workers.dev
462-
Current Version ID: Galaxy-Class"
463-
`);
451+
"Total Upload: xx KiB / gzip: xx KiB
452+
Worker Startup Time: 100 ms
453+
Your Worker has access to the following bindings:
454+
Binding Resource
455+
env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object
456+
457+
Uploaded test-name (TIMINGS)
458+
Building image my-container:Galaxy
459+
Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy
460+
Deployed test-name triggers (TIMINGS)
461+
https://test-name.test-sub-domain.workers.dev
462+
Current Version ID: Galaxy-Class"
463+
`);
464464
expect(std.err).toMatchInlineSnapshot(`""`);
465465
expect(std.warn).toMatchInlineSnapshot(`""`);
466466
});
@@ -599,7 +599,7 @@ describe("wrangler deploy with containers", () => {
599599
│ [containers.constraints]
600600
│
601601
│
602-
│ SUCCESS Modified application my-container
602+
│ SUCCESS Modified application my-container (Application ID: abc)
603603
│
604604
╰ Applied changes
605605
@@ -740,7 +740,7 @@ describe("wrangler deploy with containers", () => {
740740
│ [containers.constraints]
741741
│
742742
│
743-
│ SUCCESS Modified application my-container
743+
│ SUCCESS Modified application my-container (Application ID: abc)
744744
│
745745
╰ Applied changes
746746
@@ -908,7 +908,7 @@ describe("wrangler deploy with containers", () => {
908908
│ tier = 1
909909
│
910910
│
911-
│ SUCCESS Modified application my-container
911+
│ SUCCESS Modified application my-container (Application ID: abc)
912912
│
913913
╰ Applied changes
914914
@@ -954,7 +954,7 @@ describe("wrangler deploy with containers", () => {
954954
│ tier = 1
955955
│
956956
│
957-
│ SUCCESS Modified application my-container
957+
│ SUCCESS Modified application my-container (Application ID: abc)
958958
│
959959
╰ Applied changes
960960
@@ -1012,7 +1012,7 @@ describe("wrangler deploy with containers", () => {
10121012
│ tier = 1
10131013
│
10141014
│
1015-
│ SUCCESS Modified application my-container
1015+
│ SUCCESS Modified application my-container (Application ID: abc)
10161016
│
10171017
╰ Applied changes
10181018
@@ -1070,7 +1070,7 @@ describe("wrangler deploy with containers", () => {
10701070
│ tier = 1
10711071
│
10721072
│
1073-
│ SUCCESS Modified application my-container
1073+
│ SUCCESS Modified application my-container (Application ID: abc)
10741074
│
10751075
╰ Applied changes
10761076
@@ -1124,7 +1124,7 @@ describe("wrangler deploy with containers", () => {
11241124
│ tier = 1
11251125
│
11261126
│
1127-
│ SUCCESS Modified application my-container
1127+
│ SUCCESS Modified application my-container (Application ID: abc)
11281128
│
11291129
╰ Applied changes
11301130
@@ -1183,7 +1183,7 @@ describe("wrangler deploy with containers", () => {
11831183
│ tier = 1
11841184
│
11851185
│
1186-
│ SUCCESS Modified application my-container
1186+
│ SUCCESS Modified application my-container (Application ID: abc)
11871187
│
11881188
╰ Applied changes
11891189
@@ -1366,7 +1366,6 @@ describe("wrangler deploy with containers dry run", () => {
13661366
mockDockerImageInspectDigests("my-container", "worker")
13671367
)
13681368
.mockImplementationOnce(mockDockerLogin("mockpassword"))
1369-
.mockImplementationOnce(mockDockerManifestInspect("my-container", true))
13701369
.mockImplementationOnce(mockDockerPush("my-container", "worker"));
13711370

13721371
vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker");
@@ -1412,7 +1411,7 @@ function createDockerMockChain(
14121411
mockDockerImageInspectDigests(containerName, tag),
14131412
mockDockerImageInspectSize(containerName, tag),
14141413
mockDockerLogin("mockpassword"),
1415-
mockDockerManifestInspect("some-account-id/" + containerName, true),
1414+
// Skip manifest inspect mock - it's not being called due to empty repoDigests
14161415
mockDockerTag(containerName, "some-account-id/" + containerName, tag),
14171416
mockDockerPush("some-account-id/" + containerName, tag),
14181417
mockDockerImageDelete("some-account-id/" + containerName, tag),
@@ -1441,8 +1440,17 @@ function setupDockerMocks(
14411440
.mockImplementationOnce(mocks[4])
14421441
.mockImplementationOnce(mocks[5])
14431442
.mockImplementationOnce(mocks[6])
1444-
.mockImplementationOnce(mocks[7])
1445-
.mockImplementationOnce(mocks[8]);
1443+
.mockImplementationOnce(mocks[7]);
1444+
// Default mock for execFileSync to handle docker verification and other calls
1445+
vi.mocked(execFileSync).mockImplementation(
1446+
(_file: string, args?: readonly string[]) => {
1447+
// Handle docker info calls (for verification)
1448+
if (args && args[0] === "manifest") {
1449+
return "i promise I am an unsuccessful docker manifest call";
1450+
}
1451+
return "";
1452+
}
1453+
);
14461454
}
14471455

14481456
// Common test setup
@@ -1677,7 +1685,7 @@ function mockDockerImageInspectDigests(containerName: string, tag: string) {
16771685
"inspect",
16781686
`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
16791687
"--format",
1680-
"{{json .RepoDigests}}",
1688+
"{{ json .RepoDigests }} {{ .Id }}",
16811689
]);
16821690

16831691
const stdout = new PassThrough();
@@ -1697,7 +1705,7 @@ function mockDockerImageInspectDigests(containerName: string, tag: string) {
16971705
setImmediate(() => {
16981706
stdout.emit(
16991707
"data",
1700-
`["${getCloudflareContainerRegistry()}/${containerName}@sha256:three"]`
1708+
`["${getCloudflareContainerRegistry()}/${containerName}@sha256:three"] config-sha`
17011709
);
17021710
});
17031711

@@ -1774,36 +1782,6 @@ function mockDockerLogin(expectedPassword: string) {
17741782
};
17751783
}
17761784

1777-
function mockDockerManifestInspect(containerName: string, shouldFail = true) {
1778-
return (cmd: string, args: readonly string[]) => {
1779-
expect(cmd).toBe("/usr/bin/docker");
1780-
expect(args[0]).toBe("manifest");
1781-
expect(args[1]).toBe("inspect");
1782-
expect(args[2]).toEqual(`${containerName}@three`);
1783-
expect(args).toEqual([
1784-
"manifest",
1785-
"inspect",
1786-
`${getCloudflareContainerRegistry()}/${containerName}@three`,
1787-
]);
1788-
const readable = new Writable({
1789-
write() {},
1790-
final() {},
1791-
});
1792-
return {
1793-
stdout: Buffer.from(
1794-
"i promise I am an unsuccessful docker manifest call"
1795-
),
1796-
stdin: readable,
1797-
on: function (reason: string, cbPassed: (code: number) => unknown) {
1798-
if (reason === "close") {
1799-
cbPassed(shouldFail ? 1 : 0);
1800-
}
1801-
return this;
1802-
},
1803-
} as unknown as ChildProcess;
1804-
};
1805-
}
1806-
18071785
function mockDockerPush(containerName: string, tag: string) {
18081786
return (cmd: string, args: readonly string[]) => {
18091787
expect(cmd).toBe("/usr/bin/docker");

0 commit comments

Comments
 (0)