[dataprotection] Add controller and initContainer resource settings for AKS Backup Extension #9648
[dataprotection] Add controller and initContainer resource settings for AKS Backup Extension #9648priyansh17 wants to merge 1 commit intoAzure:mainfrom
Conversation
… pass-through in DataProtectionKubernetes Co-authored-by: priyansh17|choudharypr@microsoft.com
️✔️Azure CLI Extensions Breaking Change Test
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
Hi @priyansh17, |
|
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>
|
microsoft-github-policy-service
bot
requested review from
FumingZhang,
yanzhudd,
yonzhan and
zhoxing-ms
microsoft-github-policy-service
bot
added
the
customer-reported
|
Thank you for your contribution @priyansh17! We will review the pull request and get back to you soon. |
CodeGen Tools Feedback CollectionThank you for using our CodeGen tool. We value your feedback, and we would like to know how we can improve our product. Please take a few minutes to fill our codegen survey |
priyansh17
changed the title
There was a problem hiding this comment.
Pull request overview
This PR extends the AKS Backup (DataProtectionKubernetes) partner extension configuration mapping to support resource settings for the controller and the Velero Azure plugin initContainer, and adds unit tests to validate the new mappings.
Changes:
- Add new configuration constants and short-name mappings for controller and initContainer (veleroPluginForMicrosoftAzure) resource requests/limits.
- Enhance config normalization to accept canonical “full-path” keys case-insensitively and strip whitespace.
- Add unit tests covering short-name mappings, full-path pass-through, and BSL validation behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/k8s-extension/azext_k8s_extension/partner_extensions/DataProtectionKubernetes.py |
Adds controller/plugin resource config keys and expands config normalization/recognition logic. |
src/k8s-extension/azext_k8s_extension/tests/latest/test_data_protection_kubernetes.py |
Adds unit tests for the new mapping/normalization behavior and validation paths. |
| # -------------------------------------------------------------------------------------------- | ||
|
|
||
| import unittest | ||
| from unittest.mock import patch |
There was a problem hiding this comment.
patch is imported but never used in this test module. This will be flagged by linting and should be removed (or used if intended).
| from unittest.mock import patch |
| if validate_bsl: | ||
| for key in self.bsl_configuration_settings: | ||
| if key.lower() not in input_configuration_keys: | ||
| raise RequiredArgumentMissingError(f"Missing required configuration setting: {key}") |
There was a problem hiding this comment.
__validate_and_map_config now recognizes canonical full-path keys (including the backup storage location paths), but the required BSL validation only checks for the short-name keys (e.g., blobContainer). If a user provides the required BSL settings using the full-path keys, this will still raise RequiredArgumentMissingError even though the keys are otherwise recognized later in the method. Consider updating the validation to accept either short-name or canonical full-path variants (or restrict full-path recognition to non-required settings).
| for key in input_configuration_settings: | ||
| _key = key.lower() | ||
| if _key in self.configuration_mapping: | ||
| configuration_settings[self.configuration_mapping[_key]] = configuration_settings.pop(key).strip() | ||
| elif _key in recognized_full_paths_lower: | ||
| # User provided the full configuration path directly - normalize to canonical form | ||
| canonical = recognized_full_paths_lower[_key] | ||
| configuration_settings[canonical] = configuration_settings.pop(key).strip() |
There was a problem hiding this comment.
When both a short-name key and its corresponding canonical full-path key are provided (e.g., cpuLimit and resources.limits.cpu), the current loop can overwrite one with the other based on dict insertion order, and may even pop the value that was just normalized. Please make precedence explicit (for example: prefer the explicit full-path key, or detect duplicates and raise/ignore deterministically) to avoid surprising configuration results.
|
Hi @priyansh17 Release SuggestionsModule: k8s-extension
Notes
|
This adds support for users to modify data protection backup extension for AKS workload to modify limits of specific containers being deployed.
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: