From e74bf2cc58e0897a077f543a06479de489ee228f Mon Sep 17 00:00:00 2001 From: Akash Mukhopadhyay <43958099+mukhoakash@users.noreply.github.com> Date: Fri, 15 Mar 2024 00:30:56 -0700 Subject: [PATCH] AKS: Add changes for resource consumption for Azure Container Storage (#7215) --- src/aks-preview/HISTORY.rst | 5 + src/aks-preview/azext_aks_preview/_params.py | 21 +- .../azurecontainerstorage/_consts.py | 5 +- .../azurecontainerstorage/_helpers.py | 444 ++++++++++++--- .../azurecontainerstorage/_validators.py | 323 ++++++++--- .../azurecontainerstorage/acstor_ops.py | 529 ++++++++++++++---- src/aks-preview/azext_aks_preview/custom.py | 2 +- .../managed_cluster_decorator.py | 319 +++++++---- .../tests/latest/test_aks_commands.py | 85 +-- .../tests/latest/test_validators.py | 368 ++++++------ src/aks-preview/setup.py | 2 +- 11 files changed, 1455 insertions(+), 648 deletions(-) diff --git a/src/aks-preview/HISTORY.rst b/src/aks-preview/HISTORY.rst index ec9d8273ad9..9d7a32f244c 100644 --- a/src/aks-preview/HISTORY.rst +++ b/src/aks-preview/HISTORY.rst @@ -12,6 +12,11 @@ To release a new version, please select a new version number (usually plus 1 to Pending +++++++ +2.0.0b5 ++++++++ +* Add support to enable and disable a single type of storagepool using `--enable-azure-container-storage` and `--disable-azure-container-storage` respectively. +* Add support to define the resource allocation to Azure Container Storage applications based on the type of node pools used and storagepools enabled. + 2.0.0b4 +++++++ * Add `--enable-vtpm` to `az aks create`, `az aks nodepool add` and `az aks nodepool update`. diff --git a/src/aks-preview/azext_aks_preview/_params.py b/src/aks-preview/azext_aks_preview/_params.py index 6c76ee37e78..3ca87046f7b 100644 --- a/src/aks-preview/azext_aks_preview/_params.py +++ b/src/aks-preview/azext_aks_preview/_params.py @@ -178,6 +178,7 @@ validate_artifact_streaming, ) from azext_aks_preview.azurecontainerstorage._consts import ( + CONST_ACSTOR_ALL, CONST_STORAGE_POOL_TYPE_AZURE_DISK, CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN, @@ -316,6 +317,13 @@ CONST_STORAGE_POOL_TYPE_ELASTIC_SAN, ] +disable_storage_pool_types = [ + CONST_STORAGE_POOL_TYPE_AZURE_DISK, + CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, + CONST_STORAGE_POOL_TYPE_ELASTIC_SAN, + CONST_ACSTOR_ALL, +] + storage_pool_skus = [ CONST_STORAGE_POOL_SKU_PREMIUM_LRS, CONST_STORAGE_POOL_SKU_STANDARD_LRS, @@ -331,6 +339,13 @@ CONST_STORAGE_POOL_OPTION_SSD, ] +disable_storage_pool_options = [ + CONST_STORAGE_POOL_OPTION_NVME, + CONST_STORAGE_POOL_OPTION_SSD, + CONST_ACSTOR_ALL, +] + +# consts for guardrails level node_provisioning_modes = [ CONST_NODE_PROVISIONING_MODE_MANUAL, CONST_NODE_PROVISIONING_MODE_AUTO, @@ -1194,8 +1209,8 @@ def load_arguments(self, _): ) c.argument( "disable_azure_container_storage", - action="store_true", - help="Flag to disable azure container storage", + arg_type=get_enum_type(disable_storage_pool_types), + help="disable azure container storage or any one of the storagepool types", ) c.argument( "storage_pool_name", @@ -1212,7 +1227,7 @@ def load_arguments(self, _): ) c.argument( "storage_pool_option", - arg_type=get_enum_type(storage_pool_options), + arg_type=get_enum_type(disable_storage_pool_options), help="set ephemeral disk storage pool option for azure container storage", ) c.argument( diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_consts.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_consts.py index 0b23e1cfeb8..13f7b05f3c5 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_consts.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_consts.py @@ -3,6 +3,7 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- +CONST_ACSTOR_ALL = "all" CONST_ACSTOR_IO_ENGINE_LABEL_KEY = "acstor.azure.com/io-engine" CONST_ACSTOR_IO_ENGINE_LABEL_VAL = "acstor" CONST_ACSTOR_K8S_EXTENSION_NAME = "microsoft.azurecontainerstorage" @@ -14,7 +15,7 @@ CONST_STORAGE_POOL_DEFAULT_SIZE = "512Gi" CONST_STORAGE_POOL_NAME_PREFIX = "storagepool-" CONST_STORAGE_POOL_OPTION_NVME = "NVMe" -CONST_STORAGE_POOL_OPTION_SSD = "SSD" +CONST_STORAGE_POOL_OPTION_SSD = "Temp" CONST_STORAGE_POOL_SKU_PREMIUM_LRS = "Premium_LRS" CONST_STORAGE_POOL_SKU_PREMIUM_ZRS = "Premium_ZRS" CONST_STORAGE_POOL_SKU_PREMIUMV2_LRS = "PremiumV2_LRS" @@ -25,5 +26,3 @@ CONST_STORAGE_POOL_TYPE_AZURE_DISK = "azureDisk" CONST_STORAGE_POOL_TYPE_ELASTIC_SAN = "elasticSan" CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK = "ephemeralDisk" - -RP_REGISTRATION_POLLING_INTERVAL_IN_SEC = 5 diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py index 6ce4aa7beb4..db8d94259c2 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py @@ -3,20 +3,21 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -import time -from datetime import datetime +import re +from typing import Tuple -from azext_aks_preview._client_factory import get_providers_client_factory from azext_aks_preview.azurecontainerstorage._consts import ( + CONST_ACSTOR_IO_ENGINE_LABEL_KEY, CONST_ACSTOR_K8S_EXTENSION_NAME, CONST_EXT_INSTALLATION_NAME, CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME, CONST_K8S_EXTENSION_CUSTOM_MOD_NAME, CONST_K8S_EXTENSION_NAME, CONST_STORAGE_POOL_OPTION_NVME, + CONST_STORAGE_POOL_OPTION_SSD, + CONST_STORAGE_POOL_TYPE_AZURE_DISK, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN, CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, - RP_REGISTRATION_POLLING_INTERVAL_IN_SEC, ) from azure.cli.command_modules.acs._roleassignments import ( add_role_assignment, @@ -29,94 +30,28 @@ logger = get_logger(__name__) -def register_dependent_rps(cmd, subscription_id) -> bool: - required_rp = 'Microsoft.KubernetesConfiguration' - from azure.mgmt.resource.resources.models import ( - ProviderConsentDefinition, ProviderRegistrationRequest) - - properties = ProviderRegistrationRequest( - third_party_provider_consent=ProviderConsentDefinition( - consent_to_authorization=False - ) - ) - client = get_providers_client_factory(cmd.cli_ctx) - is_registered = False - try: - is_registered = _is_rp_registered(cmd, required_rp, subscription_id) - if is_registered: - return True - client.register(required_rp, properties=properties) - # wait for registration to finish - timeout_secs = 120 - start = datetime.utcnow() - is_registered = _is_rp_registered(cmd, required_rp, subscription_id) - while not is_registered: - 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 %s resource provider to be registered.", required_rp - ) - break - - except Exception as e: # pylint: disable=broad-except - logger.error( - "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 - - -def should_create_storagepool( +def validate_storagepool_creation( cmd, subscription_id, node_resource_group, kubelet_identity_object_id, storage_pool_type, - storage_pool_option, - agentpool_details, - nodepool_name, ): - role_assignment_success = perform_role_operations_on_managed_rg( - cmd, - subscription_id, - node_resource_group, - kubelet_identity_object_id, - True - ) - return_val = True + if storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: + role_assignment_success = perform_role_operations_on_managed_rg( + cmd, + subscription_id, + node_resource_group, + kubelet_identity_object_id, + True + ) - if not role_assignment_success: - msg = "\nUnable to add Role Assignments needed for Elastic SAN storagepools to be functional. " \ - "Please check with your admin on permissions." - if storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: - msg += "\nThis command will not create an Elastic SAN storagepool after installation." - return_val = False - msg += "\nGoing ahead with the installation of Azure Container Storage..." - logger.warning(msg) - - if not return_val: - return return_val - - if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and \ - storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME: - nodepool_list = nodepool_name.split(',') - for nodepool in nodepool_list: - agentpool_vm = agentpool_details.get(nodepool.lower()) - if agentpool_vm is not None and agentpool_vm.lower().startswith('standard_l'): - break - else: - logger.warning( - "\nNo supporting nodepool found which can support ephemeral NVMe disk " - "so this command will not create an ephemeral NVMe disk storage pool after installation." - "\nGoing ahead with the installation of Azure Container Storage..." + if not role_assignment_success: + raise UnknownError( + f"Cannot set --enable-azure-container-storage to {CONST_STORAGE_POOL_TYPE_ELASTIC_SAN}. " + "Unable to add Role Assignments needed for Elastic SAN storagepools to be functional. " + "Please check with your admin for permissions." ) - return_val = False - - return return_val def perform_role_operations_on_managed_rg( @@ -127,7 +62,7 @@ def perform_role_operations_on_managed_rg( 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"] + roles = ["Azure Container Storage Operator", "Reader"] result = True for role in roles: @@ -203,13 +138,336 @@ def check_if_extension_is_installed(cmd, resource_group, cluster_name) -> bool: return return_val -def _is_rp_registered(cmd, required_rp, subscription_id): - registered = False +def get_extension_installed_and_cluster_configs( + cmd, + resource_group, + cluster_name, + agentpool_profiles +) -> Tuple[bool, bool, bool, bool, float]: + client_factory = get_k8s_extension_module(CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME) + client = client_factory.cf_k8s_extension_operation(cmd.cli_ctx) + k8s_extension_custom_mod = get_k8s_extension_module(CONST_K8S_EXTENSION_CUSTOM_MOD_NAME) + is_extension_installed = False + is_elasticSan_enabled = False + is_azureDisk_enabled = False + is_ephemeralDisk_nvme_enabled = False + is_ephemeralDisk_localssd_enabled = False + resource_cpu_value = -1 + try: - providers_client = get_providers_client_factory(cmd.cli_ctx, subscription_id) - registration_state = getattr(providers_client.get(required_rp), 'registration_state', "NotRegistered") + extension = k8s_extension_custom_mod.show_k8s_extension( + client, + resource_group, + cluster_name, + CONST_EXT_INSTALLATION_NAME, + "managedClusters", + ) + + extension_type = extension.extension_type.lower() + is_extension_installed = extension_type == CONST_ACSTOR_K8S_EXTENSION_NAME + config_settings = extension.configuration_settings + + if is_extension_installed and config_settings is not None: + is_cli_operation_active = config_settings.get("global.cli.activeControl", "False") == "True" + if is_cli_operation_active: + is_azureDisk_enabled = ( + config_settings.get("global.cli.storagePool.azureDisk.enabled", "False") == "True" + ) + is_elasticSan_enabled = ( + config_settings.get("global.cli.storagePool.elasticSan.enabled", "False") == "True" + ) + is_ephemeralDisk_nvme_enabled = ( + config_settings.get("global.cli.storagePool.ephemeralDisk.nvme.enabled", "False") == "True" + ) + is_ephemeralDisk_localssd_enabled = ( + config_settings.get("global.cli.storagePool.ephemeralDisk.temp.enabled", "False") == "True" + ) + cpu_value = config_settings.get("global.cli.resources.ioEngine.cpu", "1") + resource_cpu_value = float(cpu_value) + else: + # For versions where "global.cli.activeControl" were not set it signifies + # that selective control of storgepool type was not yet defined. + # Hence, all the storagepool types are active and io engine core count is 1. + is_azureDisk_enabled = is_elasticSan_enabled = is_ephemeralDisk_localssd_enabled = True + resource_cpu_value = 1 + + # Determine if epehemeral NVMe was active based on the labelled nodepools present in cluster. + for agentpool in agentpool_profiles: + vm_size = agentpool.vm_size + node_labels = agentpool.node_labels + if (node_labels is not None and + node_labels.get(CONST_ACSTOR_IO_ENGINE_LABEL_KEY) is not None and + vm_size.lower().startswith('standard_l')): + is_ephemeralDisk_nvme_enabled = True + break + + except: # pylint: disable=bare-except + is_extension_installed = False + + return ( + is_extension_installed, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + resource_cpu_value + ) + + +def get_initial_resource_value_args( + storage_pool_type, + storage_pool_option, + nodepool_skus, +): + core_value = memory_value = hugepages_value = hugepages_number = 0 + if (storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK or + (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD)): + core_value = 1 + memory_value = 1 + hugepages_value = 1 + hugepages_number = 512 + elif (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME): + core_value = _get_cpu_value_based_on_vm_size(nodepool_skus) + memory_value = 2 + hugepages_value = 2 + hugepages_number = 1024 + + return _generate_k8s_extension_resource_args( + core_value, + memory_value, + hugepages_value, + hugepages_number, + ) + + +def get_current_resource_value_args( + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value=None, + nodepool_skus=None, +): + ( + current_core_value, + current_memory_value, + current_hugepages_value, + current_hugepages_number, + ) = _get_current_resource_values( + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value, + nodepool_skus, + ) + + return _generate_k8s_extension_resource_args( + current_core_value, + current_memory_value, + current_hugepages_value, + current_hugepages_number, + ) + + +def get_desired_resource_value_args( + storage_pool_type, + storage_pool_option, + current_core_value, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + nodepool_skus, + is_enabling_op, +): + ( + current_core_value, + current_memory_value, + current_hugepages_value, + current_hugepages_number, + ) = _get_current_resource_values( + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value, + nodepool_skus, + ) + + updated_core_value = updated_memory_value = \ + updated_hugepages_value = updated_hugepages_number = 0 + + if is_enabling_op: + if storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK or \ + (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD): + updated_core_value = 1 + updated_memory_value = 1 + updated_hugepages_value = 1 + updated_hugepages_number = 512 + elif (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME): + updated_core_value = _get_cpu_value_based_on_vm_size(nodepool_skus) + updated_memory_value = 2 + updated_hugepages_value = 2 + updated_hugepages_number = 1024 + + # We have decided the updated value based on the type we are enabling. + # Now, we compare and check if the current values are already greater + # than that and if so, we preserve the current values. + updated_core_value = current_core_value \ + if current_core_value > updated_core_value else updated_core_value + updated_memory_value = current_memory_value \ + if current_memory_value > updated_memory_value else updated_memory_value + updated_hugepages_value = current_hugepages_value \ + if current_hugepages_value > updated_hugepages_value else updated_hugepages_value + updated_hugepages_number = current_hugepages_number \ + if current_hugepages_number > updated_hugepages_number else updated_hugepages_number + else: + # If we are disabling AzureDisk storagepool but EphemeralDisk(any) is + # still enabled, or if we are disabling Ephemeral LocalSSD but + # AzureDisk or Ephemeral NVMe storagepool is still enabled, or + # if we are disabling ElasticSan storagepool but AzureDisk or any + # EphemeralDisk storagepool type is still enabled, + # then we will preserve the current resource values. + is_disabled_type_smaller_than_active_types = ( + (storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN and + (is_azureDisk_enabled or is_ephemeralDisk_nvme_enabled or is_ephemeralDisk_localssd_enabled)) or + (storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK and + (is_ephemeralDisk_nvme_enabled or is_ephemeralDisk_localssd_enabled)) or + (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD and + (is_azureDisk_enabled or is_ephemeralDisk_nvme_enabled)) + ) + if is_disabled_type_smaller_than_active_types: + updated_core_value = current_core_value + updated_memory_value = current_memory_value + updated_hugepages_value = current_hugepages_value + updated_hugepages_number = current_hugepages_number + elif (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME and + (is_ephemeralDisk_localssd_enabled or is_azureDisk_enabled)): + # If we are disabling Ephemeral NVMe storagepool but azureDisk is + # still enabled, we will set the azureDisk storagepool type values. + updated_core_value = 1 + updated_memory_value = 1 + updated_hugepages_value = 1 + updated_hugepages_number = 512 + + return _generate_k8s_extension_resource_args( + updated_core_value, + updated_memory_value, + updated_hugepages_value, + updated_hugepages_number, + ) + + +# get_cores_from_sku returns the number of core in the vm_size passed. +# Returns -1 if there is a problem with parsing the vm_size. +def get_cores_from_sku(vm_size): + cpu_value = -1 + pattern = r'standard_([a-z]+)(\d+)([a-z]*)_v(\d+)' + match = re.search(pattern, vm_size.lower()) + if match: + series_prefix = match.group(1) + size_val = int(match.group(2)) + version = int(match.group(4)) + + cpu_value = size_val + # https://learn.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series + # https://learn.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series-memory + if version == 2 and (series_prefix in ('d', 'ds')): + if size_val in (2, 11): + cpu_value = 2 + elif size_val in (3, 12): + cpu_value = 4 + elif size_val in (4, 13): + cpu_value = 8 + elif size_val in (5, 14): + cpu_value = 16 + elif size_val == 15: + cpu_value = 20 + + return cpu_value + + +def _get_cpu_value_based_on_vm_size(nodepool_skus): + cpu_value = -1 + for vm_size in nodepool_skus: + number_of_cores = get_cores_from_sku(vm_size) + if number_of_cores != -1: + if cpu_value == -1: + cpu_value = number_of_cores * 0.25 + else: + cpu_value = (number_of_cores * 0.25) if \ + (cpu_value > number_of_cores * 0.25) else \ + cpu_value + else: + raise UnknownError( + f"Unable to determine the number of cores in nodepool of node size: {vm_size}" + ) + + if cpu_value == -1: + cpu_value = 1 + return cpu_value + + +def _get_current_resource_values( + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value=None, + nodepool_skus=None, +): + # Setting these to default values set in the cluster when + # all the storagepools used to be enabled by default. + core_value = current_memory_value = current_hugepages_value = 1 + current_hugepages_number = 1024 + if is_elasticSan_enabled: + core_value = 0 + current_memory_value = 0 + current_hugepages_value = 0 + current_hugepages_number = 0 + if is_azureDisk_enabled or is_ephemeralDisk_localssd_enabled: + core_value = 1 + current_memory_value = 1 + current_hugepages_value = 1 + current_hugepages_number = 512 + if is_ephemeralDisk_nvme_enabled: + if current_core_value is None and nodepool_skus is not None: + core_value = _get_cpu_value_based_on_vm_size(nodepool_skus) + current_memory_value = 2 + current_hugepages_value = 2 + current_hugepages_number = 1024 + + current_core_value = current_core_value if current_core_value is not None else core_value + + return ( + current_core_value, + current_memory_value, + current_hugepages_value, + current_hugepages_number + ) + + +def _generate_k8s_extension_resource_args( + core_value, + memory_value, + hugepages_value, + hugepages_number, +): + memory_value_str = str(memory_value) + "Gi" + hugepages_value_str = str(hugepages_value) + "Gi" + + resource_args = [ + {"global.cli.resources.num_hugepages": hugepages_number}, + {"global.cli.resources.ioEngine.cpu": str(core_value)}, + {"global.cli.resources.ioEngine.memory": memory_value_str}, + {"global.cli.resources.ioEngine.hugepages2Mi": hugepages_value_str}, + ] - registered = (registration_state and registration_state.lower() == 'registered') - except Exception: # pylint: disable=broad-except - pass - return registered + return resource_args diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py index c54f035b2eb..0202df2ed9b 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py @@ -6,16 +6,26 @@ import re from azext_aks_preview.azurecontainerstorage._consts import ( + CONST_ACSTOR_ALL, + CONST_ACSTOR_IO_ENGINE_LABEL_KEY, + CONST_ACSTOR_IO_ENGINE_LABEL_VAL, + CONST_STORAGE_POOL_OPTION_NVME, 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_ELASTIC_SAN, CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, ) +from azext_aks_preview.azurecontainerstorage._helpers import ( + get_cores_from_sku +) from azure.cli.core.azclierror import ( ArgumentUsageError, InvalidArgumentValueError, MutuallyExclusiveArgumentError, + RequiredArgumentMissingError, + UnknownError, ) from knack.log import get_logger @@ -28,60 +38,23 @@ logger = get_logger(__name__) -def validate_azure_container_storage_params( - enable_azure_container_storage, - disable_azure_container_storage, - storage_pool_name, +def validate_disable_azure_container_storage_params( storage_pool_type, - storage_pool_sku, - storage_pool_option, - storage_pool_size, - nodepool_list, - agentpool_names, - is_extension_installed, -): - if enable_azure_container_storage and disable_azure_container_storage: - raise MutuallyExclusiveArgumentError( - 'Conflicting flags. Cannot set --enable-azure-container-storage ' - 'and --disable-azure-container-storage together.' - ) - - if disable_azure_container_storage: - _validate_disable_azure_container_storage_params( - storage_pool_name, - storage_pool_sku, - storage_pool_option, - storage_pool_size, - nodepool_list, - is_extension_installed, - ) - - elif enable_azure_container_storage: - _validate_enable_azure_container_storage_params( - storage_pool_name, - storage_pool_type, - storage_pool_sku, - storage_pool_option, - storage_pool_size, - is_extension_installed - ) - - _validate_nodepool_names(nodepool_list, agentpool_names) - - -def _validate_disable_azure_container_storage_params( storage_pool_name, storage_pool_sku, storage_pool_option, storage_pool_size, nodepool_list, is_extension_installed, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, ): if not is_extension_installed: raise InvalidArgumentValueError( 'Invalid usage of --disable-azure-container-storage. ' - 'Azure Container Storage is not enabled on the cluster. ' - 'Aborting disabling of Azure Container Storage.' + 'Azure Container Storage is not enabled in the cluster.' ) if storage_pool_name is not None: @@ -103,10 +76,29 @@ def _validate_disable_azure_container_storage_params( ) if storage_pool_option is not None: - raise MutuallyExclusiveArgumentError( - 'Conflicting flags. Cannot define --storage-pool-option value ' - 'when --disable-azure-container-storage is set.' - ) + if storage_pool_type != CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + raise ArgumentUsageError( + 'Cannot define --storage-pool-option value when ' + '--disable-azure-container-storage is not set ' + f'to {CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK}.' + ) + + if ((storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME and + not is_ephemeralDisk_nvme_enabled) or + (storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD and + not is_ephemeralDisk_localssd_enabled)): + raise InvalidArgumentValueError( + 'Invalid --storage-pool-option value since ephemeralDisk ' + f'of type {storage_pool_option} is not enabled in the cluster.' + ) + else: + if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and \ + is_ephemeralDisk_localssd_enabled and is_ephemeralDisk_nvme_enabled: + raise RequiredArgumentMissingError( + 'Value of --storage-pool-option must be defined since ephemeralDisk of both ' + f'the types: {CONST_STORAGE_POOL_OPTION_NVME} and {CONST_STORAGE_POOL_OPTION_SSD} ' + 'are enabled in the cluster.' + ) if nodepool_list is not None: raise MutuallyExclusiveArgumentError( @@ -114,22 +106,69 @@ def _validate_disable_azure_container_storage_params( 'when --disable-azure-container-storage is set.' ) + if storage_pool_type != CONST_ACSTOR_ALL: + is_ephemeralDisk_enabled = is_ephemeralDisk_localssd_enabled or is_ephemeralDisk_nvme_enabled + is_storagepool_type_not_active = ( + (storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK and + not is_azureDisk_enabled) or + (storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN and + not is_elasticSan_enabled) or + (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + not is_ephemeralDisk_enabled) + ) -def _validate_enable_azure_container_storage_params( - storage_pool_name, + if is_storagepool_type_not_active: + raise ArgumentUsageError( + 'Invalid --disable-azure-container-storage value. ' + 'Azure Container Storage is not enabled for storagepool ' + f'type {storage_pool_type} in the cluster.' + ) + + number_of_storagepool_types_active = 0 + if is_azureDisk_enabled: + number_of_storagepool_types_active += 1 + if is_elasticSan_enabled: + number_of_storagepool_types_active += 1 + if is_ephemeralDisk_nvme_enabled: + number_of_storagepool_types_active += 1 + if is_ephemeralDisk_localssd_enabled: + number_of_storagepool_types_active += 1 + + number_of_storagepool_types_to_be_disabled = 0 + if storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK or \ + storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN or \ + (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + storage_pool_option != CONST_ACSTOR_ALL): + number_of_storagepool_types_to_be_disabled = 1 + elif (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and + storage_pool_option == CONST_ACSTOR_ALL): + if is_ephemeralDisk_nvme_enabled: + number_of_storagepool_types_to_be_disabled += 1 + if is_ephemeralDisk_localssd_enabled: + number_of_storagepool_types_to_be_disabled += 1 + + if number_of_storagepool_types_active == number_of_storagepool_types_to_be_disabled: + raise ArgumentUsageError( + f'Since {storage_pool_type} is the only storagepool type enabled for Azure Container Storage, ' + 'disabling the storagepool type will lead to disabling Azure Container Storage from the cluster. ' + f'To disable Azure Container Storage, set --disable-azure-container-storage to {CONST_ACSTOR_ALL}.' + ) + + +def validate_enable_azure_container_storage_params( storage_pool_type, + storage_pool_name, storage_pool_sku, storage_pool_option, storage_pool_size, + nodepool_list, + agentpool_details, is_extension_installed, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, ): - if is_extension_installed: - raise InvalidArgumentValueError( - 'Invalid usage of --enable-azure-container-storage. ' - 'Azure Container Storage is already enabled on the cluster. ' - 'Aborting installation of Azure Container Storage.' - ) - if storage_pool_name is not None: pattern = r'[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*' is_pool_name_valid = re.fullmatch(pattern, storage_pool_name) @@ -155,18 +194,64 @@ def _validate_enable_azure_container_storage_params( 'when --enable-azure-container-storage is set to elasticSan.' ) - 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.' - ) + if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + if storage_pool_option is None: + raise RequiredArgumentMissingError( + 'Value of --storage-pool-option must be defined when ' + '--enable-azure-container-storage is set to ephemeralDisk.' + ) + if storage_pool_option == CONST_ACSTOR_ALL: + raise InvalidArgumentValueError( + f'Cannot set --storage-pool-option value as {CONST_ACSTOR_ALL} ' + 'when --enable-azure-container-storage is set.' + ) + else: + if storage_pool_option is not None: + 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: - raise ArgumentUsageError( - '--storage-pool-option Temp storage (SSD) currently not supported.' - ) + _validate_storage_pool_size(storage_pool_size, storage_pool_type) + + _validate_nodepools( + nodepool_list, + agentpool_details, + storage_pool_type, + storage_pool_option, + is_extension_installed + ) + if is_extension_installed: + if (is_azureDisk_enabled and + storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK) or \ + (is_elasticSan_enabled and + storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN): + raise ArgumentUsageError( + 'Invalid --enable-azure-container-storage value. ' + 'Azure Container Storage is already enabled for storagepool type ' + f'{storage_pool_type} in the cluster.' + ) + + if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and \ + ((is_ephemeralDisk_nvme_enabled and + storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME) or + (is_ephemeralDisk_localssd_enabled and + storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD)): + ephemeral_disk_type_installed = CONST_STORAGE_POOL_OPTION_SSD if \ + is_ephemeralDisk_localssd_enabled else CONST_STORAGE_POOL_OPTION_NVME + raise ArgumentUsageError( + 'Invalid --enable-azure-container-storage value. ' + 'Azure Container Storage is already enabled for storagepool type ' + f'{CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK} and option {ephemeral_disk_type_installed} ' + 'in the cluster.' + ) + + +# _Validate_storage_pool_size validates that the storage_pool_size is +# string of a combination of a float number immediately followed by +# Ti or Gi e.g. 2Ti, 512Gi, 1.5Ti. The function also validates that the +# minimum storage pool size for an elastic san storagepool should be 1Ti. +def _validate_storage_pool_size(storage_pool_size, storage_pool_type): if storage_pool_size is not None: pattern = r'^\d+(\.\d+)?[GT]i$' match = re.match(pattern, storage_pool_size) @@ -185,7 +270,8 @@ def _validate_enable_azure_container_storage_params( ): raise ArgumentUsageError( 'Value for --storage-pool-size must be at least 1Ti when ' - '--enable-azure-container-storage is elasticSan.') + '--enable-azure-container-storage is elasticSan.' + ) elif storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: logger.warning( @@ -194,34 +280,111 @@ def _validate_enable_azure_container_storage_params( ) +def _validate_nodepools( + nodepool_list, + agentpool_details, + storage_pool_type, + storage_pool_option, + is_extension_installed, +): + nodepool_arr = [] + insufficient_core_error = ( + 'Cannot operate Azure Container Storage on a node pool consisting of ' + 'nodes with cores less than 4. Node pool: {0} with node size: {1} ' + 'which is assigned for Azure Container Storage has nodes with {2} cores.' + ) + if is_extension_installed: + if nodepool_list is not None: + raise ArgumentUsageError( + 'Cannot set --azure-container-storage-nodepools while using ' + '--enable-azure-container-storage to enable a type of storagepool ' + 'in a cluster where Azure Container Storage is already installed.' + ) + + for agentpool in agentpool_details: + node_labels = agentpool.get("node_labels") + if node_labels is not None and \ + node_labels.get(CONST_ACSTOR_IO_ENGINE_LABEL_KEY) is not None: + nodepool_name = agentpool.get("name") + nodepool_arr.append(nodepool_name) + + if len(nodepool_arr) == 0: + raise ArgumentUsageError( + f'Cannot enable Azure Container Storage storagepool of type {storage_pool_type} ' + 'since none of the nodepools in the cluster are labelled for Azure Container Storage.' + ) + + insufficient_core_error = ( + f'Cannot enable Azure Container Storage storagepool type: {storage_pool_type} ' + 'on a node pool consisting of nodes with cores less than 4. ' + 'Node pool: {0} with node size: {1} has nodes with {2} cores. ' + f'Remove the label {CONST_ACSTOR_IO_ENGINE_LABEL_KEY}={CONST_ACSTOR_IO_ENGINE_LABEL_VAL} ' + 'from the node pool and use node pools which has nodes with 4 or more cores and try again.' + ) + else: + _validate_nodepool_names(nodepool_list, agentpool_details) + nodepool_arr = nodepool_list.split(',') + + nvme_nodepool_found = False + for nodepool in nodepool_arr: + for agentpool in agentpool_details: + pool_name = agentpool.get("name") + if nodepool == pool_name: + vm_size = agentpool.get("vm_size") + if vm_size is not None: + cpu_value = get_cores_from_sku(vm_size) + if cpu_value < 0: + raise UnknownError( + f'Unable to determine number of cores in node pool: {pool_name}, node size: {vm_size}' + ) + if cpu_value < 4: + raise InvalidArgumentValueError(insufficient_core_error.format(pool_name, vm_size, cpu_value)) + + if vm_size.lower().startswith('standard_l'): + nvme_nodepool_found = True + + if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and \ + storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME and \ + not nvme_nodepool_found: + raise ArgumentUsageError( + f'Cannot set --storage-pool-option as {CONST_STORAGE_POOL_OPTION_NVME} ' + 'as none of the node pools can support ephemeral NVMe disk.' + ) + + +# _validate_nodepool_names validates that the nodepool_list is a comma separated +# string consisting of valid nodepool names i.e. a lower alphanumeric +# characters and the first character should be lowercase letter. def _validate_nodepool_names(nodepool_names, agentpool_details): - # Validate that nodepool_list is a comma separated string - # consisting of valid nodepool names i.e. lower alphanumeric - # characters and the first character should be lowercase letter. pattern = r'^[a-z][a-z0-9]*(?:,[a-z][a-z0-9]*)*$' if re.fullmatch(pattern, nodepool_names) is None: raise InvalidArgumentValueError( "Invalid --azure-container-storage-nodepools value. " - "Accepted value is a comma separated string of valid nodepool " - "names without any spaces.\nA valid nodepool name may only contain lowercase " + "Accepted value is a comma separated string of valid node pool " + "names without any spaces.\nA valid node pool name may only contain lowercase " "alphanumeric characters and must begin with a lowercase letter." ) + agentpool_names = [] + for details in agentpool_details: + agentpool_names.append(details.get("name")) + nodepool_list = nodepool_names.split(',') for nodepool in nodepool_list: - if nodepool not in agentpool_details: - if len(agentpool_details) > 1: + if nodepool not in agentpool_names: + if len(agentpool_names) > 1: + agentpool_names_str = ', '.join(agentpool_names) raise InvalidArgumentValueError( - f'Nodepool: {nodepool} not found. ' - 'Please provide a comma separated string of existing nodepool names ' + f'Node pool: {nodepool} not found. ' + 'Please provide a comma separated string of existing node pool names ' 'in --azure-container-storage-nodepools.' - f"\nNodepools available in the cluster are: {', '.join(agentpool_details)}." + f'\nNode pools available in the cluster are: {agentpool_names_str}.' '\nAborting installation of Azure Container Storage.' ) raise InvalidArgumentValueError( - f'Nodepool: {nodepool} not found. ' - 'Please provide a comma separated string of existing nodepool names ' + f'Node pool: {nodepool} not found. ' + 'Please provide a comma separated string of existing node pool names ' 'in --azure-container-storage-nodepools.' - f'\nNodepool available in the cluster is: {agentpool_details[0]}.' + f'\nNode pool available in the cluster is: {agentpool_names[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 669dd9273d4..fbe54cc6b82 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py @@ -15,22 +15,25 @@ CONST_STORAGE_POOL_OPTION_NVME, CONST_STORAGE_POOL_OPTION_SSD, CONST_STORAGE_POOL_SKU_PREMIUM_LRS, + CONST_ACSTOR_ALL, CONST_STORAGE_POOL_TYPE_AZURE_DISK, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN, CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, ) from azext_aks_preview.azurecontainerstorage._helpers import ( get_k8s_extension_module, + get_current_resource_value_args, + get_desired_resource_value_args, + get_initial_resource_value_args, perform_role_operations_on_managed_rg, - register_dependent_rps, - should_create_storagepool, + validate_storagepool_creation, ) from knack.log import get_logger logger = get_logger(__name__) -def perform_enable_azure_container_storage( +def perform_enable_azure_container_storage( # pylint: disable=too-many-statements,too-many-locals,too-many-branches cmd, subscription_id, resource_group, @@ -42,101 +45,157 @@ def perform_enable_azure_container_storage( storage_pool_size, storage_pool_sku, storage_pool_option, - nodepool_names, - agentpool_details, + acstor_nodepool_skus, is_cluster_create, + is_extension_installed=False, + is_azureDisk_enabled=False, + is_elasticSan_enabled=False, + is_ephemeralDisk_localssd_enabled=False, + is_ephemeralDisk_nvme_enabled=False, + current_core_value=None, ): - # Step 1: Check and register the dependent provider for ManagedClusters i.e. - # Microsoft.KubernetesConfiguration - if not register_dependent_rps(cmd, subscription_id): - return - - if nodepool_names is None: - nodepool_names = "nodepool1" - if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: - if storage_pool_option is None: - storage_pool_option = CONST_STORAGE_POOL_OPTION_NVME - if storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD: - storage_pool_option = "temp" - - # Step 2: Validate if storagepool should be created. + # Step 1: Validate if storagepool could be created. # Depends on the following: - # 2a: Grant AKS cluster's node identity the following + # 1a: Grant AKS cluster's node identity the following # roles on the AKS managed resource group: - # 1. Reader - # 2. Network Contributor - # 3. Elastic SAN Owner - # 4. Elastic SAN Volume Group Owner + # Azure Container Storage Operator. # Ensure grant was successful if creation of # Elastic SAN storagepool is requested. - # 2b: Ensure Ls series nodepool is present if creation - # of Ephemeral NVMe Disk storagepool is requested. - create_storage_pool = should_create_storagepool( + validate_storagepool_creation( cmd, subscription_id, node_resource_group, kubelet_identity_object_id, storage_pool_type, - storage_pool_option, - agentpool_details, - nodepool_names, ) # Step 3: Configure the storagepool parameters - # Setting nodepools as empty since CLI is handling nodepool labels - config_settings = [{"cli.node.nodepools": ""}] - if create_storage_pool: - if storage_pool_name is None: - storage_pool_name = storage_pool_type.lower() - if storage_pool_size is None: - storage_pool_size = CONST_STORAGE_POOL_DEFAULT_SIZE_ESAN if \ - storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN else \ - CONST_STORAGE_POOL_DEFAULT_SIZE - config_settings.extend( - [ - {"cli.storagePool.create": True}, - {"cli.storagePool.name": storage_pool_name}, - {"cli.storagePool.size": storage_pool_size}, - {"cli.storagePool.type": storage_pool_type}, - ] + config_settings = [] + if storage_pool_name is None: + storage_pool_name = storage_pool_type.lower() + if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + storage_pool_name = storage_pool_type.lower() + "-" + storage_pool_option.lower() + if storage_pool_size is None: + storage_pool_size = CONST_STORAGE_POOL_DEFAULT_SIZE_ESAN if \ + storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN else \ + CONST_STORAGE_POOL_DEFAULT_SIZE + + azure_disk_enabled = is_azureDisk_enabled if is_extension_installed else False + elastic_san_enabled = is_elasticSan_enabled if is_extension_installed else False + ephemeral_disk_nvme_enabled = is_ephemeralDisk_nvme_enabled if is_extension_installed else False + ephemeral_disk_localssd_enabled = is_ephemeralDisk_localssd_enabled if is_extension_installed else False + + epheremaldisk_type = "" + if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + if storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME: + ephemeral_disk_nvme_enabled = True + elif storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD: + ephemeral_disk_localssd_enabled = True + epheremaldisk_type = storage_pool_option.lower() + else: + if storage_pool_sku is None: + storage_pool_sku = CONST_STORAGE_POOL_SKU_PREMIUM_LRS + if storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: + config_settings.append({"global.cli.storagePool.elasticSan.sku": storage_pool_sku}) + elastic_san_enabled = True + elif storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK: + config_settings.append({"global.cli.storagePool.azureDisk.sku": storage_pool_sku}) + azure_disk_enabled = True + + config_settings.extend( + [ + {"global.cli.activeControl": True}, + {"global.cli.storagePool.install.create": True}, + {"global.cli.storagePool.install.name": storage_pool_name}, + {"global.cli.storagePool.install.size": storage_pool_size}, + {"global.cli.storagePool.install.type": storage_pool_type}, + {"global.cli.storagePool.install.diskType": epheremaldisk_type}, + {"global.cli.storagePool.azureDisk.enabled": azure_disk_enabled}, + {"global.cli.storagePool.elasticSan.enabled": elastic_san_enabled}, + {"global.cli.storagePool.ephemeralDisk.nvme.enabled": ephemeral_disk_nvme_enabled}, + {"global.cli.storagePool.ephemeralDisk.temp.enabled": ephemeral_disk_localssd_enabled}, + # Always set cli.storagePool.disable.type to empty + # and cli.storagePool.disable.validation to False + # during enable operation so that any older disable + # operation doesn't interfere with the current state. + {"global.cli.storagePool.disable.validation": False}, + {"global.cli.storagePool.disable.type": ""}, + {"global.cli.storagePool.disable.active": False}, + # Resetting older cluster values from previous ARC version. + {"cli.storagePool.type": ""}, + {"cli.storagePool.ephemeralDisk.diskType": ""}, + ] + ) + + if is_extension_installed: + resource_args = get_desired_resource_value_args( + storage_pool_type, + storage_pool_option, + current_core_value, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + acstor_nodepool_skus, + True, ) - if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: - config_settings.append({"cli.storagePool.ephemeralDisk.diskType": storage_pool_option.lower()}) - else: - if storage_pool_sku is None: - storage_pool_sku = CONST_STORAGE_POOL_SKU_PREMIUM_LRS - if storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: - config_settings.append({"cli.storagePool.elasticSan.sku": storage_pool_sku}) - elif storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK: - config_settings.append({"cli.storagePool.azureDisk.sku": storage_pool_sku}) else: - config_settings.append({"cli.storagePool.create": False}) + resource_args = get_initial_resource_value_args( + storage_pool_type, + storage_pool_option, + acstor_nodepool_skus, + ) + + config_settings.extend(resource_args) # Step 5: Install the k8s_extension 'microsoft.azurecontainerstorage' client_factory = get_k8s_extension_module(CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME) client = client_factory.cf_k8s_extension_operation(cmd.cli_ctx) k8s_extension_custom_mod = get_k8s_extension_module(CONST_K8S_EXTENSION_CUSTOM_MOD_NAME) + update_settings = [ + {"global.cli.activeControl": True}, + {"global.cli.storagePool.install.create": False}, + {"global.cli.storagePool.install.name": ""}, + {"global.cli.storagePool.install.size": ""}, + {"global.cli.storagePool.install.type": ""}, + {"global.cli.storagePool.install.diskType": ""}, + ] try: - result = k8s_extension_custom_mod.create_k8s_extension( - cmd, - client, - resource_group, - cluster_name, - CONST_EXT_INSTALLATION_NAME, - "managedClusters", - CONST_ACSTOR_K8S_EXTENSION_NAME, - auto_upgrade_minor_version=True, - release_train="stable", - scope="cluster", - release_namespace="acstor", - configuration_settings=config_settings, - ) + if is_extension_installed: + result = k8s_extension_custom_mod.update_k8s_extension( + cmd, + client, + resource_group, + cluster_name, + CONST_EXT_INSTALLATION_NAME, + "managedClusters", + configuration_settings=config_settings, + yes=True, + no_wait=False, + ) + op_text = "Azure Container Storage successfully updated" + else: + result = k8s_extension_custom_mod.create_k8s_extension( + cmd, + client, + resource_group, + cluster_name, + CONST_EXT_INSTALLATION_NAME, + "managedClusters", + CONST_ACSTOR_K8S_EXTENSION_NAME, + auto_upgrade_minor_version=True, + release_train="stable", + scope="cluster", + release_namespace="acstor", + configuration_settings=config_settings, + ) + op_text = "Azure Container Storage successfully installed" - create_result = LongRunningOperation(cmd.cli_ctx)(result) - if create_result.provisioning_state == "Succeeded": - logger.warning("Azure Container Storage successfully installed.") + long_op_result = LongRunningOperation(cmd.cli_ctx)(result) + if long_op_result.provisioning_state == "Succeeded": + logger.warning(op_text) except Exception as ex: # pylint: disable=broad-except if is_cluster_create: logger.error("Azure Container Storage failed to install.\nError: %s", ex) @@ -146,10 +205,51 @@ def perform_enable_azure_container_storage( "to enable Azure Container Storage." ) else: - logger.error("AKS update to enable Azure Container Storage failed.\nError: %s", ex) + if is_extension_installed: + logger.error( + "AKS update to enable Azure Container Storage pool type %s failed. \n" + " Error: %s. Resetting cluster state.", storage_pool_type, ex + ) + + # If enabling of storagepool type in Azure Container Storage failed, + # define the existing resource values for ioEngine and hugepages for + # resetting the cluster state. + resource_args = get_current_resource_value_args( + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value, + ) + update_settings.extend(resource_args) + # Also, unset the type of storagepool which was supposed to enabled. + if storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK: + update_settings.append({"global.cli.storagePool.azureDisk.enabled": False}) + elif storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: + update_settings.append({"global.cli.storagePool.elasticSan.enabled": False}) + elif storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + if storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME: + update_settings.append({"global.cli.storagePool.ephemeralDisk.nvme.enabled": False}) + elif storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD: + update_settings.append({"global.cli.storagePool.ephemeralDisk.temp.enabled": False}) + else: + logger.error("AKS update to enable Azure Container Storage failed.\nError: %s", ex) -def perform_disable_azure_container_storage( + k8s_extension_custom_mod.update_k8s_extension( + cmd, + client, + resource_group, + cluster_name, + CONST_EXT_INSTALLATION_NAME, + "managedClusters", + configuration_settings=update_settings, + yes=True, + no_wait=True, + ) + + +def perform_disable_azure_container_storage( # pylint: disable=too-many-statements,too-many-locals,too-many-branches cmd, subscription_id, resource_group, @@ -157,20 +257,61 @@ def perform_disable_azure_container_storage( node_resource_group, kubelet_identity_object_id, perform_validation, + storage_pool_type, + storage_pool_option, + is_elasticSan_enabled, + is_azureDisk_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value, ): client_factory = get_k8s_extension_module(CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME) client = client_factory.cf_k8s_extension_operation(cmd.cli_ctx) k8s_extension_custom_mod = get_k8s_extension_module(CONST_K8S_EXTENSION_CUSTOM_MOD_NAME) no_wait_delete_op = False + + azure_disk_enabled = is_azureDisk_enabled + elastic_san_enabled = is_elasticSan_enabled + ephemeral_disk_nvme_enabled = is_ephemeralDisk_nvme_enabled + ephemeral_disk_localssd_enabled = is_ephemeralDisk_localssd_enabled + + # Ensure that all the install storagepool fields are turned off. + reset_install_settings = [ + {"global.cli.activeControl": True}, + {"global.cli.storagePool.install.create": False}, + {"global.cli.storagePool.install.name": ""}, + {"global.cli.storagePool.install.size": ""}, + {"global.cli.storagePool.install.type": ""}, + {"global.cli.storagePool.install.diskType": ""}, + ] + + pool_option = "" + if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + if storage_pool_option is not None: + pool_option = storage_pool_option.lower() + else: + if is_ephemeralDisk_nvme_enabled: + pool_option = CONST_STORAGE_POOL_OPTION_NVME.lower() + elif is_ephemeralDisk_localssd_enabled: + pool_option = CONST_STORAGE_POOL_OPTION_SSD.lower() + # Step 1: Perform validation if accepted by user if perform_validation: - # Set cli.node.nodepools to empty to ensure - # no post uninstall label removal takes place - # since we have added the labels to the nodepool. config_settings = [ - {"cli.storagePool.uninstallValidation": True}, - {"cli.node.nodepools": ""}, + {"global.cli.storagePool.disable.validation": True}, + {"global.cli.storagePool.disable.type": storage_pool_type}, + # Set these values to ensure cluster state incase of + # a cluster where cli operation has not yet run or older + # version of charts were installed. + {"global.cli.storagePool.azureDisk.enabled": azure_disk_enabled}, + {"global.cli.storagePool.elasticSan.enabled": elastic_san_enabled}, + {"global.cli.storagePool.ephemeralDisk.nvme.enabled": ephemeral_disk_nvme_enabled}, + {"global.cli.storagePool.ephemeralDisk.temp.enabled": ephemeral_disk_localssd_enabled}, ] + + config_settings.extend(reset_install_settings) + config_settings.append({"global.cli.storagePool.disable.diskType": pool_option.lower()}) + try: update_result = k8s_extension_custom_mod.update_k8s_extension( cmd, @@ -186,14 +327,45 @@ def perform_disable_azure_container_storage( update_long_op_result = LongRunningOperation(cmd.cli_ctx)(update_result) if update_long_op_result.provisioning_state == "Succeeded": - logger.warning("Validation succeeded. Disabling Azure Container Storage...") + logger.warning("Validation succeeded. Continuing Azure Container Storage disable operation...") # Since, pre uninstall validation will ensure deletion of storagepools, # we don't need to long wait while performing the delete operation. # Setting no_wait_delete_op = True. - no_wait_delete_op = True + # Only relevant when we are uninstalling Azure Container Storage completely. + if storage_pool_type == CONST_ACSTOR_ALL: + no_wait_delete_op = True except Exception as ex: # pylint: disable=broad-except - config_settings = [{"cli.storagePool.uninstallValidation": False}] + config_settings = [ + {"global.cli.storagePool.disable.validation": False}, + {"global.cli.storagePool.disable.type": ""}, + {"global.cli.storagePool.disable.diskType": ""}, + # Set these values to ensure cluster state incase of + # a cluster where cli operation has not yet run or older + # version of charts were installed. + {"global.cli.storagePool.azureDisk.enabled": azure_disk_enabled}, + {"global.cli.storagePool.elasticSan.enabled": elastic_san_enabled}, + {"global.cli.storagePool.ephemeralDisk.nvme.enabled": ephemeral_disk_nvme_enabled}, + {"global.cli.storagePool.ephemeralDisk.temp.enabled": ephemeral_disk_localssd_enabled}, + ] + + config_settings.extend(reset_install_settings) + + err_msg = ( + "Validation failed. " + "Please ensure that storagepools are not being used. " + "Unable to perform disable Azure Container Storage operation. " + "Resetting cluster state." + ) + + if storage_pool_type != CONST_ACSTOR_ALL: + err_msg = ( + "Validation failed. " + f"Please ensure that storagepools of type {storage_pool_type} are not being used. " + f"Unable to perform disable Azure Container Storage operation. " + "Resetting cluster state." + ) + k8s_extension_custom_mod.update_k8s_extension( cmd, client, @@ -207,42 +379,179 @@ def perform_disable_azure_container_storage( ) 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." - ) from ex + raise UnknownError(err_msg) from ex raise UnknownError( - "Validation failed. Unable to disable Azure Container Storage. Reseting cluster state." + "Validation failed. Unable to perform Azure Container Storage operation. Resetting cluster state." ) from ex - # Step 2: If the extension is installed and validation succeeded or skipped, call delete_k8s_extension - try: - delete_op_result = k8s_extension_custom_mod.delete_k8s_extension( + # Step 2: If the extension is installed and validation succeeded or skipped, + # if we want to uninstall Azure Container Storage completely, then call + # delete_k8s_extension. Else, if we want to disable a storagepool type, + # we will perform another update_k8s_extension operation on the cluster. + + if storage_pool_type == CONST_ACSTOR_ALL: + # Uninstallation operation + try: + delete_op_result = k8s_extension_custom_mod.delete_k8s_extension( + cmd, + client, + resource_group, + cluster_name, + CONST_EXT_INSTALLATION_NAME, + "managedClusters", + yes=True, + no_wait=no_wait_delete_op, + ) + + 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." + ) from delete_ex + + logger.warning("Azure Container Storage has been disabled.") + + # Revoke role assignments irrespective of whether ElasticSAN + # type was enabled on the cluster to handle older clusters where + # the role assignments were done for all storagepool type during + # installation of Azure Container Storage. + perform_role_operations_on_managed_rg( + cmd, + subscription_id, + node_resource_group, + kubelet_identity_object_id, + False + ) + else: + # Disabling a particular type of storagepool. + if storage_pool_type == CONST_STORAGE_POOL_TYPE_AZURE_DISK: + azure_disk_enabled = False + elif storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: + elastic_san_enabled = False + elif storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + if storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME: + ephemeral_disk_nvme_enabled = False + elif storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD: + ephemeral_disk_localssd_enabled = False + elif storage_pool_option == CONST_ACSTOR_ALL: + ephemeral_disk_nvme_enabled = False + ephemeral_disk_localssd_enabled = False + + config_settings = [ + {"global.cli.storagePool.disable.validation": False}, + {"global.cli.storagePool.disable.type": storage_pool_type}, + {"global.cli.storagePool.disable.active": True}, + {"global.cli.storagePool.disable.diskType": pool_option.lower()}, + # Set these values to ensure cluster state incase of + # a cluster where cli operation has not yet run or older + # version of charts were installed. + {"global.cli.storagePool.azureDisk.enabled": azure_disk_enabled}, + {"global.cli.storagePool.elasticSan.enabled": elastic_san_enabled}, + {"global.cli.storagePool.ephemeralDisk.nvme.enabled": ephemeral_disk_nvme_enabled}, + {"global.cli.storagePool.ephemeralDisk.temp.enabled": ephemeral_disk_localssd_enabled}, + ] + + config_settings.extend(reset_install_settings) + + # Define the new resource values for ioEngine and hugepages. + resource_args = get_desired_resource_value_args( + storage_pool_type, + storage_pool_option, + current_core_value, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + None, + False, + ) + config_settings.extend(resource_args) + # Creating the set of config settings which + # are required to demarcate that the disabling + # process is completed. This config variable + # will be used after the disabling operation is completed. + update_settings = [ + {"global.cli.activeControl": True}, + {"global.cli.storagePool.disable.validation": False}, + {"global.cli.storagePool.disable.type": ""}, + {"global.cli.storagePool.disable.active": False}, + {"global.cli.storagePool.disable.diskType": ""}, + ] + + update_settings.extend(reset_install_settings) + disable_op_failure = False + try: + disable_op_result = k8s_extension_custom_mod.update_k8s_extension( + cmd, + client, + resource_group, + cluster_name, + CONST_EXT_INSTALLATION_NAME, + "managedClusters", + configuration_settings=config_settings, + yes=True, + no_wait=False, + ) + LongRunningOperation(cmd.cli_ctx)(disable_op_result) + + # Revoke role assignments if we are disabling ElasticSAN storagepool type. + if storage_pool_type == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN and is_elasticSan_enabled: + perform_role_operations_on_managed_rg( + cmd, + subscription_id, + node_resource_group, + kubelet_identity_object_id, + False + ) + except Exception as disable_ex: # pylint: disable=broad-except + logger.error( + "Failure observed while disabling Azure Container Storage storagepool type: %s.\nError: %s" + "Resetting cluster state.", storage_pool_type, disable_ex + ) + + # If disabling type of storagepool in Azure Container Storage failed, + # define the existing resource values for ioEngine and hugepages for + # resetting the cluster state. + resource_args = get_current_resource_value_args( + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value, + ) + + update_settings.extend(resource_args) + # Revert back to storagepool type states which was supposed to disabled. + azure_disk_enabled = is_azureDisk_enabled + elastic_san_enabled = is_elasticSan_enabled + ephemeral_disk_nvme_enabled = is_ephemeralDisk_nvme_enabled + ephemeral_disk_localssd_enabled = is_ephemeralDisk_localssd_enabled + disable_op_failure = True + + # Set the types of storagepool type states in the cluster + # based on whether the previous operation succeeded or failed. + update_settings.extend( + [ + {"global.cli.storagePool.azureDisk.enabled": azure_disk_enabled}, + {"global.cli.storagePool.elasticSan.enabled": elastic_san_enabled}, + {"global.cli.storagePool.ephemeralDisk.nvme.enabled": ephemeral_disk_nvme_enabled}, + {"global.cli.storagePool.ephemeralDisk.temp.enabled": ephemeral_disk_localssd_enabled}, + ] + ) + # Since we are just resetting the cluster state, + # we are going to perform a non waiting operation. + k8s_extension_custom_mod.update_k8s_extension( cmd, client, resource_group, cluster_name, CONST_EXT_INSTALLATION_NAME, "managedClusters", + configuration_settings=update_settings, yes=True, - no_wait=no_wait_delete_op, + no_wait=True, ) - 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." - ) from delete_ex - - logger.warning("Azure Container Storage has been disabled.") - - # Step 3: Revoke AKS cluster's node identity the following - # roles on the AKS managed resource group: - # 1. Reader - # 2. Network Contributor - # 3. Elastic SAN Owner - # 4. Elastic SAN Volume Group Owner - perform_role_operations_on_managed_rg(cmd, subscription_id, node_resource_group, kubelet_identity_object_id, False) + if not disable_op_failure: + logger.warning("Azure Container Storage storagepool type %s has been disabled.", storage_pool_type) diff --git a/src/aks-preview/azext_aks_preview/custom.py b/src/aks-preview/azext_aks_preview/custom.py index 5269b81cfc2..97152c478c0 100644 --- a/src/aks-preview/azext_aks_preview/custom.py +++ b/src/aks-preview/azext_aks_preview/custom.py @@ -805,7 +805,7 @@ def aks_update( disable_ai_toolchain_operator=False, # azure container storage enable_azure_container_storage=None, - disable_azure_container_storage=False, + disable_azure_container_storage=None, storage_pool_name=None, storage_pool_size=None, storage_pool_sku=None, diff --git a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py index 79d8dfa729b..072936b17c8 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -3073,29 +3073,31 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: if not mc.agent_pool_profiles: raise UnknownError("Encountered an unexpected error while getting the agent pools from the cluster.") agentpool = mc.agent_pool_profiles[0] - agentpool_name_list = [agentpool.name] + agentpool_details = [] + pool_details = {} + pool_details["name"] = agentpool.name + pool_details["vm_size"] = agentpool.vm_size + agentpool_details.append(pool_details) # Marking the only agentpool name as the valid nodepool for # installing Azure Container Storage during `az aks create` nodepool_list = agentpool.name - from azext_aks_preview.azurecontainerstorage._helpers import check_if_extension_is_installed - is_extension_already_installed = check_if_extension_is_installed( - self.cmd, - self.context.get_resource_group_name(), - self.context.get_name(), + from azext_aks_preview.azurecontainerstorage._validators import ( + validate_enable_azure_container_storage_params ) - from azext_aks_preview.azurecontainerstorage._validators import validate_azure_container_storage_params - validate_azure_container_storage_params( - True, - None, - pool_name, + validate_enable_azure_container_storage_params( pool_type, + pool_name, pool_sku, pool_option, pool_size, nodepool_list, - agentpool_name_list, - is_extension_already_installed, + agentpool_details, + False, + False, + False, + False, + False, ) # Setup Azure Container Storage labels on the nodepool @@ -3504,12 +3506,11 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: pool_size = self.context.raw_param.get("storage_pool_size") kubelet_identity_object_id = cluster.identity_profile["kubeletidentity"].object_id node_resource_group = cluster.node_resource_group - nodepool_name = self.context.get_intermediate("azure_container_storage_nodepools") - agent_pool_details = {} + agent_pool_vm_sizes = [] if len(cluster.agent_pool_profiles) > 0: # Cluster creation has only 1 agentpool agentpool_profile = cluster.agent_pool_profiles[0] - agent_pool_details[agentpool_profile.name] = agentpool_profile.vm_size + agent_pool_vm_sizes.append(agentpool_profile.vm_size) self.context.external_functions.perform_enable_azure_container_storage( self.cmd, @@ -3523,8 +3524,7 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: pool_size, pool_sku, pool_option, - nodepool_name, - agent_pool_details, + agent_pool_vm_sizes, True, ) @@ -3678,109 +3678,194 @@ def update_enable_network_observability_in_network_profile(self, mc: ManagedClus ) return mc + # pylint: disable=too-many-statements,too-many-locals def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: """Update azure container storage for the Managed Cluster object :return: ManagedCluster """ self._ensure_mc(mc) # read the azure container storage values passed - pool_type = self.context.raw_param.get("enable_azure_container_storage") - disable_azure_container_storage = self.context.raw_param.get("disable_azure_container_storage") - enable_azure_container_storage = pool_type is not None + enable_pool_type = self.context.raw_param.get("enable_azure_container_storage") + disable_pool_type = self.context.raw_param.get("disable_azure_container_storage") + enable_azure_container_storage = enable_pool_type is not None + disable_azure_container_storage = disable_pool_type is not None nodepool_list = self.context.raw_param.get("azure_container_storage_nodepools") + if enable_azure_container_storage and disable_azure_container_storage: + raise MutuallyExclusiveArgumentError( + 'Conflicting flags. Cannot set --enable-azure-container-storage ' + 'and --disable-azure-container-storage together.' + ) + + # pylint: disable=too-many-nested-blocks if enable_azure_container_storage or disable_azure_container_storage: + # Require the agent pool profiles for azure container storage + # operations. Raise exception if not found. + if not mc.agent_pool_profiles: + raise UnknownError( + "Encounter an unexpected error while getting agent pool profiles from the cluster " + "in the process of updating agentpool profile." + ) pool_name = self.context.raw_param.get("storage_pool_name") pool_option = self.context.raw_param.get("storage_pool_option") pool_sku = self.context.raw_param.get("storage_pool_sku") pool_size = self.context.raw_param.get("storage_pool_size") - agentpool_names = [] - # Incase of enable_azure_container_storage, - # we will collect the agentpool_names and - # handle the default values of the nodepool - # on which we will install Azure Container Storage. - # Incase of disable_azure_container_storage, - # the expected value is None and no further - # processing is required on the nodepool list. - if enable_azure_container_storage: - if not mc.agent_pool_profiles: - raise UnknownError( - "Encounter an unexpected error while getting agent pool profiles from the cluster " - "in the process of updating agentpool profile." - ) - - for agentpool in mc.agent_pool_profiles: - if agentpool.name is None: - raise UnknownError("Unexpected error while getting agent pool name.") - agentpool_names.append(agentpool.name) - - if nodepool_list is None: - # When not defined, either pick nodepool1 as default or if only - # one nodepool exists. Choose the only nodepool by default. - nodepool_list = "nodepool1" - if len(agentpool_names) == 1: - nodepool_list = agentpool_names[0] - from azext_aks_preview.azurecontainerstorage._helpers import check_if_extension_is_installed - is_extension_already_installed = check_if_extension_is_installed( + agentpool_details = [] + from azext_aks_preview.azurecontainerstorage._helpers import get_extension_installed_and_cluster_configs + ( + is_extension_installed, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value, + ) = get_extension_installed_and_cluster_configs( self.cmd, self.context.get_resource_group_name(), self.context.get_name(), - ) - from azext_aks_preview.azurecontainerstorage._validators import validate_azure_container_storage_params - validate_azure_container_storage_params( - enable_azure_container_storage, - disable_azure_container_storage, - pool_name, - pool_type, - pool_sku, - pool_option, - pool_size, - nodepool_list, - agentpool_names, - is_extension_already_installed, + mc.agent_pool_profiles, ) - if enable_azure_container_storage: - # Set Azure Container Storage labels on the required nodepools - azure_container_storage_nodepools_list = nodepool_list.split(',') - from azext_aks_preview.azurecontainerstorage._consts import ( - CONST_ACSTOR_IO_ENGINE_LABEL_KEY, - CONST_ACSTOR_IO_ENGINE_LABEL_VAL - ) - for agentpool in mc.agent_pool_profiles: - labels = agentpool.node_labels - if agentpool.name in azure_container_storage_nodepools_list: - if labels is None: - labels = {} - labels[CONST_ACSTOR_IO_ENGINE_LABEL_KEY] = CONST_ACSTOR_IO_ENGINE_LABEL_VAL + if enable_azure_container_storage: + from azext_aks_preview.azurecontainerstorage._consts import ( + CONST_ACSTOR_IO_ENGINE_LABEL_KEY, + CONST_ACSTOR_IO_ENGINE_LABEL_VAL + ) + labelled_nodepool_arr = [] + for agentpool in mc.agent_pool_profiles: + pool_details = {} + pool_details["vm_size"] = agentpool.vm_size + node_name = agentpool.name + pool_details["name"] = node_name + if agentpool.node_labels is not None: + node_labels = agentpool.node_labels + if node_labels is not None and \ + node_labels.get(CONST_ACSTOR_IO_ENGINE_LABEL_KEY) is not None and \ + node_name is not None: + labelled_nodepool_arr.append(node_name) + pool_details["node_labels"] = node_labels + agentpool_details.append(pool_details) + + # Incase of a new installation, if the nodepool list is not defined + # then check for all the nodepools which are marked with acstor io-engine + # labels and include them for installation. If none of the nodepools are + # labelled, either pick nodepool1 as default, or if only + # one nodepool exists, choose the only nodepool by default. + if not is_extension_installed: + if nodepool_list is None: + nodepool_list = "nodepool1" + if len(labelled_nodepool_arr) > 0: + nodepool_list = ','.join(labelled_nodepool_arr) + elif len(agentpool_details) == 1: + pool_detail = agentpool_details[0] + nodepool_list = pool_detail.get("name") + + from azext_aks_preview.azurecontainerstorage._validators import ( + validate_enable_azure_container_storage_params + ) + validate_enable_azure_container_storage_params( + enable_pool_type, + pool_name, + pool_sku, + pool_option, + pool_size, + nodepool_list, + agentpool_details, + is_extension_installed, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + ) + + # If the extension is already installed, + # we expect that the Azure Container Storage + # nodes are already labelled. Use those label + # to generate the nodepool_list. + if is_extension_installed: + nodepool_list = ','.join(labelled_nodepool_arr) else: - # Remove residual Azure Container Storage labels - # from any other nodepools where its not intended - if labels is not None: - labels.pop(CONST_ACSTOR_IO_ENGINE_LABEL_KEY, None) - agentpool.node_labels = labels + # Set Azure Container Storage labels on the required nodepools. + nodepool_list_arr = nodepool_list.split(',') + for agentpool in mc.agent_pool_profiles: + labels = agentpool.node_labels + if agentpool.name in nodepool_list_arr: + if labels is None: + labels = {} + labels[CONST_ACSTOR_IO_ENGINE_LABEL_KEY] = CONST_ACSTOR_IO_ENGINE_LABEL_VAL + else: + # Remove residual Azure Container Storage labels + # from any other nodepools where its not intended + if labels is not None: + labels.pop(CONST_ACSTOR_IO_ENGINE_LABEL_KEY, None) + agentpool.node_labels = labels + + # set intermediates + self.context.set_intermediate("azure_container_storage_nodepools", nodepool_list, overwrite_exists=True) + self.context.set_intermediate("enable_azure_container_storage", True, overwrite_exists=True) + if disable_azure_container_storage: + from azext_aks_preview.azurecontainerstorage._validators import ( + validate_disable_azure_container_storage_params + ) + validate_disable_azure_container_storage_params( + disable_pool_type, + pool_name, + pool_sku, + pool_option, + pool_size, + nodepool_list, + is_extension_installed, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + ) + pre_disable_validate = False - # set intermediates - self.context.set_intermediate("enable_azure_container_storage", True, overwrite_exists=True) - self.context.set_intermediate("azure_container_storage_nodepools", nodepool_list, overwrite_exists=True) + msg = ( + "Disabling Azure Container Storage will forcefully delete all the storagepools in the cluster and " + "affect the applications using these storagepools. Forceful deletion of storagepools can also " + "lead to leaking of storage resources which are being consumed. Do you want to validate whether " + "any of the storagepools are being used before disabling Azure Container Storage?" + ) - if disable_azure_container_storage: - pre_uninstall_validate = False - msg = ( - "Disabling Azure Container Storage will forcefully delete all the storagepools on the cluster and " - "affect the applications using these storagepools. Forceful deletion of storagepools can also lead to " - "leaking of storage resources which are being consumed. Do you want to validate whether any of " - "the storagepools are being used before disabling Azure Container Storage?" - ) - if self.context.get_yes() or prompt_y_n(msg, default="y"): - pre_uninstall_validate = True + from azext_aks_preview.azurecontainerstorage._consts import ( + CONST_ACSTOR_ALL, + ) + if disable_pool_type != CONST_ACSTOR_ALL: + msg = ( + f"Disabling Azure Container Storage for storagepool type {disable_pool_type} " + "will forcefully delete all the storagepools of the same type and affect the " + "applications using these storagepools. Forceful deletion of storagepools can " + "also lead to leaking of storage resources which are being consumed. Do you want to " + f"validate whether any of the storagepools of type {disable_pool_type} are being used " + "before disabling Azure Container Storage?" + ) + if self.context.get_yes() or prompt_y_n(msg, default="y"): + pre_disable_validate = True + + # set intermediate + self.context.set_intermediate("disable_azure_container_storage", True, overwrite_exists=True) + self.context.set_intermediate( + "pre_disable_validate_azure_container_storage", + pre_disable_validate, + overwrite_exists=True + ) - # set intermediate - self.context.set_intermediate("disable_azure_container_storage", True, overwrite_exists=True) + # Set intermediates + self.context.set_intermediate("is_extension_installed", is_extension_installed, overwrite_exists=True) + self.context.set_intermediate("is_azureDisk_enabled", is_azureDisk_enabled, overwrite_exists=True) + self.context.set_intermediate("is_elasticSan_enabled", is_elasticSan_enabled, overwrite_exists=True) self.context.set_intermediate( - "pre_uninstall_validate_azure_container_storage", - pre_uninstall_validate, + "is_ephemeralDisk_nvme_enabled", + is_ephemeralDisk_nvme_enabled, overwrite_exists=True ) + self.context.set_intermediate( + "is_ephemeralDisk_localssd_enabled", + is_ephemeralDisk_localssd_enabled, + overwrite_exists=True + ) + self.context.set_intermediate("current_core_value", current_core_value, overwrite_exists=True) return mc @@ -4720,27 +4805,32 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: super().postprocessing_after_mc_created(cluster) enable_azure_container_storage = self.context.get_intermediate("enable_azure_container_storage") disable_azure_container_storage = self.context.get_intermediate("disable_azure_container_storage") + is_extension_installed = self.context.get_intermediate("is_extension_installed") + is_azureDisk_enabled = self.context.get_intermediate("is_azureDisk_enabled") + is_elasticSan_enabled = self.context.get_intermediate("is_elasticSan_enabled") + is_ephemeralDisk_localssd_enabled = self.context.get_intermediate("is_ephemeralDisk_localssd_enabled") + is_ephemeralDisk_nvme_enabled = self.context.get_intermediate("is_ephemeralDisk_nvme_enabled") + current_core_value = self.context.get_intermediate("current_core_value") + pool_option = self.context.raw_param.get("storage_pool_option") - if enable_azure_container_storage or disable_azure_container_storage: + # enable azure container storage + if enable_azure_container_storage: if cluster.identity_profile is None or cluster.identity_profile["kubeletidentity"] is None: logger.warning( "Unexpected error getting kubelet's identity for the cluster." "Unable to perform azure container storage operation." ) return - - # enable azure container storage - if enable_azure_container_storage: pool_name = self.context.raw_param.get("storage_pool_name") pool_type = self.context.raw_param.get("enable_azure_container_storage") - pool_option = self.context.raw_param.get("storage_pool_option") pool_sku = self.context.raw_param.get("storage_pool_sku") pool_size = self.context.raw_param.get("storage_pool_size") nodepool_list = self.context.get_intermediate("azure_container_storage_nodepools") kubelet_identity_object_id = cluster.identity_profile["kubeletidentity"].object_id - agent_pool_details = {} + acstor_nodepool_skus = [] for agentpool_profile in cluster.agent_pool_profiles: - agent_pool_details[agentpool_profile.name] = agentpool_profile.vm_size + if agentpool_profile.name in nodepool_list: + acstor_nodepool_skus.append(agentpool_profile.vm_size) self.context.external_functions.perform_enable_azure_container_storage( self.cmd, @@ -4754,15 +4844,21 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: pool_size, pool_sku, pool_option, - nodepool_list, - agent_pool_details, + acstor_nodepool_skus, False, + is_extension_installed, + is_azureDisk_enabled, + is_elasticSan_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value, ) # disable azure container storage if disable_azure_container_storage: + pool_type = self.context.raw_param.get("disable_azure_container_storage") kubelet_identity_object_id = cluster.identity_profile["kubeletidentity"].object_id - pre_uninstall_validate = self.context.get_intermediate("pre_uninstall_validate_azure_container_storage") + pre_disable_validate = self.context.get_intermediate("pre_disable_validate_azure_container_storage") self.context.external_functions.perform_disable_azure_container_storage( self.cmd, self.context.get_subscription_id(), @@ -4770,7 +4866,14 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: self.context.get_name(), self.context.get_node_resource_group(), kubelet_identity_object_id, - pre_uninstall_validate, + pre_disable_validate, + pool_type, + pool_option, + is_elasticSan_enabled, + is_azureDisk_enabled, + is_ephemeralDisk_localssd_enabled, + is_ephemeralDisk_nvme_enabled, + current_core_value, ) # attach keyvault to app routing addon diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py b/src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py index 9c9a389604f..e34a4146d95 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py @@ -10643,53 +10643,56 @@ def test_aks_update_with_azuremonitormetrics( ], ) - # live only due to downloading k8s-extension extension - # @live_only() - # Introduce this test back once v1.0.3-preview version of Azure container storage is released - # @AllowLargeResponse(8192) - # @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix='clitest', location='westus2') - # def test_aks_update_with_azurecontainerstorage(self, resource_group, resource_group_location): - # aks_name = self.create_random_name('cliakstest', 16) - # node_vm_size = 'standard_d4s_v3' - # self.kwargs.update({ - # 'resource_group': resource_group, - # 'name': aks_name, - # 'location': resource_group_location, - # 'ssh_key_value': self.generate_ssh_keys(), - # 'node_vm_size': node_vm_size, - # }) + @live_only() + @AllowLargeResponse(8192) + @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix='clitest', location='westus2') + def test_aks_update_with_azurecontainerstorage(self, resource_group, resource_group_location): + aks_name = self.create_random_name('cliakstest', 16) + node_vm_size = 'standard_d4s_v3' + self.kwargs.update({ + 'resource_group': resource_group, + 'name': aks_name, + 'location': resource_group_location, + 'ssh_key_value': self.generate_ssh_keys(), + 'node_vm_size': node_vm_size, + }) - # # add k8s-extension extension for azurecontainerstorage operations. - # self.cmd('extension add --name k8s-extension') + # add k8s-extension extension for azurecontainerstorage operations. + self.cmd('extension add --name k8s-extension') - # # create: without enable-azure-container-storage - # create_cmd = 'aks create --resource-group={resource_group} --name={name} --location={location} --ssh-key-value={ssh_key_value} --node-vm-size={node_vm_size} --node-count 3 --enable-managed-identity --output=json' - # self.cmd(create_cmd, checks=[ - # self.check('provisioningState', 'Succeeded'), - # ]) + # create: without enable-azure-container-storage + create_cmd = 'aks create --resource-group={resource_group} --name={name} --location={location} --ssh-key-value={ssh_key_value} --node-vm-size={node_vm_size} --node-count 3 --enable-managed-identity --output=json' + self.cmd(create_cmd, checks=[ + self.check('provisioningState', 'Succeeded'), + ]) - # # enabling or disabling azurecontainerstorage will not affect any field in the cluster. - # # the only check we should perform is to verify that the cluster is provisioned successfully. + # enabling or disabling azurecontainerstorage will not affect any field in the cluster. + # the only check we should perform is to verify that the cluster is provisioned successfully. - # # update: enable-azure-container-storage - # update_cmd = 'aks update --resource-group={resource_group} --name={name} --yes --output=json ' \ - # '--enable-azure-container-storage azureDisk' - # self.cmd(update_cmd, checks=[ - # self.check('provisioningState', 'Succeeded'), - # ]) + # update: enable-azure-container-storage + update_cmd = 'aks update --resource-group={resource_group} --name={name} --yes --output=json ' \ + '--enable-azure-container-storage azureDisk' + self.cmd(update_cmd, checks=[ + self.check('provisioningState', 'Succeeded'), + ]) - # # update: disable-azure-container-storage - # update_cmd = 'aks update --resource-group={resource_group} --name={name} --yes --output=json ' \ - # '--disable-azure-container-storage' - # self.cmd(update_cmd, checks=[ - # self.check('provisioningState', 'Succeeded'), - # ]) + # Sleep for 5 mins before next operation, + # since azure container storage operations take + # some time to post process. + time.sleep(5 * 60) - # # delete - # cmd = 'aks delete --resource-group={resource_group} --name={name} --yes --no-wait' - # self.cmd(cmd, checks=[ - # self.is_empty(), - # ]) + # update: disable-azure-container-storage + update_cmd = 'aks update --resource-group={resource_group} --name={name} --yes --output=json ' \ + '--disable-azure-container-storage all' + self.cmd(update_cmd, checks=[ + self.check('provisioningState', 'Succeeded'), + ]) + + # delete + cmd = 'aks delete --resource-group={resource_group} --name={name} --yes --no-wait' + self.cmd(cmd, checks=[ + self.is_empty(), + ]) # live only due to workspace is not mocked correctly @AllowLargeResponse() diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py b/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py index cfbaed0c7dd..0012fdc2a8e 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_validators.py @@ -13,6 +13,7 @@ ArgumentUsageError, InvalidArgumentValueError, MutuallyExclusiveArgumentError, + RequiredArgumentMissingError, ) from azure.cli.core.util import CLIError @@ -711,37 +712,16 @@ def test_valid_start_time(self): validators.validate_start_time(namespace) -class TestValidateAzureContainerStorage(unittest.TestCase): - def test_conflicting_flags_for_enable_disable(self): - err = ( - "Conflicting flags. Cannot set --enable-azure-container-storage " - "and --disable-azure-container-storage together." - ) - with self.assertRaises(MutuallyExclusiveArgumentError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, True, None, None, None, None, None, None, None, False - ) - self.assertEqual(str(cm.exception), err) - +class TestValidateDisableAzureContainerStorage(unittest.TestCase): def test_disable_when_extension_not_installed(self): is_extension_installed = False err = ( "Invalid usage of --disable-azure-container-storage. " - "Azure Container Storage is not enabled on the cluster. " - "Aborting disabling of Azure Container Storage." + "Azure Container Storage is not enabled in the cluster." ) with self.assertRaises(InvalidArgumentValueError) as cm: - acstor_validator.validate_azure_container_storage_params( - None, - True, - None, - None, - None, - None, - None, - None, - None, - is_extension_installed, + acstor_validator.validate_disable_azure_container_storage_params( + None, None, None, None, None, None, is_extension_installed, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -752,8 +732,8 @@ def test_disable_flag_with_storage_pool_name(self): "when --disable-azure-container-storage is set." ) with self.assertRaises(MutuallyExclusiveArgumentError) as cm: - acstor_validator.validate_azure_container_storage_params( - None, True, storage_pool_name, None, None, None, None, None, None, True + acstor_validator.validate_disable_azure_container_storage_params( + None, storage_pool_name, None, None, None, None, True, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -764,8 +744,8 @@ def test_disable_flag_with_storage_pool_sku(self): "when --disable-azure-container-storage is set." ) with self.assertRaises(MutuallyExclusiveArgumentError) as cm: - acstor_validator.validate_azure_container_storage_params( - None, True, None, None, storage_pool_sku, None, None, None, None, True + acstor_validator.validate_disable_azure_container_storage_params( + None, None, storage_pool_sku, None, None, None, True, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -776,71 +756,98 @@ def test_disable_flag_with_storage_pool_size(self): "when --disable-azure-container-storage is set." ) with self.assertRaises(MutuallyExclusiveArgumentError) as cm: - acstor_validator.validate_azure_container_storage_params( - None, True, None, None, None, None, storage_pool_size, None, None, True + acstor_validator.validate_disable_azure_container_storage_params( + None, None, None, None, storage_pool_size, None, True, False, False, False, False ) self.assertEqual(str(cm.exception), err) - def test_disable_flag_with_storage_pool_option(self): + def test_disable_flag_with_storage_pool_option_not_ephemeralDisk(self): storage_pool_option = acstor_consts.CONST_STORAGE_POOL_OPTION_NVME + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK err = ( - "Conflicting flags. Cannot define --storage-pool-option value " - "when --disable-azure-container-storage is set." + "Cannot define --storage-pool-option value when " + "--disable-azure-container-storage is not set to ephemeralDisk." ) - with self.assertRaises(MutuallyExclusiveArgumentError) as cm: - acstor_validator.validate_azure_container_storage_params( - None, - True, - None, - None, - None, - storage_pool_option, - None, - None, - None, - True, + with self.assertRaises(ArgumentUsageError) as cm: + acstor_validator.validate_disable_azure_container_storage_params( + storage_pool_type, None, None, storage_pool_option, None, None, True, False, False, False, False + ) + self.assertEqual(str(cm.exception), err) + + def test_disable_flag_with_storage_pool_option_not_set_both_ephemeralDisk_enabled(self): + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + err = ( + "Value of --storage-pool-option must be defined since ephemeralDisk of both " + "the types: NVMe and Temp are enabled in the cluster." + ) + with self.assertRaises(RequiredArgumentMissingError) as cm: + acstor_validator.validate_disable_azure_container_storage_params( + storage_pool_type, None, None, None, None, None, True, False, False, True, True ) self.assertEqual(str(cm.exception), err) def test_disable_flag_with_nodepool_list(self): nodepool_list = "test,test1" + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK err = ( "Conflicting flags. Cannot define --azure-container-storage-nodepools value " "when --disable-azure-container-storage is set." ) with self.assertRaises(MutuallyExclusiveArgumentError) as cm: - acstor_validator.validate_azure_container_storage_params( - None, True, None, None, None, None, None, nodepool_list, None, True + acstor_validator.validate_disable_azure_container_storage_params( + storage_pool_type, None, None, None, None, nodepool_list, True, False, False, False, False ) self.assertEqual(str(cm.exception), err) - def test_valid_disable(self): - acstor_validator.validate_azure_container_storage_params( - None, True, None, None, None, None, None, None, None, True + def test_disable_type_when_not_enabled(self): + pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK + is_azureDisk_enabled = False + err = ( + "Invalid --disable-azure-container-storage value. " + "Azure Container Storage is not enabled for storagepool " + "type {0} in the cluster.".format(pool_type) ) + with self.assertRaises(ArgumentUsageError) as cm: + acstor_validator.validate_disable_azure_container_storage_params( + pool_type, None, None, None, None, None, True, is_azureDisk_enabled, False, False, False + ) + self.assertEqual(str(cm.exception), err) - def test_enable_when_extension_installed(self): - is_extension_installed = True + def test_disable_only_storage_pool_installed(self): + pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK err = ( - "Invalid usage of --enable-azure-container-storage. " - "Azure Container Storage is already enabled on the cluster. " - "Aborting installation of Azure Container Storage." + "Since azureDisk is the only storagepool type enabled for Azure Container Storage, " + "disabling the storagepool type will lead to disabling Azure Container Storage from the cluster. " + "To disable Azure Container Storage, set --disable-azure-container-storage to all." ) - with self.assertRaises(InvalidArgumentValueError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - None, - None, - None, - None, - None, - None, - None, - is_extension_installed, + with self.assertRaises(ArgumentUsageError) as cm: + acstor_validator.validate_disable_azure_container_storage_params( + pool_type, None, None, None, None, None, True, True, False, False, False + ) + self.assertEqual(str(cm.exception), err) + + def test_disable_only_storagepool_type_enabled(self): + pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK + is_azureDisk_enabled = True + err = ( + "Since azureDisk is the only storagepool type enabled for Azure Container Storage, " + "disabling the storagepool type will lead to disabling Azure Container Storage from the cluster. " + "To disable Azure Container Storage, set --disable-azure-container-storage to all." + ) + with self.assertRaises(ArgumentUsageError) as cm: + acstor_validator.validate_disable_azure_container_storage_params( + pool_type, None, None, None, None, None, True, is_azureDisk_enabled, False, False, False ) self.assertEqual(str(cm.exception), err) + def test_valid_disable(self): + pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_ELASTIC_SAN + acstor_validator.validate_disable_azure_container_storage_params( + pool_type, None, None, None, None, None, True, False, True, True, False + ) + + +class TestValidateEnableAzureContainerStorage(unittest.TestCase): def test_enable_with_invalid_storage_pool_name(self): storage_pool_name = "my_test_pool" err = ( @@ -849,8 +856,8 @@ def test_enable_with_invalid_storage_pool_name(self): "'-' or '.', and must start and end with an alphanumeric character." ) with self.assertRaises(InvalidArgumentValueError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, None, storage_pool_name, None, None, None, None, None, None, False + acstor_validator.validate_enable_azure_container_storage_params( + None, storage_pool_name, None, None, None, None, None, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -860,17 +867,8 @@ def test_enable_with_sku_and_ephemeral_disk_pool(self): storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK err = "Cannot set --storage-pool-sku when --enable-azure-container-storage is ephemeralDisk." with self.assertRaises(ArgumentUsageError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - storage_pool_sku, - None, - None, - None, - None, - False, + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, storage_pool_sku, None, None, None, None, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -891,17 +889,8 @@ def test_enable_with_sku_and_elastic_san_pool(self): ) ) with self.assertRaises(ArgumentUsageError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - storage_pool_sku, - None, - None, - None, - None, - False, + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, storage_pool_sku, None, None, None, None, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -911,56 +900,30 @@ def test_enable_with_option_and_non_ephemeral_disk_pool(self): storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK err = "Cannot set --storage-pool-option when --enable-azure-container-storage is not ephemeralDisk." with self.assertRaises(ArgumentUsageError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - None, - storage_pool_option, - None, - None, - None, - False, + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, None, storage_pool_option, None, None, None, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) - def test_enable_with_ssd_option_and_ephemeral_disk_pool(self): + def test_enable_with_option_all_and_ephemeral_disk_pool(self): storage_pool_name = "valid-name" - storage_pool_option = acstor_consts.CONST_STORAGE_POOL_OPTION_SSD + storage_pool_option = acstor_consts.CONST_ACSTOR_ALL storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK - err = "--storage-pool-option Temp storage (SSD) currently not supported." - with self.assertRaises(ArgumentUsageError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - None, - storage_pool_option, - None, - None, - None, - False, + err = "Cannot set --storage-pool-option value as all when --enable-azure-container-storage is set." + with self.assertRaises(InvalidArgumentValueError) as cm: + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, None, storage_pool_option, None, None, None, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) def test_enable_with_invalid_storage_pool_size(self): storage_pool_name = "valid-name" storage_pool_size = "5" + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK err = "Value for --storage-pool-size should be defined with size followed by Gi or Ti e.g. 512Gi or 2Ti." with self.assertRaises(ArgumentUsageError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - None, - None, - None, - storage_pool_size, - None, - None, - False, + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, None, None, storage_pool_size, None, None, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -970,17 +933,8 @@ def test_enable_with_invalid_size_for_esan_storage_pool(self): storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_ELASTIC_SAN err = "Value for --storage-pool-size must be at least 1Ti when --enable-azure-container-storage is elasticSan." with self.assertRaises(ArgumentUsageError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - None, - None, - storage_pool_size, - None, - None, - False, + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, None, None, storage_pool_size, None, None, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -988,25 +942,16 @@ def test_invalid_comma_separated_nodepool_list(self): nodepool_list = "pool1, 1pool" storage_pool_name = "valid-name" storage_pool_size = "5Ti" - storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK err = ( "Invalid --azure-container-storage-nodepools value. " - "Accepted value is a comma separated string of valid nodepool " - "names without any spaces.\nA valid nodepool name may only contain lowercase " + "Accepted value is a comma separated string of valid node pool " + "names without any spaces.\nA valid node pool name may only contain lowercase " "alphanumeric characters and must begin with a lowercase letter." ) with self.assertRaises(InvalidArgumentValueError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - None, - None, - storage_pool_size, - nodepool_list, - None, - False, + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, None, None, storage_pool_size, nodepool_list, None, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -1014,26 +959,18 @@ def test_missing_nodepool_from_cluster_nodepool_list_single(self): storage_pool_name = "valid-name" storage_pool_size = "5Ti" storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + storage_pool_option = acstor_consts.CONST_STORAGE_POOL_OPTION_NVME nodepool_list = "pool1" - agentpools = ["nodepool1"] + agentpools = [{"name": "nodepool1", "vm_size": "Standard_L8s_v3"}] err = ( - "Nodepool: pool1 not found. Please provide a comma separated " - "string of existing nodepool names in --azure-container-storage-nodepools." - "\nNodepool available in the cluster is: nodepool1." + "Node pool: pool1 not found. Please provide a comma separated " + "string of existing node pool names in --azure-container-storage-nodepools." + "\nNode pool available in the cluster is: nodepool1." "\nAborting installation of Azure Container Storage." ) with self.assertRaises(InvalidArgumentValueError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - None, - None, - storage_pool_size, - nodepool_list, - agentpools, - False, + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, None, storage_pool_option, storage_pool_size, nodepool_list, agentpools, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -1041,26 +978,18 @@ def test_missing_nodepool_from_cluster_nodepool_list_multiple(self): storage_pool_name = "valid-name" storage_pool_size = "5Ti" storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + storage_pool_option = acstor_consts.CONST_STORAGE_POOL_OPTION_SSD nodepool_list = "pool1,pool2" - agentpools = ["nodepool1", "nodepool2"] + agentpools = [{"name": "nodepool1"}, {"name": "nodepool2"}] err = ( - "Nodepool: pool1 not found. Please provide a comma separated " - "string of existing nodepool names in --azure-container-storage-nodepools." - "\nNodepools available in the cluster are: nodepool1, nodepool2." + "Node pool: pool1 not found. Please provide a comma separated " + "string of existing node pool names in --azure-container-storage-nodepools." + "\nNode pools available in the cluster are: nodepool1, nodepool2." "\nAborting installation of Azure Container Storage." ) with self.assertRaises(InvalidArgumentValueError) as cm: - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - None, - None, - storage_pool_size, - nodepool_list, - agentpools, - False, + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, None, storage_pool_option, storage_pool_size, nodepool_list, agentpools, False, False, False, False, False ) self.assertEqual(str(cm.exception), err) @@ -1070,18 +999,9 @@ def test_valid_enable_for_azure_disk_pool(self): storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK storage_pool_sku = acstor_consts.CONST_STORAGE_POOL_SKU_PREMIUM_LRS nodepool_list = "nodepool1,nodepool2" - agentpools = ["nodepool1", "nodepool2"] - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - storage_pool_sku, - None, - storage_pool_size, - nodepool_list, - agentpools, - False, + agentpools = [{"name": "nodepool1"}, {"name": "nodepool2"}] + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, storage_pool_sku, None, storage_pool_size, nodepool_list, agentpools, False, False, False, False, False ) def test_valid_enable_for_ephemeral_disk_pool(self): @@ -1090,18 +1010,50 @@ def test_valid_enable_for_ephemeral_disk_pool(self): storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK storage_pool_option = acstor_consts.CONST_STORAGE_POOL_OPTION_NVME nodepool_list = "nodepool1" - agentpools = ["nodepool1", "nodepool2"] - acstor_validator.validate_azure_container_storage_params( - True, - None, - storage_pool_name, - storage_pool_type, - None, - storage_pool_option, - storage_pool_size, - nodepool_list, - agentpools, - False, + agentpools = [{"name": "nodepool1", "vm_size": "Standard_L8s_v3"}, {"name": "nodepool2", "vm_size": "Standard_L8s_v3"}] + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, None, storage_pool_option, storage_pool_size, nodepool_list, agentpools, False, False, False, False, False + ) + + def test_extension_installed_nodepool_list_defined(self): + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK + nodepool_list = "nodepool1,nodepool2" + err = ( + "Cannot set --azure-container-storage-nodepools while using " + "--enable-azure-container-storage to enable a type of storagepool " + "in a cluster where Azure Container Storage is already installed." + ) + with self.assertRaises(ArgumentUsageError) as cm: + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, None, None, None, None, nodepool_list, None, True, False, False, False, False + ) + self.assertEqual(str(cm.exception), err) + + def test_extension_installed_storagepool_type_installed(self): + storage_pool_name = "valid-name" + storage_pool_size = "5Ti" + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK + storage_pool_sku = acstor_consts.CONST_STORAGE_POOL_SKU_PREMIUM_LRS + agentpools = [{"name": "nodepool1", "node_labels": {"acstor.azure.com/io-engine": "acstor"}}, {"name": "nodepool2"}] + err = ( + "Invalid --enable-azure-container-storage value. " + "Azure Container Storage is already enabled for storagepool type " + "{0} in the cluster.".format(storage_pool_type) + ) + with self.assertRaises(ArgumentUsageError) as cm: + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, storage_pool_sku, None, storage_pool_size, None, agentpools, True, True, False, False, False + ) + self.assertEqual(str(cm.exception), err) + + def test_valid_cluster_update(self): + storage_pool_name = "valid-name" + storage_pool_size = "5Ti" + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK + storage_pool_sku = acstor_consts.CONST_STORAGE_POOL_SKU_PREMIUM_LRS + agentpools = [{"name": "nodepool1", "node_labels": {"acstor.azure.com/io-engine": "acstor"}}, {"name": "nodepool2"}] + acstor_validator.validate_enable_azure_container_storage_params( + storage_pool_type, storage_pool_name, storage_pool_sku, None, storage_pool_size, None, agentpools, True, False, False, False, False ) diff --git a/src/aks-preview/setup.py b/src/aks-preview/setup.py index aa5934bf9b3..b757bc8a077 100644 --- a/src/aks-preview/setup.py +++ b/src/aks-preview/setup.py @@ -9,7 +9,7 @@ from setuptools import setup, find_packages -VERSION = "2.0.0b4" +VERSION = "2.0.0b5" CLASSIFIERS = [ "Development Status :: 4 - Beta",