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

fix(health): Correctly implement spec for overall health #897

Merged
merged 2 commits into from Jan 24, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 43 additions & 15 deletions tonic-health/src/server.rs
Expand Up @@ -40,7 +40,10 @@ pub struct HealthReporter {

impl HealthReporter {
fn new() -> Self {
let statuses = Arc::new(RwLock::new(HashMap::new()));
// According to the gRPC Health Check specification, the empty service "" corresponds to the overall server health
let server_status = ("".to_string(), watch::channel(ServingStatus::Serving));

let statuses = Arc::new(RwLock::new(HashMap::from([server_status])));

HealthReporter { statuses }
}
Expand Down Expand Up @@ -166,9 +169,7 @@ mod tests {
use crate::proto::HealthCheckRequest;
use crate::server::{HealthReporter, HealthService};
use crate::ServingStatus;
use std::collections::HashMap;
use std::sync::Arc;
use tokio::sync::{watch, RwLock};
use tokio::sync::watch;
use tokio_stream::StreamExt;
use tonic::{Code, Request, Status};

Expand All @@ -183,23 +184,35 @@ mod tests {
}

async fn make_test_service() -> (HealthReporter, HealthService) {
let state = Arc::new(RwLock::new(HashMap::new()));
state.write().await.insert(
"TestService".to_string(),
watch::channel(ServingStatus::Unknown),
);
(
HealthReporter {
statuses: state.clone(),
},
HealthService::new(state.clone()),
)
let health_reporter = HealthReporter::new();

// insert test value
{
let mut statuses = health_reporter.statuses.write().await;
statuses.insert(
"TestService".to_string(),
watch::channel(ServingStatus::Unknown),
);
}

let health_service = HealthService::new(health_reporter.statuses.clone());
(health_reporter, health_service)
}

#[tokio::test]
async fn test_service_check() {
let (mut reporter, service) = make_test_service().await;

// Overall server health
let resp = service
.check(Request::new(HealthCheckRequest {
service: "".to_string(),
}))
.await;
assert!(resp.is_ok());
let resp = resp.unwrap().into_inner();
assert_serving_status(resp.status, ServingStatus::Serving);

// Unregistered service
let resp = service
.check(Request::new(HealthCheckRequest {
Expand Down Expand Up @@ -237,6 +250,21 @@ mod tests {
async fn test_service_watch() {
let (mut reporter, service) = make_test_service().await;

// Overall server health
let resp = service
.watch(Request::new(HealthCheckRequest {
service: "".to_string(),
}))
.await;
assert!(resp.is_ok());
let mut resp = resp.unwrap().into_inner();
let item = resp
.next()
.await
.expect("streamed response is Some")
.expect("response is ok");
assert_serving_status(item.status, ServingStatus::Serving);

// Unregistered service
let resp = service
.watch(Request::new(HealthCheckRequest {
Expand Down