Skip to content

Conversation

@zjpjack-github
Copy link
Contributor

Related command

az aks nodepool add/update/upgrade --max-blocked-nodes

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 May 23, 2025 21:27
@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented May 23, 2025

❌Azure CLI Extensions Breaking Change Test
❌aks-preview
rule cmd_name rule_message suggest_message
1010 - ParaPropUpdate aks nodepool add cmd aks nodepool add update parameter spot_max_price: updated property default from nan to nan please change property default from nan to nan for parameter spot_max_price of cmd aks nodepool add
⚠️ 1006 - ParaAdd aks nodepool add cmd aks nodepool add added parameter max_blocked_nodes
⚠️ 1006 - ParaAdd aks nodepool update cmd aks nodepool update added parameter max_blocked_nodes
⚠️ 1006 - ParaAdd aks nodepool upgrade cmd aks nodepool upgrade added parameter max_blocked_nodes

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

Hi @zjpjack-github,
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 May 23, 2025

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

@github-actions
Copy link

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 May 23, 2025

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

Adds support for a new --max-blocked-nodes parameter across az aks nodepool add/update/upgrade commands, including validation, decorator, CLI handling, tests, and documentation.

  • Introduce max_blocked_nodes argument to command signatures and help
  • Implement validate_max_blocked_nodes and wire it into parameter loading
  • Extend decorator and custom logic to read and apply the new setting
  • Update tests to cover validator and integration scenarios

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/aks-preview/azext_aks_preview/tests/latest/test_validators.py Add TestMaxBlockedNodes and new namespace for validator tests
src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Add integration tests for --max-blocked-nodes on add/update
src/aks-preview/azext_aks_preview/custom.py Accept max_blocked_nodes in add/update/upgrade and update error messages
src/aks-preview/azext_aks_preview/agentpool_decorator.py Read/apply max_blocked_nodes in decorator during create/update
src/aks-preview/azext_aks_preview/_validators.py Implement validate_max_blocked_nodes
src/aks-preview/azext_aks_preview/_params.py Register max_blocked_nodes argument with its validator
src/aks-preview/azext_aks_preview/_help.py Document --max-blocked-nodes in CLI help
Comments suppressed due to low confidence (1)

src/aks-preview/azext_aks_preview/custom.py:1397

  • The error message is missing a slash before --max-blocked-nodes. It should read ...max-unavailable/--max-blocked-nodes using.
"update max-surge/drain-timeout/node-soak-duration/undrainable-node-behavior/max-unavailable-max-blocked-nodes using "

Copy link
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

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

Queued live test to validate the change.

  • test_aks_nodepool_max_blocked_nodes

@FumingZhang
Copy link
Member

Please fix failed CI checks

@FumingZhang
Copy link
Member

Applied fix for failed test cases in PR #8798

@FumingZhang
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@FumingZhang
Copy link
Member

Requeued live test in staging env

@FumingZhang
Copy link
Member

live test

Failed with the following error

E azure.cli.testsdk.exceptions.JMESPathCheckAssertionError: Query 'upgradeSettings.maxBlockedNodes' doesn't yield expected value '2', instead the actual value is '1'. Data:

https://dev.azure.com/msazure/CloudNativeCompute/_build/results?buildId=125533074&view=logs&j=b162b355-d59d-5864-ce0f-0a70f12dd28b&t=a26fe913-e2be-5062-b08e-d15f1acc7ea2&l=2848

@FumingZhang
Copy link
Member

Please fix failed CI checks

@FumingZhang
Copy link
Member

please resolve merge conflict on HISTORY.rst

@FumingZhang
Copy link
Member

Requeued live test in staging env

failed with error

E azure.cli.testsdk.exceptions.JMESPathCheckAssertionError: Query 'upgradeSettings.maxBlockedNodes' doesn't yield expected value '2', instead the actual value is '1'. Data:

https://dev.azure.com/msazure/CloudNativeCompute/_build/results?buildId=125533074&view=logs&j=b162b355-d59d-5864-ce0f-0a70f12dd28b&t=a26fe913-e2be-5062-b08e-d15f1acc7ea2&l=2848

Also there's new merge conflict

FumingZhang
FumingZhang previously approved these changes Jun 3, 2025
zhoxing-ms
zhoxing-ms previously approved these changes Jun 3, 2025
@zhoxing-ms
Copy link
Contributor

Please resolve these conflicts

@zjpjack-github zjpjack-github dismissed stale reviews from zhoxing-ms and FumingZhang via 32d16aa June 3, 2025 17:19
@yanzhudd yanzhudd merged commit 86b262d into Azure:main Jun 4, 2025
24 checks passed
@azclibot
Copy link
Collaborator

azclibot commented Jun 4, 2025

[Release] Update index.json for extension [ aks-preview-18.0.0b9 ] : https://dev.azure.com/msazure/One/_build/results?buildId=126192197&view=results

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.

6 participants