-
Notifications
You must be signed in to change notification settings - Fork 1.5k
LocalDNS - Add more Live Test Cases #9237
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
LocalDNS - Add more Live Test Cases #9237
Conversation
️✔️Azure CLI Extensions Breaking Change Test
|
|
Hi @saewoni, |
|
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 adds better null value handling for the LocalDNS configuration feature in Azure AKS CLI extension. The change addresses edge cases where DNS override sections can be null in JSON configuration files, preventing crashes and ensuring proper handling of empty or partial configurations.
- Introduces a
process_dns_overrideshelper function to safely handle null values in DNS configurations - Adds comprehensive test coverage for various edge cases and invalid scenarios
- Updates test data files to support more testing scenarios
Reviewed Changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
_helpers.py |
Adds process_dns_overrides utility function for null-safe DNS override processing |
agentpool_decorator.py |
Refactors LocalDNS profile handling to use the new helper function |
test_localdns_profile.py |
Adds new expected default DNS override configurations |
test_aks_commands.py |
Extends test coverage with additional LocalDNS scenarios |
test_agentpool_decorator.py |
Adds unit tests for LocalDNS profile updates and helper functions |
test_aks_invalid_localdns_cases.py |
New test file with comprehensive invalid/edge case testing |
data/localdnsconfig/*.json |
Multiple test configuration files for various scenarios |
HISTORY.rst |
Documents the fix in release notes |
|
|
||
| class InvalidLocalDnsConfigTests: |
Copilot
AI
Sep 26, 2025
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.
The class InvalidLocalDnsConfigTests should inherit from a proper test base class (like ScenarioTest or unittest.TestCase) to ensure proper test framework integration and access to test utilities.
| class InvalidLocalDnsConfigTests: | |
| from azure.cli.testsdk import ScenarioTest | |
| class InvalidLocalDnsConfigTests(ScenarioTest): |
|
Hi @saewoni Release SuggestionsModule: aks-preview
Notes
|
| * Add blue-green upgrade strategy support for AKS node pools: | ||
| - `az aks nodepool add/update/upgrade`: Add `--upgrade-strategy` parameter to switch between rolling and blue-green nodepool upgrades. | ||
| - `az aks nodepool add/update/upgrade`: Add `--drain-batch-size`, `--drain-timeout-bg`, `--batch-soak-duration`, `--final-soak-duration` parameters to configure blue-green upgrade settings. | ||
| * Fix `--localdns-config` parameter to handle null values in JSON configuration files gracefully, preventing crashes when DNS override sections are null. |
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.
please either choose a new version number or move your history notes to the pending section
| return None | ||
|
|
||
|
|
||
| def process_dns_overrides(overrides_dict, target_dict, build_override_func): |
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.
similar to the change in #9188, is the work duplicated?
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.
this will be a follow up PR! once #9188 is merged.
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.
abandoning/closing this PR and will be making a new PR since juan's is merged: #9252
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
…atest/test_aks_commands.py
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
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.