add mc snapshot command and change nodepool snapshot #4594
Conversation
|
aks |
| @@ -206,9 +206,12 @@ def load_arguments(self, _): | |||
| c.argument('host_group_id', | |||
| validator=validate_host_group_id, is_preview=True) | |||
| c.argument('crg_id', validator=validate_crg_id, is_preview=True) | |||
| c.argument('message_of_the_day', type=str) # no validation for aks create because it already only supports Linux. | |||
There was a problem hiding this comment.
why so many irrelevant changes in many files? without these changes, will Style or linter checks fail?
There was a problem hiding this comment.
linter didn't fail. And the style change auto-saved.
src/aks-preview/HISTORY.md
Outdated
| 0.5.59 | ||
| ++++++ | ||
|
|
||
| * Add support for managed cluster snapshot commands and modify current nodepool snapshot commands |
There was a problem hiding this comment.
Maybe it's better to remind user there's a breaking change. What used to be the aks snapshot command is now the aks nodepool snapshot command.
However, this may also confuse users more...
There was a problem hiding this comment.
This is a good suggestion! @zqingqing1 Please take a look~
| raise CLIError( | ||
| "--api-server-authorized-ip-ranges cannot be disabled and simultaneously enabled") |
There was a problem hiding this comment.
Please use the specific error type (such as InvalidArgumentValueError or ArgumentUsageError) instead of CLIError. There are several similar issues below, I won't comment again
There was a problem hiding this comment.
This is not my change, but when saving file, my linter automatically formatted it with extra line.
There was a problem hiding this comment.
I wouldn't change the error type here, since it is not related to my change. I want to make this PR clean and simple.
There was a problem hiding this comment.
OK, please submit a separate PR to refine the code here
|
Could we use |
No, this is not deprecate the "az aks snapshot" command, the command will be used for cluster snapshot. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Please fix the live test error. |
|
Oh, the failed check AZCLI AKS LIVE TEST V2 - [COMPATIBILITY] should be a known issue unrelated to this PR, I'll open another PR to fix it. |
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally?For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update
src/index.jsonautomatically.The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify
src/index.json.