-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Packaging] BREAKING CHANGE: Remove unnecessary packages in docker image #27567
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
reduce docker image size |
4ea8e41
to
80389ec
Compare
80389ec
to
5516ed5
Compare
Dockerfile
Outdated
| xargs -r apk info --installed \ | ||
| sort -u \ | ||
)" \ | ||
&& apk add --virtual .rundeps $runDeps |
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.
.rundeps
makes no sense, see #27269 (comment)
Dockerfile
Outdated
# ca-certificates bash bash-completion - for convenience | ||
# libintl icu-libs - required by azure-devops https://github.com/Azure/azure-cli/pull/9683 | ||
# libc6-compat - required by az storage blob sync https://github.com/Azure/azure-cli/issues/10381 | ||
# gcc python3-dev musl-dev linux-headers libffi-dev - temporarily required by psutil | ||
|
||
WORKDIR azure-cli | ||
COPY . /azure-cli |
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 can be replaced by --mount
to save another 100MB.
I'll create a new PR for this.
|
||
# curl - required for installing jp | ||
# jq - we include jq as a useful tool |
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.
jq
is a very useful tool and we frequently see users using jq
to parse Azure CLI's output. Removing tools like this will cause a breaking change.
|
||
ARG JP_VERSION="0.2.1" | ||
|
||
RUN arch=$(arch | sed s/aarch64/arm64/ | sed s/x86_64/amd64/) && curl -L https://github.com/jmespath/jp/releases/download/${JP_VERSION}/jp-linux-$arch -o /usr/local/bin/jp \ |
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.
jp
is also demanded by some users. See the discussion at #25590.
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 added jp
and jq
again.
Should we also make ssh-keygen available by default? #1343
PS: Our docker image is more like a Linux image than a CLI image. The entrypoint is bash
rather than az
alpine.dockerfile
Outdated
# ca-certificates bash bash-completion jq jp openssh-keygen - for convenience | ||
# libintl icu-libs - required by azure-devops https://github.com/Azure/azure-cli/pull/9683 | ||
# libc6-compat - required by az storage blob sync https://github.com/Azure/azure-cli/issues/10381 | ||
# gcc python3-dev musl-dev linux-headers libffi-dev - temporarily required by psutil |
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.
python3-dev
is not in the original alpine.dockerfile
. Is it really needed?
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.
When install psutil
in alpine, it prints
psutil could not be installed from sources because gcc is not installed. Try running:
sudo apk add gcc python3-dev
Weird, it can be built without python3-dev
.
|
||
# We don't use openssl (3.0) for now. We only install it so that users can use it. | ||
RUN apk add --no-cache bash openssh ca-certificates jq curl openssl perl git zip \ | ||
&& apk add --no-cache --virtual .build-deps gcc make openssl-dev libffi-dev musl-dev linux-headers \ |
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.
We used to remove .build-deps
until #8291.
PS: az extension add -n azure-batch-cli-extensions
works without those dependencies. I guess some dependencies finally provide the wheel packages over time.
Choosing Alpine as a base image is not a good idea, as some packages do not provide binary packages and the user must build them from source, such as the psutil
package.
It's common to use cibuildwheel to build wheel. However, Alpine Linux uses musl, while manylinux
wheels are based on glibc, so some packages are not available on Alpine. (Cibuildwheel supports build musllinux
wheel, but not all package provide musl wheel)
Ref: pypa/pip#9735
Very comprehensive history analysis. Nice work! |
Multiple packages were removed as part of this breaking change. Azure/azure-cli#27567
Multiple packages were removed as part of this breaking change. Azure/azure-cli#27567
Multiple packages were removed as part of this breaking change. Azure/azure-cli#27567
Description
Remove unnecessary packages from the image to reduce its size from 1122MB to 922MB.
The reason for removing it is not only to reduce the size but also to comply with Docker's best practices.
Removed packages:
openssh curl openssl perl git zip gcc make openssl-dev libffi-dev musl-dev linux-headers
Note: After this PR,
connectedk8s, hybridaks, k8s-configuration, rdbms-connect, ml
extensions fail to install bacause of the lack of musllinux wheel. Please install the related dev packages withapk add xxx
and try again.This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.