From a007db35fe583b266cb0551bcf339d5b4bf13072 Mon Sep 17 00:00:00 2001 From: Fuming Zhang Date: Fri, 24 Jul 2026 07:16:35 +0000 Subject: [PATCH 1/2] {AKS} Harden live test stabilization helpers Use the settled resource response after provisioning retries and treat quoted-empty injected service principal credentials as missing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../acs/tests/latest/custom_preparers.py | 19 ++++++-- .../acs/tests/latest/test_aks_commands.py | 12 ++--- .../latest/test_aks_provisioning_retry.py | 29 ++++++++++++ .../acs/tests/latest/test_custom_preparers.py | 44 +++++++++++++++++++ 4 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom_preparers.py diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/custom_preparers.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/custom_preparers.py index f0bf07363a8..1ef1f5310ba 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/custom_preparers.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/custom_preparers.py @@ -18,6 +18,15 @@ from azure.cli.command_modules.acs.tests.latest.recording_processors import MOCK_GUID, MOCK_SECRET +def _normalize_optional_live_test_setting(value): + if value is None: + return None + value = value.strip() + if value in {"", "''", '""', "\\'\\'", '\\"\\"'}: + return None + return value + + class AKSCustomResourceGroupPreparer(ResourceGroupPreparer): """ Override to support overriding the default location in test cases using this custom preparer with specific @@ -176,14 +185,18 @@ def __init__( dev_setting_sp_password, key, ) + self.dev_setting_sp_name = _normalize_optional_live_test_setting(self.dev_setting_sp_name) + self.dev_setting_sp_password = _normalize_optional_live_test_setting(self.dev_setting_sp_password) def __call__(self, fn): - if not self.dev_setting_sp_password: - return unittest.skip("skip test case that requires service principal as password is not provided")(fn) + if not self.dev_setting_sp_name or not self.dev_setting_sp_password: + return unittest.skip( + "skip test case that requires service principal credentials as they are not provided" + )(fn) return super().__call__(fn) def create_resource(self, name, **kwargs): - if not self.dev_setting_sp_password: + if not self.dev_setting_sp_name or not self.dev_setting_sp_password: return else: # call AbstractPreparer.moniker to make resource counts and self.resource_moniker consistent between live diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py index c017579f382..a810c19466b 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py @@ -126,6 +126,7 @@ def _cmd_with_retry(self, command, checks, expect_failure): last_seen_etag = current_etag if current_provisioning_state == 'Succeeded': + result = poll_result break elif current_provisioning_state in {'Failed', 'Canceled'}: raise AssertionError( @@ -140,13 +141,12 @@ def _cmd_with_retry(self, command, checks, expect_failure): f"provisioningState did not reach 'Succeeded' after {max_retries} retries. " f"Final state: {current_provisioning_state}{final_etag_msg}" ) - # Polled to 'Succeeded'; don't re-check `result` (stale body). - else: - # Did not poll (already Succeeded, or missing id/state). - # Run the assertion anyway so it can't be silently dropped. - result.assert_with_checks(provisioning_checks) + # Validate the terminal response. This also keeps missing id/state + # failures loud when polling cannot start. + result.assert_with_checks(provisioning_checks) - # Run all non-provisioning checks against the original result + # After polling, validate and return the settled resource rather than + # the stale response that triggered the retry. if other_checks: result.assert_with_checks(other_checks) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_provisioning_retry.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_provisioning_retry.py index f8a973871ba..4f10d881bc2 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_provisioning_retry.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_provisioning_retry.py @@ -163,6 +163,35 @@ def test_non_provisioning_checks_still_run(self, mock_execute, _mock_random, _mo self._make_instance()._cmd_with_retry('aks show', [JMESPathCheck('provisioningState', 'Succeeded'), name_check], False) name_check.assert_called_once() + @patch.dict(os.environ, {'AZURE_CLI_TEST_PROVISIONING_MAX_RETRIES': '2', 'AZURE_CLI_TEST_PROVISIONING_BASE_DELAY': '0.01'}) + @patch('time.sleep', return_value=None) + @patch('random.uniform', return_value=0) + @patch('azure.cli.testsdk.base.execute') + def test_checks_and_return_value_use_settled_response(self, mock_execute, _mock_random, _mock_sleep): + resource_id = '/subscriptions/xxx/resourceGroups/rg/providers/Microsoft.ContainerService/managedClusters/mc' + initial_result = self._result({ + 'id': resource_id, + 'provisioningState': 'Updating', + 'feature': {'enabled': False}, + }) + settled_result = self._result({ + 'id': resource_id, + 'provisioningState': 'Succeeded', + 'feature': {'enabled': True}, + }) + mock_execute.side_effect = [initial_result, settled_result] + + result = self._make_instance()._cmd_with_retry( + 'aks show', + [ + JMESPathCheck('provisioningState', 'Succeeded'), + JMESPathCheck('feature.enabled', True), + ], + False, + ) + + self.assertIs(result, settled_result) + @patch.dict(os.environ, {'AZURE_CLI_TEST_PROVISIONING_MAX_RETRIES': '3', 'AZURE_CLI_TEST_PROVISIONING_BASE_DELAY': '0.01'}) @patch('time.sleep', return_value=None) @patch('random.uniform', return_value=0) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom_preparers.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom_preparers.py new file mode 100644 index 00000000000..1892dea63e1 --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom_preparers.py @@ -0,0 +1,44 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import os +import unittest +from unittest.mock import patch + +from azure.cli.command_modules.acs.tests.latest.custom_preparers import ( + AKSCustomRoleBasedServicePrincipalPreparer, + _normalize_optional_live_test_setting, +) + + +class TestNormalizeOptionalLiveTestSetting(unittest.TestCase): + + def test_empty_placeholders_are_normalized(self): + for value in (None, "", " ", "''", '""', "\\'\\'", '\\"\\"'): + with self.subTest(value=value): + self.assertIsNone(_normalize_optional_live_test_setting(value)) + + def test_valid_value_is_preserved(self): + self.assertEqual(_normalize_optional_live_test_setting(" value "), "value") + + @patch.dict(os.environ, { + "AZURE_CLI_TEST_DEV_SP_NAME": '\\"\\"', + "AZURE_CLI_TEST_DEV_SP_PASSWORD": '\\"\\"', + }) + def test_service_principal_preparer_rejects_quoted_empty_credentials(self): + preparer = AKSCustomRoleBasedServicePrincipalPreparer() + + self.assertIsNone(preparer.dev_setting_sp_name) + self.assertIsNone(preparer.dev_setting_sp_password) + + def test_case(): + pass + + skipped_test_case = preparer(test_case) + self.assertTrue(skipped_test_case.__unittest_skip__) + + +if __name__ == "__main__": + unittest.main() From fed9ff502d05d52e4769b8aebff754a67a88d583 Mon Sep 17 00:00:00 2001 From: Fuming Zhang Date: Tue, 28 Jul 2026 06:32:09 +0000 Subject: [PATCH 2/2] {AKS} Stabilize remaining live test scenarios Retry only known transient AKS operation conflicts, preserve required test regions, select supported Kubernetes versions, and retire Basic Load Balancer scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../acs/tests/latest/test_aks_commands.py | 164 +++++++++++++++--- .../latest/test_aks_provisioning_retry.py | 157 ++++++++++++++++- 2 files changed, 295 insertions(+), 26 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py index a810c19466b..dc9930b5e23 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py @@ -53,14 +53,20 @@ def __init__(self, method_name): def cmd(self, command, checks=None, expect_failure=False): # Live-only retry adapter: when AZURE_CLI_TEST_RETRY_PROVISIONING_CHECK - # is set during a live run, poll provisioningState until terminal so an - # Azure Policy race can't fail the test on a stale 'Updating' body. + # is set during a live run, retry AKS operation conflicts and poll + # provisioningState until terminal so asynchronous service operations + # can't fail the test on a transient conflict or stale 'Updating' body. # Recordings made with the flag enabled must NOT be committed; the # replay pipeline runs with the flag off and would assert against the # initial pre-poll response. - if (checks and self.is_live and + if (self.is_live and os.environ.get('AZURE_CLI_TEST_RETRY_PROVISIONING_CHECK') == 'true'): - normalized_checks = checks if isinstance(checks, (list, tuple)) else [checks] + if checks is None: + normalized_checks = [] + elif isinstance(checks, (list, tuple)): + normalized_checks = checks + else: + normalized_checks = [checks] return self._cmd_with_retry(command, normalized_checks, expect_failure) return super().cmd(command, checks=checks, expect_failure=expect_failure) @@ -85,6 +91,72 @@ def _should_retry_for_provisioning_state(self, result): return False, None return True, data['id'] + @staticmethod + def _is_transient_operation_conflict(ex): + message = str(ex) + return ( + "Another operation is in progress" in message or + "in-progress PutExtensionAddonHandler.PUT operation" in message + ) + + def _execute_with_operation_retry(self, command, expect_failure): + from azure.cli.testsdk.base import execute + import logging + + max_retries = max(1, int(os.environ.get('AZURE_CLI_TEST_OPERATION_MAX_RETRIES', '10'))) + base_delay = float(os.environ.get('AZURE_CLI_TEST_OPERATION_BASE_DELAY', '5.0')) + max_delay = float(os.environ.get('AZURE_CLI_TEST_OPERATION_MAX_DELAY', '60.0')) + + for attempt in range(max_retries): + try: + return execute(self.cli_ctx, command, expect_failure=expect_failure) + except (HttpResponseError, CLIError) as ex: + if ( + expect_failure or + not self._is_transient_operation_conflict(ex) or + attempt == max_retries - 1 + ): + raise + delay = min(base_delay * (2 ** attempt), max_delay) + random.uniform(0, 1) + logging.warning( + "AKS operation is still in progress; retrying command in %.1f seconds (%d/%d)", + delay, + attempt + 1, + max_retries, + ) + time.sleep(delay) + + raise AssertionError("unreachable") + + def _get_settled_aks_result(self, resource_id, fallback_result): + from azure.cli.testsdk.base import execute + + resource_parts = resource_id.strip('/').split('/') + normalized_parts = [part.lower() for part in resource_parts] + try: + resource_group = resource_parts[normalized_parts.index('resourcegroups') + 1] + cluster_index = normalized_parts.index('managedclusters') + cluster_name = resource_parts[cluster_index + 1] + except (ValueError, IndexError): + return fallback_result + + remaining_parts = normalized_parts[cluster_index + 2:] + if not remaining_parts: + show_command = f'aks show --resource-group {resource_group} --name {cluster_name}' + elif len(remaining_parts) == 2 and remaining_parts[0] == 'agentpools': + try: + nodepool_name = resource_parts[normalized_parts.index('agentpools') + 1] + except IndexError: + return fallback_result + show_command = ( + f'aks nodepool show --resource-group {resource_group} ' + f'--cluster-name {cluster_name} --name {nodepool_name}' + ) + else: + return fallback_result + + return execute(self.cli_ctx, show_command, expect_failure=False) + def _cmd_with_retry(self, command, checks, expect_failure): from azure.cli.testsdk.base import execute import logging @@ -92,7 +164,7 @@ def _cmd_with_retry(self, command, checks, expect_failure): # Apply kwargs substitution (e.g. {resource_group}) before executing, # matching what ScenarioTest.cmd() does internally. command = self._apply_kwargs(command) - result = execute(self.cli_ctx, command, expect_failure=expect_failure) + result = self._execute_with_operation_retry(command, expect_failure) # Split checks into provisioning vs everything else provisioning_checks = [c for c in (checks or []) if self._is_provisioning_state_check(c)] @@ -126,7 +198,7 @@ def _cmd_with_retry(self, command, checks, expect_failure): last_seen_etag = current_etag if current_provisioning_state == 'Succeeded': - result = poll_result + result = self._get_settled_aks_result(resource_id, result) break elif current_provisioning_state in {'Failed', 'Canceled'}: raise AssertionError( @@ -1534,6 +1606,7 @@ def test_aks_create_default_service_with_virtual_node_addon(self, resource_group self.cmd( 'aks delete -g {resource_group} -n {name} --yes --no-wait', checks=[self.is_empty()]) + @unittest.skip("Basic Load Balancer was retired on September 30, 2025") @AllowLargeResponse() @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix='clitest', location='westus2') @AKSCustomRoleBasedServicePrincipalPreparer() @@ -2709,7 +2782,12 @@ def test_aks_azure_service_mesh_with_egress_gateway( ) @AllowLargeResponse() - @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix='clitest', location='westus2') + @AKSCustomResourceGroupPreparer( + random_name_length=17, + name_prefix='clitest', + location='westus2', + preserve_default_location=True, + ) def test_aks_machine_cmds(self, resource_group, resource_group_location): aks_name = self.create_random_name('cliakstest', 16) self.kwargs.update({ @@ -5213,6 +5291,7 @@ def test_aks_create_default_service_with_monitoring_addon_msi(self, resource_gro self.cmd( 'aks delete -g {resource_group} -n {name} --yes --no-wait', checks=[self.is_empty()]) + @unittest.skip("Basic Load Balancer was retired on September 30, 2025") @AllowLargeResponse() @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix='clitest', location='westus2') def test_aks_create_blb_vmas_msi(self, resource_group, resource_group_location): @@ -7734,7 +7813,10 @@ def test_aks_managed_namespace(self, resource_group, resource_group_location): @live_only() # live only due to workspace is not mocked correctly and role assignment is not mocked @AllowLargeResponse() @AKSCustomResourceGroupPreparer( - random_name_length=17, name_prefix="clitest", location="westus2" + random_name_length=17, + name_prefix="clitest", + location="westus2", + preserve_default_location=True, ) def test_aks_automatic_sku(self, resource_group, resource_group_location): # reset the count so in replay mode the random names will start with 0 @@ -11253,7 +11335,8 @@ def test_aks_create_and_update_ipv6_count(self, resource_group, resource_group_l create_cmd = 'aks create --resource-group={resource_group} --name={name} --location={location} ' \ '--pod-cidr 172.126.0.0/16 --service-cidr 172.56.0.0/16 --dns-service-ip 172.56.0.10 ' \ '--pod-cidrs 172.126.0.0/16,2001:abcd:1234::/64 --service-cidrs 172.56.0.0/16,2001:ffff::/108 ' \ - '--ip-families IPv4,IPv6 --load-balancer-managed-outbound-ipv6-count 2 ' \ + '--ip-families IPv4,IPv6 --load-balancer-managed-outbound-ip-count 1 ' \ + '--load-balancer-managed-outbound-ipv6-count 2 ' \ '--network-plugin kubenet --ssh-key-value={ssh_key_value} --kubernetes-version {k8s_version} ' \ '--aks-custom-headers AKSHTTPCustomFeatures=Microsoft.ContainerService/AKS-EnableDualStack' self.cmd(create_cmd, checks=[ @@ -13305,7 +13388,10 @@ def test_aks_update_with_azurecontainerstorage_v1_with_ephemeral_disk_parameters @live_only() @AllowLargeResponse(99999) @AKSCustomResourceGroupPreparer( - random_name_length=17, name_prefix="clitest", location="uksouth" + random_name_length=17, + name_prefix="clitest", + location="uksouth", + preserve_default_location=True, ) def test_aks_create_with_azurecontainerstorage(self, resource_group, resource_group_location): @@ -14684,8 +14770,10 @@ def test_aks_update_remove_custom_ca_trust_certificates(self, resource_group, re random_name_length=17, name_prefix="clitest", location="eastus2euap", + preserve_default_location=True, ) def test_aks_network_isolated_cluster(self, resource_group, resource_group_location): + k8s_version = self._get_latest_non_lts_version(resource_group_location) vnet_name = self.create_random_name("clitest", 16) aks_subnet_name = "aks-subnet" acr_subnet_name = "acr-subnet" @@ -14709,6 +14797,7 @@ def test_aks_network_isolated_cluster(self, resource_group, resource_group_locat "kubelet_identity_name": kubelet_identity_name, "acr_name": acr_name, "ssh_key_value": self.generate_ssh_keys(), + "k8s_version": k8s_version, } ) @@ -14871,7 +14960,7 @@ def test_aks_network_isolated_cluster(self, resource_group, resource_group_locat # create AKS cluster to enable network isolated cluster with BYO ACR and outbound type none create_cmd_1 = ( "aks create --resource-group {resource_group} --name {aks_name_1} -c 1 --ssh-key-value={ssh_key_value} " - "-k 1.30 " + "-k {k8s_version} " "--enable-private-cluster " "--network-plugin azure --vnet-subnet-id {vnet_id}/subnets/{aks_subnet_name} " "--assign-identity {cluster_identity_id} " @@ -14890,7 +14979,7 @@ def test_aks_network_isolated_cluster(self, resource_group, resource_group_locat # create AKS cluster to use Direct as artifact source create_cmd_2 = ( "aks create --resource-group {resource_group} --name {aks_name_2} -c 1 --ssh-key-value={ssh_key_value} " - "-k 1.30 " + "-k {k8s_version} " "--enable-private-cluster " "--network-plugin azure --vnet-subnet-id {vnet_id}/subnets/{aks_subnet_name} " "--assign-identity {cluster_identity_id} " @@ -14918,7 +15007,7 @@ def test_aks_network_isolated_cluster(self, resource_group, resource_group_locat # create AKS cluster to enable network isolated cluster with managed ACR and outbound type none create_cmd_3 = ( "aks create --resource-group {resource_group} --name {aks_name_3} -c 1 --ssh-key-value={ssh_key_value} " - "-k 1.30 " + "-k {k8s_version} " "--enable-private-cluster " "--network-plugin azure " "--outbound-type=none " @@ -15322,6 +15411,7 @@ def test_vms_agentpool_type(self, resource_group, resource_group_location): ) + @unittest.skip("Basic Load Balancer was retired on September 30, 2025") @AllowLargeResponse() @AKSCustomResourceGroupPreparer( random_name_length=17, @@ -15519,9 +15609,18 @@ def test_aks_update_node_provisioning_profile( checks=[self.is_empty()], ) + # LocalDNS requires a currently supported Kubernetes version, which cannot + # be kept stable in a recording as regional version support advances. + @live_only() @AllowLargeResponse() - @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix="clitest", location="westus2") + @AKSCustomResourceGroupPreparer( + random_name_length=17, + name_prefix="clitest", + location="westus2", + preserve_default_location=True, + ) def test_aks_nodepool_add_with_localdns_config(self, resource_group, resource_group_location): + k8s_version = self._get_latest_non_lts_version(resource_group_location) aks_name = self.create_random_name("cliakstest", 16) nodepool_name = self.create_random_name("np", 6) localdns_config_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), "data", "localdnsconfig", "localdnsconfig.json") @@ -15532,13 +15631,14 @@ def test_aks_nodepool_add_with_localdns_config(self, resource_group, resource_gr "ssh_key_value": self.generate_ssh_keys(), "localdns_config": localdns_config_path, "location": resource_group_location, + "k8s_version": k8s_version, }) # Create AKS cluster create_cmd = ( "aks create --resource-group={resource_group} --name={name} --location={location} " "--node-count 1 --ssh-key-value={ssh_key_value} --generate-ssh-keys " - "--kubernetes-version 1.33.0" # k8s version > 1.33 to support localDNS + "--kubernetes-version {k8s_version}" ) self.cmd(create_cmd, checks=[self.check("provisioningState", "Succeeded")]) @@ -15547,7 +15647,7 @@ def test_aks_nodepool_add_with_localdns_config(self, resource_group, resource_gr "aks nodepool add --resource-group={resource_group} --cluster-name={name} " "--name={nodepool_name} --node-count 1 --localdns-config={localdns_config} " "--aks-custom-headers AKSHTTPCustomFeatures=Microsoft.ContainerService/LocalDNSPreview " - "--kubernetes-version 1.33.0" # k8s version > 1.33 to support localDNS + "--kubernetes-version {k8s_version}" ) self.cmd(add_cmd, checks=[self.check("provisioningState", "Succeeded")]) @@ -15566,9 +15666,16 @@ def test_aks_nodepool_add_with_localdns_config(self, resource_group, resource_gr checks=[self.is_empty()], ) + @live_only() @AllowLargeResponse() - @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix="clitest", location="westus2") + @AKSCustomResourceGroupPreparer( + random_name_length=17, + name_prefix="clitest", + location="westus2", + preserve_default_location=True, + ) def test_aks_nodepool_update_with_localdns_config(self, resource_group, resource_group_location): + k8s_version = self._get_latest_non_lts_version(resource_group_location) aks_name = self.create_random_name("cliakstest", 16) nodepool_name = self.create_random_name("np", 6) localdns_config_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), "data", "localdnsconfig", "localdnsconfig.json") @@ -15577,14 +15684,15 @@ def test_aks_nodepool_update_with_localdns_config(self, resource_group, resource "name": aks_name, "nodepool_name": nodepool_name, "ssh_key_value": self.generate_ssh_keys(), - "localdns_config": localdns_config_path + "localdns_config": localdns_config_path, + "k8s_version": k8s_version, }) # Create AKS cluster create_cmd = ( "aks create --resource-group={resource_group} --name={name} " "--node-count 1 --ssh-key-value={ssh_key_value} --generate-ssh-keys " - "--kubernetes-version 1.33.0" # k8s version > 1.33 to support localDNS + "--kubernetes-version {k8s_version}" ) self.cmd(create_cmd, checks=[self.check("provisioningState", "Succeeded")]) @@ -15592,7 +15700,7 @@ def test_aks_nodepool_update_with_localdns_config(self, resource_group, resource add_cmd = ( "aks nodepool add --resource-group={resource_group} --cluster-name={name} " "--name={nodepool_name} --node-count 1 " - "--kubernetes-version 1.33.0" # k8s version > 1.33 to support localDNS + "--kubernetes-version {k8s_version}" ) self.cmd(add_cmd, checks=[self.check("provisioningState", "Succeeded")]) @@ -15619,9 +15727,16 @@ def test_aks_nodepool_update_with_localdns_config(self, resource_group, resource checks=[self.is_empty()], ) + @live_only() @AllowLargeResponse() - @AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix="clitest", location="westus2") + @AKSCustomResourceGroupPreparer( + random_name_length=17, + name_prefix="clitest", + location="westus2", + preserve_default_location=True, + ) def test_aks_nodepool_add_with_localdns_required_mode(self, resource_group, resource_group_location): + k8s_version = self._get_latest_non_lts_version(resource_group_location) aks_name = self.create_random_name("cliakstest", 16) nodepool_name = self.create_random_name("np", 6) required_config_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), "data", "localdnsconfig", "required_mode_only.json") @@ -15630,14 +15745,15 @@ def test_aks_nodepool_add_with_localdns_required_mode(self, resource_group, reso "name": aks_name, "nodepool_name": nodepool_name, "ssh_key_value": self.generate_ssh_keys(), - "required_config": required_config_path + "required_config": required_config_path, + "k8s_version": k8s_version, }) # Create AKS cluster create_cmd = ( "aks create --resource-group={resource_group} --name={name} " "--node-count 1 --ssh-key-value={ssh_key_value} --generate-ssh-keys " - "--kubernetes-version 1.33.0" + "--kubernetes-version {k8s_version}" ) self.cmd(create_cmd, checks=[self.check("provisioningState", "Succeeded")]) @@ -15646,7 +15762,7 @@ def test_aks_nodepool_add_with_localdns_required_mode(self, resource_group, reso "aks nodepool add --resource-group={resource_group} --cluster-name={name} " "--name={nodepool_name} --node-count 1 --localdns-config={required_config} " "--aks-custom-headers AKSHTTPCustomFeatures=Microsoft.ContainerService/LocalDNSPreview " - "--kubernetes-version 1.33.0" + "--kubernetes-version {k8s_version}" ) self.cmd(add_cmd, checks=[self.check("provisioningState", "Succeeded")]) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_provisioning_retry.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_provisioning_retry.py index 4f10d881bc2..6aca1306a19 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_provisioning_retry.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_provisioning_retry.py @@ -9,6 +9,7 @@ from unittest.mock import MagicMock, patch from azure.cli.testsdk.checkers import JMESPathCheck +from knack.util import CLIError class MockExecutionResult: @@ -83,6 +84,22 @@ def test_list_response_returns_false(self): self.assertFalse(should_retry) +class TestCmdRetryDispatch(unittest.TestCase): + + @patch.dict(os.environ, {'AZURE_CLI_TEST_RETRY_PROVISIONING_CHECK': 'true'}) + def test_live_command_without_checks_uses_retry_path(self): + from azure.cli.command_modules.acs.tests.latest.test_aks_commands import ( + AzureKubernetesServiceScenarioTest, + ) + instance = object.__new__(AzureKubernetesServiceScenarioTest) + instance.is_live = True + instance._cmd_with_retry = MagicMock() + + instance.cmd('aks delete', checks=None, expect_failure=False) + + instance._cmd_with_retry.assert_called_once_with('aks delete', [], False) + + class TestCmdWithRetry(unittest.TestCase): def _make_instance(self): @@ -117,9 +134,10 @@ def test_retries_until_succeeded(self, mock_execute, _mock_random, _mock_sleep): self._result({'id': resource_id, 'provisioningState': 'Updating'}), self._result({'provisioningState': 'Updating'}), self._result({'provisioningState': 'Succeeded'}), + self._result({'provisioningState': 'Succeeded'}), ] self._make_instance()._cmd_with_retry('aks show', [JMESPathCheck('provisioningState', 'Succeeded')], False) - self.assertEqual(mock_execute.call_count, 3) + self.assertEqual(mock_execute.call_count, 4) @patch.dict(os.environ, {'AZURE_CLI_TEST_PROVISIONING_MAX_RETRIES': '2', 'AZURE_CLI_TEST_PROVISIONING_BASE_DELAY': '0.01'}) @patch('time.sleep', return_value=None) @@ -180,8 +198,10 @@ def test_checks_and_return_value_use_settled_response(self, mock_execute, _mock_ 'feature': {'enabled': True}, }) mock_execute.side_effect = [initial_result, settled_result] + instance = self._make_instance() + instance._get_settled_aks_result = MagicMock(return_value=settled_result) - result = self._make_instance()._cmd_with_retry( + result = instance._cmd_with_retry( 'aks show', [ JMESPathCheck('provisioningState', 'Succeeded'), @@ -191,6 +211,7 @@ def test_checks_and_return_value_use_settled_response(self, mock_execute, _mock_ ) self.assertIs(result, settled_result) + instance._get_settled_aks_result.assert_called_once_with(resource_id, initial_result) @patch.dict(os.environ, {'AZURE_CLI_TEST_PROVISIONING_MAX_RETRIES': '3', 'AZURE_CLI_TEST_PROVISIONING_BASE_DELAY': '0.01'}) @patch('time.sleep', return_value=None) @@ -247,6 +268,7 @@ def test_delay_is_clamped_to_max_delay(self, mock_execute, _mock_random, mock_sl self._result({'provisioningState': 'Updating'}), self._result({'provisioningState': 'Updating'}), self._result({'provisioningState': 'Succeeded'}), + self._result({'provisioningState': 'Succeeded'}), ] self._make_instance()._cmd_with_retry('aks show', [JMESPathCheck('provisioningState', 'Succeeded')], False) # Without the cap, attempts 3 and 4 would sleep 16s and 32s. @@ -255,5 +277,136 @@ def test_delay_is_clamped_to_max_delay(self, mock_execute, _mock_random, mock_sl self.assertLessEqual(call.args[0], 10.0) +class TestExecuteWithOperationRetry(unittest.TestCase): + + def _make_instance(self): + from azure.cli.command_modules.acs.tests.latest.test_aks_commands import ( + AzureKubernetesServiceScenarioTest, + ) + instance = object.__new__(AzureKubernetesServiceScenarioTest) + instance.cli_ctx = MagicMock() + return instance + + @patch.dict(os.environ, { + 'AZURE_CLI_TEST_OPERATION_MAX_RETRIES': '3', + 'AZURE_CLI_TEST_OPERATION_BASE_DELAY': '0.01', + }) + @patch('time.sleep', return_value=None) + @patch('random.uniform', return_value=0) + @patch('azure.cli.testsdk.base.execute') + def test_retries_transient_operation_conflict(self, mock_execute, _mock_random, mock_sleep): + expected = MockExecutionResult({'provisioningState': 'Succeeded'}) + mock_execute.side_effect = [ + CLIError('Operation is not allowed: Another operation is in progress.'), + expected, + ] + + result = self._make_instance()._execute_with_operation_retry('aks update', False) + + self.assertIs(result, expected) + self.assertEqual(mock_execute.call_count, 2) + mock_sleep.assert_called_once() + + @patch.dict(os.environ, {'AZURE_CLI_TEST_OPERATION_MAX_RETRIES': '3'}) + @patch('time.sleep', return_value=None) + @patch('azure.cli.testsdk.base.execute') + def test_does_not_retry_other_errors(self, mock_execute, mock_sleep): + mock_execute.side_effect = CLIError('Invalid parameter') + + with self.assertRaisesRegex(CLIError, 'Invalid parameter'): + self._make_instance()._execute_with_operation_retry('aks update', False) + + mock_execute.assert_called_once() + mock_sleep.assert_not_called() + + @patch.dict(os.environ, {'AZURE_CLI_TEST_OPERATION_MAX_RETRIES': '3'}) + @patch('time.sleep', return_value=None) + @patch('azure.cli.testsdk.base.execute') + def test_does_not_retry_expected_failure(self, mock_execute, mock_sleep): + mock_execute.side_effect = CLIError( + 'Operation is not allowed: in-progress PutExtensionAddonHandler.PUT operation' + ) + + with self.assertRaises(CLIError): + self._make_instance()._execute_with_operation_retry('aks update', True) + + mock_execute.assert_called_once() + mock_sleep.assert_not_called() + + +class TestGetSettledAksResult(unittest.TestCase): + + def _make_instance(self): + from azure.cli.command_modules.acs.tests.latest.test_aks_commands import ( + AzureKubernetesServiceScenarioTest, + ) + instance = object.__new__(AzureKubernetesServiceScenarioTest) + instance.cli_ctx = MagicMock() + return instance + + @patch('azure.cli.testsdk.base.execute') + def test_refetches_managed_cluster_with_native_show(self, mock_execute): + expected = MockExecutionResult({'provisioningState': 'Succeeded'}) + mock_execute.return_value = expected + resource_id = ( + '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.ContainerService/' + 'managedClusters/cluster' + ) + instance = self._make_instance() + + result = instance._get_settled_aks_result(resource_id, MagicMock()) + + self.assertIs(result, expected) + mock_execute.assert_called_once_with( + instance.cli_ctx, + 'aks show --resource-group rg --name cluster', + expect_failure=False, + ) + + @patch('azure.cli.testsdk.base.execute') + def test_refetches_agent_pool_with_native_show(self, mock_execute): + expected = MockExecutionResult({'provisioningState': 'Succeeded'}) + mock_execute.return_value = expected + resource_id = ( + '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.ContainerService/' + 'managedClusters/cluster/agentPools/pool' + ) + instance = self._make_instance() + + result = instance._get_settled_aks_result(resource_id, MagicMock()) + + self.assertIs(result, expected) + mock_execute.assert_called_once_with( + instance.cli_ctx, + 'aks nodepool show --resource-group rg --cluster-name cluster --name pool', + expect_failure=False, + ) + + @patch('azure.cli.testsdk.base.execute') + def test_keeps_original_result_for_non_aks_resource(self, mock_execute): + original = MagicMock() + + result = self._make_instance()._get_settled_aks_result( + '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/vnet', + original, + ) + + self.assertIs(result, original) + mock_execute.assert_not_called() + + @patch('azure.cli.testsdk.base.execute') + def test_keeps_original_result_for_other_aks_child_resource(self, mock_execute): + original = MagicMock() + resource_id = ( + '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.ContainerService/' + 'managedClusters/cluster/trustedAccessRoleBindings/binding' + ) + + result = self._make_instance()._get_settled_aks_result(resource_id, original) + + self.assertIs(result, original) + mock_execute.assert_not_called() + + if __name__ == '__main__': unittest.main()