diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py index 1e258f18a40..6ce4aa7beb4 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py @@ -3,12 +3,9 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -from azure.cli.core.azclierror import UnknownError -from azure.cli.command_modules.acs._roleassignments import ( - add_role_assignment, - build_role_scope, - delete_role_assignments, -) +import time +from datetime import datetime + from azext_aks_preview._client_factory import get_providers_client_factory from azext_aks_preview.azurecontainerstorage._consts import ( CONST_ACSTOR_K8S_EXTENSION_NAME, @@ -16,25 +13,32 @@ CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME, CONST_K8S_EXTENSION_CUSTOM_MOD_NAME, CONST_K8S_EXTENSION_NAME, - CONST_STORAGE_POOL_NAME_PREFIX, CONST_STORAGE_POOL_OPTION_NVME, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN, CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, RP_REGISTRATION_POLLING_INTERVAL_IN_SEC, ) - -from datetime import datetime +from azure.cli.command_modules.acs._roleassignments import ( + add_role_assignment, + build_role_scope, + delete_role_assignments, +) +from azure.cli.core.azclierror import UnknownError from knack.log import get_logger -import time logger = get_logger(__name__) def register_dependent_rps(cmd, subscription_id) -> bool: required_rp = 'Microsoft.KubernetesConfiguration' - from azure.mgmt.resource.resources.models import ProviderRegistrationRequest, ProviderConsentDefinition + from azure.mgmt.resource.resources.models import ( + ProviderConsentDefinition, ProviderRegistrationRequest) - properties = ProviderRegistrationRequest(third_party_provider_consent=ProviderConsentDefinition(consent_to_authorization=False)) + properties = ProviderRegistrationRequest( + third_party_provider_consent=ProviderConsentDefinition( + consent_to_authorization=False + ) + ) client = get_providers_client_factory(cmd.cli_ctx) is_registered = False try: @@ -50,16 +54,17 @@ def register_dependent_rps(cmd, subscription_id) -> bool: is_registered = _is_rp_registered(cmd, required_rp, subscription_id) time.sleep(RP_REGISTRATION_POLLING_INTERVAL_IN_SEC) if (datetime.utcnow() - start).seconds >= timeout_secs: - logger.error("Timed out while waiting for the {0} resource provider to be registered.".format(required_rp)) + logger.error( + "Timed out while waiting for the %s resource provider to be registered.", required_rp + ) break - except Exception as e: + except Exception as e: # pylint: disable=broad-except logger.error( - "Installation of Azure Container Storage requires registering to the following resource provider: {0}. " - "We were unable to perform the registration on your behalf due to the following error: {1}\n" - "Please check with your admin on permissions, " - "or try running registration manually with: `az provider register --namespace {0}` command." - .format(required_rp, e.msg) + "Installation of Azure Container Storage requires registering to the following resource provider: %s. " + "We were unable to perform the registration on your behalf due to the following error: %s\n" + "Please check with your admin on permissions, or try running registration manually with: " + "`az provider register --namespace %s` command.", required_rp, e, required_rp ) return is_registered @@ -75,7 +80,13 @@ def should_create_storagepool( agentpool_details, nodepool_name, ): - role_assignment_success = perform_role_operations_on_managed_rg(cmd, subscription_id, node_resource_group, kubelet_identity_object_id, True) + role_assignment_success = perform_role_operations_on_managed_rg( + cmd, + subscription_id, + node_resource_group, + kubelet_identity_object_id, + True + ) return_val = True if not role_assignment_success: @@ -108,7 +119,13 @@ def should_create_storagepool( return return_val -def perform_role_operations_on_managed_rg(cmd, subscription_id, node_resource_group, kubelet_identity_object_id, assign): +def perform_role_operations_on_managed_rg( + cmd, + subscription_id, + node_resource_group, + kubelet_identity_object_id, + assign +): managed_rg_role_scope = build_role_scope(node_resource_group, None, subscription_id) roles = ["Reader", "Network Contributor", "Elastic SAN Owner", "Elastic SAN Volume Group Owner"] result = True @@ -136,7 +153,7 @@ def perform_role_operations_on_managed_rg(cmd, subscription_id, node_resource_gr if not result: break - except Exception as ex: + except Exception: # pylint: disable=broad-except break else: return True @@ -156,8 +173,8 @@ def get_k8s_extension_module(module_name): from importlib import import_module azext_custom = import_module(module_name) return azext_custom - except ImportError as ie: - raise UnknownError( + except ImportError: + raise UnknownError( # pylint: disable=raise-missing-from "Please add CLI extension `k8s-extension` for performing Azure Container Storage operations.\n" "Run command `az extension add --name k8s-extension`" ) @@ -180,7 +197,7 @@ def check_if_extension_is_installed(cmd, resource_group, cluster_name) -> bool: extension_type = extension.extension_type.lower() if extension_type != CONST_ACSTOR_K8S_EXTENSION_NAME: return_val = False - except: + except: # pylint: disable=bare-except return_val = False return return_val diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py index 7a1f4a5b98f..c54f035b2eb 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py @@ -3,23 +3,22 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- +import re + from azext_aks_preview.azurecontainerstorage._consts import ( CONST_STORAGE_POOL_OPTION_SSD, CONST_STORAGE_POOL_SKU_PREMIUM_LRS, CONST_STORAGE_POOL_SKU_PREMIUM_ZRS, - CONST_STORAGE_POOL_TYPE_AZURE_DISK, - CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN, + CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, ) - from azure.cli.core.azclierror import ( ArgumentUsageError, InvalidArgumentValueError, MutuallyExclusiveArgumentError, ) - from knack.log import get_logger -import re + elastic_san_supported_skus = [ CONST_STORAGE_POOL_SKU_PREMIUM_LRS, @@ -142,20 +141,25 @@ def _validate_enable_azure_container_storage_params( if storage_pool_sku is not None: if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: - raise ArgumentUsageError('Cannot set --storage-pool-sku when --enable-azure-container-storage is ephemeralDisk.') - elif storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN and \ - storage_pool_sku not in elastic_san_supported_skus: + raise ArgumentUsageError( + 'Cannot set --storage-pool-sku when --enable-azure-container-storage is ephemeralDisk.' + ) + if ( + storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN and + storage_pool_sku not in elastic_san_supported_skus + ): supported_skus_str = ", ".join(elastic_san_supported_skus) raise ArgumentUsageError( 'Invalid --storage-pool-sku value. ' - 'Supported value for --storage-pool-sku are {0} ' + f'Supported value for --storage-pool-sku are {supported_skus_str} ' 'when --enable-azure-container-storage is set to elasticSan.' - .format(supported_skus_str) ) if storage_pool_type != CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and \ storage_pool_option is not None: - raise ArgumentUsageError('Cannot set --storage-pool-option when --enable-azure-container-storage is not ephemeralDisk.') + raise ArgumentUsageError( + 'Cannot set --storage-pool-option when --enable-azure-container-storage is not ephemeralDisk.' + ) if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and \ storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD: @@ -171,25 +175,23 @@ def _validate_enable_azure_container_storage_params( 'Value for --storage-pool-size should be defined ' 'with size followed by Gi or Ti e.g. 512Gi or 2Ti.' ) - - else: - if storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: - pool_size_qty = float(storage_pool_size[:-2]) - pool_size_unit = storage_pool_size[-2:] - - if ( - (pool_size_unit == "Gi" and pool_size_qty < 1024) or - (pool_size_unit == "Ti" and pool_size_qty < 1) - ): - raise ArgumentUsageError( - 'Value for --storage-pool-size must be at least 1Ti when ' - '--enable-azure-container-storage is elasticSan.') - - elif storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: - logger.warning( - 'Storage pools using Ephemeral disk use all capacity available on the local device. ' - ' --storage-pool-size will be ignored.' - ) + if storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: + pool_size_qty = float(storage_pool_size[:-2]) + pool_size_unit = storage_pool_size[-2:] + + if ( + (pool_size_unit == "Gi" and pool_size_qty < 1024) or + (pool_size_unit == "Ti" and pool_size_qty < 1) + ): + raise ArgumentUsageError( + 'Value for --storage-pool-size must be at least 1Ti when ' + '--enable-azure-container-storage is elasticSan.') + + elif storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + logger.warning( + 'Storage pools using Ephemeral disk use all capacity available on the local device. ' + ' --storage-pool-size will be ignored.' + ) def _validate_nodepool_names(nodepool_names, agentpool_details): @@ -210,19 +212,16 @@ def _validate_nodepool_names(nodepool_names, agentpool_details): if nodepool not in agentpool_details: if len(agentpool_details) > 1: raise InvalidArgumentValueError( - 'Nodepool: {0} not found. ' + f'Nodepool: {nodepool} not found. ' 'Please provide a comma separated string of existing nodepool names ' 'in --azure-container-storage-nodepools.' - '\nNodepools available in the cluster are: {1}.' + f"\nNodepools available in the cluster are: {', '.join(agentpool_details)}." '\nAborting installation of Azure Container Storage.' - .format(nodepool, ', '.join(agentpool_details)) - ) - else: - raise InvalidArgumentValueError( - 'Nodepool: {0} not found. ' - 'Please provide a comma separated string of existing nodepool names ' - 'in --azure-container-storage-nodepools.' - '\nNodepool available in the cluster is: {1}.' - '\nAborting installation of Azure Container Storage.' - .format(nodepool, agentpool_details[0]) ) + raise InvalidArgumentValueError( + f'Nodepool: {nodepool} not found. ' + 'Please provide a comma separated string of existing nodepool names ' + 'in --azure-container-storage-nodepools.' + f'\nNodepool available in the cluster is: {agentpool_details[0]}.' + '\nAborting installation of Azure Container Storage.' + ) diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py index 837141c6cc4..669dd9273d4 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py @@ -20,14 +20,12 @@ CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, ) from azext_aks_preview.azurecontainerstorage._helpers import ( - check_if_extension_is_installed, get_k8s_extension_module, perform_role_operations_on_managed_rg, register_dependent_rps, should_create_storagepool, ) from knack.log import get_logger -from knack.prompting import prompt_y_n logger = get_logger(__name__) @@ -139,16 +137,16 @@ def perform_enable_azure_container_storage( create_result = LongRunningOperation(cmd.cli_ctx)(result) if create_result.provisioning_state == "Succeeded": logger.warning("Azure Container Storage successfully installed.") - except Exception as ex: + except Exception as ex: # pylint: disable=broad-except if is_cluster_create: - logger.error("Azure Container Storage failed to install.\nError: {0}".format(ex.message)) + logger.error("Azure Container Storage failed to install.\nError: %s", ex) logger.warning( "AKS cluster is created. " "Please run `az aks update` along with `--enable-azure-container-storage` " "to enable Azure Container Storage." ) else: - logger.error("AKS update to enable Azure Container Storage failed.\nError: {0}".format(ex.message)) + logger.error("AKS update to enable Azure Container Storage failed.\nError: %s", ex) def perform_disable_azure_container_storage( @@ -194,7 +192,7 @@ def perform_disable_azure_container_storage( # we don't need to long wait while performing the delete operation. # Setting no_wait_delete_op = True. no_wait_delete_op = True - except Exception as ex: + except Exception as ex: # pylint: disable=broad-except config_settings = [{"cli.storagePool.uninstallValidation": False}] k8s_extension_custom_mod.update_k8s_extension( cmd, @@ -208,15 +206,16 @@ def perform_disable_azure_container_storage( no_wait=True, ) - if ex.message.__contains__("pre-upgrade hooks failed"): + if "pre-upgrade hooks failed" in str(ex): raise UnknownError( "Validation failed. " "Please ensure that storagepools are not being used. " "Unable to disable Azure Container Storage. " "Reseting cluster state." - ) - else: - raise UnknownError("Validation failed. Unable to disable Azure Container Storage. Reseting cluster state.") + ) from ex + raise UnknownError( + "Validation failed. Unable to disable Azure Container Storage. Reseting cluster state." + ) from ex # Step 2: If the extension is installed and validation succeeded or skipped, call delete_k8s_extension try: @@ -234,7 +233,9 @@ def perform_disable_azure_container_storage( if not no_wait_delete_op: LongRunningOperation(cmd.cli_ctx)(delete_op_result) except Exception as delete_ex: - raise UnknownError("Failure observed while disabling Azure Container Storage.\nError: {0}".format(delete_ex.message)) + raise UnknownError( + "Failure observed while disabling Azure Container Storage." + ) from delete_ex logger.warning("Azure Container Storage has been disabled.")