-
Couldn't load subscription status.
- Fork 259
ci: Populate missing fields in CNS configmap #3503
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
Conversation
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 missing fields in various CNS configmaps used for integration testing in Azure Container Networking. The changes add additional configuration keys to ensure a more complete set of fields across different manifest files.
- Added "EnableStateMigration": false, "EnableSubnetScarcity": false, and "MetricsBindAddress": ":10092" to multiple configmaps.
- Some Windows-specific configmaps receive only a subset of these new fields.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/manifests/cnsconfig/overlayconfigmap.yaml | Added three new keys to the overlay config. |
| test/integration/manifests/cnsconfig/swiftlinuxconfigmap.yaml | Added three new keys to the Swift Linux config. |
| test/integration/manifests/cnsconfig/swiftwindowsconfigmap.yaml | Added three new keys to the Swift Windows config. |
| test/integration/manifests/cnsconfig/azurecnioverlaylinuxconfigmap.yaml | Added three new keys to the Azure CNI overlay Linux config. |
| test/integration/manifests/cnsconfig/ciliumnodesubnetconfigmap.yaml | Added three new keys to the Cilium Node Subnet config. |
| test/integration/manifests/cnsconfig/ciliumconfigmap.yaml | Added three new keys to the Cilium config. |
| test/integration/manifests/cnsconfig/azurecnidualstackoverlaylinuxconfigmap.yaml | Added three new keys to the dual-stack overlay Linux config. |
| test/integration/manifests/cnsconfig/azurestatelesscnioverlaywindowsconfigmap.yaml | Added two new keys – missing "EnableSubnetScarcity". |
| test/integration/manifests/cnsconfig/azurecnioverlaywindowsconfigmap.yaml | Added only one new key. |
| test/integration/manifests/cnsconfig/azurecnidualstackoverlaywindowsconfigmap.yaml | Added only one new key. |
Comments suppressed due to low confidence (3)
test/integration/manifests/cnsconfig/azurestatelesscnioverlaywindowsconfigmap.yaml:35
- The addition of new fields in this configmap is inconsistent with similar files (e.g., Swift Windows) which include 'EnableSubnetScarcity'. Consider adding "EnableSubnetScarcity": false for consistency across Windows configmaps.
"MetricsBindAddress": ":10092", test/integration/manifests/cnsconfig/azurecnioverlaywindowsconfigmap.yaml:34
- Only 'EnableStateMigration' is added here whereas other similar Windows configmaps include additional fields such as "EnableSubnetScarcity" and "MetricsBindAddress". Review whether these fields should be added for consistency.
"EnableStateMigration": false, test/integration/manifests/cnsconfig/azurecnidualstackoverlaywindowsconfigmap.yaml:34
- This change adds only 'EnableStateMigration', while other dual-stack overlays include additional configuration keys. Consider adding the missing fields (e.g., "MetricsBindAddress" and possibly "EnableSubnetScarcity") for consistency.
"EnableStateMigration": false, | /azp run Azure Container Networking PR |
| Azure Pipelines successfully started running 1 pipeline(s). |
| This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
| Pull request closed due to inactivity. |
5417470 to 4f802bb Compare 4f802bb to d5a102c Compare | /azp run Azure Container Networking PR |
| Azure Pipelines successfully started running 1 pipeline(s). |
ci: add in missing fields
Reason for Change:
Adding in fields that have 0 impact to current E2E as they default to false or are not used.
Issue Fixed:
Requirements:
Notes:
Will sort and organize bottom block to match default managed aks offering after #3502 has merged.