[ACS] az aks create/update: Introduce changes for Azure container storage in ACS cli
#28251
Conversation
This file contains 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
mukhoakash
requested review from
zhoxing-ms,
andyliuliming,
jsntcy and
yanzhudd
as code owners
️✔️AzureCLI-FullTest
|
|
Hi @mukhoakash, |
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| aks create | cmd aks create added parameter enable_azure_container_storage |
||
| aks create | cmd aks create added parameter storage_pool_name |
||
| aks create | cmd aks create added parameter storage_pool_option |
||
| aks create | cmd aks create added parameter storage_pool_size |
||
| aks create | cmd aks create added parameter storage_pool_sku |
||
| aks update | cmd aks update added parameter azure_container_storage_nodepools |
||
| aks update | cmd aks update added parameter disable_azure_container_storage |
||
| aks update | cmd aks update added parameter enable_azure_container_storage |
||
| aks update | cmd aks update added parameter storage_pool_name |
||
| aks update | cmd aks update added parameter storage_pool_option |
||
| aks update | cmd aks update added parameter storage_pool_size |
||
| aks update | cmd aks update added parameter storage_pool_sku |
microsoft-github-policy-service
bot
requested review from
evelyn-ys,
calvinhzy and
yonzhan
|
azure-container-storage |
mukhoakash
changed the title
|
Could you please fix the failed CI checks? |
zioproto
reviewed
Jan 29, 2024
| err_msg = "Validation failed. " \ | ||
| "Please ensure that storagepools are not being used. " \ | ||
| "Unable to perform disable Azure Container Storage operation. " \ | ||
| "Reseting cluster state." |
| update_settings.append({"cli.storagePool.ephemeralDisk.enabled": True}) | ||
| disable_op_failure = True | ||
|
|
||
| # Since we are just reseting the cluster state, |
|
|
||
| # If disabling type of storagepool in Azure Container Storage failed, | ||
| # define the existing resource values for ioEngine and hugepages for | ||
| # reseting the cluster state. |
| if re.fullmatch(pattern, nodepool_names) is None: | ||
| raise InvalidArgumentValueError( | ||
| "Invalid --azure-container-storage-nodepools value. " | ||
| "Accepted value is a comma separated string of valid nodepool " |
There was a problem hiding this comment.
consistency: "node pools"
should be two distinct words in English, consistent with the AKS docs:
https://learn.microsoft.com/en-us/azure/aks/create-node-pools
in the command it is correct to use the form nodepool
| f'Nodepool: {nodepool} not found. ' | ||
| 'Please provide a comma separated string of existing nodepool names ' | ||
| 'in --azure-container-storage-nodepools.' | ||
| f"\nNodepools available in the cluster are: {', '.join(agentpool_names)}." |
| else: | ||
| raise ArgumentUsageError( | ||
| f'Cannot set --storage-pool-option as {CONST_STORAGE_POOL_OPTION_NVME} ' | ||
| 'as no supporting nodepool found which can support ephemeral NVMe disk.' |
There was a problem hiding this comment.
readibility: "as none of the node pools can support ephemeral NVMe disk."
| err_msg = "Validation failed. " \ | ||
| f"Please ensure that storagepools of type {storage_pool_type} are not being used. " \ | ||
| f"Unable to perform disable Azure Container Storage operation. " \ | ||
| "Reseting cluster state." |
| if "pre-upgrade hooks failed" in str(ex): | ||
| raise UnknownError(err_msg) from ex | ||
| raise UnknownError( | ||
| "Validation failed. Unable to perform Azure Container Storage operation. Reseting cluster state." |
| except Exception as disable_ex: | ||
| logger.error( | ||
| "Failure observed while disabling Azure Container Storage storagepool type: %s.\nError: %s" | ||
| "Reseting cluster state.", storage_pool_type, disable_ex |
andyliuliming
suggested changes
Feb 20, 2024
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
mukhoakash
changed the title
mukhoakash
force-pushed
the
mukhoakash/acstor
branch
from
e50a41e
to
08a0b06
Compare
mukhoakash
force-pushed
the
mukhoakash/acstor
branch
from
08a0b06
to
fe3b857
Compare
mukhoakash
changed the title
az aks create/update: Introduce changes for Azure container storage in ACS cli
mukhoakash
force-pushed
the
mukhoakash/acstor
branch
from
8f4f401
to
793e417
Compare
|
Please fix the CI issues |
mukhoakash
force-pushed
the
mukhoakash/acstor
branch
from
6a5ca6f
to
3bb1221
Compare
CIs fixed. |
|
Live test triggered:
|
|
Please note that CLI is only responsible for the code style and specification, but not business logic. So it it better to get approval from the service team first. |
Link to updated live tests. |
FumingZhang
approved these changes
Mar 19, 2024
src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py
Outdated
Show resolved
Hide resolved
src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py
Outdated
Show resolved
Hide resolved
andyliuliming
approved these changes
Mar 19, 2024
yanzhudd
approved these changes
Mar 19, 2024
4 tasks
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.
Related command
Description
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.