-
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} Use deb822 format in DEB installation script #28250
Conversation
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
️✔️AzureCLI-FullTest
|
Hi @konstruktoid, |
️✔️AzureCLI-BreakingChangeTest
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
/retest |
This is the script I used for testing:
|
Co-authored-by: Hang <bebound@gmail.com>
Co-authored-by: Hang <bebound@gmail.com>
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
#28250 (comment) #!/bin/bash
CLI_VERSION="2.57.0"
set -euo pipefail
# use deb_install.sh in current directry
images=("ubuntu:18.04" "ubuntu:20.04" "ubuntu:22.04" "debian:10" "debian:11" "debian:12")
for image in "${images[@]}"; do
echo "Testing ${image}"
docker run -v "$(pwd)/deb_install.sh:/deb_install.sh" --rm "${image}" bash -c "bash /deb_install.sh"
if ! az version | grep "\"azure-cli\": \"${CLI_VERSION}\""; then
echo "Test failed in ${image}"
exit 1
fi
done |
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
I don't have a preference regarding adding it to the repo. @jiasli, what's your opinion? PS: deb822 is supported by apt since version 1.1, making it compatible with Ubuntu 16.04, Debian 9 and later. |
Yeah, it was more of a concept, if it was worth spending time on. |
@@ -42,12 +42,15 @@ setup() { | |||
set -v | |||
export DEBIAN_FRONTEND=noninteractive | |||
apt-get update | |||
apt-get install -y apt-transport-https lsb-release gnupg curl | |||
apt-get install --assume-yes --no-install-recommends apt-transport-https ca-certificates curl gnupg lsb-release |
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.
Why is ca-certificates
required?
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's specified in https://github.com/konstruktoid/azure-docs-cli/blob/main/docs-ref-conceptual/includes/cli-install-linux-apt.md#option-2-step-by-step-installation-instructions, and (needed or not) most often included in installation scripts just-in-case, just as gnupg
and curl
that tends to be installed by default as well.
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
Suites: ${CLI_REPO} | ||
Components: main | ||
Architectures: $(dpkg --print-architecture) | ||
Signed-by: /etc/apt/keyrings/microsoft.gpg" | tee /etc/apt/sources.list.d/azure-cli.sources |
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 /etc/apt/sources.list.d/azure-cli.list
-> /etc/apt/sources.list.d/azure-cli.sources
renaming breaks the logic of check_distro_consistency
from #5755:
LIST_FILE_PATH = os.path.join(os.sep, 'etc', 'apt', 'sources.list.d', 'azure-cli.list') |
azure-cli/src/azure-cli-core/azure/cli/core/extension/operations.py
Lines 564 to 591 in b6ab754
def check_distro_consistency(): | |
if IS_WINDOWS: | |
return | |
try: | |
logger.debug('Linux distro check: Reading from: %s', LIST_FILE_PATH) | |
with open(LIST_FILE_PATH, 'r') as list_file: | |
package_source = list_file.read() | |
stored_linux_dist_name = package_source.split(" ")[3] | |
logger.debug('Linux distro check: Found in list file: %s', stored_linux_dist_name) | |
current_linux_dist_name = get_lsb_release() | |
logger.debug('Linux distro check: Reported by API: %s', current_linux_dist_name) | |
except Exception as err: # pylint: disable=broad-except | |
current_linux_dist_name = None | |
stored_linux_dist_name = None | |
logger.debug('Linux distro check: An error occurred while checking ' | |
'linux distribution version source list consistency.') | |
logger.debug(err) | |
if current_linux_dist_name != stored_linux_dist_name: | |
logger.debug("Linux distro check: Mismatch distribution " | |
"name in %s file", LIST_FILE_PATH) | |
logger.debug("Linux distro check: If command fails, install the appropriate package " | |
"for your distribution or change the above file accordingly.") | |
logger.debug("Linux distro check: %s has '%s', current distro is '%s'", | |
LIST_FILE_PATH, stored_linux_dist_name, current_linux_dist_name) |
and result in error in debug log:
cli.azure.cli.core.extension.operations: [Errno 2] No such file or directory: '/etc/apt/sources.list.d/azure-cli.list'
Description
This PR:
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.