-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[AKS] Throw proper error message during HOBO to Base conversion when no AP #9821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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. | ||||||
| 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": | ||||||
|
||||||
| 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: |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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
|
||||||||||||||
| 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)) |
There was a problem hiding this comment.
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.