Skip to content

Conversation

@lilypan26
Copy link
Contributor

@lilypan26 lilypan26 commented May 21, 2025


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

Related command

--skip-gpu-driver-install, --gpu-driver None/Install

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

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

❌Azure CLI Extensions Breaking Change Test
❌aks-preview
rule cmd_name rule_message suggest_message
1007 - ParaRemove aks nodepool add cmd aks nodepool add removed parameter skip_gpu_driver_install please add back parameter skip_gpu_driver_install for cmd aks nodepool add
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

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

Hi @lilypan26,
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 21, 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>

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

This PR updates and re-enables the GPU driver install tests to use the new --gpu-driver parameter and removes the deprecated skip_gpu_driver_install logic and tests.

  • Re-enable and adapt the test_aks_skip_gpu_driver_install in test_aks_commands.py to use --gpu-driver None/Install and assert on gpuProfile.driver.
  • Remove all skip_gpu_driver_install methods in agentpool_decorator.py and associated tests in test_agentpool_decorator.py.
  • Align CLI tests with the new API flag semantics for GPU driver configuration.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Re-enabled and updated GPU driver skip/install test using new flag.
src/aks-preview/azext_aks_preview/tests/latest/test_agentpool_decorator.py Removed obsolete skip_gpu_driver_install test helpers and calls.
src/aks-preview/azext_aks_preview/agentpool_decorator.py Deleted deprecated get_skip_gpu_driver_install and set_up_skip_gpu_driver_install.

"--resource-group={resource_group} "
"--cluster-name={name} "
"--name={nodepool_name_second} "
"--node-vm-size={node_vm_size} "
Copy link

Copilot AI May 21, 2025

Choose a reason for hiding this comment

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

The placeholder {node_vm_size} is never set in self.kwargs. Add "node_vm_size": <desired_value> to self.kwargs.update(...) before using it, or remove the placeholder if it's not needed.

Copilot uses AI. Check for mistakes.
# "aks delete -g {resource_group} -n {name} --yes --no-wait",
# checks=[self.is_empty()],
# )
# nodepool delete the second
Copy link

Copilot AI May 21, 2025

Choose a reason for hiding this comment

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

The comment is misleading: this command deletes the first node pool ({nodepool_name}), not the second. Update the comment to accurately describe the deletion target.

Suggested change
# nodepool delete the second
# delete the first node pool

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented May 21, 2025

Hi @lilypan26

Release Suggestions

Module: aks-preview

  • Update VERSION to 18.0.0b3 in src/aks-preview/setup.py

Notes

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_skip_gpu_driver_install

BTW, please commit new recording file for the new test case to pass CI checks.

Pending
+++++++

18.0.0b3
Copy link
Member

Choose a reason for hiding this comment

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

Please choose a new major version like 19.0.0b1 as you're making a breaking change.

https://github.com/Azure/azure-cli/blob/release/doc/extensions/versioning_guidelines.md

@lilypan26 lilypan26 changed the title [AKS] chore: update gpuProfile.driver test [AKS] chore: remove --skip-gpu-driver-install May 27, 2025
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