From 6ee7ce17e90afcfa49e5bc8ae22c469309cace56 Mon Sep 17 00:00:00 2001 From: wenxuanW Date: Fri, 7 Nov 2025 10:18:40 -0800 Subject: [PATCH] Address comments --- src/aks-preview/HISTORY.rst | 5 + src/aks-preview/azext_aks_preview/custom.py | 26 +++--- .../managed_cluster_decorator.py | 68 +++++++++----- .../tests/latest/test_custom.py | 84 +++++++++++++++++ .../latest/test_managed_cluster_decorator.py | 92 +++++++++++++++++++ src/aks-preview/setup.py | 2 +- 6 files changed, 241 insertions(+), 36 deletions(-) diff --git a/src/aks-preview/HISTORY.rst b/src/aks-preview/HISTORY.rst index af8e1e35a69..24f54943038 100644 --- a/src/aks-preview/HISTORY.rst +++ b/src/aks-preview/HISTORY.rst @@ -12,6 +12,11 @@ To release a new version, please select a new version number (usually plus 1 to Pending +++++++ +19.0.0b15 ++++++++ +* Fix `NoneType` error when performing operations on automatic clusters that have hosted system components enabled. + + 19.0.0b14 +++++++ * `az aks safeguards`: Add support for Deployment Safeguards with Pod Security Standards (PSS). New `--pss-level` parameter allows setting PSS enforcement level to Privileged, Baseline, or Restricted. Commands now support both `-g/-n` and `--cluster` argument patterns. diff --git a/src/aks-preview/azext_aks_preview/custom.py b/src/aks-preview/azext_aks_preview/custom.py index 191f1e7468c..217c5610986 100644 --- a/src/aks-preview/azext_aks_preview/custom.py +++ b/src/aks-preview/azext_aks_preview/custom.py @@ -1578,8 +1578,8 @@ def aks_scale(cmd, # pylint: disable=unused-argument "Please specify nodepool name or use az aks nodepool command to scale node pool" ) - for agent_profile in instance.agent_pool_profiles: - if agent_profile.name == nodepool_name or (nodepool_name == "" and len(instance.agent_pool_profiles) == 1): + for agent_profile in (instance.agent_pool_profiles or []): + if agent_profile.name == nodepool_name or (nodepool_name == "" and instance.agent_pool_profiles and len(instance.agent_pool_profiles) == 1): if agent_profile.enable_auto_scaling: raise CLIError( "Cannot scale cluster autoscaler enabled node pool.") @@ -1629,7 +1629,7 @@ def aks_upgrade(cmd, _fill_defaults_for_pod_identity_profile(instance.pod_identity_profile) vmas_cluster = False - for agent_profile in instance.agent_pool_profiles: + for agent_profile in (instance.agent_pool_profiles or []): if agent_profile.type.lower() == "availabilityset": vmas_cluster = True break @@ -1646,7 +1646,7 @@ def aks_upgrade(cmd, # This only provide convenience for customer at client side so they can run az aks upgrade to upgrade all # nodepools of a cluster. The SDK only support upgrade single nodepool at a time. - for agent_pool_profile in instance.agent_pool_profiles: + for agent_pool_profile in (instance.agent_pool_profiles or []): if vmas_cluster: raise CLIError('This cluster is not using VirtualMachineScaleSets. Node image upgrade only operation ' 'can only be applied on VirtualMachineScaleSets and VirtualMachines(Preview) cluster.') @@ -1715,7 +1715,7 @@ def aks_upgrade(cmd, return None if upgrade_all: - for agent_profile in instance.agent_pool_profiles: + for agent_profile in (instance.agent_pool_profiles or []): agent_profile.orchestrator_version = kubernetes_version agent_profile.creation_data = None @@ -3044,12 +3044,13 @@ def aks_enable_addons( if enable_virtual_node: # All agent pool will reside in the same vnet, we will grant vnet level Contributor role # in later function, so using a random agent pool here is OK - random_agent_pool = result.agent_pool_profiles[0] - if random_agent_pool.vnet_subnet_id != "": - add_virtual_node_role_assignment( - cmd, result, random_agent_pool.vnet_subnet_id) - # Else, the cluster is not using custom VNet, the permission is already granted in AKS RP, - # we don't need to handle it in client side in this case. + if result.agent_pool_profiles and len(result.agent_pool_profiles) > 0: + random_agent_pool = result.agent_pool_profiles[0] + if random_agent_pool.vnet_subnet_id != "": + add_virtual_node_role_assignment( + cmd, result, random_agent_pool.vnet_subnet_id) + # Else, the cluster is not using custom VNet, the permission is already granted in AKS RP, + # we don't need to handle it in client side in this case. else: result = sdk_no_wait(no_wait, client.begin_create_or_update, @@ -4470,6 +4471,9 @@ def aks_check_network_outbound( if not cluster: raise ValidationError("Can not get cluster information!") + if not cluster.agent_pool_profiles or len(cluster.agent_pool_profiles) == 0: + raise ValidationError("No agent pool profiles found in the cluster!") + vm_set_type = cluster.agent_pool_profiles[0].type if not vm_set_type: raise ValidationError("Can not get VM set type of the cluster!") 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 cf95430ebd6..934cc77d25f 100644 --- a/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/managed_cluster_decorator.py @@ -5201,6 +5201,45 @@ def check_raw_parameters(self): error_msg = f"Please specify one or more of {' or '.join(option_names)}." raise RequiredArgumentMissingError(error_msg) + def update_agentpool_profile(self, mc: ManagedCluster) -> ManagedCluster: + """Update agentpool profile for the ManagedCluster object. + + Preview override to handle empty agent_pool_profiles gracefully. + + :return: the ManagedCluster object + """ + self._ensure_mc(mc) + + # Preview-specific change: an AKS ManagedCluster of automatic + # cluster with hosted system components may not have agent pools + # When transitioning from hosted to non-hosted automatic clusters, + # customers must first add a system node pool before disabling + # the hosted system profile. + if not mc.agent_pool_profiles: + if mc.hosted_system_profile and mc.hosted_system_profile.enabled: + return mc + raise UnknownError( + "Encounter an unexpected error while getting agent pool profiles from the cluster in the process of " + "updating agentpool profile." + ) + + # Call preview agentpool decorator method instead of default + agentpool_profile = self.agentpool_decorator.update_agentpool_profile_preview(mc.agent_pool_profiles) + mc.agent_pool_profiles[0] = agentpool_profile + + # Update nodepool labels for all nodepools + nodepool_labels = self.context.get_nodepool_labels() + if nodepool_labels is not None: + for agent_profile in mc.agent_pool_profiles: + agent_profile.node_labels = nodepool_labels + + # Update nodepool taints for all nodepools + nodepool_taints = self.context.get_nodepool_taints() + if nodepool_taints is not None: + for agent_profile in mc.agent_pool_profiles: + agent_profile.node_taints = nodepool_taints + return mc + def update_network_profile(self, mc: ManagedCluster) -> ManagedCluster: self._ensure_mc(mc) @@ -6522,34 +6561,17 @@ def update_upgrade_settings(self, mc: ManagedCluster) -> ManagedCluster: return mc - def update_nodepool_taints_mc(self, mc: ManagedCluster) -> ManagedCluster: - self._ensure_mc(mc) - - 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." - ) - - # update nodepool taints for all nodepools - nodepool_taints = self.context.get_nodepool_taints() - if nodepool_taints is not None: - for agent_profile in mc.agent_pool_profiles: - agent_profile.node_taints = nodepool_taints - return mc - def update_nodepool_initialization_taints_mc(self, mc: ManagedCluster) -> ManagedCluster: self._ensure_mc(mc) - 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." - ) - # update nodepool taints for all nodepools nodepool_initialization_taints = self.context.get_nodepool_initialization_taints() if nodepool_initialization_taints is not None: + 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 agent_profile in mc.agent_pool_profiles: if agent_profile.mode is not None and agent_profile.mode.lower() == "user": agent_profile.node_initialization_taints = nodepool_initialization_taints @@ -7245,8 +7267,6 @@ def update_mc_profile_preview(self) -> ManagedCluster: mc = self.update_auto_upgrade_profile(mc) # update cluster upgrade settings profile mc = self.update_upgrade_settings(mc) - # update nodepool taints - mc = self.update_nodepool_taints_mc(mc) # update nodepool initialization taints mc = self.update_nodepool_initialization_taints_mc(mc) # update acns in network_profile diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_custom.py b/src/aks-preview/azext_aks_preview/tests/latest/test_custom.py index 49580ab3cde..23c981d139c 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_custom.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_custom.py @@ -12,6 +12,9 @@ ) from azext_aks_preview.custom import ( aks_stop, + aks_scale, + aks_upgrade, + aks_enable_addons, ) from azext_aks_preview.tests.latest.mocks import MockCLI, MockClient, MockCmd @@ -49,6 +52,87 @@ def test_aks_stop(self): ) self.assertEqual(aks_stop(self.cmd, self.client, "rg", "name", False), None) + def test_aks_scale_with_none_agent_pool_profiles(self): + """Test aks_scale handles None agent_pool_profiles gracefully""" + # Test case: automatic cluster with hosted system components, no agent pools + mc = self.models.ManagedCluster(location="test_location") + mc.agent_pool_profiles = None # This is the key scenario + mc.pod_identity_profile = None + + self.client.get = Mock(return_value=mc) + + # Should not raise NoneType error and should return without crashing + try: + result = aks_scale(self.cmd, self.client, "rg", "name", 3, "nodepool1") + # We expect this to complete without NoneType errors + except Exception as e: + # Should not be a NoneType error + self.assertNotIn("NoneType", str(type(e))) + + def test_aks_upgrade_with_none_agent_pool_profiles(self): + """Test aks_upgrade handles None agent_pool_profiles gracefully""" + mc = self.models.ManagedCluster(location="test_location") + mc.agent_pool_profiles = None # Key test scenario + mc.pod_identity_profile = None + mc.kubernetes_version = "1.24.0" + mc.provisioning_state = "Succeeded" + mc.max_agent_pools = 10 + + self.client.get = Mock(return_value=mc) + + # Should not raise NoneType error + try: + result = aks_upgrade( + self.cmd, self.client, "rg", "name", + kubernetes_version="1.25.0", yes=True + ) + except Exception as e: + self.assertNotIn("NoneType", str(type(e))) + + def test_aks_enable_addons_with_none_agent_pool_profiles(self): + """Test aks_enable_addons handles None agent_pool_profiles gracefully""" + mc = self.models.ManagedCluster(location="test_location") + mc.agent_pool_profiles = None # Key test scenario + mc.addon_profiles = {} + mc.service_principal_profile = self.models.ManagedClusterServicePrincipalProfile( + client_id="msi" + ) + mc.api_server_access_profile = None + + self.client.get = Mock(return_value=mc) + self.client.begin_create_or_update = Mock(return_value=mc) + + # Should not raise NoneType error + try: + result = aks_enable_addons( + self.cmd, self.client, "rg", "name", "monitoring", + workspace_resource_id="/subscriptions/test/resourceGroups/test/providers/Microsoft.OperationalInsights/workspaces/test" + ) + except Exception as e: + self.assertNotIn("NoneType", str(type(e))) + + def test_aks_enable_addons_virtual_node_with_none_agent_pool_profiles(self): + """Test aks_enable_addons for virtual-node handles None agent_pool_profiles""" + mc = self.models.ManagedCluster(location="test_location") + mc.agent_pool_profiles = None # Key test scenario for virtual node addon + mc.addon_profiles = {} + mc.service_principal_profile = self.models.ManagedClusterServicePrincipalProfile( + client_id="msi" + ) + mc.api_server_access_profile = None + + self.client.get = Mock(return_value=mc) + self.client.begin_create_or_update = Mock(return_value=mc) + + # Virtual node addon should handle None agent_pool_profiles gracefully + try: + result = aks_enable_addons( + self.cmd, self.client, "rg", "name", "virtual-node", + subnet_name="test-subnet" + ) + except Exception as e: + self.assertNotIn("NoneType", str(type(e))) + if __name__ == '__main__': unittest.main() diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py index c4d663b5e90..9a791f94908 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py @@ -12903,5 +12903,97 @@ def test_azure_keyvault_kms_network_access_parameter_fix(self): "Public" ) + def test_update_agentpool_profile_with_none_agent_pool_profiles(self): + """Test update_agentpool_profile handles None agent_pool_profiles with hosted system components""" + # Test case 1: None agent_pool_profiles with hosted system components enabled (should succeed) + dec_1 = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + {}, + CUSTOM_MGMT_AKS_PREVIEW, + ) + + # Create a managed cluster with None agent_pool_profiles but hosted system components enabled + hosted_system_profile = self.models.ManagedClusterHostedSystemProfile(enabled=True) + mc_1 = self.models.ManagedCluster( + location="test_location", + agent_pool_profiles=None, # This is the key scenario + hosted_system_profile=hosted_system_profile + ) + dec_1.context.attach_mc(mc_1) + + # Should return the MC unchanged without raising an error + result_1 = dec_1.update_agentpool_profile(mc_1) + self.assertEqual(result_1, mc_1) + self.assertIsNone(result_1.agent_pool_profiles) + self.assertTrue(result_1.hosted_system_profile.enabled) + + def test_update_agentpool_profile_with_none_agent_pool_profiles_no_hosted_system(self): + """Test update_agentpool_profile raises UnknownError for None agent_pool_profiles without hosted system components""" + # Test case 2: None agent_pool_profiles without hosted system components (should raise UnknownError) + dec_2 = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + {}, + CUSTOM_MGMT_AKS_PREVIEW, + ) + + # Create a managed cluster with None agent_pool_profiles and no hosted system components + mc_2 = self.models.ManagedCluster( + location="test_location", + agent_pool_profiles=None, # This is the key scenario + hosted_system_profile=None + ) + dec_2.context.attach_mc(mc_2) + + # Should raise UnknownError + with self.assertRaises(UnknownError): + dec_2.update_agentpool_profile(mc_2) + + def test_update_agentpool_profile_with_none_agent_pool_profiles_hosted_system_disabled(self): + """Test update_agentpool_profile raises UnknownError for None agent_pool_profiles with hosted system components disabled""" + # Test case 3: None agent_pool_profiles with hosted system components disabled (should raise UnknownError) + dec_3 = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + {}, + CUSTOM_MGMT_AKS_PREVIEW, + ) + + # Create a managed cluster with None agent_pool_profiles and hosted system components disabled + hosted_system_profile_disabled = self.models.ManagedClusterHostedSystemProfile(enabled=False) + mc_3 = self.models.ManagedCluster( + location="test_location", + agent_pool_profiles=None, # This is the key scenario + hosted_system_profile=hosted_system_profile_disabled + ) + dec_3.context.attach_mc(mc_3) + + # Should raise UnknownError + with self.assertRaises(UnknownError): + dec_3.update_agentpool_profile(mc_3) + + def test_update_agentpool_profile_with_empty_agent_pool_profiles(self): + """Test update_agentpool_profile raises UnknownError for empty agent_pool_profiles list""" + # Test case 4: Empty agent_pool_profiles list (should raise UnknownError) + dec_4 = AKSPreviewManagedClusterUpdateDecorator( + self.cmd, + self.client, + {}, + CUSTOM_MGMT_AKS_PREVIEW, + ) + + # Create a managed cluster with empty agent_pool_profiles + mc_4 = self.models.ManagedCluster( + location="test_location", + agent_pool_profiles=[], # Empty list scenario + hosted_system_profile=None + ) + dec_4.context.attach_mc(mc_4) + + # Should raise UnknownError + with self.assertRaises(UnknownError): + dec_4.update_agentpool_profile(mc_4) + if __name__ == "__main__": unittest.main() diff --git a/src/aks-preview/setup.py b/src/aks-preview/setup.py index 6c0d0e96c63..f6116b92e8f 100644 --- a/src/aks-preview/setup.py +++ b/src/aks-preview/setup.py @@ -9,7 +9,7 @@ from setuptools import find_packages, setup -VERSION = "19.0.0b14" +VERSION = "19.0.0b15" CLASSIFIERS = [ "Development Status :: 4 - Beta",