[AKS] Throw proper error message during HOBO to Base conversion when no AP#9821
[AKS] Throw proper error message during HOBO to Base conversion when no AP#9821reneeli123 wants to merge 1 commit intoAzure:mainfrom
Conversation
️✔️Azure CLI Extensions Breaking Change Test
|
|
Hi @reneeli123, |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
Bot
requested review from
yanzhudd,
yonzhan and
zhoxing-ms
CodeGen Tools Feedback CollectionThank you for using our CodeGen tool. We value your feedback, and we would like to know how we can improve our product. Please take a few minutes to fill our codegen survey |
There was a problem hiding this comment.
Pull request overview
Updates AKS preview’s managed cluster update flow to emit a clearer, actionable CLI error when converting an Automatic SKU cluster to Base without any agent pools, and adds unit tests to cover the new behavior.
Changes:
- Add SKU-aware handling in
update_agentpool_profileto raiseRequiredArgumentMissingErroronly when--sku baseis explicitly requested on an automatic cluster with no agent pools. - Add unit tests for (1) automatic clusters with no agent pools when not converting to base and (2) automatic→base conversion with no agent pools.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/aks-preview/azext_aks_preview/managed_cluster_decorator.py |
Adds explicit automatic→base conversion check and raises a more actionable error when no agent pools exist. |
src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py |
Adds tests validating pass-through vs. error behavior for automatic SKU clusters with no agent pools. |
| 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": |
There was a problem hiding this comment.
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).
| 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: |
| # 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. |
There was a problem hiding this comment.
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.
| with self.assertRaises(RequiredArgumentMissingError): | ||
| dec.update_agentpool_profile(mc) |
There was a problem hiding this comment.
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.
| 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)) |
|
Hi @reneeli123 Release SuggestionsModule: aks-preview
Notes
|
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az aks update -n "${RESOURCE_NAME}" -g "${RESOURCE_GROUP}" --sku base
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.