Skip to content

RTECO-1052 - Fix docker performance#3447

Merged
fluxxBot merged 4 commits intomasterfrom
RTECO-1052
Apr 21, 2026
Merged

RTECO-1052 - Fix docker performance#3447
fluxxBot merged 4 commits intomasterfrom
RTECO-1052

Conversation

@fluxxBot
Copy link
Copy Markdown
Contributor

@fluxxBot fluxxBot commented Apr 20, 2026

  • All tests have passed. If this feature is not already covered by the tests, new tests have been added.
  • The pull request is targeting the master branch.
  • The code has been validated to compile successfully by running go vet ./....
  • The code has been formatted properly using go fmt ./....

RTECO-1052 — Docker image id verification: fix OOM, make slow path opt-out
PR #410 introduced daemon.WithUnbufferedOpener() in containerManager.Id() to stop the Docker push flow from OOMing on large images — daemon.Image(ref) buffers the entire docker save tarball in memory. That change fixed the OOM but caused a second regression: on Docker 29 (which most customers now run) and on resource-constrained agents, the unbuffered opener stalls the push for 5+ minutes on multi-GB images. During that stall the jf docker push appears hung, sitting between the Pushed line from the Docker daemon and the subsequent Setting properties... build-info step.

The root cause is daemon.Image(...).Manifest() invoking a full docker save over the whole image just to read the config digest. On Docker 29 the docker images --format {{.ID}} fallback cannot be used because it now reports the manifest digest, not the config digest, so we can't cheaply substitute it.

What this PR does
Two changes, both scoped to the Docker-client path inside containerManager.Id():

Replace WithUnbufferedOpener with WithFileBufferedOpener — spools the docker save stream to a temp file instead of RAM, permanently removing the OOM risk without needing any flag. Memory usage stays bounded regardless of image size.
Add a Push-only opt-in to skip the local id lookup entirely via a new env var JFROG_CLI_SKIP_DOCKER_IMAGE_ID_VERIFICATION. When set to a truthy value on a docker push, Id() short-circuits and returns an empty string; the build-info builder then adopts the Artifactory-side manifest's Config.Digest for layer lookups, bypassing docker save completely. Pull and all other flows ignore the flag and always verify.
Default behavior is unchanged: we still verify the local id, just with file-backed buffering instead of in-memory buffering.

Depends on:

  1. RTECO-1052 - Docker verification customisable through flags jfrog-cli-artifactory#428

@fluxxBot fluxxBot added the improvement Automatically generated release notes label Apr 20, 2026
@fluxxBot fluxxBot added the safe to test Approve running integration tests on a pull request label Apr 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@fluxxBot fluxxBot added safe to test Approve running integration tests on a pull request and removed safe to test Approve running integration tests on a pull request labels Apr 20, 2026
@fluxxBot fluxxBot added safe to test Approve running integration tests on a pull request and removed safe to test Approve running integration tests on a pull request labels Apr 20, 2026
@fluxxBot
Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@fluxxBot fluxxBot requested a review from bhanurp April 21, 2026 08:42
@fluxxBot fluxxBot added safe to test Approve running integration tests on a pull request and removed safe to test Approve running integration tests on a pull request labels Apr 21, 2026
@fluxxBot fluxxBot added safe to test Approve running integration tests on a pull request and removed safe to test Approve running integration tests on a pull request labels Apr 21, 2026
@fluxxBot fluxxBot enabled auto-merge (squash) April 21, 2026 10:23
@fluxxBot fluxxBot merged commit d53cc6a into master Apr 21, 2026
79 of 82 checks passed
@fluxxBot fluxxBot deleted the RTECO-1052 branch April 21, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Automatically generated release notes safe to test Approve running integration tests on a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants