Skip to content

[aks-preview] Fix operator precedence bug in get_container_network_logs() validation#9575

Open
ShantingLiu wants to merge 3 commits intoAzure:mainfrom
ShantingLiu:jennyliu/fix-container-network-logs-validation
Open

[aks-preview] Fix operator precedence bug in get_container_network_logs() validation#9575
ShantingLiu wants to merge 3 commits intoAzure:mainfrom
ShantingLiu:jennyliu/fix-container-network-logs-validation

Conversation

@ShantingLiu
Copy link
Contributor

@ShantingLiu ShantingLiu commented Feb 4, 2026

The validation logic in get_container_network_logs() had two issues:

  1. Operator precedence bug - the condition evaluated as (enable_cnl AND acns_not_enabled) OR monitoring_not_enabled which would error if monitoring wasn't configured, regardless of enable_cnl
  2. Timing issue during cluster create - the validation only checked mc.addon_profiles, but during create this is empty even when --enable-addons monitoring is specified

Fix: Check both mc.addon_profiles (for update) AND --enable-addons parameter (for create), with correct operator precedence.


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

az aks create --enable-container-network-logs
az aks update --enable-container-network-logs

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)
  • My extension version conforms to the Extension version schema

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.json automatically.
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.

Copilot AI review requested due to automatic review settings February 4, 2026 19:36
@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Feb 4, 2026

️✔️Azure CLI Extensions Breaking Change Test
️✔️Non Breaking Changes

@azure-client-tools-bot-prd
Copy link

Hi @ShantingLiu,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@yonzhan
Copy link
Collaborator

yonzhan commented Feb 4, 2026

Thank you for your contribution! We will review the pull request and get back to you soon.

@github-actions
Copy link

github-actions bot commented Feb 4, 2026

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).
After that please run the following commands to enable git hooks:

pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>

@github-actions
Copy link

github-actions bot commented Feb 4, 2026

CodeGen Tools Feedback Collection

Thank 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

The validation logic had two issues:
1. Operator precedence bug - the condition evaluated as
   (enable_cnl AND acns_not_enabled) OR monitoring_not_enabled
   which would error if monitoring wasn't configured, regardless of enable_cnl
2. Timing issue during cluster create - the validation only checked
   mc.addon_profiles, but during create this is empty even when
   --enable-addons monitoring is specified

Fix: Check both mc.addon_profiles (for update) AND --enable-addons
parameter (for create), with correct operator precedence.
@ShantingLiu ShantingLiu force-pushed the jennyliu/fix-container-network-logs-validation branch from e8747b5 to aee6e23 Compare February 4, 2026 19:39
@github-actions
Copy link

github-actions bot commented Feb 4, 2026

Hi @ShantingLiu

Release Suggestions

Module: aks-preview

  • Update VERSION to 19.0.0b22 in src/aks-preview/setup.py

Notes

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes validation in get_container_network_logs() to avoid incorrect failures caused by operator precedence and to correctly detect monitoring enablement during cluster create.

Changes:

  • Restructures validation so dependency checks only run when container network logs are being enabled.
  • Adds create-time detection for monitoring enablement via the --enable-addons parameter in addition to existing mc.addon_profiles checks.

ShantingLiu and others added 2 commits February 4, 2026 15:21
Address Copilot feedback: improve monitoring enablement detection

- Use normalized addon list via _get_enable_addons() when available
- Handle both string and list formats for enable_addons parameter  
- Also check --enable-azure-monitor-logs flag as alternative way to enable monitoring
- Use CONST_MONITORING_ADDON_NAME_CAMELCASE for proper constant matching

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ADDON_NAME_CAMELCASE

The enable_addons parameter contains the user-facing addon name 'monitoring',
not the profile key name 'omsAgent' (CONST_MONITORING_ADDON_NAME_CAMELCASE).
This caused the check to always fail since 'omsAgent' is never in the
enable_addons list.
"Container network logs requires '--enable-acns', advanced networking "
"to be enabled, and the monitoring addon to be enabled."

if enable_cnl:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related to #9530

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AKS Auto-Assign Auto assign by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants