-
Notifications
You must be signed in to change notification settings - Fork 1.5k
{AKS} populate location of managed namespaces using managed cluster's location #8821
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.
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
{AKS} populate location of managed namespaces using managed cluster's location #8821
Conversation
️✔️Azure CLI Extensions Breaking Change Test
|
|
Hi @xmzhao0822, |
|
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR populates managed namespace resources with the location of their parent AKS cluster by fetching and assigning the cluster’s location in both add and update commands.
- Fetch the AKS cluster location in
aks_managed_namespace_addandaks_managed_namespace_update. - Assign the cluster’s location to the
locationfield onnamespace_config. - Adjust tests to mock
get_mgmt_service_clientand simulate cluster location.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/aks-preview/azext_aks_preview/tests/latest/test_managednamespace.py | Added get_mgmt_service_client patch and mock cluster client in tests to simulate location. |
| src/aks-preview/azext_aks_preview/managednamespace.py | Imported client factory & cluster client, fetched the cluster’s location, and set namespace_config.location in both add/update commands. |
Comments suppressed due to low confidence (1)
src/aks-preview/azext_aks_preview/managednamespace.py:43
- The new
locationproperty assignment isn't covered by any tests. Add tests to verify thatnamespace_config.locationis correctly set based on the cluster's location.
namespace_config.location = cluster_location
|
FumingZhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
live test failed with the following error
|
|
due to change in Azure/azure-cli#31589, the CI checks failed, could you please replace |
…preview/tests/latest/recordings/test_aks_create_and_update_outbound_ips.yaml
|
live test passed |
|
[Release] Update index.json for extension [ aks-preview-18.0.0b8 ] : https://dev.azure.com/msazure/One/_build/results?buildId=126094133&view=results |
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az aks namespace add
az aks namespace update
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)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.jsonautomatically.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.