diff --git a/src/azure-cli/azure/cli/command_modules/acs/_params.py b/src/azure-cli/azure/cli/command_modules/acs/_params.py index 0dcc370a04e..6e47f7ddb3d 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_params.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_params.py @@ -558,7 +558,7 @@ def load_arguments(self, _): # azure container storage c.argument( "enable_azure_container_storage", - arg_type=_get_enable_azure_container_storage_type(), + arg_type=_get_container_storage_enum_type(storage_pool_types), help="enable azure container storage. Can be used as a flag (defaults to True) or with a storage pool type value: (azureDisk, ephemeralDisk, elasticSan)", ) c.argument( @@ -764,12 +764,12 @@ def load_arguments(self, _): # azure container storage c.argument( "enable_azure_container_storage", - arg_type=_get_enable_azure_container_storage_type(), + arg_type=_get_container_storage_enum_type(storage_pool_types), help="enable azure container storage. Can be used as a flag (defaults to True) or with a storage pool type value: (azureDisk, ephemeralDisk, elasticSan)", ) c.argument( "disable_azure_container_storage", - arg_type=_get_disable_azure_container_storage_type(), + arg_type=_get_container_storage_enum_type(disable_storage_pool_types), help="disable azure container storage or any one of the storage pool types. Can be used as a flag (defaults to True) or with a storagepool type value: azureDisk, ephemeralDisk, elasticSan, all (to disable all storage pools).", ) c.argument( @@ -1286,63 +1286,33 @@ def _get_default_install_location(exe_name): return install_location -def _get_enable_azure_container_storage_type(): +def _get_container_storage_enum_type(choices): """Custom argument type that accepts both None and enum values""" import argparse from azure.cli.core.azclierror import InvalidArgumentValueError class AzureContainerStorageAction(argparse.Action): def __call__(self, parser, namespace, values, option_string=None): - if values is None: + if values in [[], None]: # When used as a flag without value, set as True setattr(namespace, self.dest, True) return - if isinstance(values, str): - # Handle enum values (case insensitive) - for storage_type in storage_pool_types: - if values.lower() == storage_type.lower(): - setattr(namespace, self.dest, storage_type) - return - - # Invalid value - valid_values = storage_pool_types - raise InvalidArgumentValueError( - f"Invalid value '{values}'. Valid values are: {', '.join(valid_values)}" - ) - - return CLIArgumentType( - nargs='?', # Optional argument - action=AzureContainerStorageAction, - ) - - -def _get_disable_azure_container_storage_type(): - """Custom argument type that accepts both None and enum values""" - import argparse - from azure.cli.core.azclierror import InvalidArgumentValueError - - class AzureContainerStorageAction(argparse.Action): - def __call__(self, parser, namespace, values, option_string=None): - if values is None: - # When used as a flag without value, set as True - setattr(namespace, self.dest, True) + # Allow multiple enum values in a case insensitive manner + normalized_value_arr = values if isinstance(values, list) else str(values).split(',') + normalized_value_arr = [str(v).lower().strip() for v in normalized_value_arr] + valid_value_arr = [v for v in choices if v.lower() in normalized_value_arr] + if len(valid_value_arr) == len(normalized_value_arr): + normalized_values = valid_value_arr[0] if len(valid_value_arr) == 1 else valid_value_arr + setattr(namespace, self.dest, normalized_values) return - if isinstance(values, str): - # Handle enum values (case insensitive) - for storage_type in disable_storage_pool_types: - if values.lower() == storage_type.lower(): - setattr(namespace, self.dest, storage_type) - return - # Invalid value - valid_values = disable_storage_pool_types raise InvalidArgumentValueError( - f"Invalid value '{values}'. Valid values are: {', '.join(valid_values)}" + f"Invalid value '{values}'. Valid values are: {', '.join(choices)}" ) return CLIArgumentType( - nargs='?', # Optional argument + nargs='*', # Allow multiple values action=AzureContainerStorageAction, ) diff --git a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_helpers.py b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_helpers.py index 6ed673c4b56..e642488e800 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_helpers.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_helpers.py @@ -7,7 +7,9 @@ from azure.cli.command_modules.acs.azurecontainerstorage._consts import ( CONST_ACSTOR_ALL, + CONST_ACSTOR_EXT_INSTALLATION_NAME, CONST_ACSTOR_IO_ENGINE_LABEL_KEY, + CONST_ACSTOR_K8S_EXTENSION_NAME, CONST_ACSTOR_V1_K8S_EXTENSION_NAME, CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY, CONST_DISK_TYPE_PV_WITH_ANNOTATION, @@ -146,7 +148,7 @@ def check_if_extension_is_installed(cmd, resource_group, cluster_name) -> bool: return return_val -def get_extension_installed_and_cluster_configs( +def get_extension_installed_and_cluster_configs_v1( cmd, resource_group, cluster_name, @@ -245,6 +247,56 @@ def get_extension_installed_and_cluster_configs( ) +def get_extension_installed_and_cluster_configs( + cmd, + resource_group, + cluster_name +) -> Tuple[bool, bool, bool]: + 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_ephemeral_disk_enabled = False + is_elastic_san_enabled = False + + try: + extension = k8s_extension_custom_mod.show_k8s_extension( + client, + resource_group, + cluster_name, + CONST_ACSTOR_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_ephemeral_disk_enabled = ( + config_settings.get("csiDriverConfigs.local-csi-driver.enabled", "False") == "True" + ) + is_elastic_san_enabled = ( + config_settings.get("csiDriverConfigs.azuresan-csi-driver.enabled", "False") == "True" + ) + + except: # pylint: disable=bare-except + is_extension_installed = False + + return ( + is_extension_installed, + is_ephemeral_disk_enabled, + is_elastic_san_enabled, + ) + + +def should_delete_extension(storage_options_to_remove) -> bool: + return ( + storage_options_to_remove in [True, CONST_ACSTOR_ALL] or + (isinstance(storage_options_to_remove, list) and CONST_ACSTOR_ALL in storage_options_to_remove) + ) + + def get_container_storage_extension_installed( cmd, resource_group, diff --git a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_validators.py b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_validators.py index ed3774581d4..a817519b376 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_validators.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_validators.py @@ -432,10 +432,12 @@ def validate_enable_azure_container_storage_v1_params( # pylint: disable=too-ma def validate_enable_azure_container_storage_params( + enablement_option, is_extension_installed, + is_ephemeral_disk_enabled, + is_elastic_san_enabled, is_v1_extension_installed, v1_extension_version, - storage_pool_type, storage_pool_name, storage_pool_sku, storage_pool_option, @@ -450,18 +452,28 @@ def validate_enable_azure_container_storage_params( 'that depend on Azure Container Storage.' ) - if is_extension_installed: - raise InvalidArgumentValueError( - 'Cannot enable Azure Container Storage as it is already enabled on the cluster.' - ) - - # Todo: Remove this for the 2.1.0 release - if storage_pool_type is not None and not isinstance(storage_pool_type, bool): - raise InvalidArgumentValueError( - 'The latest version of Azure Container Storage only supports ephemeral nvme storage and does not ' - 'require or support a storage-pool-type value for --enable-azure-container-storage parameter. ' - f'Please remove {storage_pool_type} from the command and try again.' - ) + if not enablement_option or enablement_option is True: + if is_extension_installed: + raise InvalidArgumentValueError( + 'Cannot enable Azure Container Storage as it is already enabled on the cluster.' + ) + else: + enablement_option_arr = enablement_option if isinstance(enablement_option, list) else [enablement_option] + enable_ephemeral_disk = CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK in enablement_option + enable_elastic_san = CONST_STORAGE_POOL_TYPE_ELASTIC_SAN in enablement_option + for option in enablement_option_arr: + if option not in [CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN]: + raise InvalidArgumentValueError( + f"Unsupported storage option '{option}'. " + f"Supported values are '{CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK}' " + f"and '{CONST_STORAGE_POOL_TYPE_ELASTIC_SAN}'." + ) + if is_ephemeral_disk_enabled == enable_ephemeral_disk and is_elastic_san_enabled == enable_elastic_san: + options_display = "', '".join(enablement_option_arr) + raise InvalidArgumentValueError( + f"Cannot enable the requested storage options ('{options_display}') " + "as they are already enabled on the cluster." + ) if storage_pool_name is not None: raise InvalidArgumentValueError( @@ -493,8 +505,10 @@ def validate_enable_azure_container_storage_params( def validate_disable_azure_container_storage_params( + disablement_option, is_extension_installed, - storage_pool_type, + is_ephemeral_disk_enabled, + is_elastic_san_enabled, storage_pool_name, storage_pool_sku, storage_pool_option, @@ -502,16 +516,31 @@ def validate_disable_azure_container_storage_params( ): if not is_extension_installed: raise InvalidArgumentValueError( - 'Cannot disable Azure Container Storage as it is not enabled on the cluster.' + 'Cannot disable Azure Container Storage as it could not be found on the cluster.' ) - # Todo: Remove this for the 2.1.0 release - if storage_pool_type is not None and not isinstance(storage_pool_type, bool): - raise InvalidArgumentValueError( - 'The latest version of Azure Container Storage only supports ephemeral nvme storage and does not ' - 'require or support a storage-pool-type value for --disable-azure-container-storage parameter. ' - f'Please remove {storage_pool_type} from the command and try again.' - ) + if disablement_option and disablement_option not in [True, CONST_ACSTOR_ALL]: + actionable = False + disablement_option_arr = disablement_option if isinstance(disablement_option, list) else [disablement_option] + for disable_option in disablement_option_arr: + if disable_option == CONST_ACSTOR_ALL: + actionable = True + elif disable_option == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: + actionable = actionable or is_ephemeral_disk_enabled + elif disable_option == CONST_STORAGE_POOL_TYPE_ELASTIC_SAN: + actionable = actionable or is_elastic_san_enabled + else: + raise InvalidArgumentValueError( + f"Cannot disable unsupported storage option '{disable_option}'. " + f"Supported values are '{CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK}', " + f"'{CONST_STORAGE_POOL_TYPE_ELASTIC_SAN}' and '{CONST_ACSTOR_ALL}'." + ) + if not actionable: + options_display = "', '".join(disablement_option_arr) + raise InvalidArgumentValueError( + f"Cannot disable the requested storage options ('{options_display}') " + "as they could not be found on the cluster." + ) if storage_pool_name is not None: raise InvalidArgumentValueError( diff --git a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/acstor_ops.py b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/acstor_ops.py index 5c033bf738b..adeed2909b1 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/acstor_ops.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/acstor_ops.py @@ -34,6 +34,7 @@ get_initial_resource_value_args, perform_role_operations_on_managed_rg, validate_storagepool_creation, + should_delete_extension, ) from knack.log import get_logger @@ -682,43 +683,80 @@ def perform_disable_azure_container_storage_v1( # pylint: disable=too-many-stat logger.warning("Azure Container Storage storagepool type %s has been disabled.", storage_pool_type) -def perform_enable_azure_container_storage( +def perform_azure_container_storage_update( cmd, resource_group, cluster_name, + storage_options_to_add, + is_extension_installed=False, + storage_options_to_remove=None, ): 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) - config_settings = [] - delete_extension = False - try: - result = k8s_extension_custom_mod.create_k8s_extension( - cmd, - client, - resource_group, - cluster_name, - CONST_ACSTOR_EXT_INSTALLATION_NAME, - "managedClusters", - CONST_ACSTOR_K8S_EXTENSION_NAME, - auto_upgrade_minor_version=True, - release_train="stable", - scope="cluster", - release_namespace=CONST_ACSTOR_EXT_INSTALLATION_NAMESPACE, - configuration_settings=config_settings, - ) - op_text = "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 - logger.error("Azure Container Storage failed to install.\nError: %s", ex) - delete_extension = True + delete_extension = should_delete_extension(storage_options_to_remove) + delete_extension_auto = False - if delete_extension: - logger.warning("Cleaning up the cluster by disabling Azure Container Storage") + if not delete_extension: + config_settings = [] + + storage_options_to_add = storage_options_to_add if isinstance(storage_options_to_add, (list, str)) else [] + storage_options_to_remove = storage_options_to_remove \ + if isinstance(storage_options_to_remove, (list, str)) else [] + + if CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK in storage_options_to_remove: + config_settings.append({"csiDriverConfigs.local-csi-driver.enabled": "False"}) + elif CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK in storage_options_to_add: + config_settings.append({"csiDriverConfigs.local-csi-driver.enabled": "True"}) + + if CONST_STORAGE_POOL_TYPE_ELASTIC_SAN in storage_options_to_remove: + config_settings.append({"csiDriverConfigs.azuresan-csi-driver.enabled": "False"}) + elif CONST_STORAGE_POOL_TYPE_ELASTIC_SAN in storage_options_to_add: + config_settings.append({"csiDriverConfigs.azuresan-csi-driver.enabled": "True"}) + + try: + if is_extension_installed: + result = k8s_extension_custom_mod.update_k8s_extension( + cmd, + client, + resource_group, + cluster_name, + CONST_ACSTOR_EXT_INSTALLATION_NAME, + "managedClusters", + configuration_settings=config_settings, + yes=True, + ) + op_text = "Azure Container Storage successfully updated" + else: + result = k8s_extension_custom_mod.create_k8s_extension( + cmd, + client, + resource_group, + cluster_name, + CONST_ACSTOR_EXT_INSTALLATION_NAME, + "managedClusters", + CONST_ACSTOR_K8S_EXTENSION_NAME, + auto_upgrade_minor_version=True, + release_train="stable", + scope="cluster", + release_namespace=CONST_ACSTOR_EXT_INSTALLATION_NAMESPACE, + configuration_settings=config_settings, + ) + op_text = "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_extension_installed: + logger.error("Azure Container Storage failed to update.\nError: %s.", ex) + else: + logger.error("Azure Container Storage failed to install.\nError: %s", ex) + delete_extension_auto = True + + if delete_extension or delete_extension_auto: + if delete_extension_auto: + logger.warning("Cleaning up the cluster by disabling Azure Container Storage") try: delete_op_result = k8s_extension_custom_mod.delete_k8s_extension( cmd, @@ -732,40 +770,12 @@ def perform_enable_azure_container_storage( LongRunningOperation(cmd.cli_ctx)(delete_op_result) logger.warning("Azure Container Storage has been disabled.") - logger.warning( - "Please retry enabling Azure Container Storage by running " - "`az aks update` along with `--enable-azure-container-storage`" - ) + if delete_extension_auto: + logger.warning( + "Please retry enabling Azure Container Storage by running " + "`az aks update` along with `--enable-azure-container-storage`" + ) except Exception as delete_ex: raise UnknownError( "Failed to disable Azure Container Storage with error: %s" % delete_ex ) from delete_ex - - -def perform_disable_azure_container_storage( - cmd, - resource_group, - cluster_name, -): - 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) - - try: - delete_op_result = k8s_extension_custom_mod.delete_k8s_extension( - cmd, - client, - resource_group, - cluster_name, - CONST_ACSTOR_EXT_INSTALLATION_NAME, - "managedClusters", - yes=True, - no_wait=False, - ) - - LongRunningOperation(cmd.cli_ctx)(delete_op_result) - logger.warning("Azure Container Storage has been disabled.") - except Exception as delete_ex: - raise UnknownError( - "Failed to disable Azure Container Storage with error: %s" % delete_ex - ) from delete_ex diff --git a/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py b/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py index 56c6a008b57..86953e7cc4c 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py +++ b/src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py @@ -97,8 +97,7 @@ from azure.cli.command_modules.acs.azurecontainerstorage.acstor_ops import ( perform_disable_azure_container_storage_v1, perform_enable_azure_container_storage_v1, - perform_enable_azure_container_storage, - perform_disable_azure_container_storage, + perform_azure_container_storage_update, ) from azure.cli.command_modules.acs.azuremonitormetrics.azuremonitorprofile import ( ensure_azure_monitor_profile_prerequisites @@ -340,8 +339,7 @@ def external_functions(self) -> SimpleNamespace: # azure container storage functions external_functions["perform_enable_azure_container_storage_v1"] = perform_enable_azure_container_storage_v1 external_functions["perform_disable_azure_container_storage_v1"] = perform_disable_azure_container_storage_v1 - external_functions["perform_enable_azure_container_storage"] = perform_enable_azure_container_storage - external_functions["perform_disable_azure_container_storage"] = perform_disable_azure_container_storage + external_functions["perform_azure_container_storage_update"] = perform_azure_container_storage_update self.__external_functions = SimpleNamespace(**external_functions) return self.__external_functions @@ -6900,13 +6898,14 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: """ self._ensure_mc(mc) - if self.context.raw_param.get("enable_azure_container_storage") is not None: - self.context.set_intermediate("enable_azure_container_storage", True, overwrite_exists=True) + enable_azure_container_storage_param = self.context.raw_param.get("enable_azure_container_storage") + if enable_azure_container_storage_param: + self.context.set_intermediate("enable_azure_container_storage", enable_azure_container_storage_param, overwrite_exists=True) container_storage_version = self.context.raw_param.get("container_storage_version") if container_storage_version is not None and container_storage_version == CONST_ACSTOR_VERSION_V1: # read the azure container storage values passed - pool_type = self.context.raw_param.get("enable_azure_container_storage") + pool_type = enable_azure_container_storage_param enable_azure_container_storage = pool_type is not None ephemeral_disk_volume_type = self.context.raw_param.get("ephemeral_disk_volume_type") ephemeral_disk_nvme_perf_tier = self.context.raw_param.get("ephemeral_disk_nvme_perf_tier") @@ -7000,7 +6999,6 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: overwrite_exists=True ) else: - enable_azure_container_storage = self.context.raw_param.get("enable_azure_container_storage") storage_pool_name = self.context.raw_param.get("storage_pool_name") pool_sku = self.context.raw_param.get("storage_pool_sku") pool_option = self.context.raw_param.get("storage_pool_option") @@ -7010,10 +7008,12 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: validate_enable_azure_container_storage_params, ) validate_enable_azure_container_storage_params( + enable_azure_container_storage_param, + False, + False, False, False, "", - enable_azure_container_storage, storage_pool_name, pool_sku, pool_option, @@ -7547,10 +7547,11 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: existing_ephemeral_nvme_perf_tier, ) else: - self.context.external_functions.perform_enable_azure_container_storage( + self.context.external_functions.perform_azure_container_storage_update( self.cmd, self.context.get_resource_group_name(), self.context.get_name(), + enable_azure_container_storage, ) # Add role assignments for automatic sku @@ -9003,7 +9004,7 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: pool_sku = self.context.raw_param.get("storage_pool_sku") pool_size = self.context.raw_param.get("storage_pool_size") agentpool_details = {} - from azure.cli.command_modules.acs.azurecontainerstorage._helpers import get_extension_installed_and_cluster_configs + from azure.cli.command_modules.acs.azurecontainerstorage._helpers import get_extension_installed_and_cluster_configs_v1 ( is_extension_installed, is_azureDisk_enabled, @@ -9013,7 +9014,7 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: current_core_value, existing_ephemeral_disk_volume_type, existing_perf_tier, - ) = get_extension_installed_and_cluster_configs( + ) = get_extension_installed_and_cluster_configs_v1( self.cmd, self.context.get_resource_group_name(), self.context.get_name(), @@ -9223,12 +9224,11 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: 'and --disable-azure-container-storage together.' ) - from azure.cli.command_modules.acs.azurecontainerstorage._helpers import get_container_storage_extension_installed - is_extension_installed, _ = get_container_storage_extension_installed( + from azure.cli.command_modules.acs.azurecontainerstorage._helpers import get_extension_installed_and_cluster_configs + is_extension_installed, is_ephemeralDisk_enabled, is_elasticSan_enabled = get_extension_installed_and_cluster_configs( self.cmd, self.context.get_resource_group_name(), self.context.get_name(), - CONST_ACSTOR_EXT_INSTALLATION_NAME, ) from azure.cli.command_modules.acs.azurecontainerstorage._helpers import get_container_storage_extension_installed @@ -9250,10 +9250,12 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: validate_enable_azure_container_storage_params, ) validate_enable_azure_container_storage_params( + enable_azure_container_storage_param, is_extension_installed, + is_ephemeralDisk_enabled, + is_elasticSan_enabled, is_containerstorage_v1_installed, v1_extension_version, - enable_azure_container_storage_param, storage_pool_name, pool_sku, pool_option, @@ -9268,6 +9270,23 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: "The PVs and PVCs can only be cleaned up after re-enabling it by running 'az aks update --enable-azure-container-storage'. " "Would you like to proceed with the disabling?" ) + from azure.cli.command_modules.acs.azurecontainerstorage._helpers import should_delete_extension + if not should_delete_extension(disable_azure_container_storage_param): + storage_option_display = storage_option_param_str = disable_azure_container_storage_param + if isinstance(disable_azure_container_storage_param, list): + storage_options = disable_azure_container_storage_param + storage_option_param_str = " ".join(storage_options) + if len(storage_options) > 2: + storage_options = [storage_options[:-1].join("', '"), storage_options[-1]] + storage_option_display = "' and '".join(storage_options) + msg = ( + "Please make sure there are no existing PVs and PVCs that are provisioned by Azure Container Storage " + f"for '{storage_option_display}' before disabling. If storage options are disabled " + "with remaining PVs and PVCs, any data associated with those PVs and PVCs will not be erased " + "and the nodes will be left in an unclean state. The PVs and PVCs can only be cleaned up " + f"after re-enabling it by running 'az aks update --enable-azure-container-storage {storage_option_param_str}'. " + "Would you like to proceed with the disabling?" + ) if not self.context.get_yes() and not prompt_y_n(msg, default="n"): raise DecoratorEarlyExitException() @@ -9275,19 +9294,19 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: validate_disable_azure_container_storage_params ) validate_disable_azure_container_storage_params( - is_extension_installed, disable_azure_container_storage_param, + is_extension_installed, + is_ephemeralDisk_enabled, + is_elasticSan_enabled, storage_pool_name, pool_sku, pool_option, pool_size ) - if enable_azure_container_storage_param: - self.context.set_intermediate("enable_azure_container_storage", True) - - if disable_azure_container_storage_param: - self.context.set_intermediate("disable_azure_container_storage", True) + self.context.set_intermediate("enable_azure_container_storage", enable_azure_container_storage_param, overwrite_exists=True) + self.context.set_intermediate("disable_azure_container_storage", disable_azure_container_storage_param, overwrite_exists=True) + self.context.set_intermediate("is_extension_installed", is_extension_installed, overwrite_exists=True) return mc @@ -9777,10 +9796,12 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: current_core_value, ) else: - self.context.external_functions.perform_enable_azure_container_storage( + self.context.external_functions.perform_azure_container_storage_update( self.cmd, self.context.get_resource_group_name(), self.context.get_name(), + enable_azure_container_storage, + is_extension_installed, ) # disable azure container storage @@ -9810,10 +9831,13 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: ) else: - self.context.external_functions.perform_disable_azure_container_storage( + self.context.external_functions.perform_azure_container_storage_update( self.cmd, self.context.get_resource_group_name(), self.context.get_name(), + None, + is_extension_installed, + disable_azure_container_storage, ) # attach keyvault to app routing addon diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_validators.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_validators.py index 044edaff95d..52a54e5c802 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_validators.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_validators.py @@ -1634,26 +1634,37 @@ def test_valid_cluster_update(self): ) class TestValidateDisableAzureContainerStorage(unittest.TestCase): - def test_disable_with_storagepool_type_params(self): + def test_disable_unsupported_type(self): storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_disable_azure_container_storage_params( - True, storage_pool_type, "", "", "", "", + storage_pool_type, True, False, False, "", "", "", "", ) err = ( - 'The latest version of Azure Container Storage only supports ephemeral nvme storage and does not ' - 'require or support a storage-pool-type value for --disable-azure-container-storage parameter. ' - f'Please remove {storage_pool_type} from the command and try again.' + f"Cannot disable unsupported storage option '{storage_pool_type}'. " + "Supported values are 'ephemeralDisk', 'elasticSan' and 'all'." + ) + self.assertEqual(str(cm.exception), err) + + def test_disable_already_disabled_type(self): + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + with self.assertRaises(InvalidArgumentValueError) as cm: + acstor_validator.validate_disable_azure_container_storage_params( + storage_pool_type, True, False, False, "", "", "", "", + ) + err = ( + f"Cannot disable the requested storage options ('{storage_pool_type}') " + "as they could not be found on the cluster." ) self.assertEqual(str(cm.exception), err) def test_disable_when_acstor_not_enabled(self): with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_disable_azure_container_storage_params( - False, "", "", "", "", "", + "", False, False, False, "", "", "", "", ) err = ( - 'Cannot disable Azure Container Storage as it is not enabled on the cluster.' + 'Cannot disable Azure Container Storage as it could not be found on the cluster.' ) self.assertEqual(str(cm.exception), err) @@ -1661,7 +1672,7 @@ def test_disable_with_storage_pool_name(self): storage_pool_name = "valid-name" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_disable_azure_container_storage_params( - True, True, storage_pool_name, "", "", "", + True, True, False, False, storage_pool_name, "", "", "", ) err = ( 'The latest version of Azure Container Storage does not ' @@ -1674,7 +1685,7 @@ def test_disable_with_storage_pool_sku(self): storage_pool_sku = "valid-sku" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_disable_azure_container_storage_params( - True, True, None, storage_pool_sku, "", "", + True, True, False, False, None, storage_pool_sku, "", "", ) err = ( 'The latest version of Azure Container Storage does not ' @@ -1687,7 +1698,7 @@ def test_disable_with_storage_pool_option(self): storage_pool_option = "valid-option" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_disable_azure_container_storage_params( - True, True, None, None, storage_pool_option, "", + True, True, False, False, None, None, storage_pool_option, "", ) err = ( 'The latest version of Azure Container Storage does not ' @@ -1700,7 +1711,7 @@ def test_disable_with_storage_pool_size(self): storage_pool_size = "valid-size" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_disable_azure_container_storage_params( - True, True, None, None, None, storage_pool_size + True, True, False, False, None, None, None, storage_pool_size ) err = ( 'The latest version of Azure Container Storage does not ' @@ -1710,10 +1721,15 @@ def test_disable_with_storage_pool_size(self): self.assertEqual(str(cm.exception), err) class TestValidateEnableAzureContainerStorage(unittest.TestCase): - def test_enable_when_already_enabled(self): + def test_enable_extension(self): + acstor_validator.validate_enable_azure_container_storage_params( + True, False, False, False, False, "", None, None, None, None, + ) + + def test_enable_extension_when_already_enabled(self): with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_enable_azure_container_storage_params( - True, "", "", "", "", "", "", "", + "", True, False, False, False, "", None, None, None, None, ) err = ( 'Cannot enable Azure Container Storage as it is already enabled on the cluster.' @@ -1724,7 +1740,7 @@ def test_enable_when_v1_installed(self): v1_extension_version = "1.3.0" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_enable_azure_container_storage_params( - False, True, v1_extension_version, "", "", "", "", "", + "", False, False, False, True, v1_extension_version, None, None, None, None, ) err = ( f'Failed to enable the latest version of Azure Container Storage as version {v1_extension_version} ' @@ -1735,16 +1751,77 @@ def test_enable_when_v1_installed(self): ) self.assertEqual(str(cm.exception), err) - def test_enable_with_storagepool_type(self): - storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK + def test_enable_supported_type_when_extension_disabled(self): + storage_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + acstor_validator.validate_enable_azure_container_storage_params( + storage_type, False, False, False, False, "", None, None, None, None, + ) + + def test_enable_supported_type_when_extension_enabled(self): + storage_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + acstor_validator.validate_enable_azure_container_storage_params( + storage_type, True, False, False, False, "", None, None, None, None, + ) + + def test_enable_supported_type_when_already_enabled(self): + storage_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + with self.assertRaises(InvalidArgumentValueError) as cm: + acstor_validator.validate_enable_azure_container_storage_params( + storage_type, True, True, False, False, "", None, None, None, None, + ) + err = ( + f"Cannot enable the requested storage options ('{storage_type}') " + "as they are already enabled on the cluster." + ) + self.assertEqual(str(cm.exception), err) + + def test_enable_multiple_types_when_extension_disabled(self): + storage_types = [acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, acstor_consts.CONST_STORAGE_POOL_TYPE_ELASTIC_SAN] + acstor_validator.validate_enable_azure_container_storage_params( + storage_types, False, False, False, False, "", None, None, None, None, + ) + + def test_enable_multiple_types_when_some_are_not_enabled(self): + storage_types = [acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, acstor_consts.CONST_STORAGE_POOL_TYPE_ELASTIC_SAN] + acstor_validator.validate_enable_azure_container_storage_params( + storage_types, True, True, False, False, "", None, None, None, None, + ) + + def test_enable_multiple_types_when_all_are_already_enabled(self): + storage_types = [acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, acstor_consts.CONST_STORAGE_POOL_TYPE_ELASTIC_SAN] with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_enable_azure_container_storage_params( - False, False, "", storage_pool_type, None, None, None, None, + storage_types, True, True, True, False, "", None, None, None, None, ) err = ( - 'The latest version of Azure Container Storage only supports ephemeral nvme storage and does not ' - 'require or support a storage-pool-type value for --enable-azure-container-storage parameter. ' - f'Please remove {storage_pool_type} from the command and try again.' + f"Cannot enable the requested storage options ('{storage_types[0]}', '{storage_types[1]}') " + "as they are already enabled on the cluster." + ) + self.assertEqual(str(cm.exception), err) + + def test_enable_multiple_types_when_some_are_unsupported(self): + unsupported_storage_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK + supported_storage_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + storage_types = [supported_storage_type, unsupported_storage_type] + with self.assertRaises(InvalidArgumentValueError) as cm: + acstor_validator.validate_enable_azure_container_storage_params( + storage_types, False, False, False, False, "", None, None, None, None, + ) + err = ( + f"Unsupported storage option '{unsupported_storage_type}'. " + "Supported values are 'ephemeralDisk' and 'elasticSan'." + ) + self.assertEqual(str(cm.exception), err) + + def test_enable_unsupported_type(self): + storage_type = acstor_consts.CONST_STORAGE_POOL_TYPE_AZURE_DISK + with self.assertRaises(InvalidArgumentValueError) as cm: + acstor_validator.validate_enable_azure_container_storage_params( + storage_type, False, False, False, False, "", None, None, None, None, + ) + err = ( + f"Unsupported storage option '{storage_type}'. " + "Supported values are 'ephemeralDisk' and 'elasticSan'." ) self.assertEqual(str(cm.exception), err) @@ -1752,7 +1829,7 @@ def test_enable_with_storage_pool_name(self): storage_pool_name = "valid-name" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_enable_azure_container_storage_params( - False, False, "", None, storage_pool_name, None, None, None, + None, False, False, False, False, "", storage_pool_name, None, None, None, ) err = ( 'The latest version of Azure Container Storage does not ' @@ -1765,7 +1842,7 @@ def test_enable_with_storage_pool_sku(self): storage_pool_sku = "valid-sku" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_enable_azure_container_storage_params( - False, False, "", None, None, storage_pool_sku, None, None, + None, False, False, False, False, "", None, storage_pool_sku, None, None, ) err = ( 'The latest version of Azure Container Storage does not ' @@ -1773,12 +1850,12 @@ def test_enable_with_storage_pool_sku(self): f'Please remove --storage-pool-sku {storage_pool_sku} from the command and try again.' ) self.assertEqual(str(cm.exception), err) - + def test_enable_with_storage_pool_option(self): storage_pool_option = "valid-option" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_enable_azure_container_storage_params( - False, False, "", None, None, None, storage_pool_option, None, + None, False, False, False, False, "", None, None, storage_pool_option, None, ) err = ( 'The latest version of Azure Container Storage does not ' @@ -1791,7 +1868,7 @@ def test_enable_with_storagepool_size(self): storage_pool_size = "valid-size" with self.assertRaises(InvalidArgumentValueError) as cm: acstor_validator.validate_enable_azure_container_storage_params( - False, False, "", None, None, None, None, storage_pool_size, + None, False, False, False, False, "", None, None, None, storage_pool_size, ) err = ( 'The latest version of Azure Container Storage does not '