Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding the cli command option for --pod-ip-allocation-mode for cluster creation and nodepool addition in AKS #7288

Merged
merged 34 commits into from
Feb 29, 2024
Merged

Adding the cli command option for --pod-ip-allocation-mode for cluster creation and nodepool addition in AKS #7288

merged 34 commits into from
Feb 29, 2024

Conversation

nairashu
Copy link
Contributor

This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

az aks create -g MyResourceGroup -n MyManagedCluster --os-sku Ubuntu --max-pods MaxPodsPerNode --network-plugin azure --vnet-subnet-id /subscriptions/00000/resourceGroups/AnotherResourceGroup/providers/Microsoft.Network/virtualNetworks/MyVnet/subnets/NodeSubnet --pod-subnet-id /subscriptions/00000/resourceGroups/AnotherResourceGroup/providers/Microsoft.Network/virtualNetworks/MyVnet/subnets/PodSubnet --pod-ip-allocation-mode StaticBlock

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)
  • My extension version conforms to the Extension version schema

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.json automatically.
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.

@azure-client-tools-bot-prd azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Feb 14, 2024 •
edited
Loading

❌Azure CLI Extensions Breaking Change Test
❌aks-preview
rule cmd_name rule_message suggest_message
❌ 1010 - ParaPropUpdate aks nodepool add cmd aks nodepool add update parameter spot_max_price: updated property default from nan to nan please change property default from nan to nan for parameter spot_max_price of cmd aks nodepool add
⚠️ 1006 - ParaAdd aks create cmd aks create added parameter pod_ip_allocation_mode
⚠️ 1006 - ParaAdd aks nodepool add cmd aks nodepool add added parameter pod_ip_allocation_mode

@azure-client-tools-bot-prd azure-client-tools-bot-prd
Copy link

Hi @nairashu,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@yonzhan
Copy link
Collaborator

yonzhan commented Feb 14, 2024

WIP

@nairashu nairashu changed the title [WIP]: Adding the cli command option for --pod-ip-allocation-mode for cluster creation and nodepool addition in AKS Adding the cli command option for --pod-ip-allocation-mode for cluster creation and nodepool addition in AKS Feb 15, 2024
@nairashu
Copy link
Contributor Author

WIP

Removed the WIP after adding all the changes for this agentpool option.

@nairashu
Copy link
Contributor Author

This PR is a placeholder and waiting on this property to get released in the SDK. Requesting early feedback.

FumingZhang
FumingZhang reviewed Feb 18, 2024
View reviewed changes
Copy link
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some unit test cases (see examples in test_agentpool_decorator.py) and at least one live test case (see examples in test_aks_commands.py) for the newly added option.

src/aks-preview/azext_aks_preview/_validators.py Show resolved Hide resolved
src/aks-preview/azext_aks_preview/managed_cluster_decorator.py Outdated Show resolved Hide resolved
FumingZhang
FumingZhang reviewed Feb 21, 2024
View reviewed changes
Copy link
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve merge conflicts

@nairashu
Copy link
Contributor Author

Please add some unit test cases (see examples in test_agentpool_decorator.py) and at least one live test case (see examples in test_aks_commands.py) for the newly added option.

I have added the requested UT and live test case.

FumingZhang
FumingZhang reviewed Feb 21, 2024
View reviewed changes
Copy link
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix failed UT

FAILED src/aks-preview/azext_aks_preview/tests/latest/test_agentpool_decorator.py::AKSPreviewAgentPoolContextStandaloneModeTestCase::test_get_pod_ip_allocation_mode
FAILED src/aks-preview/azext_aks_preview/tests/latest/test_agentpool_decorator.py::AKSPreviewAgentPoolContextManagedClusterModeTestCase::test_get_pod_ip_allocation_mode

src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Show resolved Hide resolved
src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Show resolved Hide resolved
@nairashu
Copy link
Contributor Author

Please resolve merge conflicts

Merge conflicts were resolved

@yonzhan
Copy link
Collaborator

yonzhan commented Feb 21, 2024

Please fix CI issues

@azclibot
Copy link
Collaborator

⚠️ Suggestions

Module: aks-preview

  • Update version to 1.0.0b7 in setup.py
  • Set azext.isPreview to true in azext_aks-preview/azext_metadata.json

Notes

  • Stable/preview tag is inherited from last release. If needed, please add stable/preview label to modify it.
  • Major/minor/patch/pre increment of version number is calculated by pull request code changes automatically. If needed, please add major/minor/patch/pre label to adjust it.
  • For more info about extension versioning, please refer to Extension version schema

Ashish Nair added 21 commits February 29, 2024 04:01
Attempting to not initialize any pod-ip-allocation-mode by default an…
4516aaa 
…d then attach an agentpool with StaticBlock
Fixed param list
da0e473
Adding the live only tag for the live test added
4d6de3f
Fixed the missing '-' from the max-pods argument
6882dfd
Attempting to fix the resource ID for the node subnet
3ad89a1
Attempting to fix the resource ID for the node subnet
24a52a2
Fixed the property name of the agentpoolprofile
5953766
Updated the expected value for podSubnetID
4cad504
Added the logic to add the podIPAllocationMode to the agentpool netwo…
88ff1e1 
…rk profile which was what was failing the CI
Removed the reference to agentpool network profile
0c1276b
Added a test case for managed_cluster properties generation
256a1bf
Added methods to update the pod ip allocation mode property of the ag…
83bc419 
…entpool to both the create and update methods
Fixing the variable name in the live test aks_name
15ebd05
Added the live test recording to the PR
3035ee2
Updated the PR based on the review comments to reorder the logic in t…
5bbc405 
…he get method for the pod-ip-allocation-mode property and also removed the definition and call to set the property in the AgentPoolUpdateDecorator as the property option is only available in the aks create and aks nodepool add commands.
Updated the test cases based on the suggestions made in the PR review
c587768
Upated the History and setup.py files after rebasing to merge the new…
30feb66 
… property in the next version
Fixed the assertion value
c50184d
Fixed the azdev styles issue
3572460
Removed the liveonly tag
b1e48ee
Excluding the test from the live test as it requires feature registra…
d519a51 
…tion
nairashu and others added 2 commits February 28, 2024 20:09
@nairashu @zhoxing-ms
Update src/aks-preview/azext_aks_preview/_help.py based on the sugges…
1703660 
…ted change

Co-authored-by: Xing Zhou <Zhou.Xing@microsoft.com>
@nairashu @zhoxing-ms
Update src/aks-preview/azext_aks_preview/_help.py to use first-person…
39ad6cb 
… voice.

Co-authored-by: Xing Zhou <Zhou.Xing@microsoft.com>
@wangzelin007 wangzelin007 merged commit c4eaa7d into Azure:main Feb 29, 2024
13 of 15 checks passed
@azclibot
Copy link
Collaborator

[Release] Update index.json for extension [ aks-preview ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=135128&view=results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyler-lloyd tyler-lloyd tyler-lloyd left review comments

@FumingZhang FumingZhang FumingZhang left review comments

@zhoxing-ms zhoxing-ms zhoxing-ms approved these changes

@wangzelin007 wangzelin007 wangzelin007 approved these changes

@andyliuliming andyliuliming Awaiting requested review from andyliuliming andyliuliming is a code owner

@yanzhudd yanzhudd Awaiting requested review from yanzhudd

@yonzhan yonzhan Awaiting requested review from yonzhan

Assignees

@zhoxing-ms zhoxing-ms

@yanzhudd yanzhudd

Labels
AKS Auto-Assign Auto assign by bot auto-cal-version
Projects
None yet
Milestone
February 2024 (2024-03-05)
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@nairashu @yonzhan @azclibot @FumingZhang @zhoxing-ms @wangzelin007 @tyler-lloyd @AllyW @yanzhudd