-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
update best practices #22169
base: main
Are you sure you want to change the base?
update best practices #22169
Conversation
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
something like `wget` and `tar`, because it ensures a more precise build cache. | ||
`ADD` also has built-in support for checksum validation of the remote | ||
resources, and a protocol for parsing branches, tags, and subdirectories from | ||
[Git URLs](/reference/cli/docker/buildx/build.md#git-repositories). | ||
|
||
> [!NOTE] | ||
> | ||
> `ADD` redownloads the file every time the image is built to verify the checksum |
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.
This is not correct for servers that support etags. Only HEAD
request is needed to know the file has not changed if there is local cache.
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.
Is this better?
|
||
If the file being downloaded is supposed to be part of the image and is okay to be redownloaded on each build (to verify changes), using `ADD` as part of the image build is more suitable. | ||
|
||
If the file is an archive being extracted, or not supposed to be part of the final image, using `ADD` by itself would add an additional layer and subsequently removing it using `RUN rm` will not decrease the image size. In this case, look at the below example to use a multi stage build with a `scratch` image to download the file using `ADD` and bind mounting it where needed in the final image. |
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 don't see how "supposed to be part of final image" matters in here. Layers in the target stage end up in the exported image, and RUN rm
does not release data from the parent layers. This is true for all instructions and nothing specific about ADD
or RUN
.
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.
It was to clarify that always using ADD in your image build process is not the recommendation, since it adds a layer when you might not want one, which is how saying ADD is always better than curl/wget in the best practice reads to me. Is there a better way to phrase that using ADD without a multi stage build to bring it in when the artifact is not needed in the final image is not a best practice?
Description
Update the
ADD
recommendation to be clear on the behavior overRUN curl
and counterparts.Related issues or tickets
Closes #22168
Related to #18582
Reviews
Initially changed as a result of discussion in moby/buildkit#4125