Skip to content

Conversation

bimmlerd
Copy link
Member

When setting logging up in such a way that logrus would attempt to serialise entries to JSON, it could fail with errors such as:

Failed to obtain reader, failed to marshal fields to JSON, json: unsupported type: map[types.Identity]*types.Node

Replace the broken Info log with logging just the number of deleted nodes and prepare a serializable slice of strings for the debug log with more detail.

Fixes: b855b25 (node/manager: synthesize node deletion events)

@bimmlerd bimmlerd added the release-note/misc This PR makes changes that have no direct user impact. label Nov 11, 2024
@bimmlerd bimmlerd marked this pull request as ready for review November 11, 2024 16:17
@bimmlerd bimmlerd requested a review from a team as a code owner November 11, 2024 16:17
@bimmlerd bimmlerd requested a review from joamaki November 11, 2024 16:17
@bimmlerd
Copy link
Member Author

/test

When setting logging up in such a way that logrus would attempt to
serialise entries to JSON, it could fail with errors such as:

    Failed to obtain reader, failed to marshal fields to JSON, json: unsupported type: map[types.Identity]*types.Node

Replace the broken Info log with logging just the number of deleted
nodes and prepare a serializable slice of strings for the debug log with
more detail.

Fixes: b855b25 (node/manager: synthesize node deletion events)

Reported-by: Simon Dickhoven <sdickhoven@everquote.com>
Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
@bimmlerd bimmlerd force-pushed the pr/bimmlerd/fix-nodemanager-logging-json branch from 6500f09 to ac60c80 Compare November 12, 2024 08:28
@bimmlerd
Copy link
Member Author

bimmlerd commented Nov 12, 2024

/test (rebased due to #35889 linter fix getting merged)

@aanm aanm enabled auto-merge November 12, 2024 08:58
@aanm aanm added this pull request to the merge queue Nov 12, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 12, 2024
Merged via the queue into cilium:main with commit 0cbe208 Nov 12, 2024
64 checks passed
@bimmlerd bimmlerd deleted the pr/bimmlerd/fix-nodemanager-logging-json branch February 20, 2025 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants