Skip to content
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

Misleading node name in the "Updated lending limits" log message #253

Closed
tardieu opened this issue Oct 8, 2024 · 3 comments
Closed

Misleading node name in the "Updated lending limits" log message #253

tardieu opened this issue Oct 8, 2024 · 3 comments
Assignees

Comments

@tardieu
Copy link
Member

tardieu commented Oct 8, 2024

The "Updated lending limits" log message is emitted upon successful etcd update of the slack queue. If this update fails due to an update conflict, we repeat the update when reconciling the next queued node event which is often about another node. As a result, the node name embedded in the "updated lending limits" message is misleading suggesting the limit was updated because of the specific node whereas there is no such relationship.

We need to log the cause of lending limit changes in a way that identifies the responsible node. The "Updated node health information" log message only accounts for autopilot label changes.

@tardieu
Copy link
Member Author

tardieu commented Oct 8, 2024

We should also include before and after lending limit values (or deltas) in these messages as opposed to only the final state.

@dgrove-oss dgrove-oss self-assigned this Oct 14, 2024
@dgrove-oss
Copy link
Collaborator

dgrove-oss commented Oct 16, 2024

The primary logging issues about misleading nodes and no logging for cordoning nodes were fixed as part of #255.

@dgrove-oss
Copy link
Collaborator

#256 logs the delta on changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants