Skip to content

fix(spur-cli): show the controller reason when a node update fails - #992

Open
pre wants to merge 1 commit into
ROCm:mainfrom
silogen:fix/cli-node-update-reason
Open

pre wants to merge 1 commit into
ROCm:mainfrom
silogen:fix/cli-node-update-reason

Conversation

@pre

@pre pre commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Related:

Fixes #990

What this fixes

When spurctld refuses a node update, scontrol update nodename=... printed only node update failed. The CLI dropped the message from the gRPC status, so the administrator had to read the controller log to find the cause.

Approach

update_node now adds the status message to its error, so the CLI prints node update failed: <reason>. It uses Status::message() and not the Display output of tonic::Status, because that output also has the status code and quoting.

Testing

A new unit test makes the mock controller refuse an update. It checks that the error from update_node contains the controller reason. cargo fmt --check, cargo clippy --workspace --exclude spur-ffi --all-targets --locked and cargo test --locked passed with no warnings: 4581 passed, 0 failed, 35 ignored.

I ran these commands on a single-node cluster in two cycles:

Command main dcce7c7 this branch
scontrol update nodename=nosuch state=drain reason=t error: nosuch: node update failed error: nosuch: node update failed: node nosuch not found
scontrol update nodename=nosuch,<node> state=bogus unknown node state 'bogus' ... same

scontrol update printed only "node update failed" and dropped the
message of the gRPC status. An administrator could not see why the
controller refused the update.
@codecov-commenter

codecov-commenter commented Oct 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #992   +/-   ##
=======================================
  Coverage   83.01%   83.01%           
=======================================
  Files         216      216           
  Lines      121503   121515   +12     
=======================================
+ Hits       100854   100868   +14     
+ Misses      20649    20647    -2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

scontrol update node does not show why the controller refused the update

2 participants