[AKS] az aks create/update: Add new parameters --enable-app-routing-istio and --disable-app-routing-istio to manage App Routing Istio gateway implementation#33254
Conversation
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
Hi @meecethereese, |
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
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>
|
There was a problem hiding this comment.
Pull request overview
Adds support in az aks create/update (and new az aks approuting gateway istio subcommands) to manage the Gateway API installation and the App Routing “Istio” Gateway API implementation, backed by a newer azure-mgmt-containerservice SDK version.
Changes:
- Bump
azure-mgmt-containerservicedependency to41.1.0across setup + OS-specific requirements. - Add new CLI args for managed Gateway API installation (
--enable/--disable-gateway-api) and App Routing Istio implementation (--enable/--disable-app-routing-istio,--enable/--disable-ari). - Implement create/update wiring in the managed cluster decorators and add unit + scenario tests and help entries, plus new
az aks approuting gateway istio enable/disablecommands.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/azure-cli/setup.py | Bumps mgmt-containerservice SDK floor to pick up new model surface. |
| src/azure-cli/requirements.py3.windows.txt | Pins mgmt-containerservice to 41.1.0 on Windows. |
| src/azure-cli/requirements.py3.Linux.txt | Pins mgmt-containerservice to 41.1.0 on Linux. |
| src/azure-cli/requirements.py3.Darwin.txt | Pins mgmt-containerservice to 41.1.0 on macOS. |
| src/azure-cli/azure/cli/command_modules/acs/_consts.py | Adds constants for gateway installation + App Routing Istio modes. |
| src/azure-cli/azure/cli/command_modules/acs/_params.py | Registers new CLI parameters for aks create/update. |
| src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py | Wires new params into create/update request shaping. |
| src/azure-cli/azure/cli/command_modules/acs/custom.py | Adds new approuting gateway istio enable/disable custom commands; passes new params into decorator flow. |
| src/azure-cli/azure/cli/command_modules/acs/commands.py | Registers new aks approuting gateway istio command group. |
| src/azure-cli/azure/cli/command_modules/acs/_help.py | Documents new args and new approuting gateway istio commands. |
| src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py | Adds unit tests for new context getters and decorator mutations. |
| src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py | Adds new scenario tests covering gateway api + app routing istio flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mc.ingress_profile = self.models.ManagedClusterIngressProfile() | ||
| if mc.ingress_profile.gateway_api is None: | ||
| mc.ingress_profile.gateway_api = ( | ||
| self.models.ManagedClusterIngressProfileGatewayConfiguration( |
There was a problem hiding this comment.
set_up_ingress_profile_gateway_api instantiates ManagedClusterIngressProfile/ManagedClusterIngressProfileGatewayConfiguration without the per-line # pylint: disable=no-member used throughout this file, which can cause pylint CI failures. Align with the surrounding patterns (either add the disables here or refactor to reuse an existing helper that already has them).
| mc.ingress_profile = self.models.ManagedClusterIngressProfile() | |
| if mc.ingress_profile.gateway_api is None: | |
| mc.ingress_profile.gateway_api = ( | |
| self.models.ManagedClusterIngressProfileGatewayConfiguration( | |
| mc.ingress_profile = self.models.ManagedClusterIngressProfile() # pylint: disable=no-member | |
| if mc.ingress_profile.gateway_api is None: | |
| mc.ingress_profile.gateway_api = ( | |
| self.models.ManagedClusterIngressProfileGatewayConfiguration( # pylint: disable=no-member |
| if mc.ingress_profile.web_app_routing is None: | ||
| mc.ingress_profile.web_app_routing = ( | ||
| self.models.ManagedClusterIngressProfileWebAppRouting() # pylint: disable=no-member | ||
| ) |
There was a problem hiding this comment.
When --enable-app-routing-istio is set during create, this method will create ingress_profile.web_app_routing even if App Routing itself isn't enabled (it leaves enabled unset). That can produce an inconsistent request payload. Consider validating that App Routing is being enabled (e.g., --enable-app-routing / addon enabled) or explicitly setting web_app_routing.enabled=True (or raising a user-friendly error) when enabling the Istio implementation.
| ) | |
| ) | |
| mc.ingress_profile.web_app_routing.enabled = True |
| if enable_app_routing_istio or disable_app_routing_istio: | ||
| if mc.ingress_profile is None: | ||
| mc.ingress_profile = self.models.ManagedClusterIngressProfile() # pylint: disable=no-member | ||
| if mc.ingress_profile.web_app_routing is None: | ||
| mc.ingress_profile.web_app_routing = ( | ||
| self.models.ManagedClusterIngressProfileWebAppRouting() # pylint: disable=no-member | ||
| ) | ||
| if mc.ingress_profile.web_app_routing.gateway_api_implementations is None: | ||
| mc.ingress_profile.web_app_routing.gateway_api_implementations = ( | ||
| self.models.ManagedClusterWebAppRoutingGatewayAPIImplementations() # pylint: disable=no-member | ||
| ) |
There was a problem hiding this comment.
update_ingress_profile_app_routing_istio currently creates ingress_profile.web_app_routing and gateway_api_implementations even when App Routing isn't enabled on the cluster. This means --disable-app-routing-istio can still mutate the cluster by adding new objects, and --enable-app-routing-istio may send an invalid config. Please gate this on App Routing being enabled (existing web_app_routing.enabled or an explicit enable in the same request), and otherwise no-op or raise a clear CLIError instructing users to enable App Routing first.
| @AllowLargeResponse() | ||
| @AKSCustomResourceGroupPreparer( | ||
| random_name_length=17, name_prefix="clitest", location="centraluseuap" | ||
| ) | ||
| def test_aks_create_with_gateway_api_and_azureservicemesh( | ||
| self, resource_group, resource_group_location | ||
| ): | ||
| # reset the count so in replay mode the random names will start with 0 |
There was a problem hiding this comment.
These new ScenarioTest cases appear to require VCR recordings for playback CI, but there are no corresponding YAML files under acs/tests/latest/recordings/ (e.g., no recordings for test_aks_create_with_gateway_api_and_azureservicemesh, test_aks_create_and_update_with_app_routing_istio, etc.). Either add the recordings, or mark the tests appropriately (e.g., @live_only() / @record_only()) so CI doesn't fail in replay mode.
| helps["aks approuting gateway istio enable"] = """ | ||
| type: command | ||
| short-summary: Enable Gateway API based ingress on App Routing via Istio without service mesh functionality. | ||
| long-summary: | | ||
| This command enables an ingress-only version of Istio as a Gateway API implementation for App Routing | ||
| in the given cluster. This Istio instance only reconciles Gateway API resources and does not provide | ||
| service mesh functionality (e.g. mTLS, traffic management between services). Cannot be used | ||
| simultaneously with Azure Service Mesh (az aks mesh enable). | ||
| examples: |
There was a problem hiding this comment.
The help text for az aks approuting gateway istio enable/disable doesn't mention that the command does not enable App Routing itself (it only toggles the Gateway API implementation). Since the implementation is updated via _aks_approuting_update without setting enable_app_routing, users need to enable App Routing first (e.g., az aks approuting enable). Please clarify this prerequisite in the long-summary to prevent confusing failures.
| from azure.cli.command_modules.acs._consts import CONST_WORKLOAD_RUNTIME_KATA_VM_ISOLATION | ||
| from azure.cli.command_modules.acs.tests.latest.utils import get_test_data_file_path | ||
| from azure.cli.core.azclierror import ClientRequestError, CLIInternalError, InvalidArgumentValueError | ||
| from azure.cli.core.azclierror import BadRequestError, ClientRequestError, CLIInternalError, InvalidArgumentValueError, MutuallyExclusiveArgumentError |
There was a problem hiding this comment.
MutuallyExclusiveArgumentError is imported here but not used anywhere in this test file, which can cause lint failures in environments that check unused imports. Please remove it (or add an assertion that actually exercises/validates this error type).
| from azure.cli.core.azclierror import BadRequestError, ClientRequestError, CLIInternalError, InvalidArgumentValueError, MutuallyExclusiveArgumentError | |
| from azure.cli.core.azclierror import BadRequestError, ClientRequestError, CLIInternalError, InvalidArgumentValueError |
Related command
Description
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.