[AKS] az aks create/update: Add ephemeralDisk and elasticSan storag…#9576
[AKS] az aks create/update: Add ephemeralDisk and elasticSan storag…#9576yanzhudd merged 7 commits intoAzure:mainfrom
az aks create/update: Add ephemeralDisk and elasticSan storag…#9576Conversation
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| aks create | cmd aks create update parameter enable_azure_container_storage: updated property nargs from ? to * |
||
| aks update | cmd aks update update parameter disable_azure_container_storage: updated property nargs from ? to * |
||
| aks update | cmd aks update update parameter enable_azure_container_storage: updated property nargs from ? to * |
|
Hi @NedAnd1, |
|
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
yanzhudd,
yonzhan and
zhoxing-ms
CodeGen Tools Feedback CollectionThank 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 |
|
There was a problem hiding this comment.
Pull request overview
This pull request adds support for ephemeralDisk and elasticSan storage options for Azure Container Storage (ACStor) v2 in the AKS preview CLI extension, providing parity with the main CLI PR #32558. The changes enable users to selectively enable or disable specific storage driver options rather than managing the entire extension as a single unit.
Changes:
- Refactored enable/disable operations into a unified function that can handle individual storage driver configuration
- Updated validators to support multiple storage types and improved error messages
- Modified argument parsing to accept multiple storage type values (space or comma-separated)
- Added comprehensive test coverage for new validation scenarios
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test_validators.py | Updated test cases to reflect new validation logic with multiple storage types and improved parameter signatures |
| managed_cluster_decorator.py | Refactored to use unified update function, updated parameter passing, and added logic for granular storage option prompts |
| acstor_ops.py | Merged enable/disable functions into single update function with config settings for individual CSI drivers |
| _validators.py | Enhanced validation to support multiple storage types, improved error messages, and better handling of already-enabled scenarios |
| _helpers.py | Added new helper functions to check extension installation status and individual driver enablement states |
| _params.py | Consolidated argument type functions and updated to support multiple storage type values with case-insensitive matching |
Comments suppressed due to low confidence (1)
src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py:708
- The comment references the old function name "perform_disable_azure_container_storage" but the function has been renamed to "perform_azure_container_storage_update" which handles both enable and disable operations. Please update this comment to reflect the current function name.
# This will be set true only when aks-preview extension is used
# and we want the aks-preview ManagedClusterDecorator to call the
# perform_disable_azure_container_storage function.
src/aks-preview/azext_aks_preview/azurecontainerstorage/_helpers.py
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
The integration failures look like they are unrelated & fixed by PR #9575 |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
github-actions
bot
added
the
release-version-block
NedAnd1
force-pushed
the
ned/acstor-2.1
branch
from
3e67978 to
1a5da0d
Compare
github-actions
bot
removed
the
release-version-block
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
FumingZhang
left a comment
FumingZhang
left a comment
There was a problem hiding this comment.
Queued live test to validate the change
- test_aks_create_with_azurecontainerstorage_with_ephemeral_disk_parameters
- test_aks_create_with_azurecontainerstorage_with_elastic_san_parameters
- test_aks_create_with_azurecontainerstorage_with_multiple_parameters
- test_aks_update_with_azurecontainerstorage_with_ephemeral_disk
- test_aks_update_with_azurecontainerstorage_with_elastic_san
- test_aks_update_with_azurecontainerstorage_with_multiple_parameters
One of the test cases failed due to a transient helm install issue, could they be retried? |
Re-queued live test for The test failed with error
You may follow the examples in other test cases to install the extension you need |
Weird that that error is cropping up now, since the k8s-extension installs were previously commented out in our scenario tests for |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Re-queued live test |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Co-authored-by: Yan Zhu <105691024+yanzhudd@users.noreply.github.com>
|
@yanzhudd @FumingZhang Thanks for the reviews, could this PR be reapproved & merged soon? |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
[Release] Update index.json for extension [ aks-preview-19.0.0b23 ] : https://dev.azure.com/msazure/One/_build/results?buildId=156058189&view=results |
…e options for ACStor v2
Created for parity with PR Azure/azure-cli#32558
to support CLI-driven storage option enablement for ACStor 2.1 within the AKS preview CLI extension
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az aks create/updateGeneral Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)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.jsonautomatically.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.