[Container] az container create: Remove default values for container group to support standby pool reuse scenario#31824
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
microsoft-github-policy-service
bot
requested review from
yanzhudd and
zhoxing-ms
|
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>
|
There was a problem hiding this comment.
Pull Request Overview
Updates the az container create command to avoid setting default network values when a standby pool profile ID is provided, enabling proper reuse of standby pools.
- Clear
vnet_address_prefixandsubnet_address_prefixwhenstandby_pool_profile_idis supplied - Move default assignment of
portsandprotocolinto the non-standby code path - Only create
cgroup_ip_addressif not in a standby pool scenario
Comments suppressed due to low confidence (1)
src/azure-cli/azure/cli/command_modules/container/custom.py:146
- Add a test case for the standby pool reuse scenario to verify that no default ports, protocol, or IP address are set when
standby_pool_profile_idis provided.
if standby_pool_profile_id != None:
There was a problem hiding this comment.
Use is not None when comparing to None for better readability and to follow PEP8 conventions.
| if standby_pool_profile_id != None: | |
| if standby_pool_profile_id is not None: |
There was a problem hiding this comment.
This assignment is redundant and can be removed since it does not change the value of ports.
| ports = ports |
There was a problem hiding this comment.
This assignment is a no-op and should be removed to simplify the code.
| protocol = protocol |
There was a problem hiding this comment.
Use is None when checking against None to align with Python best practices.
| if standby_pool_profile_id == None: | |
| if standby_pool_profile_id is None: |
drishtipeshwani09
changed the title
az container create: Remove default values for container group to support standby pool reuse scenario
|
Please add some test cases for this PR change |
There was a problem hiding this comment.
May I ask why design this assignment logic?
There was a problem hiding this comment.
Thanks for pointing this out!! There was a mistake in this logic, I have modified this such that only when the standby pool profile id is null, and the port/protocol variables are null as well, we want to assign the variables default values, otherwise they will remain null.
Co-authored-by: Xing Zhou <Zhou.Xing@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
4 participants
Related command
The changes are related to this command -
az container createDescription
Testing Guide
Currently, while we try to reuse a cg from a given standby pool with delegated subnet, we get the below error -
Code: BadRequest
Message: ContainerGroup properties other than config map are not allowed
This change would fix that, now if a customer runs the following command for example, they should be able to reuse a cg successfully -
az container create --resource-group rg --name ctn --container-group-profile-id {cgprofile-id} --container-group-profile-revision {revision-number} --standby-pool-profile-id {standbypoolprofile-id} --vnet {vnet-name/id} --subnet {subnet-name/id}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.