-
Notifications
You must be signed in to change notification settings - Fork 260
Fix logging response from NMAgent in syncHostNCVersion function #3747
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: thatmattlong <[email protected]>
cns/restserver/internalapi.go
Outdated
|
|
||
| // Track NCs missing from NMAgent response and outdated NCs for better error reporting | ||
| missingNCs := make(map[string]string) // ncID -> expected version | ||
| outdatedNMaNCs := make(map[string]string) // ncID -> "expected:actual" version info |
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.
you should define a new struct for this instead of using a string. You should name the struct ncVersionInfo and it should have two fields, an expected field and an actual field
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.
Implemented as requested. Created ncVersionInfo struct with expected and actual fields to replace the formatted string approach. This provides better type safety and clearer separation of version information. b35be04
Co-authored-by: thatmattlong <[email protected]>
|
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. |
Problem
The
syncHostNCVersionfunction logged a generic error message when some NCs couldn't be updated:This message was not useful because it didn't distinguish between:
Solution
Enhanced the error logging to separately track and report missing vs outdated NCs with detailed version information:
Changes
Core Implementation (
cns/restserver/internalapi.go)missingNCs: Maps NC ID → expected versionoutdatedNMaNCs: Maps NC ID → "expected:X,actual:Y"Test Coverage (
cns/restserver/internalapi_test.go)TestSyncHostNCVersionErrorMessagescovering both scenariosExample Output
Before:
After:
This provides operators with actionable information to distinguish between missing NCs (potential NMAgent issues) and outdated NCs (version synchronization issues), along with specific version details for effective troubleshooting.
Fixes #3746.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.