-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip downloading base image layers if they exists in target repository #1840
Conversation
I have addressed all the comments, AFAIKT. Good for another review pass. I decided to update CHANGELOG in a follow-up PR. |
jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/StepsRunner.java
Outdated
Show resolved
Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/StepsRunner.java
Show resolved
Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/global/JibSystemProperties.java
Show resolved
Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/ObtainBaseImageLayerStep.java
Show resolved
Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/ObtainBaseImageLayerStep.java
Show resolved
Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/global/JibSystemProperties.java
Outdated
Show resolved
Hide resolved
…-download-base-layers
.newTargetImageRegistryClientFactory() | ||
.setAuthorization(pushAuthorization) | ||
.newRegistryClient(); | ||
// TODO: also check if cross-repo blob mount is possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this is as easy as adding buildConfiguration.getBaseImageRegistry().equals(buildConfiguration.getTargetImageRegistry())
on in the existing-checker below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it involves more than that, like checking
JibSystemProperties.useCrossRepositoryBlobMounts()
&& canAttemptBlobMount(authorization, sourceRepository))
to ensure that we will attempt the blob mount. And I think it is that we can attempt blob mount, and it might be possible that a server may not allow/support the mount by returning 202 Accepted
instead of 201 Created
, in which case RegistryClient.pushBlob()
will fall back to do the usual pushing. So I think this is actually complicated.
But I think we are already pretty good without blob mount. Most of the time, the base image will be in the target repository. The blob mount can be a further optimization, but I think it does not give a huge value.
Hi team, Have any time for release it? Sounds good to my CI/CD method. Thanks !!! |
Fixes #1673.
This will give substantial speedup for CI/CD environments where people usually don't persist the Jib base image cache.
Design
I tried to do intelligent, fine-grained BLOb checking on each layer. For example, if only a subset of the layers are missing in the target repo, it downloads and caches only those layers.
I also made it avoid checking the layer twice in the situation where it should push missing layers, because currently
PushBlobStep
always does a Blob check before pushing.I spent a lot of time tinkering with several different designs and implementations in order to maintain generality and loose coupling between build steps and components, and I think I can settle with this design. So, other advantages from this are
Speedup
For a large image like
adoptopenjdk/openjdk9-openj9
,(However, I admit
adoptopenjdk/openjdk9-openj9
is 466MB and way larger thanopenjdk:8-slim
or Distroless.)Interaction with
--offline
As pointed out before, not caching the base image layers may cause a problem for
--offline
when layers are missing. However, in reality, this would be extremely rare, because--offline
can be used only withjib:dockerBuild
andjib:buildTar
; for those goals, Jib always downloads base image layers. And people do understand they have to do an online build at least once before they can do--offline
.But in an extremely rare case where the user
jib:dockerBuild
orjib:buildTar
on their local dev machinejib:build
which skipped downloading layers (i.e., manifest JSON is cached but layers are not)--offline jib:dockerBuild
without ever trying onlinejib:dockerBuild
then this case is still covered, as Jib will show the following error message.
[ERROR] Failed to execute goal com.google.cloud.tools:jib-maven-plugin:1.3.1-SNAPSHOT:dockerBuild (default-cli) on project helloworld: Cannot run Jib in offline mode; local Jib cache for base image is missing image layer sha256:.... Rerun Jib in online mode with "-Djib.cacheBaseImage=always" to re-download the base image layers. -> [Help 1]
(
-Djib.cacheBaseImage=always
is actually unnecessary becausejib:dockerBuild
andjib:buildTar
always download layers and--offline
is effective only for those goals, but I added it just for the unlikely case someone triesjib:build
instead ofjib:dockerBuild
to do one-time online caching in this situation.)