Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions src/aks-preview/azext_aks_preview/managed_cluster_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -5692,13 +5692,24 @@ def update_agentpool_profile(self, mc: ManagedCluster) -> ManagedCluster:
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.
# cluster with hosted system components may not have agent pools.
# When converting from automatic to base SKU, customers must first
# add a system node pool before changing the SKU.
Comment on lines 5694 to +5697
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated comment reads awkwardly (“an AKS ManagedCluster of automatic …”). Consider rephrasing (e.g., “an AKS ManagedCluster of automatic SKU …”) to improve clarity and grammar.

Copilot uses AI. Check for mistakes.
if not mc.agent_pool_profiles:
if mc.hosted_system_profile and mc.hosted_system_profile.enabled:
return mc
if mc.sku and mc.sku.name and mc.sku.name.lower() == "automatic":
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the existing CONST_MANAGED_CLUSTER_SKU_NAME_AUTOMATIC constant instead of the hard-coded string "automatic" to match the rest of this module’s SKU comparisons (and avoid drift if constants change).

Suggested change
if mc.sku and mc.sku.name and mc.sku.name.lower() == "automatic":
if mc.sku and mc.sku.name and mc.sku.name.lower() == CONST_MANAGED_CLUSTER_SKU_NAME_AUTOMATIC:

Copilot uses AI. Check for mistakes.
# Check if the user is explicitly requesting a conversion to base SKU
requested_sku = self.context.raw_param.get("sku")
if requested_sku is not None and requested_sku.lower() == CONST_MANAGED_CLUSTER_SKU_NAME_BASE:
raise RequiredArgumentMissingError(
"The cluster does not have any agent pool profiles. "
"Please add a system node pool to the cluster before converting to base SKU. "
"You can add a system node pool by running "
"'az aks nodepool add --cluster-name <cluster-name> -g <resource-group> "
"-n <nodepool-name> --mode System'."
)
return mc
raise UnknownError(
"Encounter an unexpected error while getting agent pool profiles from the cluster in the process of "
"updating agentpool profile."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14515,6 +14515,51 @@ def test_update_agentpool_profile_with_none_agent_pool_profiles_hosted_system_di
with self.assertRaises(UnknownError):
dec_3.update_agentpool_profile(mc_3)

def test_update_agentpool_profile_with_none_agent_pool_profiles_automatic_sku(self):
"""Test update_agentpool_profile passes through for automatic SKU cluster with no agent pools when not converting to base"""
dec = AKSPreviewManagedClusterUpdateDecorator(
self.cmd,
self.client,
{},
CUSTOM_MGMT_AKS_PREVIEW,
)

# Create an automatic SKU cluster with None agent_pool_profiles and no hosted system profile
mc = self.models.ManagedCluster(
location="test_location",
agent_pool_profiles=None,
hosted_system_profile=None,
sku=self.models.ManagedClusterSKU(name="Automatic", tier="Standard"),
)
dec.context.attach_mc(mc)

# Should return the MC unchanged (no --sku base requested)
result = dec.update_agentpool_profile(mc)
self.assertEqual(result, mc)
self.assertIsNone(result.agent_pool_profiles)

def test_update_agentpool_profile_automatic_to_base_no_agent_pools(self):
"""Test update_agentpool_profile raises RequiredArgumentMissingError when converting automatic to base with no agent pools"""
dec = AKSPreviewManagedClusterUpdateDecorator(
self.cmd,
self.client,
{"sku": "base"},
CUSTOM_MGMT_AKS_PREVIEW,
)

# Create an automatic SKU cluster with None agent_pool_profiles
mc = self.models.ManagedCluster(
location="test_location",
agent_pool_profiles=None,
hosted_system_profile=None,
sku=self.models.ManagedClusterSKU(name="Automatic", tier="Standard"),
)
dec.context.attach_mc(mc)

# Should raise RequiredArgumentMissingError asking user to add a system node pool first
with self.assertRaises(RequiredArgumentMissingError):
dec.update_agentpool_profile(mc)
Comment on lines +14560 to +14561
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the PR’s goal is to surface a specific improved error message for automatic→base conversion, this test should also assert key parts of the exception message (similar to other tests in this file that use as cm + assertIn). Otherwise the message could regress without failing tests.

Suggested change
with self.assertRaises(RequiredArgumentMissingError):
dec.update_agentpool_profile(mc)
with self.assertRaises(RequiredArgumentMissingError) as cm:
dec.update_agentpool_profile(mc)
self.assertIn("system node pool", str(cm.exception))
self.assertIn("base", str(cm.exception))

Copilot uses AI. Check for mistakes.

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)
Expand Down
Loading