-
Couldn't load subscription status.
- Fork 259
[feat] synchronize NC version with NMA programmed goal state #3790
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
[feat] synchronize NC version with NMA programmed goal state #3790
Conversation
update values add client test scenarios . . . . .
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
Implements delegated network container version updates by combining NMAgent and IMDS version sources and expands related unit tests.
- Removes obsolete log calls in CNS service initializers.
- Enhances
syncHostNCVersionto fetch NC versions from IMDS, merge with NMAgent data, and update host state. - Adds new
GetNCVersionsFromIMDSsupport in IMDS client and covers success and error scenarios with unit tests.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cns/service/main.go | Removed redundant logger.Printf calls in controller initializers |
| cns/restserver/restserver.go | Added GetNCVersionsFromIMDS to imdsClient interface |
| cns/restserver/internalapi.go | Merged IMDS NC versions into syncHostNCVersion and added helper methods |
| cns/restserver/internalapi_test.go | Expanded SyncHostNCVersion tests to cover IMDS fallback and error cases |
| cns/imds/client.go | Introduced GetNCVersionsFromIMDS and associated types for network metadata |
| cns/imds/client_test.go | Added tests for GetNCVersionsFromIMDS, including success and failure cases |
| cns/fakes/imdsclientfake.go | Mock implementation for GetNCVersionsFromIMDS |
Comments suppressed due to low confidence (1)
cns/restserver/internalapi.go:25
- The code uses strings (e.g., strings.ToLower, strings.Contains) and fmt.Errorf but the import block does not include
"strings"or"fmt". Add these imports to avoid compile errors.
"github.com/pkg/errors" 563b8f4 to 2d45308 Compare 81cac82 to 6d116a2 Compare ce26e18 to 2d920ca Compare 48e6cfd to dfdf5e5 Compare dfdf5e5 to 2927cca Compare | /azp run Azure Container Networking PR |
| Azure Pipelines successfully started running 1 pipeline(s). |
) * Make nma api calls and imds call to update delegated NC goal state update values add client test scenarios . . . . . * Address comments * fix lint error * address code review comments * update imds version and * Check imds version to confirm if imds changes are available * fix formatting errors * . * Update naming convenction * remove nmagent api check
* Make nma api calls and imds call to update delegated NC goal state update values add client test scenarios . . . . . * Address comments * fix lint error * address code review comments * update imds version and * Check imds version to confirm if imds changes are available * fix formatting errors * . * Update naming convenction * remove nmagent api check
Reason for Change:
Swiftv2 network containers (NCs) are published to a different path in pubsub compared to Swiftv1 NCs, and the NMA getNCDetails API only returns programmed Swiftv1 NC details. For Swiftv2, the programmed NC goal state is exposed through IMDS, so we need to query IMDS to retrieve those NC details. In this code review, I am adding a call to NMA SupportedAPIs to verify that the build includes the required features. If the expected API is present, I proceed to call the IMDS NC details API and combine that information with the Swiftv1 NMA API results to ensure any outdated NCs are updated with the latest details from IMDS or NMA. If the latest build is not detected, the IMDS NCVersions API call is skipped, and only the NMA NCVersions API is used for the query swiftv1 NC goal state
Issue Fixed:
Requirements:
Notes: