Skip to content

Support registering multiple health checks. #59

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

Merged
merged 3 commits into from
May 14, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/register_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ async fn main() {
Port: Some(42424),
Namespace: None,
}),
Check: None,
Checks: Vec::new(),
SkipNodeUpdate: None,
};
consul.register_entity(&payload).await.unwrap();
Expand Down
5 changes: 2 additions & 3 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,8 @@ pub struct RegisterEntityPayload {
/// Optional service to register.
#[serde(skip_serializing_if = "Option::is_none")]
pub Service: Option<RegisterEntityService>,
/// Optional check to register
#[serde(skip_serializing_if = "Option::is_none")]
pub Check: Option<RegisterEntityCheck>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey.
In the official API docs, Check is not marked as deprecated or as legacy.
I think someone reading the docs will naturally use Check, so it's better to keep it and document that Checks is preferred instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @x0rw, we discussed this.
The official documentation states that
Multiple checks can be provided by replacing Check with Checks and sending an array of Check objects.
When we examined the server code for consul however, it accepts both as well and appends Check to Checks.

This behavior is confusing/surprising. We're opting to expose just one field instead, which can be empty to represent the original None state.

Copy link
Contributor Author

@giladwolff giladwolff May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/// Checks to register.
pub Checks: Vec<RegisterEntityCheck>,
/// Whether to skip updating the nodes information in the registration.
#[serde(skip_serializing_if = "Option::is_none")]
pub SkipNodeUpdate: Option<bool>,
Expand Down
32 changes: 32 additions & 0 deletions tests/test_runner.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use rs_consul::*;
use std::collections::HashMap;

#[path = "utils/test_setup.rs"]
mod test_setup;
Expand Down Expand Up @@ -304,4 +305,35 @@ mod tests {
assert_eq!(string_value4, &value.unwrap());
assert_ne!(mod_idx3, mod_idx4);
}

#[tokio::test(flavor = "multi_thread")]
async fn test_register_with_health_checks() {
let consul = get_client();

let new_service_name = "test-service-99".to_string();
let checks = [
RegisterEntityCheck {
Node: None,
CheckID: None,
Name: "Service Check".to_string(),
Notes: None,
Status: Some("passing".to_string()),
ServiceID: Some(service_id(&new_service_name)),
Definition: HashMap::new(),
},
RegisterEntityCheck {
Node: Some("local".to_string()),
CheckID: None,
Name: "Node check".to_string(),
Notes: None,
Status: Some("passing".to_string()),
ServiceID: None,
Definition: HashMap::new(),
},
]
.to_vec();
register_entity_with_checks(&consul, &new_service_name, "local", checks).await;

assert!(is_registered(&consul, &new_service_name).await);
}
}
46 changes: 45 additions & 1 deletion tests/utils/test_setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub(crate) async fn register_entity(consul: &Consul, service_name: &String, node
Port: Some(42424),
Namespace: None,
}),
Check: None,
Checks: Vec::new(),
SkipNodeUpdate: None,
};
consul
Expand All @@ -40,6 +40,50 @@ pub(crate) async fn register_entity(consul: &Consul, service_name: &String, node
.expect("expected register_entity request to succeed");
}

pub(crate) async fn register_entity_with_checks(
consul: &Consul,
service_name: &String,
node_id: &str,
checks: Vec<RegisterEntityCheck>,
) {
let ResponseMeta {
response: service_names_before_register,
..
} = consul
.get_all_registered_service_names(None)
.await
.expect("expected get_registered_service_names request to succeed");
assert!(!service_names_before_register.contains(service_name));

let payload = RegisterEntityPayload {
ID: None,
Node: node_id.to_string(),
Address: "127.0.0.1".to_string(),
Datacenter: None,
TaggedAddresses: Default::default(),
NodeMeta: Default::default(),
Service: Some(RegisterEntityService {
ID: Some(service_id(service_name)),
Service: service_name.clone(),
Tags: vec![],
TaggedAddresses: Default::default(),
Meta: Default::default(),
Port: Some(42424),
Namespace: None,
}),
Checks: checks,
SkipNodeUpdate: None,
};
consul
.register_entity(&payload)
.await
.expect("expected register_entity request to succeed");
}

pub(crate) fn service_id(service_name: &str) -> String {
format!("{service_name}-ID")
}

pub(crate) async fn is_registered(consul: &Consul, service_name: &String) -> bool {
let ResponseMeta {
response: service_names_after_register,
Expand Down
Loading