feat(cli): add CRN VM control commands#106
Conversation
Adds `aleph crn {start,stop,reboot,erase,logs}` for direct
communication with Compute Resource Nodes. Log output is sanitized
to strip ANSI escape sequences and control characters from QEMU
serial console output.
foxpatch-aleph
left a comment
There was a problem hiding this comment.
The PR adds useful CRN VM control commands with good test coverage for the log sanitization logic. However, there's a string formatting bug in handle_operation that produces incorrect output ("rebootped" instead of "rebooted"), and the function doesn't properly propagate API errors to the user - it always reports success even when the underlying operation may have failed. These issues should be fixed before merging.
crates/aleph-cli/src/commands/crn.rs (line 83): String formatting bug: {operation}ped produces incorrect output for "reboot" → "rebootped" (should be "rebooted"). Consider using a match expression or proper past tense mapping:
let past_tense = match operation {
"stop" => "stopped",
"reboot" => "rebooted",
"erase" => "erased",
_ => unreachable!(),
};
eprintln!("Instance {} {past_tense} on {}", args.vm_id, args.crn_url);crates/aleph-cli/src/commands/crn.rs (line 66): The handle_operation function doesn't capture or report the response from the SDK. If perform_operation returns an error, the ? operator propagates it but the success message on line 83 is printed before any error handling. Consider restructuring to only print success after the operation completes, or use match to handle the Result explicitly.
crates/aleph-cli/src/commands/crn.rs (line 129): Minor: In JSON log output, the message is not sanitized. If the consumer expects clean output, consider applying sanitize_log here too, or document that JSON output contains raw ANSI sequences.
foxpatch-aleph
left a comment
There was a problem hiding this comment.
This PR adds well-implemented CRN VM control commands with proper error handling, authentication, and comprehensive tests. The sanitize_log function correctly handles ANSI escape sequences from QEMU serial console output. The code follows existing patterns in the codebase, has clear documentation, and includes 6 test cases covering edge cases for log sanitization. The dependency on futures-util is correctly added to both workspace and crate level.
crates/aleph-cli/src/commands/crn.rs (line 105): The let chains syntax (if let Some(next) = chars.next() && next == '[') requires Rust 1.84+. This is fine for the project, but note that bare escapes consume the following character (e.g., "a\x1bb" → "a"). This behavior is intentional and tested, though it could drop valid characters in edge cases. For QEMU serial console output this should be acceptable.
crates/aleph-cli/src/commands/crn.rs (line 89): Consider using println! instead of eprintln! for the success message. Status messages like "Instance X stopped" are informational rather than errors, and stdout is more appropriate for programmatic parsing when not using --json.
Adds
aleph crn {start,stop,reboot,erase,logs}for directcommunication with Compute Resource Nodes. Log output is sanitized
to strip ANSI escape sequences and control characters from QEMU
serial console output.