{AKS} fix InvalidParameter error when add Machines nodepool (#9689)

Author: HuShaoRu

src/aks-preview/HISTORY.rst

@@ -11,6 +11,11 @@ To release a new version, please select a new version number (usually plus 1 to
 
 Pending
 +++++++
+
+19.0.0b27
++++++++
+* `az aks nodepool add`: Fix `InvalidParameter` error when `mode` is `Machines`.
+
 19.0.0b26
 +++++++
 * `az aks create/update`: Add `--enable-app-routing-istio` / `--disable-app-routing-istio` (short: `--enable-ari` / `--disable-ari`) flags to enable or disable Istio as a Gateway API implementation for App Routing.

src/aks-preview/azext_aks_preview/agentpool_decorator.py

@@ -46,6 +46,7 @@
     CONST_SSH_ACCESS_LOCALUSER,
     CONST_GPU_DRIVER_NONE,
     CONST_NODEPOOL_MODE_MANAGEDSYSTEM,
+    CONST_NODEPOOL_MODE_MACHINES,
 )
 from azext_aks_preview._helpers import (
     get_nodepool_snapshot_by_snapshot_id,
@@ -1452,6 +1453,25 @@ def set_up_managed_system_mode(self, agentpool: AgentPool) -> AgentPool:
 
         return agentpool
 
+    def set_up_machines_mode(self, agentpool: AgentPool) -> AgentPool:
+        """Handle the special Machines mode by resetting all properties except name and mode.
+
+        :param agentpool: the AgentPool object
+        :return: the AgentPool object
+        """
+        self._ensure_agentpool(agentpool)
+
+        mode = self.context.get_mode()
+        if mode == CONST_NODEPOOL_MODE_MACHINES:
+            agentpool.mode = CONST_NODEPOOL_MODE_MACHINES
+            # Make sure all other attributes are None
+            for attr in vars(agentpool):
+                if attr != 'name' and attr != 'mode' and not attr.startswith('_'):
+                    if hasattr(agentpool, attr):
+                        setattr(agentpool, attr, None)
+
+        return agentpool
+
     def set_up_localdns_profile(self, agentpool: AgentPool) -> AgentPool:
         """Set up local DNS profile for the AgentPool object if provided via --localdns-config."""
         self._ensure_agentpool(agentpool)
@@ -1468,11 +1488,12 @@ def construct_agentpool_profile_preview(self) -> AgentPool:
         # DO NOT MOVE: keep this on top, construct the default AgentPool profile
         agentpool = self.construct_agentpool_profile_default(bypass_restore_defaults=True)
 
-        # Check if mode is ManagedSystem, if yes, reset all properties
+        # Check if mode is ManagedSystem or Machines, if yes, reset all other properties
         agentpool = self.set_up_managed_system_mode(agentpool)
+        agentpool = self.set_up_machines_mode(agentpool)
 
-        # If mode is ManagedSystem, skip all other property setups
-        if agentpool.mode == CONST_NODEPOOL_MODE_MANAGEDSYSTEM:
+        # If mode is ManagedSystem or Machines, skip all other property setups
+        if agentpool.mode == CONST_NODEPOOL_MODE_MANAGEDSYSTEM or agentpool.mode == CONST_NODEPOOL_MODE_MACHINES:
             return agentpool
 
         # set up preview vm properties

src/aks-preview/azext_aks_preview/tests/latest/recordings/test_aks_machine_add_cmds.yaml

@@ -37,6 +37,7 @@
     CONST_MANAGED_CLUSTER_SKU_NAME_AUTOMATIC,
     CONST_GPU_DRIVER_NONE,
     CONST_NODEPOOL_MODE_MANAGEDSYSTEM,
+    CONST_NODEPOOL_MODE_MACHINES,
 )
 from azure.cli.command_modules.acs.agentpool_decorator import AKSAgentPoolParamDict
 from azure.cli.command_modules.acs.tests.latest.mocks import (
@@ -1724,6 +1725,41 @@ def common_set_up_managed_system_mode(self):
         # Verify that agentpool is returned unchanged
         self.assertEqual(dec_agentpool_3, original_agentpool_3)
 
+    def common_set_up_machines_mode(self):
+        dec_1 = AKSPreviewAgentPoolAddDecorator(
+            self.cmd,
+            self.client,
+            {"mode": CONST_NODEPOOL_MODE_MACHINES},
+            self.resource_type,
+            self.agentpool_decorator_mode,
+        )
+
+        # fail on passing the wrong agentpool object
+        with self.assertRaises(CLIInternalError):
+            dec_1.set_up_machines_mode(None)
+
+        # Create an agentpool with various properties set
+        agentpool_1 = self.create_initialized_agentpool_instance(
+            restore_defaults=False,
+            count=3,
+            vm_size="Standard_D2s_v3",
+            os_type="Linux",
+            enable_auto_scaling=True,
+            min_count=1,
+            max_count=5,
+        )
+        dec_1.context.attach_agentpool(agentpool_1)
+
+        original_name = agentpool_1.name
+        dec_agentpool_1 = dec_1.set_up_machines_mode(agentpool_1)
+        self.assertEqual(dec_agentpool_1.name, original_name)
+        self.assertEqual(dec_agentpool_1.mode, CONST_NODEPOOL_MODE_MACHINES)
+        for attr_name in vars(dec_agentpool_1):
+            if attr_name not in ['name', 'mode'] and not attr_name.startswith('_'):
+                attr_value = getattr(dec_agentpool_1, attr_name)
+                self.assertIsNone(attr_value,
+                    f"Attribute '{attr_name}' should be None but was '{attr_value}'")
+
     def common_construct_agentpool_profile_preview_with_managed_system_mode(self):
         """Test that construct_agentpool_profile_preview properly handles ManagedSystem mode"""
 
@@ -1765,6 +1801,47 @@ def common_construct_agentpool_profile_preview_with_managed_system_mode(self):
                 self.assertIsNone(attr_value,
                     f"Attribute '{attr_name}' should be None but was '{attr_value}' when mode is ManagedSystem")
 
+    def common_construct_agentpool_profile_preview_with_machines_mode(self):
+        """Test that construct_agentpool_profile_preview properly handles Machines mode"""
+
+        # Test that when mode is Machines, only name and mode are preserved,
+        # and all other property setup methods are bypassed
+        dec = AKSPreviewAgentPoolAddDecorator(
+            self.cmd,
+            self.client,
+            {
+                "nodepool_name": "testnp",
+                "mode": CONST_NODEPOOL_MODE_MACHINES,
+                # Add some parameters that would normally set properties
+                "node_count": 3,
+                "node_vm_size": "Standard_D2s_v3",
+                "crg_id": "test_crg_id",
+                "enable_artifact_streaming": True,
+            },
+            self.resource_type,
+            self.agentpool_decorator_mode,
+        )
+
+        # Construct the agentpool profile with mocked Azure API calls
+        with patch(
+            "azext_aks_preview.agentpool_decorator.cf_agent_pools",
+            return_value=Mock(list=Mock(return_value=[])),
+        ):
+            agentpool = dec.construct_agentpool_profile_preview()
+
+        # Verify that name is preserved
+        self.assertEqual(agentpool.name, "testnp")
+
+        # Verify that mode is set to Machines
+        self.assertEqual(agentpool.mode, CONST_NODEPOOL_MODE_MACHINES)
+
+        # Verify that all other properties are None (bypassed)
+        for attr_name in vars(agentpool):
+            if attr_name not in ['name', 'mode'] and not attr_name.startswith('_'):
+                attr_value = getattr(agentpool, attr_name)
+                self.assertIsNone(attr_value,
+                    f"Attribute '{attr_name}' should be None but was '{attr_value}' when mode is Machines")
+
     def common_set_up_upgrade_strategy(self):
         # Test case 1: No upgrade strategy provided
         dec_1 = AKSPreviewAgentPoolAddDecorator(
@@ -1946,6 +2023,9 @@ def test_set_up_virtual_machines_profile(self):
     def test_set_up_managed_system_mode(self):
         self.common_set_up_managed_system_mode()
 
+    def test_set_up_machines_mode(self):
+        self.common_set_up_machines_mode()
+
     def test_set_up_upgrade_strategy(self):
         self.common_set_up_upgrade_strategy()
 
@@ -2034,6 +2114,9 @@ def test_set_up_blue_green_upgrade_settings(self):
     def test_construct_agentpool_profile_preview_with_managed_system_mode(self):
         self.common_construct_agentpool_profile_preview_with_managed_system_mode()
 
+    def test_construct_agentpool_profile_preview_with_machines_mode(self):
+        self.common_construct_agentpool_profile_preview_with_machines_mode()
+
 
 class AKSPreviewAgentPoolAddDecoratorManagedClusterModeTestCase(
     AKSPreviewAgentPoolAddDecoratorCommonTestCase
@@ -2083,6 +2166,9 @@ def test_set_up_virtual_machines_profile(self):
     def test_set_up_managed_system_mode(self):
         self.common_set_up_managed_system_mode()
 
+    def test_set_up_machines_mode(self):
+        self.common_set_up_machines_mode()
+
     def test_set_up_upgrade_strategy(self):
         self.common_set_up_upgrade_strategy()