-
Notifications
You must be signed in to change notification settings - Fork 180
RUST-979 Non-unified spec tests for load balancer support behavior #473
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
Changes from all commits
7d887e2
68a985b
de57789
a4089e0
640e90c
9b097f2
4995848
2a5f866
0a3d228
b2d99b6
0adda62
007e461
78394b8
d4994d4
573e171
11b2fff
bbedec2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,8 @@ use strsim::jaro_winkler; | |
use typed_builder::TypedBuilder; | ||
use webpki_roots::TLS_SERVER_ROOTS; | ||
|
||
#[cfg(test)] | ||
use crate::srv::LookupHosts; | ||
use crate::{ | ||
bson::{doc, Bson, Document}, | ||
bson_util, | ||
|
@@ -543,20 +545,30 @@ pub struct ClientOptions { | |
#[serde(skip)] | ||
pub(crate) resolver_config: Option<ResolverConfig>, | ||
|
||
/// Used by tests to override MIN_HEARTBEAT_FREQUENCY. | ||
#[builder(default)] | ||
#[cfg(test)] | ||
pub(crate) heartbeat_freq_test: Option<Duration>, | ||
/// Whether or not the client is connecting to a MongoDB cluster through a load balancer. | ||
#[builder(default, setter(skip))] | ||
#[serde(rename = "loadbalanced")] | ||
pub(crate) load_balanced: Option<bool>, | ||
|
||
/// Allow use of the `load_balanced` option. | ||
// TODO RUST-653 Remove this when load balancer work is ready for release. | ||
/// Control test behavior of the client. | ||
#[cfg(test)] | ||
#[builder(default, setter(skip))] | ||
#[serde(skip)] | ||
pub(crate) allow_load_balanced: bool, | ||
#[derivative(PartialEq = "ignore")] | ||
pub(crate) test_options: Option<TestOptions>, | ||
} | ||
|
||
/// Whether or not the client is connecting to a MongoDB cluster through a load balancer. | ||
#[builder(default, setter(skip))] | ||
pub(crate) load_balanced: Option<bool>, | ||
#[cfg(test)] | ||
#[derive(Debug, Clone, Default)] | ||
pub(crate) struct TestOptions { | ||
/// Override MIN_HEARTBEAT_FREQUENCY. | ||
pub(crate) heartbeat_freq: Option<Duration>, | ||
|
||
/// Disable server and SRV-polling monitor threads. | ||
pub(crate) disable_monitoring_threads: bool, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously known as |
||
|
||
/// Mock response for `SrvPollingMonitor::lookup_hosts`. | ||
pub(crate) mock_lookup_hosts: Option<Result<LookupHosts>>, | ||
} | ||
|
||
fn default_hosts() -> Vec<ServerAddress> { | ||
|
@@ -629,6 +641,8 @@ impl Serialize for ClientOptions { | |
writeconcern: &'a Option<WriteConcern>, | ||
|
||
zlibcompressionlevel: &'a Option<i32>, | ||
|
||
loadbalanced: &'a Option<bool>, | ||
} | ||
|
||
let client_options = ClientOptionsHelper { | ||
|
@@ -652,6 +666,7 @@ impl Serialize for ClientOptions { | |
tls: &self.tls, | ||
writeconcern: &self.write_concern, | ||
zlibcompressionlevel: &self.zlib_compression, | ||
loadbalanced: &self.load_balanced, | ||
}; | ||
|
||
client_options.serialize(serializer) | ||
|
@@ -921,11 +936,10 @@ impl From<ClientOptionsParser> for ClientOptions { | |
original_uri: Some(parser.original_uri), | ||
resolver_config: None, | ||
server_api: None, | ||
#[cfg(test)] | ||
heartbeat_freq_test: None, | ||
allow_load_balanced: false, | ||
load_balanced: parser.load_balanced, | ||
sdam_event_handler: None, | ||
#[cfg(test)] | ||
test_options: None, | ||
} | ||
} | ||
} | ||
|
@@ -1132,7 +1146,9 @@ impl ClientOptions { | |
write_concern.validate()?; | ||
} | ||
|
||
if !self.allow_load_balanced && self.load_balanced.is_some() { | ||
// Disallow use of load-balanced configurations in non-test code. | ||
// TODO RUST-653 Remove this when load balancer work is ready for release. | ||
if !cfg!(test) && self.load_balanced.is_some() { | ||
return Err(ErrorKind::InvalidArgument { | ||
message: "loadBalanced is not supported".to_string(), | ||
} | ||
|
@@ -1200,6 +1216,11 @@ impl ClientOptions { | |
] | ||
); | ||
} | ||
|
||
#[cfg(test)] | ||
pub(crate) fn test_options_mut(&mut self) -> &mut TestOptions { | ||
self.test_options.get_or_insert_with(Default::default) | ||
} | ||
} | ||
|
||
/// Splits a string into a section before a given index and a section exclusively after the index. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ use crate::{ | |
error::Result, | ||
options::{ClientOptions, ServerAddress}, | ||
sdam::Topology, | ||
RUNTIME, | ||
}; | ||
|
||
fn localhost_test_build_10gen(port: u16) -> ServerAddress { | ||
|
@@ -26,22 +27,23 @@ lazy_static::lazy_static! { | |
async fn run_test(new_hosts: Result<Vec<ServerAddress>>, expected_hosts: HashSet<ServerAddress>) { | ||
let mut options = ClientOptions::new_srv(); | ||
options.hosts = DEFAULT_HOSTS.clone(); | ||
let topology = Topology::new_mocked(options); | ||
options.test_options_mut().disable_monitoring_threads = true; | ||
let topology = Topology::new(options).unwrap(); | ||
let mut monitor = SrvPollingMonitor::new(topology.downgrade()).unwrap(); | ||
|
||
monitor | ||
.update_hosts( | ||
new_hosts.map(|hosts| LookupHosts { | ||
hosts: hosts.into_iter().map(Result::Ok).collect(), | ||
min_ttl: Duration::from_secs(60), | ||
}), | ||
topology.clone(), | ||
) | ||
.update_hosts(new_hosts.and_then(make_lookup_hosts), topology.clone()) | ||
.await; | ||
|
||
assert_eq!(expected_hosts, topology.servers().await); | ||
} | ||
|
||
fn make_lookup_hosts(hosts: Vec<ServerAddress>) -> Result<LookupHosts> { | ||
Ok(LookupHosts { | ||
hosts: hosts.into_iter().map(Result::Ok).collect(), | ||
min_ttl: Duration::from_secs(60), | ||
}) | ||
} | ||
|
||
// If a new DNS record is returned, it should be reflected in the topology. | ||
#[cfg_attr(feature = "tokio-runtime", tokio::test)] | ||
#[cfg_attr(feature = "async-std-runtime", async_std::test)] | ||
|
@@ -102,3 +104,25 @@ async fn timeout_error() { | |
async fn no_results() { | ||
run_test(Ok(Vec::new()), DEFAULT_HOSTS.iter().cloned().collect()).await; | ||
} | ||
|
||
// SRV polling is not done for load-balanced clusters (as per spec at | ||
// https://github.com/mongodb/specifications/blob/master/source/polling-srv-records-for-mongos-discovery/tests/README.rst#test-that-srv-polling-is-not-done-for-load-balalanced-clusters). | ||
#[cfg_attr(feature = "tokio-runtime", tokio::test)] | ||
#[cfg_attr(feature = "async-std-runtime", async_std::test)] | ||
async fn load_balanced_no_srv_polling() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has a different form than the other tests because those directly fire server updates, skipping the rest of the srv-polling machinery, whereas this one needs to test that the dns polling doesn't happen at all. |
||
let hosts = vec![localhost_test_build_10gen(27017)]; | ||
let mut options = ClientOptions::new_srv(); | ||
let rescan_interval = options.original_srv_info.as_ref().cloned().unwrap().min_ttl; | ||
options.hosts = hosts.clone(); | ||
options.load_balanced = Some(true); | ||
options.test_options_mut().mock_lookup_hosts = Some(make_lookup_hosts(vec![ | ||
localhost_test_build_10gen(27017), | ||
localhost_test_build_10gen(27018), | ||
])); | ||
let topology = Topology::new(options).unwrap(); | ||
RUNTIME.delay_for(rescan_interval * 2).await; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unfortunately long, but also mandated by the spec :( |
||
assert_eq!( | ||
hosts.into_iter().collect::<HashSet<_>>(), | ||
topology.servers().await | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This particular gate key turned out to be more hassle than it was worth - it meant that attempting to parse options from test json would fail without jumping through a lot of hoops. The options validation now enforces that the
load_balanced
option is test-only, which seems a reasonable degree of protection.