Some more validations added. More lint fixes

src/aks-preview/HISTORY.rst

@@ -9,14 +9,11 @@ If there is no rush to release a new version, please just add a description of t
 
 To release a new version, please select a new version number (usually plus 1 to last patch version, X.Y.Z -> Major.Minor.Patch, more details in `\doc <https://semver.org/>`_), and then add a new section named as the new version number in this file, the content should include the new modifications and everything from the *Pending* section. Finally, update the `VERSION` variable in `setup.py` with this new version number.
 
-Pending
-+++++++
-* Vendor new SDK and bump API version to 2024-04-02-preview.
-
 5.0.0b2
 ++++++++
 * Add option `--ephemeral-disk-volume-type` to `az aks create` and `az aks update` for Azure Container Storage operations.
 * Add option `--azure-container-storage-perf-tier` to `az aks create` and `az aks update` to define resource tiers for Azure Container Storage performance.
+* Vendor new SDK and bump API version to 2024-04-02-preview.
 
 5.0.0b1
 ++++++++

src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py

@@ -192,9 +192,14 @@ def get_extension_installed_and_cluster_configs(
                 )
                 cpu_value = config_settings.get("global.cli.resources.ioEngine.cpu", "1")
                 enable_ephemeral_bypass_annotation = (
-                    config_settings.get("global.cli.storagePool.ephemeralDisk.enableEphemeralBypassAnnotation", "False") == "True"
+                    config_settings.get(
+                        "global.cli.storagePool.ephemeralDisk.enableEphemeralBypassAnnotation", "False"
+                    ) == "True"
+                )
+                perf_tier = config_settings.get(
+                    "global.cli.storagePool.ephemeralDisk.nvme.perfTier",
+                    CONST_EPHEMERAL_NVME_PERF_TIER_STANDARD
                 )
-                perf_tier = config_settings.get("global.cli.storagePool.ephemeralDisk.nvme.perfTier", CONST_EPHEMERAL_NVME_PERF_TIER_STANDARD)
 
                 if perf_tier.lower() == CONST_EPHEMERAL_NVME_PERF_TIER_BASIC.lower():
                     perf_tier = CONST_EPHEMERAL_NVME_PERF_TIER_BASIC
@@ -339,7 +344,10 @@ def get_desired_resource_value_args(
             updated_hugepages_number = 512
         elif (storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and
               storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME):
-            updated_core_value = _get_ephemeral_nvme_cpu_value_based_on_vm_size_perf_tier(nodepool_skus, ephemeral_nvme_perf_tier)
+            updated_core_value = _get_ephemeral_nvme_cpu_value_based_on_vm_size_perf_tier(
+                nodepool_skus,
+                ephemeral_nvme_perf_tier,
+            )
             updated_memory_value = 2
             updated_hugepages_value = 2
             updated_hugepages_number = 1024
@@ -501,7 +509,10 @@ def _get_current_resource_values(
         current_hugepages_number = 512
     if is_ephemeralDisk_nvme_enabled:
         if current_core_value is None and nodepool_skus is not None:
-            core_value = _get_ephemeral_nvme_cpu_value_based_on_vm_size_perf_tier(nodepool_skus, ephemeral_nvme_perf_tier)
+            core_value = _get_ephemeral_nvme_cpu_value_based_on_vm_size_perf_tier(
+                nodepool_skus,
+                ephemeral_nvme_perf_tier,
+            )
         current_memory_value = 2
         current_hugepages_value = 2
         current_hugepages_number = 1024

src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py

@@ -5,6 +5,9 @@
 
 import re
 
+from azext_aks_preview._consts import (
+    CONST_DEFAULT_NODE_OS_TYPE
+)
 from azext_aks_preview.azurecontainerstorage._consts import (
     CONST_ACSTOR_ALL,
     CONST_ACSTOR_IO_ENGINE_LABEL_KEY,
@@ -38,7 +41,7 @@
 logger = get_logger(__name__)
 
 
-def validate_disable_azure_container_storage_params(
+def validate_disable_azure_container_storage_params(  # pylint: disable=too-many-branches
     storage_pool_type,
     storage_pool_name,
     storage_pool_sku,
@@ -169,7 +172,7 @@ def validate_disable_azure_container_storage_params(
             )
 
 
-def validate_enable_azure_container_storage_params(
+def validate_enable_azure_container_storage_params(  # pylint: disable=too-many-locals,too-many-branches
     storage_pool_type,
     storage_pool_name,
     storage_pool_sku,
@@ -261,6 +264,7 @@ def validate_enable_azure_container_storage_params(
 
             if (storage_pool_name is not None or storage_pool_sku is not None or
                 storage_pool_size is not None or nodepool_list is not None):
+                # pylint: disable=too-many-boolean-expressions
                 if (ephemeral_disk_volume_type is not None and
                     required_type_installed_for_disk_vol_type and ephemeral_disk_nvme_perf_tier is None) or \
                    (ephemeral_disk_volume_type is None and
@@ -356,6 +360,7 @@ def validate_enable_azure_container_storage_params(
                 f'{storage_pool_type} in the cluster.'
             )
 
+        # pylint: disable=too-many-boolean-expressions
         if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and \
            ephemeral_disk_volume_type is None and \
            ((is_ephemeralDisk_nvme_enabled and
@@ -448,14 +453,42 @@ def _validate_nodepools(
             'from the node pool and use node pools which has nodes with 4 or more cores and try again.'
         )
     else:
-        _validate_nodepool_names(nodepool_list, agentpool_details)
+        agentpool_names = []
+        for details in agentpool_details:
+            agentpool_names.append(details.get("name"))
+        if not nodepool_list:
+            agentpool_names_str = ', '.join(agentpool_names)
+            raise RequiredArgumentMissingError(
+                'Multiple node pools present. Please define the node pools on which you want '
+                'to enable Azure Container Storage using --azure-container-storage-nodepools.'
+                f'\nNode pools available in the cluster are: {agentpool_names_str}.'
+                '\nAborting Azure Container Storage operation.'
+            )
+        _validate_nodepool_names(nodepool_list, agentpool_names)
         nodepool_arr = nodepool_list.split(',')
 
     nvme_nodepool_found = False
+    available_node_count = 0
     for nodepool in nodepool_arr:
         for agentpool in agentpool_details:
             pool_name = agentpool.get("name")
             if nodepool == pool_name:
+                os_type = agentpool.get("os_type")
+                if os_type is not None and os_type.lower() != CONST_DEFAULT_NODE_OS_TYPE.lower():
+                    raise InvalidArgumentValueError(
+                        f'Azure Container Storage can be enabled only on {CONST_DEFAULT_NODE_OS_TYPE} nodepools. '
+                        f'Node pool: {pool_name}, os type: {os_type} does not meet the criteria.'
+                    )
+                mode = agentpool.get("mode")
+                node_taints = agentpool.get("node_taints")
+                if mode.lower() == "system" and node_taints is not None:
+                    critical_taint = "CriticalAddonsOnly=true:NoSchedule"
+                    if critical_taint.casefold() in (taint.casefold() for taint in node_taints):
+                        raise InvalidArgumentValueError(
+                            f'Unable to install Azure Container Storage on system nodepool: {pool_name} '
+                            f'since it has a taint {critical_taint}. Remove the taint from the node pool '
+                            'and retry the Azure Container Storage operation.'
+                        )
                 vm_size = agentpool.get("vm_size")
                 if vm_size is not None:
                     cpu_value = get_cores_from_sku(vm_size)
@@ -469,6 +502,14 @@ def _validate_nodepools(
                     if vm_size.lower().startswith('standard_l'):
                         nvme_nodepool_found = True
 
+                node_count = agentpool.get("count")
+                available_node_count = available_node_count + node_count
+
+    if available_node_count < 3:
+        raise UnknownError(
+            'Insufficient nodes present. Azure Container Storage requires atleast 3 nodes to be enabled.'
+        )
+
     if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK and \
        storage_pool_option == CONST_STORAGE_POOL_OPTION_NVME and \
        not nvme_nodepool_found:
@@ -481,7 +522,7 @@ def _validate_nodepools(
 # _validate_nodepool_names validates that the nodepool_list is a comma separated
 # string consisting of valid nodepool names i.e. a lower alphanumeric
 # characters and the first character should be lowercase letter.
-def _validate_nodepool_names(nodepool_names, agentpool_details):
+def _validate_nodepool_names(nodepool_names, agentpool_names):
     pattern = r'^[a-z][a-z0-9]*(?:,[a-z][a-z0-9]*)*$'
     if re.fullmatch(pattern, nodepool_names) is None:
         raise InvalidArgumentValueError(
@@ -491,10 +532,6 @@ def _validate_nodepool_names(nodepool_names, agentpool_details):
             "alphanumeric characters and must begin with a lowercase letter."
         )
 
-    agentpool_names = []
-    for details in agentpool_details:
-        agentpool_names.append(details.get("name"))
-
     nodepool_list = nodepool_names.split(',')
     for nodepool in nodepool_list:
         if nodepool not in agentpool_names:

src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py

@@ -297,7 +297,10 @@ def perform_enable_azure_container_storage(  # pylint: disable=too-many-statemen
                         existing_ephemeral_disk_volume_type.lower() == CONST_DISK_TYPE_PV_WITH_ANNOTATION.lower()
                     )
                     update_settings.append(
-                        {"global.cli.storagePool.ephemeralDisk.enableEphemeralBypassAnnotation": enable_ephemeral_bypass_annotation}
+                        {
+                            "global.cli.storagePool.ephemeralDisk.enableEphemeralBypassAnnotation":
+                            enable_ephemeral_bypass_annotation
+                        }
                     )
             else:
                 logger.error("AKS update to enable Azure Container Storage failed.\nError: %s", ex)
@@ -549,7 +552,7 @@ def perform_disable_azure_container_storage(  # pylint: disable=too-many-stateme
         resource_args = get_desired_resource_value_args(
             storage_pool_type,
             storage_pool_option,
-            ephemeral_nvme_perf_tier,
+            ephemeral_disk_nvme_perf_tier,
             current_core_value,
             is_azureDisk_enabled,
             is_elasticSan_enabled,

src/aks-preview/azext_aks_preview/managed_cluster_decorator.py

@@ -3347,20 +3347,37 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
         # 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")
-            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 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["name"] = agentpool.name
             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
             agentpool_details.append(pool_details)
             # Marking the only agentpool name as the valid nodepool for
             # installing Azure Container Storage during `az aks create`
@@ -4084,6 +4101,19 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
                 'and --disable-azure-container-storage together.'
             )
 
+        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.'
+            )
+
         # pylint: disable=too-many-nested-blocks
         if enable_azure_container_storage or disable_azure_container_storage:
             # Require the agent pool profiles for azure container storage
@@ -4124,8 +4154,12 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
                 for agentpool in mc.agent_pool_profiles:
                     pool_details = {}
                     pool_details["vm_size"] = agentpool.vm_size
+                    pool_details["count"] = agentpool.count
                     node_name = agentpool.name
                     pool_details["name"] = node_name
+                    pool_details["os_type"] = agentpool.os_type
+                    pool_details["mode"] = agentpool.mode
+                    pool_details["node_taints"] = agentpool.node_taints
                     if agentpool.node_labels is not None:
                         node_labels = agentpool.node_labels
                         if node_labels is not None and \
@@ -4142,7 +4176,7 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
                 # one nodepool exists, choose the only nodepool by default.
                 if not is_extension_installed:
                     if nodepool_list is None:
-                        nodepool_list = "nodepool1"
+                        nodepool_list = ""
                         if len(labelled_nodepool_arr) > 0:
                             nodepool_list = ','.join(labelled_nodepool_arr)
                         elif len(agentpool_details) == 1:

src/aks-preview/linter_exclusions.yml

@@ -60,6 +60,12 @@ aks create:
     enable_azure_container_storage:
       rule_exclusions:
         - option_length_too_long
+    ephemeral_disk_volume_type:
+      rule_exclusions:
+        - option_length_too_long
+    ephemeral_disk_nvme_perf_tier:
+      rule_exclusions:
+        - option_length_too_long
     enable_ai_toolchain_operator:
       rule_exclusions:
         - option_length_too_long
@@ -173,6 +179,12 @@ aks update:
     azure_container_storage_nodepools:
       rule_exclusions:
         - option_length_too_long
+    ephemeral_disk_volume_type:
+      rule_exclusions:
+        - option_length_too_long
+    ephemeral_disk_nvme_perf_tier:
+      rule_exclusions:
+        - option_length_too_long
     enable_ai_toolchain_operator:
       rule_exclusions:
         - option_length_too_long