[Service Fabric] add sf cluster update command #4328 #29908
Conversation
mwesigwaguma
requested review from
zhoxing-ms,
jsntcy and
yanzhudd
as code owners
️✔️AzureCLI-FullTest
|
❌AzureCLI-BreakingChangeTest
|
|
ServiceFabric |
microsoft-github-policy-service
bot
added
Service Fabric
please add description of the changes in this pr here |
src/azure-cli/azure/cli/command_modules/servicefabric/custom.py
Outdated
Show resolved
Hide resolved
src/azure-cli/azure/cli/command_modules/servicefabric/custom.py
Outdated
Show resolved
Hide resolved
|
Please note that Azure CLI will have a code freeze for the upcoming release on 09/24/2024 10:00 UTC. If you want to catch this release train, please address the comments and CI issues ASAP. Otherwise, it has to be postponed to next sprint (11-05). |
mwesigwaguma
changed the title
mwesigwaguma
changed the title
zhoxing-ms
modified the milestones:
September 2024 (2024-10-08),
October 2024 (2024-11-05)
zhoxing-ms
modified the milestones:
October 2024 (2024-11-05),
November 2024 (2024-11-19)❗ Breaking change release ❗
| @@ -147,6 +139,29 @@ def load_arguments(self, _): # pylint: disable=too-many-statements | |||
| help='JSON encoded parameters configuration. Use @{file} to load from a file. ' | |||
| 'For example: [{"section": "NamingService","parameter": "MaxOperationTimeout"}]') | |||
|
|
|||
| with self.argument_context('sf cluster node') as c: | |||
| c.argument('number_of_nodes_to_add', options_list=['--number-of-nodes-to-add', '--nodes-to-add'], help='number of nodes to add.') | |||
| c.argument('number_of_nodes_to_remove', options_list=['--number-of-nodes-to-rem', '--nodes-to-remove'], help='number of nodes to remove.') | |||
There was a problem hiding this comment.
Why do you need to rename --number-of-nodes-to-remove to --number-of-nodes-to-rem, I personally think that --number-of-nodes-to-remove has better readability than --number-of-nodes-to-rem
There was a problem hiding this comment.
There was a complaint about length of the options in pipeline I believe. Also if the options are too long, the help output is not readable. This is a problem with one of our other commands az sf managed-applicatioin update -h
There was a problem hiding this comment.
Please note that this will cause a breaking change for the customer, perhaps you can add a few shorter option names to bypass the complaint of length
| self.cmd('sf cluster update --resource-group {rg} -c {cluster_name} --upgrade-mode manual --version {version}', | ||
| checks=[self.check('upgradeMode', 'Manual')]) | ||
|
|
||
| @live_only() |
There was a problem hiding this comment.
May I ask why do you need to mark this test as @live_only()?
There was a problem hiding this comment.
after running test live it fails in playback with following errors and workaround to replaced mismatched values does not work, in this case keyvault name
E AssertionError: Can't overwrite existing cassette ('e:\repos\azure-cli\src\azure-cli\azure\cli\command_modules\servicefabric\tests\latest\recordings\test_reliability_and_durability.yaml') in your current record mode ('once').
E No match for the request (<Request (GET) https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrg000001?api-version=2023-02-01>) was found.
E Found 3 similar requests with 1 different matcher(s) :
E
E 1 - (<Request (GET) https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrgojwj2hjwlnme?api-version=2023-02-01>).
E Matchers succeeded : ['method', 'scheme', 'host', 'port', '_custom_request_query_matcher']
E Matchers failed :
E path - assertion failure :
E /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrg000001 != /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrgojwj2hjwlnme
E
E 2 - (<Request (GET) https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrgojwj2hjwlnme?api-version=2023-02-01>).
E Matchers succeeded : ['method', 'scheme', 'host', 'port', '_custom_request_query_matcher']
E Matchers failed :
E path - assertion failure :
E /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrg000001 != /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrgojwj2hjwlnme
E
E 3 - (<Request (GET) https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrgojwj2hjwlnme?api-version=2023-02-01>).
E Matchers succeeded : ['method', 'scheme', 'host', 'port', '_custom_request_query_matcher']
E Matchers failed :
E path - assertion failure :
E /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrg000001 != /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/clitest.rg000001/providers/Microsoft.KeyVault/vaults/clitestrgojwj2hjwlnme
workaround of switching
In fact, the deprecation of commands is not considered a real breaking change because customers can still use them normally, but they will receive relevant warning logs. Only by completely removing these commands can it be considered a real breaking change. Therefore, please remove the |
zhoxing-ms
modified the milestones:
November 2024 (2024-11-19)❗ Breaking change release ❗,
December 2024 (2024-12-10)
…solidate_cluster_commands
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
removed breaking change file |
|
Actually, my suggestion is to restore the breaking change file b1bfe8a, as it will not cause a real breaking change to the user, but will only notify users with the deprecation information of the parameters through warning logs |
|
|
zhoxing-ms
modified the milestones:
January 2025 (2025-01-14),
February 2025 (2025-02-11)
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@mwesigwaguma please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
|
Please reply with above information. |
|
please note that Azure CLI will freeze the code on 02/25/2025 10:00 UTC for the upcoming release. If you want to catch this release train, please resolve the comments ASAP, otherwise this PR has to be postponed to next sprint. |
|
Please take a look at this comment #29908 (comment) |
Related command
sf cluster upgrade-type
sf cluster setting
sf cluster durability
f cluster reliability update
sf cluster setting remove
sf cluster setting set
sf cluster upgrade-type set
Description
The commands groups and commands above are being deprecated and moved under one command sf cluster update
Testing Guide
History Notes
[Service Fabric]
az sf cluster upgrade-type set: Deprecate command. Please useaz sf cluster updatecommand[Service Fabric]
az sf cluster durability: Deprecate command. Please useaz sf cluster updatecommand[Service Fabric]
az sf cluster reliability update: Deprecate command. Please useaz sf cluster updatecommand[Service Fabric]
az sf cluster setting remove: Deprecate command. Please useaz sf cluster updatecommand[Service Fabric]
az sf cluster setting set: Deprecate command. Please useaz sf cluster updatecommandThis 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.