test(admin): cover rebalance status dispatch#187
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a non-Windows CLI integration test to ensure the direct rc admin rebalance status command path dispatches to the correct admin endpoint and returns the expected JSON payload, preventing regressions that parser/client unit tests might not catch.
Changes:
- Introduces a loopback admin test server that captures the HTTP method/path and returns a stubbed JSON response.
- Adds an end-to-end test executing the built
rcbinary with--json admin rebalance statusand asserting both output JSON and request routing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+17
to
+104
| fn rc_binary() -> PathBuf { | ||
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_rc") { | ||
| return PathBuf::from(path); | ||
| } | ||
|
|
||
| let workspace_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) | ||
| .parent() | ||
| .expect("cli crate has parent directory") | ||
| .parent() | ||
| .expect("workspace root exists") | ||
| .to_path_buf(); | ||
|
|
||
| let debug_binary = workspace_root.join("target/debug/rc"); | ||
| if debug_binary.exists() { | ||
| return debug_binary; | ||
| } | ||
|
|
||
| workspace_root.join("target/release/rc") | ||
| } | ||
|
|
||
| fn read_admin_request(stream: &mut TcpStream) -> CapturedAdminRequest { | ||
| stream | ||
| .set_read_timeout(Some(Duration::from_secs(5))) | ||
| .expect("set read timeout"); | ||
|
|
||
| let mut request = Vec::new(); | ||
| let mut buffer = [0_u8; 8192]; | ||
| loop { | ||
| let bytes_read = stream.read(&mut buffer).expect("read admin request"); | ||
| if bytes_read == 0 { | ||
| break; | ||
| } | ||
| request.extend_from_slice(&buffer[..bytes_read]); | ||
| if request.windows(4).any(|window| window == b"\r\n\r\n") { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| let request = String::from_utf8(request).expect("admin request should be UTF-8"); | ||
| let request_line = request.lines().next().expect("request line"); | ||
| let mut parts = request_line.split_whitespace(); | ||
| let method = parts.next().expect("request method").to_string(); | ||
| let target = parts.next().expect("request target").to_string(); | ||
|
|
||
| CapturedAdminRequest { method, target } | ||
| } | ||
|
|
||
| fn start_admin_test_server( | ||
| response_body: &'static str, | ||
| ) -> (String, Receiver<CapturedAdminRequest>, JoinHandle<()>) { | ||
| let listener = TcpListener::bind("127.0.0.1:0").expect("bind admin test server"); | ||
| listener | ||
| .set_nonblocking(true) | ||
| .expect("set admin test server nonblocking"); | ||
| let endpoint = format!("http://{}", listener.local_addr().expect("server address")); | ||
| let (sender, receiver) = mpsc::channel(); | ||
|
|
||
| let handle = thread::spawn(move || { | ||
| let deadline = Instant::now() + Duration::from_secs(10); | ||
| let (mut stream, _) = loop { | ||
| match listener.accept() { | ||
| Ok(accepted) => break accepted, | ||
| Err(e) if e.kind() == ErrorKind::WouldBlock && Instant::now() < deadline => { | ||
| thread::sleep(Duration::from_millis(10)); | ||
| } | ||
| Err(e) => panic!("accept admin request: {e}"), | ||
| } | ||
| }; | ||
| let request = read_admin_request(&mut stream); | ||
| sender.send(request).expect("send captured request"); | ||
|
|
||
| let response = format!( | ||
| "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", | ||
| response_body.len(), | ||
| response_body | ||
| ); | ||
| stream | ||
| .write_all(response.as_bytes()) | ||
| .expect("write admin response"); | ||
| }); | ||
|
|
||
| (endpoint, receiver, handle) | ||
| } | ||
|
|
||
| fn rc_host_alias(endpoint: &str) -> String { | ||
| let (_, endpoint_authority) = endpoint.split_once("://").expect("endpoint has scheme"); | ||
| format!("http://ACCESS_KEY:SECRET_KEY@{endpoint_authority}") | ||
| } |
Comment on lines
+74
to
+84
| let handle = thread::spawn(move || { | ||
| let deadline = Instant::now() + Duration::from_secs(10); | ||
| let (mut stream, _) = loop { | ||
| match listener.accept() { | ||
| Ok(accepted) => break accepted, | ||
| Err(e) if e.kind() == ErrorKind::WouldBlock && Instant::now() < deadline => { | ||
| thread::sleep(Duration::from_millis(10)); | ||
| } | ||
| Err(e) => panic!("accept admin request: {e}"), | ||
| } | ||
| }; |
Comment on lines
+88
to
+92
| let response = format!( | ||
| "HTTP/1.1 200 OK\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", | ||
| response_body.len(), | ||
| response_body | ||
| ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue(s)
None.
Problem Background and User Impact
Recent admin rebalance work added
rc admin rebalance statusas a direct CLI command. Existing coverage verifies command parsing and the lower-level S3 admin route, while the expansion wrapper path is already covered separately. The directAdminCommands::Rebalanceexecution path still lacked an end-to-end command test.Root Cause Summary
No integration test executed the built
rcbinary foradmin rebalance status, so a future regression could misroute or remove that command branch while parser and lower-level client tests still passed.Solution Overview
This PR adds one focused non-Windows integration test. It runs
rc --json admin rebalance statusagainst a loopback admin test server, asserts the returned JSON status payload, and verifies the CLI sends aGETrequest to/rustfs/admin/v3/rebalance/status.Test Status
cargo test -p rustfs-cli --test admin_rebalance rebalance_status_dispatches_to_rebalance_status_json -- --nocapturemake pre-commit