Skip to content

{AKS} Fix azdev style issues for diagnostics & draft #7121

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.

Sign up for GitHub

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

Merged
yanzhudd merged 2 commits into from
Dec 29, 2023
Merged

{AKS} Fix azdev style issues for diagnostics & draft #7121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d435c87
fix for diagnostics & draft
FumingZhang Dec 26, 2023
0c39e9e
update
FumingZhang Dec 26, 2023
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
67 changes: 34 additions & 33 deletions src/aks-preview/azext_aks_preview/aks_diagnostics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,37 +3,37 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

import colorama
import datetime
import json
import os
import subprocess
import tempfile
import time

from azure.cli.core.commands.client_factory import get_mgmt_service_client, get_subscription_id
from azure.cli.command_modules.acs.custom import k8s_install_kubelogin
from azure.cli.command_modules.acs._params import _get_default_install_location
from enum import Flag, auto
from knack.log import get_logger
from knack.prompting import prompt_y_n
from knack.util import CLIError
from msrestazure.azure_exceptions import CloudError
from packaging import version
from tabulate import tabulate

import colorama
from azext_aks_preview._client_factory import cf_agent_pools, get_storage_client

from azext_aks_preview._consts import (
CONST_CONTAINER_NAME_MAX_LENGTH,
CONST_PERISCOPE_REPO_ORG,
CONST_PERISCOPE_CONTAINER_REGISTRY,
CONST_PERISCOPE_RELEASE_TAG,
CONST_PERISCOPE_IMAGE_VERSION,
CONST_PERISCOPE_NAMESPACE,
CONST_PERISCOPE_RELEASE_TAG,
CONST_PERISCOPE_REPO_ORG,
)

from azext_aks_preview._helpers import which, print_or_merge_credentials
from azext_aks_preview._helpers import print_or_merge_credentials, which
from azure.cli.command_modules.acs._params import _get_default_install_location
from azure.cli.command_modules.acs.custom import k8s_install_kubelogin
from azure.cli.core.commands.client_factory import (
get_mgmt_service_client,
get_subscription_id,
)
from knack.log import get_logger
from knack.prompting import prompt_y_n
from knack.util import CLIError
from msrestazure.azure_exceptions import CloudError
from packaging import version
from tabulate import tabulate

logger = get_logger(__name__)

Expand Down Expand Up @@ -87,9 +87,9 @@ def aks_kollect_cmd(cmd, # pylint: disable=too-many-statements,too-many-local
try:
parsed_storage_account = parse_resource_id(storage_account_id)
except CloudError as ex:
raise CLIError(ex.message)
raise CLIError(ex.message) from ex
else:
raise CLIError("Invalid storage account id %s" % storage_account_id)
raise CLIError(f"Invalid storage account id {storage_account_id}")

storage_account_name = parsed_storage_account['name']

Expand Down Expand Up @@ -133,11 +133,11 @@ def aks_kollect_cmd(cmd, # pylint: disable=too-many-statements,too-many-local
return

print()
print("Getting credentials for cluster %s " % name)
print(f"Getting credentials for cluster {name}")
temp_kubeconfig_path = _get_temp_kubeconfig_path(cmd, client, resource_group_name, name, mc.aad_profile is not None)

print()
print("Starts collecting diag info for cluster %s " % name)
print(f"Starts collecting diag info for cluster {name}")

# Base the container name on the fqdn (or private fqdn) of the managed cluster
container_name = _generate_container_name(mc.fqdn, mc.private_fqdn)
Expand Down Expand Up @@ -193,7 +193,7 @@ def aks_kollect_cmd(cmd, # pylint: disable=too-many-statements,too-many-local
subprocess.check_output(["kubectl", "--kubeconfig", temp_kubeconfig_path, "apply", "-k",
kustomize_folder, "-n", CONST_PERISCOPE_NAMESPACE], stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as err:
raise CLIError(err.output)
raise CLIError(err.output) from err
finally:
os.remove(kustomize_file_path)
os.rmdir(kustomize_folder)
Expand Down Expand Up @@ -322,9 +322,10 @@ def _get_storage_account_from_diag_settings(cli_ctx, resource_group_name, name):
diag_settings_client = get_mgmt_service_client(
cli_ctx, MonitorManagementClient).diagnostic_settings
subscription_id = get_subscription_id(cli_ctx)
aks_resource_id = '/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.ContainerService' \
'/managedClusters/{2}'.format(subscription_id,
resource_group_name, name)
aks_resource_id = (
f"/subscriptions/{subscription_id}/resourceGroups/{resource_group_name}/providers/"
f"Microsoft.ContainerService/managedClusters/{name}"
)
diag_settings = diag_settings_client.list(aks_resource_id)
for _, diag_setting in enumerate(diag_settings):
if diag_setting:
Expand Down Expand Up @@ -374,7 +375,7 @@ def _is_windows_hpc_supported(agent_pools):
# The full (major.minor.patch) version *may* be stored in currentOrchestratorVersion.
# If not, it'll be in orchestratorVersion.
windows_k8s_versions = [p.current_orchestrator_version or p.orchestrator_version for p in agent_pools if p.os_type.casefold() == "Windows".casefold()]
return all([version.parse(v) >= version.parse("1.23.0") for v in windows_k8s_versions])
return all((version.parse(v) >= version.parse("1.23.0") for v in windows_k8s_versions))


def _display_diagnostics_report(temp_kubeconfig_path): # pylint: disable=too-many-statements
Expand Down Expand Up @@ -420,17 +421,15 @@ def _display_diagnostics_report(temp_kubeconfig_path): # pylint: disable=too-m
if apd_lines and 'No resources found' in apd_lines[0]:
apd_lines.pop(0)

print("Got {} diagnostic results for {} ready nodes{}\r".format(len(apd_lines),
len(ready_nodes),
'.' * retry), end='')
print(f"Got {len(apd_lines)} diagnostic results for {len(ready_nodes)} ready nodes{'.' * retry}\r", end='')
if len(apd_lines) < len(ready_nodes):
time.sleep(3)
else:
apds_created = True
print()
else:
for node_name in ready_nodes:
if ready_nodes[node_name]:
for node_name, node_ready in ready_nodes.items():
if node_ready:
continue
apdName = "aks-periscope-diagnostic-" + node_name
try:
Expand All @@ -450,8 +449,10 @@ def _display_diagnostics_report(temp_kubeconfig_path): # pylint: disable=too-m
node_name, network_status)

if not network_config or not network_status:
print("The diagnostics information for node {} is not ready yet. "
"Will try again in 10 seconds.".format(node_name))
print(
f"The diagnostics information for node {node_name} is not ready yet. "
"Will try again in 10 seconds."
)
time.sleep(10)
break

Expand All @@ -462,7 +463,7 @@ def _display_diagnostics_report(temp_kubeconfig_path): # pylint: disable=too-m
network_status_object)
ready_nodes[node_name] = True
except subprocess.CalledProcessError as err:
raise CLIError(err.output)
raise CLIError(err.output) from err

print()
if network_config_array:
Expand Down
53 changes: 25 additions & 28 deletions src/aks-preview/azext_aks_preview/aks_draft/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,16 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

import json
from posixpath import dirname
from typing import Dict, List, Optional, Tuple
import subprocess
import requests
import logging
import os
import platform
import subprocess
from pathlib import Path
from typing import Dict, List, Optional, Tuple

import requests
from azext_aks_preview._consts import CONST_DRAFT_CLI_VERSION
from knack.prompting import prompt_y_n
import logging
from azext_aks_preview._consts import (
CONST_DRAFT_CLI_VERSION
)


# `az aks draft create` function
Expand Down Expand Up @@ -147,10 +144,10 @@ def _run(binary_path: str, command: str, arguments: List[str]) -> bool:
if binary_path is None:
raise ValueError('The given Binary path was null or empty')

logging.info(f'Running `az aks draft {command}`')
logging.info("Running `az aks draft %s`", command)
cmd = [binary_path, command] + arguments
process = subprocess.Popen(cmd)
exit_code = process.wait()
with subprocess.Popen(cmd) as process:
exit_code = process.wait()
return exit_code == 0


Expand Down Expand Up @@ -189,25 +186,23 @@ def _binary_pre_check(download_path: str) -> Optional[str]:
if response:
return _download_binary()
return draft_binary_path
else: # prompt the user to download binary
# If users says no, we error out and tell them that this requires the binary
msg = 'The required binary was not found. Would you like us to download the required binary for you?'

if not prompt_y_n(msg, default='n'):
raise ValueError('`az aks draft` requires the missing dependency')

return _download_binary()
# prompt the user to download binary
# If users says no, we error out and tell them that this requires the binary
msg = 'The required binary was not found. Would you like us to download the required binary for you?'
if not prompt_y_n(msg, default='n'):
raise ValueError('`az aks draft` requires the missing dependency')
return _download_binary()


# Returns True if the local binary is the latest version, False otherwise
def _is_latest_version(binary_path: str) -> bool:
latest_version = CONST_DRAFT_CLI_VERSION
process = subprocess.Popen([binary_path, 'version'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
stdout, stderr = process.communicate()
with subprocess.Popen([binary_path, 'version'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process:
stdout, stderr = process.communicate()
if stderr.decode():
return False
# return string of result is "version: v0.0.x"
current_version = stdout.decode().split('\n')[0].strip().split()[-1]
current_version = stdout.decode().split('\n', maxsplit=1)[0].strip().split()[-1]
return latest_version == current_version


Expand All @@ -220,7 +215,9 @@ def _get_filename() -> Optional[str]:
if architecture == 'x86_64':
architecture = 'amd64'
if architecture not in ['arm64', 'amd64']:
logging.error('Cannot find a suitable download for the current system architecture. Draft only supports AMD64 and ARM64.')
logging.error(
"Cannot find a suitable download for the current system architecture. Draft only supports AMD64 and ARM64."
)
return None

file_suffix = ".exe" if operating_system == "windows" else ""
Expand All @@ -243,7 +240,7 @@ def _get_existing_path() -> Optional[str]:
for path in paths:
binary_file_path = path + '/' + filename
if os.path.exists(binary_file_path):
logging.info('Existing binary found at: ' + binary_file_path)
logging.info("Existing binary found at: %s", binary_file_path)
return binary_file_path
return None

Expand Down Expand Up @@ -277,15 +274,15 @@ def _download_binary(download_path: str = '~/.aksdraft') -> Optional[str]:
# Directory
if os.path.exists(download_path) is False:
Path(download_path).mkdir(parents=True, exist_ok=True)
logging.info(f'Directory {download_path} was created inside of your HOME directory')
logging.info("Directory %s was created inside of your HOME directory", download_path)
full_path = f'{download_path}/{filename}'

# Writing the file to the local file system
with open(full_path, 'wb') as output_file:
output_file.write(response.content)
logging.info(f'Download of Draft binary was successful with a status code: {response.status_code}')
logging.info("Download of Draft binary was successful with a status code: %s", response.status_code)
os.chmod(full_path, 0o755)
return full_path

logging.error(f'Download of Draft binary was unsuccessful with a status code: {response.status_code}')
logging.error("Download of Draft binary was unsuccessful with a status code: %s", response.status_code)
return None