Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 1 addition & 20 deletions src/azure-cli-core/azure/cli/core/extension/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from packaging.version import parse

from azure.cli.core import CommandIndex
from azure.cli.core.util import CLIError, reload_module
from azure.cli.core.util import CLIError, reload_module, rmtree_with_retry
from azure.cli.core.extension import (extension_exists, build_extension_path, get_extensions, get_extension_modname,
get_extension, ext_compat_with_cli,
EXT_METADATA_ISPREVIEW, EXT_METADATA_ISEXPERIMENTAL,
Expand Down Expand Up @@ -559,22 +559,3 @@ def check_distro_consistency():
"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)


def rmtree_with_retry(path):
# A workaround for https://bugs.python.org/issue33240
# Retry shutil.rmtree several times, but even if it fails after several retries, don't block the command execution.
retry_num = 3
import time
while True:
try:
shutil.rmtree(path)
return
except OSError as err:
if retry_num > 0:
logger.warning("Failed to delete '%s': %s. Retrying ...", path, err)
retry_num -= 1
time.sleep(1)
else:
logger.warning("Failed to delete '%s': %s. You may try to delete it manually.", path, err)
break
20 changes: 20 additions & 0 deletions src/azure-cli-core/azure/cli/core/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -1272,3 +1272,23 @@ def is_modern_terminal():
"""In addition to knack.util.is_modern_terminal, detect Cloud Shell."""
import knack.util
return knack.util.is_modern_terminal() or in_cloud_console()


def rmtree_with_retry(path):
# A workaround for https://bugs.python.org/issue33240
# Retry shutil.rmtree several times, but even if it fails after several retries, don't block the command execution.
retry_num = 3
import time
while True:
try:
import shutil
shutil.rmtree(path)
return
except OSError as err:
if retry_num > 0:
logger.warning("Failed to delete '%s': %s. Retrying ...", path, err)
retry_num -= 1
time.sleep(1)
else:
logger.warning("Failed to delete '%s': %s. You may try to delete it manually.", path, err)
break
53 changes: 49 additions & 4 deletions src/azure-cli/azure/cli/command_modules/util/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

import os

from knack.log import get_logger
from knack.util import CLIError

logger = get_logger(__name__)

Expand Down Expand Up @@ -32,7 +35,6 @@ def show_version(cmd): # pylint: disable=unused-argument


def upgrade_version(cmd, update_all=None, yes=None): # pylint: disable=too-many-locals, too-many-statements, too-many-branches, no-member, unused-argument
import os
import platform
import sys
import subprocess
Expand All @@ -41,7 +43,6 @@ def upgrade_version(cmd, update_all=None, yes=None): # pylint: disable=too-many
from azure.cli.core._environment import _ENV_AZ_INSTALLER
from azure.cli.core.extension import get_extensions, WheelExtension
from packaging.version import parse
from knack.util import CLIError

update_cli = True
from azure.cli.core.util import get_latest_from_github
Expand Down Expand Up @@ -131,8 +132,7 @@ def upgrade_version(cmd, update_all=None, yes=None): # pylint: disable=too-many
logger.warning("Exit the container to pull latest image with 'docker pull mcr.microsoft.com/azure-cli' "
"or run 'pip install --upgrade azure-cli' in this container")
elif installer == 'MSI':
logger.debug("Update azure cli with MSI from https://aka.ms/installazurecliwindows")
exit_code = subprocess.call(['powershell.exe', '-NoProfile', "Start-Process msiexec.exe -Wait -ArgumentList '/i https://aka.ms/installazurecliwindows'"]) # pylint: disable=line-too-long
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no need to launch PowerShell and let it launch msiexec.exe. Directly launching msiexec.exe is sufficient.

exit_code = _upgrade_on_windows()
else:
logger.warning(UPGRADE_MSG)
if exit_code:
Expand Down Expand Up @@ -175,6 +175,51 @@ def upgrade_version(cmd, update_all=None, yes=None): # pylint: disable=too-many
else auto_upgrade_msg)


def _upgrade_on_windows():
"""Download MSI to a temp folder and install it with msiexec.exe.
Directly installing from URL may be blocked by policy: https://github.com/Azure/azure-cli/issues/19171
This also gives the user a chance to manually install the MSI in case of msiexec.exe failure.
"""
logger.warning("Updating Azure CLI with MSI from https://aka.ms/installazurecliwindows")
Copy link
Member

Choose a reason for hiding this comment

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

This was logger.debug before. Intentional change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

I want the command to be more explicit and give the user more information about what it is doing internally. Otherwise, in case of failure, all that the user can see is CLI upgrade failed. (No different from Something occurred.)

To view the details, the user must specify --debug which is definitely not user-friendly.

tmp_dir, msi_path = _download_from_url('https://aka.ms/installazurecliwindows')

logger.warning("Installing MSI")
import subprocess
exit_code = subprocess.call(['msiexec.exe', '/i', msi_path])

if exit_code:
logger.warning("Installation Failed. You may manually install %s", msi_path)
else:
from azure.cli.core.util import rmtree_with_retry
logger.warning("Succeeded. Deleting %s", tmp_dir)
rmtree_with_retry(tmp_dir)
return exit_code


def _download_from_url(url):
import requests
from azure.cli.core.util import should_disable_connection_verify
r = requests.get(url, stream=True, verify=(not should_disable_connection_verify()))
if r.status_code != 200:
raise CLIError("Request to {} failed with {}".format(url, r.status_code))

# r.url is the real path of the msi, like'https://azcliprod.blob.core.windows.net/msi/azure-cli-2.27.1.msi'
file_name = r.url.rsplit('/')[-1]
import tempfile
tmp_dir = tempfile.mkdtemp()
msi_path = os.path.join(tmp_dir, file_name)
logger.warning("Downloading MSI to %s", msi_path)
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, isn't this more of a logger.debug?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above.


with open(msi_path, 'wb') as f:
for chunk in r.iter_content(chunk_size=1024):
f.write(chunk)

# Return both the temp directory and MSI path, like
# 'C:\Users\<name>\AppData\Local\Temp\tmpzv4pelsf',
# 'C:\Users\<name>\AppData\Local\Temp\tmpzv4pelsf\azure-cli-2.27.1.msi'
return tmp_dir, msi_path


def demo_style(cmd, theme=None): # pylint: disable=unused-argument
from azure.cli.core.style import Style, print_styled_text, format_styled_text
if theme:
Expand Down