Skip to content

Conversation

@khluu
Copy link
Contributor

@khluu khluu commented Oct 22, 2025

Add Azure CLI and dependencies into base-extra images

p
Signed-off-by: kevin <[email protected]>
@khluu khluu requested a review from a team as a code owner October 22, 2025 20:30
cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds Azure CLI to the base-extra image by including necessary packages and configurations in the Dockerfile. The changes involve adding curl, gnupg, and lsb-release to the list of APT packages, setting up the Microsoft package repository, and installing Azure CLI. The added packages and configurations are essential for installing and using Azure CLI within the Docker image.

Comment on lines +37 to +38
curl -sLS https://packages.microsoft.com/keys/microsoft.asc |
gpg --dearmor | sudo tee /etc/apt/keyrings/microsoft.gpg > /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It's generally better to combine multiple commands into a single RUN instruction using && to reduce the number of layers in the Docker image. This can improve build speed and reduce image size.

Consider combining the curl and gpg commands into a single RUN instruction.

RUN curl -sLS https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor | sudo tee /etc/apt/keyrings/microsoft.gpg > /dev/null

Comment on lines +41 to +51
AZ_VER=2.72.0
AZ_DIST="$(lsb_release -cs)"
echo "Types: deb
URIs: https://packages.microsoft.com/repos/azure-cli/
Suites: ${AZ_DIST}
Components: main
Architectures: $(dpkg --print-architecture)
Signed-by: /etc/apt/keyrings/microsoft.gpg" | sudo tee /etc/apt/sources.list.d/azure-cli.sources

sudo apt-get update -y
sudo apt-get install -y azure-cli="${AZ_VER}"-1~"${AZ_DIST}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider using ARG to define the Azure CLI version (AZ_VER) so that it can be easily overridden during the build process. This makes the Dockerfile more flexible and configurable.

Also, it would be better to combine all the commands related to setting up the Azure CLI repository and installing Azure CLI into a single RUN instruction to reduce the number of layers in the Docker image.

ARG AZ_VER=2.72.0
RUN AZ_DIST="$(lsb_release -cs)" && \
    echo "Types: deb\nURIs: https://packages.microsoft.com/repos/azure-cli/\nSuites: ${AZ_DIST}\nComponents: main\nArchitectures: $(dpkg --print-architecture)\nSigned-by: /etc/apt/keyrings/microsoft.gpg" | sudo tee /etc/apt/sources.list.d/azure-cli.sources && \
    sudo apt-get update -y && \
    sudo apt-get install -y azure-cli="${AZ_VER}"-1~"${AZ_DIST}"

Comment on lines 33 to 34
sudo apt-get autoclean
sudo rm -rf /etc/apt/sources.list.d/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

After installing the Azure CLI, it's a good practice to clean up the APT package lists to reduce the image size. This can be done by adding sudo apt-get clean to the Dockerfile.

Also, consider combining the autoclean and rm commands into a single RUN instruction.

RUN sudo apt-get autoclean && sudo rm -rf /etc/apt/sources.list.d/*

p
Signed-off-by: kevin <[email protected]>
Comment on lines 19 to 20
curl
gnupg
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gnupg and curl are already inside the image. they do not need to be reinstalled here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed!

@aslonnie aslonnie added the go add ONLY when ready to merge, run all tests label Oct 22, 2025
@aslonnie
Copy link
Collaborator

could you add a PR description?

p
Signed-off-by: kevin <[email protected]>
@khluu khluu requested a review from aslonnie October 24, 2025 10:03
@aslonnie aslonnie merged commit af33918 into master Oct 24, 2025
6 checks passed
@aslonnie aslonnie deleted the khluu/azure_cli_baseextra branch October 24, 2025 16:52
xinyuangui2 pushed a commit to xinyuangui2/ray that referenced this pull request Oct 27, 2025
Add Azure CLI and dependencies into `base-extra` images

---------

Signed-off-by: kevin <[email protected]>
Signed-off-by: xgui <[email protected]>
landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025
Add Azure CLI and dependencies into `base-extra` images

---------

Signed-off-by: kevin <[email protected]>
Aydin-ab pushed a commit to Aydin-ab/ray-aydin that referenced this pull request Nov 19, 2025
Add Azure CLI and dependencies into `base-extra` images

---------

Signed-off-by: kevin <[email protected]>
Signed-off-by: Aydin Abiar <[email protected]>
Future-Outlier pushed a commit to Future-Outlier/ray that referenced this pull request Dec 7, 2025
Add Azure CLI and dependencies into `base-extra` images

---------

Signed-off-by: kevin <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

azure core Issues that should be addressed in Ray Core devprod go add ONLY when ready to merge, run all tests

Development

Successfully merging this pull request may close these issues.

4 participants