{AKS} Preserve existing ACNS settings during partial updates#33049
Merged
{AKS} Preserve existing ACNS settings during partial updates#33049
Conversation
nddq
requested review from
FumingZhang,
NoriZC,
teresaritorto and
yanzhudd
as code owners
️✔️AzureCLI-FullTest
|
|
Hi @nddq, |
️✔️AzureCLI-BreakingChangeTest
|
Collaborator
|
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
yonzhan and
zhoxing-ms
microsoft-github-policy-service
bot
added
Auto-Assign
Contributor
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.
nddq
force-pushed
the
fix/acns-update-preserve-existing-state-v2
branch
from
22be663 to
37d1788
Compare
Member
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
FumingZhang
reviewed
Mar 26, 2026
Member
FumingZhang
left a comment
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.
nddq
force-pushed
the
fix/acns-update-preserve-existing-state-v2
branch
2 times, most recently
from
fdb827a to
e8ddb8e
Compare
Member
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Member
|
There's a merge conflict in your PR; please resolve it first. Also, the code cutoff for the upcoming official azure-cli release (03/31/2026 06:00 UTC) is approaching. Make sure both CI and live tests pass to meet the minimum requirements for the PR to be accepted and merged. @nddq |
nddq
force-pushed
the
fix/acns-update-preserve-existing-state-v2
branch
from
e8ddb8e to
9ca3063
Compare
Member
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
nddq
changed the title
nddq
force-pushed
the
fix/acns-update-preserve-existing-state-v2
branch
from
9ca3063 to
f80b386
Compare
Member
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Member
|
Re-queued live test |
FumingZhang
changed the title
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. The live test adds an aks wait between sequential updates to avoid 409 OperationNotAllowed from in-progress addon operations. Signed-off-by: Quang Nguyen <nguyenquang@microsoft.com>
nddq
force-pushed
the
fix/acns-update-preserve-existing-state-v2
branch
from
31c5999 to
5865e72
Compare
Member
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Member
Re-queued |
Member
Author
|
@FumingZhang looks like everything passed |
Member
Author
|
@yanzhudd can you help me merge this? Thanks! |
FumingZhang
approved these changes
Apr 7, 2026
yanzhudd
approved these changes
Apr 8, 2026
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
yanzhudd
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: