Open
Conversation
ea412ab to
5991aad
Compare
5991aad to
d6901ff
Compare
d6901ff to
013efed
Compare
02675f9 to
b886175
Compare
b886175 to
0271641
Compare
c02f5fd to
e869067
Compare
0f7b630 to
087ba15
Compare
joh-klein
reviewed
Sep 25, 2024
joh-klein
reviewed
Sep 25, 2024
7f2cebb to
69e8b98
Compare
joh-klein
reviewed
Sep 30, 2024
images/Dockerfile
Outdated
| # The second `apt-get update` isn't strictly necessary, but let's leave it here to force the | ||
| # image build to break if the ppa is suddenly unavailable. | ||
| RUN apt-get update \ | ||
| && apt-get install -y --no-install-recommends curl unzip sudo lsb-release gpg-agent software-properties-common \ |
There was a problem hiding this comment.
Could you maybe add the comment
# 'gpg-agent' and 'software-properties-common' are needed for the 'add-apt-repository' command that follows
here so it is clear, why you need this?
But why the others? Is sudo and lsb-release already needed at this stage?
joh-klein
reviewed
Sep 30, 2024
images/Dockerfile
Outdated
| # Install a sane set of base utilities and upgrade all system packages | ||
| RUN apt-get update \ | ||
| && apt-get install -y --no-install-recommends sudo lsb-release gpg-agent software-properties-common curl git jq unzip \ | ||
| && apt-get upgrade -y \ |
There was a problem hiding this comment.
This is probably a question of style – but as far as I can see (and what I read from them), microsoft keeps the base image very up to date – so, maybe this is not really needed?
This reduces the overall runtime image size by >300MB. - Use heredoc shell steps. It's just more readable. - Move git-core/ppa installs to the build stage. - Move Docker CLI install to a simple COPY from the upstream docker.io/docker:cli image - Remove dockerd components. This image assumes that your dockerd is running outside of this container, probably as part of a k8s Pod.
69e8b98 to
1836384
Compare
Author
|
@joh-klein Since the whole Dockerfile has been updated, I went all-in. I took into account your suggestions. Have a look. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reduces the overall on-disk image size by ~348MB (amd64) and the download size by ~146MB. For those of us who use runners in containers/k8s, the network traffic is reduced by a considerable 29%.