fix(aks): preserve existing ACNS settings during partial updates#33049
fix(aks): preserve existing ACNS settings during partial updates#33049
Conversation
❌AzureCLI-FullTest
|
|
Hi @nddq, |
️✔️AzureCLI-BreakingChangeTest
|
|
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
This PR addresses an AKS update-path issue where partial ACNS updates could unintentionally drop existing advancedNetworking sub-settings by replacing the entire AdvancedNetworking object, instead of updating only user-specified fields.
Changes:
- Update decorator logic to mutate
mc.network_profile.advanced_networkingin-place and only overwrite explicitly provided ACNS fields. - Add/extend unit tests to validate partial updates preserve existing ACNS state.
- Add a live test that performs sequential partial updates (network policies, then transit encryption) and verifies settings persist.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py |
Switches ACNS update logic from “replace object” to “mutate existing object” for partial updates. |
src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py |
Adds unit coverage for preserving existing ACNS sub-properties across partial updates. |
src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py |
Adds a live test ensuring sequential partial ACNS updates preserve previously-set values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py
Show resolved
Hide resolved
src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py
Show resolved
Hide resolved
22be663 to
37d1788
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
FumingZhang
left a comment
There was a problem hiding this comment.
Please update the PR title or description to meet the required history note format so the CI check can pass.
| User-Agent: | ||
| - AZURECLI/2.84.0 azsdk-python-core/1.38.3 Python/3.12.3 (Linux-6.17.0-1008-azure-x86_64-with-glibc2.39) | ||
| method: GET | ||
| uri: https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest000001/providers/Microsoft.ContainerService/managedClusters/cliakstest000001?api-version=2025-10-02-preview |
There was a problem hiding this comment.
The recording file is outdated—the API version used is 2025-10-02-preview, but it should be a stable API version, which is now 2026-01-01. Please generate a new recording file.
| name_prefix="clitest", | ||
| location="westcentralus", | ||
| ) | ||
| def test_aks_update_acns_preserves_existing_settings( |
There was a problem hiding this comment.
Queued live test to validate the change.
- test_aks_update_acns_preserves_existing_settings
You might consider running the test with other related existing test cases to make sure the change doesn't cause any regression.
| User-Agent: | ||
| - AZURECLI/2.84.0 azsdk-python-core/1.38.3 Python/3.12.3 (Linux-6.17.0-1008-azure-x86_64-with-glibc2.39) | ||
| method: GET | ||
| uri: https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest000001/providers/Microsoft.ContainerService/managedClusters/cliakstest000001?api-version=2025-10-02-preview |
There was a problem hiding this comment.
The recording file is outdated—the API version used is 2025-10-02-preview, but it should be a stable API version, which is now 2026-01-01. Please generate a new recording file.
37d1788 to
fdb827a
Compare
The update_network_profile_advanced_networking method was creating a new AdvancedNetworking object on every update, discarding existing sub-properties (observability, security, transit encryption) that the user didn't explicitly specify. This changes the method to modify the existing object in-place, only overwriting fields the user provided. When disabling ACNS, sub-features are explicitly set to disabled to ensure a consistent payload. Signed-off-by: Quang Nguyen <nguyenquang@microsoft.com>
fdb827a to
e8ddb8e
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Currently
update_network_profile_advanced_networkingcreates a newAdvancedNetworkingobject on every update call, replacing the existing one wholesale. When a user updates only a single ACNS sub-property (e.g.--acns-advanced-networkpolicies L7), existing sub-properties like observability, security, and transit encryption are discarded because they weren't re-specified.This PR changes the update path to modify the existing
AdvancedNetworkingobject in-place, only overwriting fields the user explicitly provided.mc.network_profile.advanced_networkingin-place instead of constructing a fresh object and replacing ittest_update_network_profile_advanced_networking_preserves_existing_statecovering partial updates to network policies, observability, and disable flowstest_aks_update_acns_preserves_existing_settings— creates a cluster with ACNS, updates only network policies, then updates only transit encryption, verifying all existing settings survive each stepTesting
Unit tests:
Live test: