From 339aaeb54b264cdd7a74d237d99b17fb37c2ca21 Mon Sep 17 00:00:00 2001 From: Akash Mukhopadhyay Date: Mon, 24 Jun 2024 22:15:20 -0700 Subject: [PATCH] Some more updates --- .../azurecontainerstorage/_helpers.py | 34 +++++++++++-------- .../azurecontainerstorage/acstor_ops.py | 15 +++++--- .../managed_cluster_decorator.py | 22 ++++++++---- 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py index 8991ee0be3e..a4e8453e920 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py @@ -10,15 +10,15 @@ CONST_ACSTOR_ALL, CONST_ACSTOR_IO_ENGINE_LABEL_KEY, CONST_ACSTOR_K8S_EXTENSION_NAME, + CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY, + CONST_DISK_TYPE_PV_WITH_ANNOTATION, CONST_EPHEMERAL_NVME_PERF_TIER_BASIC, CONST_EPHEMERAL_NVME_PERF_TIER_PREMIUM, CONST_EPHEMERAL_NVME_PERF_TIER_STANDARD, - CONST_DISK_TYPE_EPHEMERAL_VOLUME_ONLY, CONST_EXT_INSTALLATION_NAME, CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME, CONST_K8S_EXTENSION_CUSTOM_MOD_NAME, CONST_K8S_EXTENSION_NAME, - CONST_DISK_TYPE_PV_WITH_ANNOTATION, CONST_STORAGE_POOL_OPTION_NVME, CONST_STORAGE_POOL_OPTION_SSD, CONST_STORAGE_POOL_TYPE_AZURE_DISK, @@ -313,6 +313,7 @@ def get_desired_resource_value_args( is_elasticSan_enabled, is_ephemeralDisk_localssd_enabled, is_ephemeralDisk_nvme_enabled, + perf_tier_updated, nodepool_skus, is_enabling_op, ): @@ -343,7 +344,8 @@ def get_desired_resource_value_args( 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): + (storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME or + is_ephemeralDisk_nvme_enabled or perf_tier_updated)): updated_core_value = _get_ephemeral_nvme_cpu_value_based_on_vm_size_perf_tier( nodepool_skus, ephemeral_nvme_perf_tier, @@ -352,11 +354,13 @@ def get_desired_resource_value_args( 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 + if not perf_tier_updated: + # For an operation where we are not modifying the perf tiers for nvme, + # 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 \ @@ -437,9 +441,10 @@ def get_cores_from_sku(vm_size): def check_if_new_storagepool_creation_required( storage_pool_type, - storage_pool_option, ephemeral_disk_volume_type, ephemeral_disk_nvme_perf_tier, + existing_ephemeral_disk_volume_type, + existing_ephemeral_nvme_perf_tier, is_extension_installed, is_ephemeralDisk_nvme_enabled, is_ephemeralDisk_localssd_enabled, @@ -447,14 +452,13 @@ def check_if_new_storagepool_creation_required( if not is_extension_installed or \ not (is_ephemeralDisk_localssd_enabled or is_ephemeralDisk_nvme_enabled) or \ storage_pool_type != CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK or \ - (ephemeral_disk_nvme_perf_tier is None and ephemeral_disk_volume_type is None): + ((ephemeral_disk_volume_type is None or + (existing_ephemeral_nvme_perf_tier.lower() == ephemeral_disk_volume_type.lower())) and + (ephemeral_disk_nvme_perf_tier is None or + (existing_ephemeral_nvme_perf_tier.lower() == ephemeral_disk_nvme_perf_tier.lower()))): return True - if (storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD and is_ephemeralDisk_localssd_enabled) or \ - (storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME and is_ephemeralDisk_nvme_enabled): - return False - - return True + return False def _get_ephemeral_nvme_cpu_value_based_on_vm_size_perf_tier(nodepool_skus, perf_tier): 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 7e2ca3472f2..706912400f3 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py @@ -6,18 +6,18 @@ from azure.cli.core.azclierror import UnknownError from azure.cli.core.commands import LongRunningOperation from azext_aks_preview.azurecontainerstorage._consts import ( + CONST_ACSTOR_ALL, CONST_ACSTOR_K8S_EXTENSION_NAME, + CONST_DISK_TYPE_PV_WITH_ANNOTATION, CONST_EPHEMERAL_NVME_PERF_TIER_STANDARD, CONST_EXT_INSTALLATION_NAME, CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME, CONST_K8S_EXTENSION_CUSTOM_MOD_NAME, - CONST_DISK_TYPE_PV_WITH_ANNOTATION, CONST_STORAGE_POOL_DEFAULT_SIZE, CONST_STORAGE_POOL_DEFAULT_SIZE_ESAN, 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, @@ -84,9 +84,10 @@ def perform_enable_azure_container_storage( # pylint: disable=too-many-statemen is_storagepool_create_op_required = check_if_new_storagepool_creation_required( storage_pool_type, - storage_pool_option, ephemeral_disk_volume_type, ephemeral_disk_nvme_perf_tier, + existing_ephemeral_disk_volume_type, + existing_ephemeral_nvme_perf_tier, is_extension_installed, is_ephemeralDisk_nvme_enabled, is_ephemeralDisk_localssd_enabled, @@ -146,6 +147,10 @@ def perform_enable_azure_container_storage( # pylint: disable=too-many-statemen if ephemeral_disk_nvme_perf_tier is None: ephemeral_disk_nvme_perf_tier = existing_ephemeral_nvme_perf_tier + perf_tier_updated = False + if existing_ephemeral_nvme_perf_tier.lower() != ephemeral_disk_nvme_perf_tier.lower(): + perf_tier_updated = True + config_settings.extend( [ {"global.cli.activeControl": True}, @@ -182,6 +187,7 @@ def perform_enable_azure_container_storage( # pylint: disable=too-many-statemen is_elasticSan_enabled, is_ephemeralDisk_localssd_enabled, is_ephemeralDisk_nvme_enabled, + perf_tier_updated, acstor_nodepool_skus, True, ) @@ -339,7 +345,7 @@ def perform_disable_azure_container_storage( # pylint: disable=too-many-stateme ): # This will be set true only when aks-preview extension is used # and we want the aks-preview ManagedClusterDecorator to call the - # perform_enable_azure_container_storage function. + # perform_disable_azure_container_storage function. if not is_called_from_extension: return client_factory = get_k8s_extension_module(CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME) @@ -557,6 +563,7 @@ def perform_disable_azure_container_storage( # pylint: disable=too-many-stateme is_elasticSan_enabled, is_ephemeralDisk_localssd_enabled, is_ephemeralDisk_nvme_enabled, + False, None, False, ) 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 ec3cb4ae7ca..7033efda88c 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -4206,13 +4206,23 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: ) if is_ephemeralDisk_nvme_enabled and ephemeral_disk_nvme_perf_tier is not None: - msg = ( - "Changing ephemeralDisk NVMe performance tier may result in a temporary " - "interruption to the applications using Azure Container Storage. Do you " - "want to continue with this operation?" + is_azure_container_storage_perf_tier_op_set = self.context.get_intermediate( + "azure_container_storage_perf_tier_op_set", + default_value="default", ) - if not (self.context.get_yes() or prompt_y_n(msg, default="n")): - raise DecoratorEarlyExitException() + + if is_azure_container_storage_perf_tier_op_set == "default": + msg = ( + "Changing ephemeralDisk NVMe performance tier may result in a temporary " + "interruption to the applications using Azure Container Storage. Do you " + "want to continue with this operation?" + ) + + if not (self.context.get_yes() or prompt_y_n(msg, default="n")): + raise DecoratorEarlyExitException() + + self.context.set_intermediate("azure_container_storage_perf_tier_op_set", True, overwrite_exists=True) + # If the extension is already installed, # we expect that the Azure Container Storage # nodes are already labelled. Use those label