[AKS] az aks create/update: Fix handling for --container-storage-version parameter (#32015)

Author: nearora-msft

src/azure-cli/azure/cli/command_modules/acs/_params.py

@@ -259,6 +259,11 @@
 ]
 
 # azure container storage
+container_storage_versions = [
+    "1",
+    "2"
+]
+
 storage_pool_types = [
     CONST_STORAGE_POOL_TYPE_AZURE_DISK,
     CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK,
@@ -516,6 +521,7 @@ def load_arguments(self, _):
         )
         c.argument(
             "container_storage_version",
+            arg_type=get_enum_type(container_storage_versions),
             help="set azure container storage version, the latest version will be installed by default",
         )
         c.argument(
@@ -724,6 +730,7 @@ def load_arguments(self, _):
         )
         c.argument(
             "container_storage_version",
+            arg_type=get_enum_type(container_storage_versions),
             help="set azure container storage version, the latest version will be installed by default",
         )
         c.argument(

src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py

@@ -6781,102 +6781,102 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
         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)
             container_storage_version = self.context.raw_param.get("container_storage_version")
-            if container_storage_version is not None:
-                if 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")
-                    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")
-                    if (ephemeral_disk_volume_type is not None or ephemeral_disk_nvme_perf_tier is not None) and \
-                            not enable_azure_container_storage:
-                        params_defined_arr = []
-                        if ephemeral_disk_volume_type is not None:
-                            params_defined_arr.append('--ephemeral-disk-volume-type')
-                        if ephemeral_disk_nvme_perf_tier is not None:
-                            params_defined_arr.append('--ephemeral-disk-nvme-perf-tier')
-
-                        params_defined = 'and '.join(params_defined_arr)
-                        raise RequiredArgumentMissingError(
-                            f'Cannot set {params_defined} without the parameter --enable-azure-container-storage.'
-                        )
 
-                    if enable_azure_container_storage:
-                        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")
-                        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_details = {}
-                        pool_details = {}
-                        pool_details["vm_size"] = agentpool.vm_size
-                        pool_details["count"] = agentpool.count
-                        pool_details["os_type"] = agentpool.os_type
-                        pool_details["mode"] = agentpool.mode
-                        pool_details["node_taints"] = agentpool.node_taints
-                        pool_details["zoned"] = agentpool.availability_zones is not None
-                        agentpool_details[agentpool.name] = pool_details
-                        # Marking the only agentpool name as the valid nodepool for
-                        # installing Azure Container Storage during `az aks create`
-                        nodepool_list = agentpool.name
+            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")
+                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")
+                if (ephemeral_disk_volume_type is not None or ephemeral_disk_nvme_perf_tier is not None) and \
+                        not enable_azure_container_storage:
+                    params_defined_arr = []
+                    if ephemeral_disk_volume_type is not None:
+                        params_defined_arr.append('--ephemeral-disk-volume-type')
+                    if ephemeral_disk_nvme_perf_tier is not None:
+                        params_defined_arr.append('--ephemeral-disk-nvme-perf-tier')
 
-                        from azure.cli.command_modules.acs.azurecontainerstorage._validators import (
-                            validate_enable_azure_container_storage_v1_params
-                        )
-                        from azure.cli.command_modules.acs.azurecontainerstorage._consts import (
-                            CONST_ACSTOR_IO_ENGINE_LABEL_KEY,
-                            CONST_ACSTOR_IO_ENGINE_LABEL_VAL,
-                            CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY,
-                            CONST_EPHEMERAL_NVME_PERF_TIER_STANDARD,
-                        )
-                        from azure.cli.command_modules.acs.azurecontainerstorage._helpers import generate_vm_sku_cache_for_region
-                        generate_vm_sku_cache_for_region(self.cmd.cli_ctx, self.context.get_location())
-
-                        default_ephemeral_disk_volume_type = CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY
-                        default_ephemeral_disk_nvme_perf_tier = CONST_EPHEMERAL_NVME_PERF_TIER_STANDARD
-                        validate_enable_azure_container_storage_v1_params(
-                            pool_type,
-                            pool_name,
-                            pool_sku,
-                            pool_option,
-                            pool_size,
-                            nodepool_list,
-                            agentpool_details,
-                            False,
-                            False,
-                            "",
-                            False,
-                            False,
-                            False,
-                            False,
-                            ephemeral_disk_volume_type,
-                            ephemeral_disk_nvme_perf_tier,
-                            default_ephemeral_disk_volume_type,
-                            default_ephemeral_disk_nvme_perf_tier,
-                        )
+                    params_defined = 'and '.join(params_defined_arr)
+                    raise RequiredArgumentMissingError(
+                        f'Cannot set {params_defined} without the parameter --enable-azure-container-storage.'
+                    )
 
-                        # Setup Azure Container Storage labels on the nodepool
-                        nodepool_labels = agentpool.node_labels
-                        if nodepool_labels is None:
-                            nodepool_labels = {}
-                        nodepool_labels[CONST_ACSTOR_IO_ENGINE_LABEL_KEY] = CONST_ACSTOR_IO_ENGINE_LABEL_VAL
-                        agentpool.node_labels = nodepool_labels
-
-                        # set intermediates
-                        self.context.set_intermediate("container_storage_version", container_storage_version, overwrite_exists=True)
-                        self.context.set_intermediate("azure_container_storage_nodepools", nodepool_list, overwrite_exists=True)
-                        self.context.set_intermediate(
-                            "current_ephemeral_nvme_perf_tier",
-                            default_ephemeral_disk_nvme_perf_tier,
-                            overwrite_exists=True
-                        )
-                        self.context.set_intermediate(
-                            "existing_ephemeral_disk_volume_type",
-                            default_ephemeral_disk_volume_type,
-                            overwrite_exists=True
-                        )
+                if enable_azure_container_storage:
+                    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")
+                    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_details = {}
+                    pool_details = {}
+                    pool_details["vm_size"] = agentpool.vm_size
+                    pool_details["count"] = agentpool.count
+                    pool_details["os_type"] = agentpool.os_type
+                    pool_details["mode"] = agentpool.mode
+                    pool_details["node_taints"] = agentpool.node_taints
+                    pool_details["zoned"] = agentpool.availability_zones is not None
+                    agentpool_details[agentpool.name] = 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 azure.cli.command_modules.acs.azurecontainerstorage._validators import (
+                        validate_enable_azure_container_storage_v1_params
+                    )
+                    from azure.cli.command_modules.acs.azurecontainerstorage._consts import (
+                        CONST_ACSTOR_IO_ENGINE_LABEL_KEY,
+                        CONST_ACSTOR_IO_ENGINE_LABEL_VAL,
+                        CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY,
+                        CONST_EPHEMERAL_NVME_PERF_TIER_STANDARD,
+                    )
+                    from azure.cli.command_modules.acs.azurecontainerstorage._helpers import generate_vm_sku_cache_for_region
+                    generate_vm_sku_cache_for_region(self.cmd.cli_ctx, self.context.get_location())
+
+                    default_ephemeral_disk_volume_type = CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY
+                    default_ephemeral_disk_nvme_perf_tier = CONST_EPHEMERAL_NVME_PERF_TIER_STANDARD
+                    validate_enable_azure_container_storage_v1_params(
+                        pool_type,
+                        pool_name,
+                        pool_sku,
+                        pool_option,
+                        pool_size,
+                        nodepool_list,
+                        agentpool_details,
+                        False,
+                        False,
+                        "",
+                        False,
+                        False,
+                        False,
+                        False,
+                        ephemeral_disk_volume_type,
+                        ephemeral_disk_nvme_perf_tier,
+                        default_ephemeral_disk_volume_type,
+                        default_ephemeral_disk_nvme_perf_tier,
+                    )
+
+                    # Setup Azure Container Storage labels on the nodepool
+                    nodepool_labels = agentpool.node_labels
+                    if nodepool_labels is None:
+                        nodepool_labels = {}
+                    nodepool_labels[CONST_ACSTOR_IO_ENGINE_LABEL_KEY] = CONST_ACSTOR_IO_ENGINE_LABEL_VAL
+                    agentpool.node_labels = nodepool_labels
+
+                    # set intermediates
+                    self.context.set_intermediate("container_storage_version", container_storage_version, overwrite_exists=True)
+                    self.context.set_intermediate("azure_container_storage_nodepools", nodepool_list, overwrite_exists=True)
+                    self.context.set_intermediate(
+                        "current_ephemeral_nvme_perf_tier",
+                        default_ephemeral_disk_nvme_perf_tier,
+                        overwrite_exists=True
+                    )
+                    self.context.set_intermediate(
+                        "existing_ephemeral_disk_volume_type",
+                        default_ephemeral_disk_volume_type,
+                        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")
@@ -8800,8 +8800,15 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
         # check if we are trying to enable container storage v1
         enable_azure_container_storage_param = self.context.raw_param.get("enable_azure_container_storage")
         disable_azure_container_storage_param = self.context.raw_param.get("disable_azure_container_storage")
+        container_storage_version = self.context.raw_param.get("container_storage_version")
+
+        if disable_azure_container_storage_param is not None and container_storage_version is not None:
+            raise InvalidArgumentValueError(
+                'The --container-storage-version parameter is not required when disabling Azure Container Storage.'
+                ' Please remove this parameter and try again.'
+            )
+
         if enable_azure_container_storage_param is not None or disable_azure_container_storage_param is not None:
-            container_storage_version = self.context.raw_param.get("container_storage_version")
             self.context.set_intermediate("container_storage_version", container_storage_version, overwrite_exists=True)
 
             enable_azure_container_storage_v1 = enable_azure_container_storage_param is not None and container_storage_version == CONST_ACSTOR_VERSION_V1