From c0fa361edfd97f006cc8cb7b327b06026cc036e2 Mon Sep 17 00:00:00 2001 From: Neha Arora Date: Wed, 27 Aug 2025 14:50:19 -0700 Subject: [PATCH 1/2] az aks create/update: Allow latest version to be passed as --container-storage-version parameter --- .../acs/azurecontainerstorage/_consts.py | 1 + .../acs/managed_cluster_decorator.py | 209 ++++++++++-------- 2 files changed, 116 insertions(+), 94 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_consts.py b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_consts.py index dc8770471d5..6c763c99ac5 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_consts.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_consts.py @@ -9,6 +9,7 @@ CONST_ACSTOR_V1_K8S_EXTENSION_NAME = "microsoft.azurecontainerstorage" CONST_ACSTOR_V1_EXT_INSTALLATION_NAME = "azurecontainerstorage" CONST_ACSTOR_VERSION_V1 = "1" +CONST_SUPPORTED_ACSTOR_VERSIONS = ("1", "2") CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY = "EphemeralVolumeOnly" CONST_DISK_TYPE_PV_WITH_ANNOTATION = "PersistentVolumeWithAnnotation" CONST_EPHEMERAL_NVME_PERF_TIER_BASIC = "Basic" 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 fa3b2ebe961..f86afb541d8 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 @@ -52,6 +52,7 @@ CONST_ACSTOR_EXT_INSTALLATION_NAME, CONST_ACSTOR_V1_EXT_INSTALLATION_NAME, CONST_ACSTOR_VERSION_V1, + CONST_SUPPORTED_ACSTOR_VERSIONS, ) from azure.cli.command_modules.acs._helpers import ( check_is_managed_aad_cluster, @@ -6781,102 +6782,109 @@ 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.' - ) + has_container_storage_version = container_storage_version is not None - 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 has_container_storage_version and container_storage_version not in CONST_SUPPORTED_ACSTOR_VERSIONS: + raise InvalidArgumentValueError( + f'Version {container_storage_version} is not supported for enabling Azure Container Storage. ' + f'The only supported versions are {", ".join(CONST_SUPPORTED_ACSTOR_VERSIONS)}' + ) - 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, - ) + if has_container_storage_version 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') - # 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 - ) + 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 + + 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 +8808,21 @@ 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 container_storage_version is not None and container_storage_version not in CONST_SUPPORTED_ACSTOR_VERSIONS: + raise InvalidArgumentValueError( + f'Version {container_storage_version} is not supported for enabling Azure Container Storage. ' + f'The only supported versions are {CONST_SUPPORTED_ACSTOR_VERSIONS}' + ) + 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 From f472334ee632e9f652ef49d1b94f57ab3701b5a1 Mon Sep 17 00:00:00 2001 From: Neha Arora Date: Thu, 28 Aug 2025 13:04:49 -0700 Subject: [PATCH 2/2] Resolve PR comments --- .../azure/cli/command_modules/acs/_params.py | 7 +++++++ .../acs/azurecontainerstorage/_consts.py | 1 - .../acs/managed_cluster_decorator.py | 16 +--------------- 3 files changed, 8 insertions(+), 16 deletions(-) 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 a6a127358ef..c5062dd7344 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_params.py +++ b/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( diff --git a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_consts.py b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_consts.py index 6c763c99ac5..dc8770471d5 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_consts.py +++ b/src/azure-cli/azure/cli/command_modules/acs/azurecontainerstorage/_consts.py @@ -9,7 +9,6 @@ CONST_ACSTOR_V1_K8S_EXTENSION_NAME = "microsoft.azurecontainerstorage" CONST_ACSTOR_V1_EXT_INSTALLATION_NAME = "azurecontainerstorage" CONST_ACSTOR_VERSION_V1 = "1" -CONST_SUPPORTED_ACSTOR_VERSIONS = ("1", "2") CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY = "EphemeralVolumeOnly" CONST_DISK_TYPE_PV_WITH_ANNOTATION = "PersistentVolumeWithAnnotation" CONST_EPHEMERAL_NVME_PERF_TIER_BASIC = "Basic" 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 f86afb541d8..3cf9e588d35 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 @@ -52,7 +52,6 @@ CONST_ACSTOR_EXT_INSTALLATION_NAME, CONST_ACSTOR_V1_EXT_INSTALLATION_NAME, CONST_ACSTOR_VERSION_V1, - CONST_SUPPORTED_ACSTOR_VERSIONS, ) from azure.cli.command_modules.acs._helpers import ( check_is_managed_aad_cluster, @@ -6782,15 +6781,8 @@ 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") - has_container_storage_version = container_storage_version is not None - if has_container_storage_version and container_storage_version not in CONST_SUPPORTED_ACSTOR_VERSIONS: - raise InvalidArgumentValueError( - f'Version {container_storage_version} is not supported for enabling Azure Container Storage. ' - f'The only supported versions are {", ".join(CONST_SUPPORTED_ACSTOR_VERSIONS)}' - ) - - if has_container_storage_version and container_storage_version == CONST_ACSTOR_VERSION_V1: + 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 @@ -8816,12 +8808,6 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: ' Please remove this parameter and try again.' ) - if container_storage_version is not None and container_storage_version not in CONST_SUPPORTED_ACSTOR_VERSIONS: - raise InvalidArgumentValueError( - f'Version {container_storage_version} is not supported for enabling Azure Container Storage. ' - f'The only supported versions are {CONST_SUPPORTED_ACSTOR_VERSIONS}' - ) - if enable_azure_container_storage_param is not None or disable_azure_container_storage_param is not None: self.context.set_intermediate("container_storage_version", container_storage_version, overwrite_exists=True)