From 3b2cefb17482ac01197932da106ff17b38a2a851 Mon Sep 17 00:00:00 2001 From: frostebite Date: Sat, 29 Aug 2026 00:54:29 +0100 Subject: [PATCH] fix(unity-test-runner): pull the docker image explicitly before starting Unity's license session Root-caused game-ci/unity-test-runner#310's remaining CI failures (after the results-check ENOENT fix): "Test all modes" windows-2022 jobs were failing with "Unable to find image ... locally" - but that's just Docker's own harmless pre-pull notice, and the image genuinely exists (confirmed against the registry, 7-8GB Windows tags). The real problem: docker run's implicit pull-on-miss folds pull time into the same session as Unity's license activation/hold/return inside the container. A partial cache miss on one of these huge Windows images took 16 minutes to pull in one observed run, long enough that Unity's own ephemeral ULF license session failed to return cleanly ("Serial number unavailable for ULF return") once the container finally started - a real failure, but caused by pull time eating into the license window, not by anything about the test itself or the image being unavailable. Docker.run now pulls the image explicitly before that window opens. A pull failure (bad tag, registry down) is a genuine, non-retryable problem and is left to fail with Docker's own error rather than swallowed or retried. --- .../dist/unity-test-runner/model/docker.d.ts | 16 +++++++++ .../dist/unity-test-runner/model/docker.js | 19 ++++++++++ .../unity-test-runner/model/docker.test.ts | 35 ++++++++++++++++--- .../src/unity-test-runner/model/docker.ts | 21 +++++++++++ 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/plugins/unity/dist/unity-test-runner/model/docker.d.ts b/plugins/unity/dist/unity-test-runner/model/docker.d.ts index 355ed4ac..f69121da 100644 --- a/plugins/unity/dist/unity-test-runner/model/docker.d.ts +++ b/plugins/unity/dist/unity-test-runner/model/docker.d.ts @@ -4,6 +4,22 @@ declare const Docker: { * Remove a possible leftover container created by `Docker.run`. */ ensureContainerRemoval(parameters: RunnerContext): Promise; + /** + * `docker run` pulls an uncached image implicitly, but that folds the pull + * time into the same session as Unity's license activation/hold/return + * inside the container - and these images are huge (7-8GB+ for Windows + * tags). A partial cache miss can take 15+ minutes to pull, and observed + * in practice (unity-test-runner#310's CI) that's long enough for Unity's + * own ephemeral ULF license session to fail to return cleanly + * ("Serial number unavailable for ULF return") once the container + * finally gets to run - a real failure, but one caused by pull time + * eating into the license window, not by anything about the test itself. + * Pulling explicitly first, before that window opens, avoids the whole + * class of failure. A pull failure here is a real, non-retryable-by-us + * problem (bad tag, registry down) and is left to fail with Docker's own + * error rather than swallowed. + */ + pull(image: any): Promise; run(image: any, parameters: any, silent?: boolean): Promise; getLinuxCommand(image: any, parameters: any): string; getWindowsCommand(image: any, parameters: any): string; diff --git a/plugins/unity/dist/unity-test-runner/model/docker.js b/plugins/unity/dist/unity-test-runner/model/docker.js index 4a26ed64..07c6e916 100644 --- a/plugins/unity/dist/unity-test-runner/model/docker.js +++ b/plugins/unity/dist/unity-test-runner/model/docker.js @@ -73,11 +73,30 @@ const Docker = { (0, fs_1.rmSync)(cidfile); } }, + /** + * `docker run` pulls an uncached image implicitly, but that folds the pull + * time into the same session as Unity's license activation/hold/return + * inside the container - and these images are huge (7-8GB+ for Windows + * tags). A partial cache miss can take 15+ minutes to pull, and observed + * in practice (unity-test-runner#310's CI) that's long enough for Unity's + * own ephemeral ULF license session to fail to return cleanly + * ("Serial number unavailable for ULF return") once the container + * finally gets to run - a real failure, but one caused by pull time + * eating into the license window, not by anything about the test itself. + * Pulling explicitly first, before that window opens, avoids the whole + * class of failure. A pull failure here is a real, non-retryable-by-us + * problem (bad tag, registry down) and is left to fail with Docker's own + * error rather than swallowed. + */ + async pull(image) { + await (0, exec_1.exec)('docker', ['pull', String(image)]); + }, async run(image, parameters, silent = false) { let runCommand = ''; if (parameters.unityLicensingServer !== '') { licensing_server_setup_1.default.Setup(parameters.unityLicensingServer, parameters.actionFolder); } + await this.pull(image); switch (process.platform) { case 'linux': runCommand = this.getLinuxCommand(image, parameters); diff --git a/plugins/unity/src/unity-test-runner/model/docker.test.ts b/plugins/unity/src/unity-test-runner/model/docker.test.ts index 7e38e9a0..61e7bcc5 100644 --- a/plugins/unity/src/unity-test-runner/model/docker.test.ts +++ b/plugins/unity/src/unity-test-runner/model/docker.test.ts @@ -63,38 +63,65 @@ describe('Docker.run retry behavior', () => { it('retries a launch failure when a githubToken is set (USE_EXIT_CODE=false, exit code cannot mean a test failure)', async () => { execMock + .mockResolvedValueOnce(0) // docker pull .mockRejectedValueOnce(new Error('docker.exe failed with exit code 1')) .mockRejectedValueOnce(new Error('docker.exe failed with exit code 1')) .mockResolvedValueOnce(0); await Docker.run('some-image', buildParameters({ githubToken: 'gh-token' })); - expect(execMock).toHaveBeenCalledTimes(3); + // 1 pull + 3 run attempts + expect(execMock).toHaveBeenCalledTimes(4); }); it('gives up after exhausting retries when a githubToken is set', async () => { - execMock.mockRejectedValue(new Error('docker.exe failed with exit code 1')); + execMock + .mockResolvedValueOnce(0) // docker pull + .mockRejectedValue(new Error('docker.exe failed with exit code 1')); await expect( Docker.run('some-image', buildParameters({ githubToken: 'gh-token' })), ).rejects.toThrow('docker.exe failed with exit code 1'); - expect(execMock).toHaveBeenCalledTimes(3); + // 1 pull + 3 run attempts + expect(execMock).toHaveBeenCalledTimes(4); }); it('does not retry when no githubToken is set (a nonzero exit there means a real test failure)', async () => { - execMock.mockRejectedValue(new Error('tests failed')); + execMock + .mockResolvedValueOnce(0) // docker pull + .mockRejectedValue(new Error('tests failed')); await expect( Docker.run('some-image', buildParameters({ githubToken: undefined })), ).rejects.toThrow('tests failed'); + // 1 pull + 1 run attempt + expect(execMock).toHaveBeenCalledTimes(2); + }); + + it('pulls the image explicitly before running, so pull time is not folded into the license-hold window', async () => { + execMock.mockResolvedValue(0); + + await Docker.run('unityci/editor:some-tag', buildParameters({ githubToken: 'gh-token' })); + + expect(execMock).toHaveBeenNthCalledWith(1, 'docker', ['pull', 'unityci/editor:some-tag']); + }); + + it('does not attempt to run if the pull itself fails - a pull failure is not launch-retryable', async () => { + execMock.mockRejectedValueOnce(new Error('manifest unknown')); + + await expect( + Docker.run('some-image', buildParameters({ githubToken: 'gh-token' })), + ).rejects.toThrow('manifest unknown'); + expect(execMock).toHaveBeenCalledTimes(1); }); it('cleans up a stale cidfile between retry attempts so --cidfile does not immediately fail again', async () => { fsState.cidfileExists = true; execMock + .mockResolvedValueOnce(0) // docker pull .mockRejectedValueOnce(new Error('docker.exe failed with exit code 1')) .mockResolvedValueOnce(0); diff --git a/plugins/unity/src/unity-test-runner/model/docker.ts b/plugins/unity/src/unity-test-runner/model/docker.ts index aa672c76..15d7b7fe 100644 --- a/plugins/unity/src/unity-test-runner/model/docker.ts +++ b/plugins/unity/src/unity-test-runner/model/docker.ts @@ -39,6 +39,25 @@ const Docker = { } }, + /** + * `docker run` pulls an uncached image implicitly, but that folds the pull + * time into the same session as Unity's license activation/hold/return + * inside the container - and these images are huge (7-8GB+ for Windows + * tags). A partial cache miss can take 15+ minutes to pull, and observed + * in practice (unity-test-runner#310's CI) that's long enough for Unity's + * own ephemeral ULF license session to fail to return cleanly + * ("Serial number unavailable for ULF return") once the container + * finally gets to run - a real failure, but one caused by pull time + * eating into the license window, not by anything about the test itself. + * Pulling explicitly first, before that window opens, avoids the whole + * class of failure. A pull failure here is a real, non-retryable-by-us + * problem (bad tag, registry down) and is left to fail with Docker's own + * error rather than swallowed. + */ + async pull(image) { + await exec('docker', ['pull', String(image)]); + }, + async run(image, parameters, silent = false) { let runCommand = ''; @@ -46,6 +65,8 @@ const Docker = { LicensingServerSetup.Setup(parameters.unityLicensingServer, parameters.actionFolder); } + await this.pull(image); + switch (process.platform) { case 'linux': runCommand = this.getLinuxCommand(image, parameters);