diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index e916974332..a76980f27d 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -1610,11 +1610,19 @@ impl super::Nexus { if let Some(max_bytes) = params.max_bytes { request = request.max_bytes(max_bytes); } - if let Some(from_start) = params.from_start { - request = request.from_start(from_start); - } - if let Some(most_recent) = params.most_recent { - request = request.most_recent(most_recent); + match (params.from_start, params.most_recent) { + (Some(from_start), None) => { + request = request.from_start(from_start); + } + (None, Some(most_recent)) => { + request = request.most_recent(most_recent); + } + _ => { + return Err(Error::invalid_request( + "Exactly one of 'from_start' \ + or 'most_recent' must be specified.", + )); + } } let data = request .send() @@ -1714,29 +1722,36 @@ impl super::Nexus { match vmm.runtime.state { DbVmmState::Running | DbVmmState::Rebooting - | DbVmmState::Migrating => { - Ok((vmm.clone(), SocketAddr::new(vmm.propolis_ip.ip(), vmm.propolis_port.into()))) - } + | DbVmmState::Migrating => Ok(( + vmm.clone(), + SocketAddr::new( + vmm.propolis_ip.ip(), + vmm.propolis_port.into(), + ), + )), DbVmmState::Starting | DbVmmState::Stopping | DbVmmState::Stopped | DbVmmState::Failed - | DbVmmState::Creating => { + | DbVmmState::Creating + | DbVmmState::Destroyed + | DbVmmState::SagaUnwound => { Err(Error::invalid_request(format!( - "cannot connect to serial console of instance in state \"{}\"", + "cannot administer instance in state \"{}\"", state.effective_state(), ))) } - DbVmmState::Destroyed | DbVmmState::SagaUnwound => Err(Error::invalid_request( - "cannot connect to serial console of instance in state \"Stopped\"", - )), + DbVmmState::Destroyed | DbVmmState::SagaUnwound => { + Err(Error::invalid_request( + "cannot administer instance in state \"Stopped\"", + )) + } } } else { Err(Error::invalid_request(format!( - "instance is {} and has no active serial console \ - server", + "instance is {} and cannot be administered", state.effective_state(), ))) } diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 163869896f..a19d6ef27a 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -5430,6 +5430,8 @@ async fn test_instance_serial(cptestctx: &ControlPlaneTestContext) { // Make sure we get a 404 if we try to access the serial console before creation. let instance_serial_url = get_instance_url(format!("{}/serial-console", instance_name).as_str()); + let instance_serial_stream_url = + get_instance_url(format!("{}/serial-console", instance_name).as_str()); let error: HttpErrorResponseBody = NexusRequest::expect_failure( client, StatusCode::NOT_FOUND, @@ -5517,6 +5519,47 @@ async fn test_instance_serial(cptestctx: &ControlPlaneTestContext) { } assert_eq!(&actual[..expected.len()], expected); + // Now try querying the serial output history endpoint with bad parameters. + // Check that the errors are indicative here. + let builder = + RequestBuilder::new(client, http::Method::GET, &instance_serial_url) + .expect_status(Some(http::StatusCode::BAD_REQUEST)); + + let error = NexusRequest::new(builder) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(); + + // We provided neither from_start nor most_recent... + assert_eq!( + error.message, + "Exactly one of 'from_start' or 'most_recent' must be specified.", + ); + + let builder = RequestBuilder::new( + client, + http::Method::GET, + &format!("{}&from_start=0&most_recent=0", instance_serial_url), + ) + .expect_status(Some(http::StatusCode::BAD_REQUEST)); + + let error = NexusRequest::new(builder) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(); + + // We provided both from_start and most_recent... + assert_eq!( + error.message, + "Exactly one of 'from_start' or 'most_recent' must be specified.", + ); + // Request a halt and verify both the immediate state and the finished state. let instance = instance_next; let instance_next = @@ -5527,6 +5570,53 @@ async fn test_instance_serial(cptestctx: &ControlPlaneTestContext) { > instance.runtime.time_run_state_updated ); + // As the instance is now stopping, we can't connect to its serial console + // anymore. We also can't get its cached data; the (simulated) Propolis is + // going away. + + let builder = RequestBuilder::new( + client, + http::Method::GET, + &instance_serial_stream_url, + ) + .expect_status(Some(http::StatusCode::BAD_REQUEST)); + + let error = NexusRequest::new(builder) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(); + + assert_eq!( + error.message, + "cannot administer instance in state \"stopping\"", + ); + + // Have to pass some offset for the cached data request otherwise we'll + // error out of Nexus early on while validating parameters, before + // discovering the serial console is unreachable. + let builder = RequestBuilder::new( + client, + http::Method::GET, + &format!("{}&from_start=0", instance_serial_url), + ) + .expect_status(Some(http::StatusCode::BAD_REQUEST)); + + let error = NexusRequest::new(builder) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(); + + assert_eq!( + error.message, + "cannot administer instance in state \"stopping\"", + ); + let instance = instance_next; instance_simulate(nexus, &instance_id).await; let instance_next =