diff --git a/src/aks-preview/HISTORY.rst b/src/aks-preview/HISTORY.rst index 13511344868..9fa03caea41 100644 --- a/src/aks-preview/HISTORY.rst +++ b/src/aks-preview/HISTORY.rst @@ -13,6 +13,11 @@ Pending +++++++ * Vendor new SDK and bump API version to 2023-09-02-preview. +0.5.167 ++++++++ +* Fix the default storagepool name value created for Azure Container Storage. +* Ensure the correct nodepool name is picked and labelled by Azure Container Storage while installing with `az aks create`. + 0.5.166 +++++++ * Add `--network-policy` to the `az aks update` command. diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_consts.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_consts.py index f4f62233f2a..0b23e1cfeb8 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_consts.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_consts.py @@ -3,6 +3,8 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- +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" CONST_EXT_INSTALLATION_NAME = "azurecontainerstorage" CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME = "azext_k8s_extension._client_factory" @@ -24,5 +26,4 @@ CONST_STORAGE_POOL_TYPE_ELASTIC_SAN = "elasticSan" CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK = "ephemeralDisk" -CONST_STORAGE_POOL_RANDOM_LENGTH = 7 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 46a9b099a45..1e258f18a40 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py @@ -18,7 +18,6 @@ CONST_K8S_EXTENSION_NAME, CONST_STORAGE_POOL_NAME_PREFIX, CONST_STORAGE_POOL_OPTION_NVME, - CONST_STORAGE_POOL_RANDOM_LENGTH, CONST_STORAGE_POOL_TYPE_ELASTIC_SAN, CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK, RP_REGISTRATION_POLLING_INTERVAL_IN_SEC, @@ -26,8 +25,6 @@ from datetime import datetime from knack.log import get_logger -import random -import string import time logger = get_logger(__name__) @@ -150,11 +147,6 @@ def perform_role_operations_on_managed_rg(cmd, subscription_id, node_resource_gr return False -def generate_random_storage_pool_name(): - random_name = CONST_STORAGE_POOL_NAME_PREFIX + ''.join(random.choices(string.ascii_lowercase, k=CONST_STORAGE_POOL_RANDOM_LENGTH)) - return random_name - - def get_k8s_extension_module(module_name): try: # adding the installed extension in the path diff --git a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py index d372c4f7a8f..7a1f4a5b98f 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py @@ -29,19 +29,6 @@ logger = get_logger(__name__) -def validate_nodepool_names_with_cluster_nodepools(nodepool_names, agentpool_details): - nodepool_list = nodepool_names.split(',') - for nodepool in nodepool_list: - if nodepool not in agentpool_details: - raise InvalidArgumentValueError( - 'Nodepool: {} not found. ' - 'Please provide existing nodepool names in --azure-container-storage-nodepools.' - '\nUse command `az nodepool list` to get the list of nodepools in the cluster.' - '\nAborting installation of Azure Container Storage.' - .format(nodepool) - ) - - def validate_azure_container_storage_params( enable_azure_container_storage, disable_azure_container_storage, @@ -51,6 +38,8 @@ def validate_azure_container_storage_params( 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( @@ -65,6 +54,7 @@ def validate_azure_container_storage_params( storage_pool_option, storage_pool_size, nodepool_list, + is_extension_installed, ) elif enable_azure_container_storage: @@ -74,8 +64,11 @@ def validate_azure_container_storage_params( 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, @@ -83,7 +76,15 @@ def _validate_disable_azure_container_storage_params( storage_pool_option, storage_pool_size, nodepool_list, + is_extension_installed, ): + 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.' + ) + if storage_pool_name is not None: raise MutuallyExclusiveArgumentError( 'Conflicting flags. Cannot define --storage-pool-name value ' @@ -121,7 +122,15 @@ def _validate_enable_azure_container_storage_params( storage_pool_sku, storage_pool_option, storage_pool_size, + is_extension_installed, ): + 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) @@ -181,3 +190,39 @@ def _validate_enable_azure_container_storage_params( 'Storage pools using Ephemeral disk use all capacity available on the local device. ' ' --storage-pool-size will be ignored.' ) + + +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 " + "alphanumeric characters and must begin with a lowercase letter." + ) + + nodepool_list = nodepool_names.split(',') + for nodepool in nodepool_list: + if nodepool not in agentpool_details: + if len(agentpool_details) > 1: + raise InvalidArgumentValueError( + 'Nodepool: {0} not found. ' + 'Please provide a comma separated string of existing nodepool names ' + 'in --azure-container-storage-nodepools.' + '\nNodepools available in the cluster are: {1}.' + '\nAborting installation of Azure Container Storage.' + .format(nodepool, ', '.join(agentpool_details)) + ) + else: + raise InvalidArgumentValueError( + 'Nodepool: {0} not found. ' + 'Please provide a comma separated string of existing nodepool names ' + 'in --azure-container-storage-nodepools.' + '\nNodepool available in the cluster is: {1}.' + '\nAborting installation of Azure Container Storage.' + .format(nodepool, agentpool_details[0]) + ) 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 7dfe79dbac8..837141c6cc4 100644 --- a/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py +++ b/src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py @@ -21,13 +21,11 @@ ) from azext_aks_preview.azurecontainerstorage._helpers import ( check_if_extension_is_installed, - generate_random_storage_pool_name, get_k8s_extension_module, perform_role_operations_on_managed_rg, register_dependent_rps, should_create_storagepool, ) -from azext_aks_preview.azurecontainerstorage._validators import validate_nodepool_names_with_cluster_nodepools from knack.log import get_logger from knack.prompting import prompt_y_n @@ -55,15 +53,6 @@ def perform_enable_azure_container_storage( if not register_dependent_rps(cmd, subscription_id): return - # Step 2: Check if extension already installed incase of an update call - if not is_cluster_create and check_if_extension_is_installed(cmd, resource_group, cluster_name): - logger.error( - "Extension type {0} already installed on cluster." - "\nAborting installation of Azure Container Storage." - .format(CONST_ACSTOR_K8S_EXTENSION_NAME) - ) - return - if nodepool_names is None: nodepool_names = "nodepool1" if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK: @@ -72,11 +61,9 @@ def perform_enable_azure_container_storage( if storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD: storage_pool_option = "temp" - validate_nodepool_names_with_cluster_nodepools(nodepool_names, agentpool_details) - - # Step 3: Validate if storagepool should be created. + # Step 2: Validate if storagepool should be created. # Depends on the following: - # 3a: Grant AKS cluster's node identity the following + # 2a: Grant AKS cluster's node identity the following # roles on the AKS managed resource group: # 1. Reader # 2. Network Contributor @@ -84,7 +71,7 @@ def perform_enable_azure_container_storage( # 4. Elastic SAN Volume Group Owner # Ensure grant was successful if creation of # Elastic SAN storagepool is requested. - # 3b: Ensure Ls series nodepool is present if creation + # 2b: Ensure Ls series nodepool is present if creation # of Ephemeral NVMe Disk storagepool is requested. create_storage_pool = should_create_storagepool( cmd, @@ -97,11 +84,12 @@ def perform_enable_azure_container_storage( nodepool_names, ) - # Step 4: Configure the storagepool parameters - config_settings = [] + # 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 = generate_random_storage_pool_name() + 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 \ @@ -112,7 +100,6 @@ def perform_enable_azure_container_storage( {"cli.storagePool.name": storage_pool_name}, {"cli.storagePool.size": storage_pool_size}, {"cli.storagePool.type": storage_pool_type}, - {"cli.node.nodepools": nodepool_names}, ] ) @@ -173,21 +160,19 @@ def perform_disable_azure_container_storage( kubelet_identity_object_id, perform_validation, ): - # Step 1: Check if show_k8s_extension returns an extension already installed - if not check_if_extension_is_installed(cmd, resource_group, cluster_name): - raise UnknownError( - "Extension type {0} not installed on cluster." - "\nAborting disabling of Azure Container Storage." - .format(CONST_ACSTOR_K8S_EXTENSION_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) no_wait_delete_op = False - # Step 2: Perform validation if accepted by user + # Step 1: Perform validation if accepted by user if perform_validation: - config_settings = [{"cli.storagePool.uninstallValidation": True}] + # 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": ""}, + ] try: update_result = k8s_extension_custom_mod.update_k8s_extension( cmd, @@ -233,7 +218,7 @@ def perform_disable_azure_container_storage( else: raise UnknownError("Validation failed. Unable to disable Azure Container Storage. Reseting cluster state.") - # Step 3: If the extension is installed and validation succeeded or skipped, call delete_k8s_extension + # 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( cmd, @@ -253,7 +238,7 @@ def perform_disable_azure_container_storage( logger.warning("Azure Container Storage has been disabled.") - # Step 4: Revoke AKS cluster's node identity the following + # Step 3: Revoke AKS cluster's node identity the following # roles on the AKS managed resource group: # 1. Reader # 2. Network Contributor 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 76d4c9e9db4..af27f356f8f 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -2759,9 +2759,9 @@ def set_up_azure_monitor_profile(self, mc: ManagedCluster) -> ManagedCluster: self.context.set_intermediate("azuremonitormetrics_addon_enabled", True, overwrite_exists=True) return mc - def set_up_azure_container_storage(self, mc: ManagedCluster) -> None: + def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: """Set up azure container storage for the Managed Cluster object - :return: None + :return: ManagedCluster """ self._ensure_mc(mc) # read the azure container storage values passed @@ -2772,6 +2772,20 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> None: 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_name_list = [agentpool.name] + # 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_azure_container_storage_params validate_azure_container_storage_params( True, @@ -2781,11 +2795,27 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> None: pool_sku, pool_option, pool_size, - None, + nodepool_list, + agentpool_name_list, + is_extension_already_installed, + ) + + # Setup Azure Container Storage labels on the nodepool + from azext_aks_preview.azurecontainerstorage._consts import ( + CONST_ACSTOR_IO_ENGINE_LABEL_KEY, + CONST_ACSTOR_IO_ENGINE_LABEL_VAL ) + 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("enable_azure_container_storage", True, overwrite_exists=True) + self.context.set_intermediate("azure_container_storage_nodepools", nodepool_list, overwrite_exists=True) + + return mc def set_up_auto_upgrade_profile(self, mc: ManagedCluster) -> ManagedCluster: """Set up auto upgrade profile for the ManagedCluster object. @@ -2928,7 +2958,7 @@ def construct_mc_profile_preview(self, bypass_restore_defaults: bool = False) -> # set up metrics profile mc = self.set_up_metrics_profile(mc) # set up for azure container storage - self.set_up_azure_container_storage(mc) + mc = self.set_up_azure_container_storage(mc) # DO NOT MOVE: keep this at the bottom, restore defaults mc = self._restore_defaults_in_mc(mc) @@ -3114,8 +3144,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 = {} - for agentpool_profile in cluster.agent_pool_profiles: + 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 self.context.external_functions.perform_enable_azure_container_storage( @@ -3130,7 +3163,7 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: pool_size, pool_sku, pool_option, - "nodepool1", + nodepool_name, agent_pool_details, True, ) @@ -3266,21 +3299,53 @@ def update_enable_network_observability_in_network_profile(self, mc: ManagedClus ) return mc - def update_azure_container_storage(self, mc: ManagedCluster) -> None: + def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster: """Update azure container storage for the Managed Cluster object - :return: None + :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 + nodepool_list = self.context.raw_param.get("azure_container_storage_nodepools") if enable_azure_container_storage or disable_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") - nodepool_list = self.context.raw_param.get("azure_container_storage_nodepools") + 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( + 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, @@ -3291,11 +3356,33 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> None: pool_option, pool_size, nodepool_list, + agentpool_names, + is_extension_already_installed, ) 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 + 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("enable_azure_container_storage", True, overwrite_exists=True) + self.context.set_intermediate("azure_container_storage_nodepools", nodepool_list, overwrite_exists=True) if disable_azure_container_storage: pre_uninstall_validate = False @@ -3310,6 +3397,8 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> None: self.context.set_intermediate("disable_azure_container_storage", True, overwrite_exists=True) self.context.set_intermediate("pre_uninstall_validate_azure_container_storage", pre_uninstall_validate, overwrite_exists=True) + return mc + def update_load_balancer_profile(self, mc: ManagedCluster) -> ManagedCluster: """Update load balancer profile for the ManagedCluster object. @@ -3947,7 +4036,7 @@ def update_mc_profile_preview(self) -> ManagedCluster: # update metrics profile mc = self.update_metrics_profile(mc) # update azure container storage - self.update_azure_container_storage(mc) + mc = self.update_azure_container_storage(mc) return mc @@ -3989,7 +4078,7 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None: 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.raw_param.get("azure_container_storage_nodepools") + nodepool_list = self.context.get_intermediate("azure_container_storage_nodepools") kubelet_identity_object_id = cluster.identity_profile["kubeletidentity"].object_id agent_pool_details = {} for agentpool_profile in cluster.agent_pool_profiles: 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 61e6bdcb23b..dc5698331cf 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 @@ -6759,6 +6759,47 @@ def test_aks_create_with_azurecontainerstorage(self, resource_group, resource_gr self.is_empty(), ]) + # live only due to downloading k8s-extension extension + @live_only() + @AllowLargeResponse(8192) + @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix='clitest', location='westus2') + def test_aks_create_with_azurecontainerstorage_with_nodepool_name(self, resource_group, resource_group_location): + # reset the count so in replay mode the random names will start with 0 + self.test_resources_count = 0 + # kwargs for string formatting + aks_name = self.create_random_name('cliakstest', 16) + nodepool_name = self.create_random_name('n', 6) + + node_vm_size = 'standard_d4s_v3' + self.kwargs.update({ + 'resource_group': resource_group, + 'name': aks_name, + 'location': resource_group_location, + 'resource_type': 'Microsoft.ContainerService/ManagedClusters', + 'ssh_key_value': self.generate_ssh_keys(), + 'node_vm_size': node_vm_size, + 'nodepool_name': nodepool_name + }) + + # add k8s-extension extension for azurecontainerstorage operations. + self.cmd('extension add --name k8s-extension') + + 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 --nodepool-name {nodepool_name} --enable-managed-identity --enable-azure-container-storage azureDisk --output=json' + + # enabling azurecontainerstorage will not affect any field in the cluster. + # the only check we should perform is to verify that the cluster is provisioned successfully. + self.cmd(create_cmd, checks=[ + self.check('provisioningState', 'Succeeded'), + self.check('agentPoolProfiles[0].name', nodepool_name), + ]) + + # delete + cmd = 'aks delete --resource-group={resource_group} --name={name} --yes --no-wait' + self.cmd(cmd, checks=[ + self.is_empty(), + ]) + @AllowLargeResponse() @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix='clitest', location='westus2') def test_aks_update_with_azuremonitormetrics(self, resource_group, resource_group_location): 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 9537dde9f90..b7156d30378 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 @@ -626,7 +626,16 @@ 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) + acstor_validator.validate_azure_container_storage_params(True, True, None, None, None, None, None, None, None, False) + self.assertEqual(str(cm.exception), err) + + 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.' + 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) self.assertEqual(str(cm.exception), err) def test_disable_flag_with_storage_pool_name(self): @@ -634,7 +643,7 @@ def test_disable_flag_with_storage_pool_name(self): err = 'Conflicting flags. Cannot define --storage-pool-name value '\ '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) + acstor_validator.validate_azure_container_storage_params(None, True, storage_pool_name, None, None, None, None, None, None, True) self.assertEqual(str(cm.exception), err) def test_disable_flag_with_storage_pool_sku(self): @@ -642,7 +651,7 @@ def test_disable_flag_with_storage_pool_sku(self): err = 'Conflicting flags. Cannot define --storage-pool-sku 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, storage_pool_sku, None, None, None) + acstor_validator.validate_azure_container_storage_params(None, True, None, None, storage_pool_sku, None, None, None, None, True) self.assertEqual(str(cm.exception), err) def test_disable_flag_with_storage_pool_size(self): @@ -650,7 +659,7 @@ def test_disable_flag_with_storage_pool_size(self): err = 'Conflicting flags. Cannot define --storage-pool-size 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, storage_pool_size, None) + acstor_validator.validate_azure_container_storage_params(None, True, None, None, None, None, storage_pool_size, None, None, True) self.assertEqual(str(cm.exception), err) def test_disable_flag_with_storage_pool_option(self): @@ -658,7 +667,7 @@ def test_disable_flag_with_storage_pool_option(self): err = 'Conflicting flags. Cannot define --storage-pool-option 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, storage_pool_option, None, None) + acstor_validator.validate_azure_container_storage_params(None, True, None, None, None, storage_pool_option, None, None, None, True) self.assertEqual(str(cm.exception), err) def test_disable_flag_with_nodepool_list(self): @@ -666,11 +675,20 @@ def test_disable_flag_with_nodepool_list(self): 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) + acstor_validator.validate_azure_container_storage_params(None, True, None, None, None, None, None, nodepool_list, None, True) 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) + acstor_validator.validate_azure_container_storage_params(None, True, None, None, None, None, None, None, None, True) + + def test_enable_when_extension_installed(self): + is_extension_installed = True + err = 'Invalid usage of --enable-azure-container-storage. '\ + 'Azure Container Storage is already enabled on the cluster. ' \ + 'Aborting installation of Azure Container Storage.' + 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) + self.assertEqual(str(cm.exception), err) def test_enable_with_invalid_storage_pool_name(self): storage_pool_name = "my_test_pool" @@ -678,7 +696,7 @@ def test_enable_with_invalid_storage_pool_name(self): "Accepted values are lowercase alphanumeric characters, " \ "'-' 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) + acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, None, None, None, None, None, None, False) self.assertEqual(str(cm.exception), err) def test_enable_with_sku_and_ephemeral_disk_pool(self): @@ -687,7 +705,7 @@ 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) + acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, storage_pool_type, storage_pool_sku, None, None, None, None, False) self.assertEqual(str(cm.exception), err) def test_enable_with_sku_and_elastic_san_pool(self): @@ -701,7 +719,7 @@ def test_enable_with_sku_and_elastic_san_pool(self): 'when --enable-azure-container-storage is set to elasticSan.' \ .format(supported_skus) 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) + acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, storage_pool_type, storage_pool_sku, None, None, None, None, False) self.assertEqual(str(cm.exception), err) def test_enable_with_option_and_non_ephemeral_disk_pool(self): @@ -710,7 +728,7 @@ 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) + acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, storage_pool_type, None, storage_pool_option, None, None, None, False) self.assertEqual(str(cm.exception), err) def test_enable_with_ssd_option_and_ephemeral_disk_pool(self): @@ -719,7 +737,7 @@ def test_enable_with_ssd_option_and_ephemeral_disk_pool(self): 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) + acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, storage_pool_type, None, storage_pool_option, None, None, None, False) self.assertEqual(str(cm.exception), err) def test_enable_with_invalid_storage_pool_size(self): @@ -727,7 +745,7 @@ def test_enable_with_invalid_storage_pool_size(self): storage_pool_size = "5" 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) + acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, None, None, None, storage_pool_size, None, None, False) self.assertEqual(str(cm.exception), err) def test_enable_with_invalid_size_for_esan_storage_pool(self): @@ -736,37 +754,70 @@ 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) + acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, storage_pool_type, None, None, storage_pool_size, None, None, False) self.assertEqual(str(cm.exception), err) - def test_valid_enable_for_azure_disk_pool(self): + 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_AZURE_DISK - storage_pool_sku = acstor_consts.CONST_STORAGE_POOL_SKU_PREMIUM_LRS - acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, storage_pool_type, storage_pool_sku, None, storage_pool_size, None) + storage_pool_type = acstor_consts.CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK + storage_pool_option = acstor_consts.CONST_STORAGE_POOL_OPTION_NVME + 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 " \ + "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) + self.assertEqual(str(cm.exception), err) - def test_valid_enable_for_ephemeral_disk_pool(self): + 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 - acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, storage_pool_type, None, storage_pool_option, storage_pool_size, None) + nodepool_list = "pool1" + agentpools = ["nodepool1"] + 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.' \ + '\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) + self.assertEqual(str(cm.exception), err) - def test_missing_nodepool_from_cluster_nodepool_list(self): + 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_NVME nodepool_list = "pool1,pool2" - agentpools = {"nodepool1": "NODEPOOL1", "nodepool2": "NODEPOOL2"} - err = 'Nodepool: pool1 not found. Please provide existing nodepool names in --azure-container-storage-nodepools.' \ - '\nUse command `az nodepool list` to get the list of nodepools in the cluster.' \ + agentpools = ["nodepool1", "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.' \ '\nAborting installation of Azure Container Storage.' with self.assertRaises(InvalidArgumentValueError) as cm: - acstor_validator.validate_nodepool_names_with_cluster_nodepools(nodepool_list, agentpools) + acstor_validator.validate_azure_container_storage_params(True, None, storage_pool_name, storage_pool_type, None, None, storage_pool_size, nodepool_list, agentpools, False) self.assertEqual(str(cm.exception), err) - def test_valid_nodepool_list_in_cluster_nodepool(self): + def test_valid_enable_for_azure_disk_pool(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 nodepool_list = "nodepool1,nodepool2" - agentpools = {"nodepool1": "NODEPOOL1", "nodepool2": "NODEPOOL2"} - acstor_validator.validate_nodepool_names_with_cluster_nodepools(nodepool_list, agentpools) + 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) + + def test_valid_enable_for_ephemeral_disk_pool(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 = "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) if __name__ == "__main__": diff --git a/src/aks-preview/setup.py b/src/aks-preview/setup.py index a9aea831709..42aae3d1f6b 100644 --- a/src/aks-preview/setup.py +++ b/src/aks-preview/setup.py @@ -9,7 +9,7 @@ from setuptools import setup, find_packages -VERSION = "0.5.166" +VERSION = "0.5.167" CLASSIFIERS = [ "Development Status :: 4 - Beta",