Skip to content

Commit

Permalink
Blueprint execution test: mock sled-agents should ignore unrelated re…
Browse files Browse the repository at this point in the history
…quests (#6590)

Fixes #6568
  • Loading branch information
jgallagher authored Sep 17, 2024
1 parent 8f7b5bd commit d0d953f
Showing 1 changed file with 40 additions and 20 deletions.
60 changes: 40 additions & 20 deletions nexus/src/app/background/tasks/blueprint_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl BackgroundTask for BlueprintExecutor {
mod test {
use super::BlueprintExecutor;
use crate::app::background::{Activator, BackgroundTask};
use httptest::matchers::{all_of, request};
use httptest::matchers::{not, request};
use httptest::responders::status_code;
use httptest::Expectation;
use nexus_db_model::{
Expand Down Expand Up @@ -431,17 +431,38 @@ mod test {

blueprint_tx.send(Some(Arc::new(blueprint.clone()))).unwrap();

// Make sure that requests get made to the sled agent. This is not a
// careful check of exactly what gets sent. For that, see the tests in
// nexus-reconfigurator-execution.
// The only sled-agent endpoint we care about in this test is `PUT
// /omicron-zones`. Add a closure to avoid repeating it multiple times
// below. We don't do a careful check of the _contents_ of what's being
// sent; for that, see the tests in nexus-reconfigurator-execution.
let match_put_omicron_zones =
|| request::method_path("PUT", "/omicron-zones");

// Helper for our mock sled-agent http servers to blanket ignore and
// return 200 OK for anything _except_ `PUT /omciron-zones`, which is
// the endpoint we care about in this test.
//
// Other Nexus background tasks created by our test context may notice
// the sled-agent records we created and query them (e.g., for
// inventory, vpc routing info, ...). We don't want those to cause
// spurious test failures, so just tell our sled-agents to accept any
// number of them.
let mock_server_ignore_spurious_http_requests =
|s: &mut httptest::Server| {
s.expect(
Expectation::matching(not(match_put_omicron_zones()))
.times(..)
.respond_with(status_code(200)),
);
};

// Make sure that requests get made to the sled agent.
for s in [&mut s1, &mut s2] {
s.expect(
Expectation::matching(all_of![request::method_path(
"PUT",
"/omicron-zones"
),])
.respond_with(status_code(204)),
Expectation::matching(match_put_omicron_zones())
.respond_with(status_code(204)),
);
mock_server_ignore_spurious_http_requests(s);
}

// Activate the task to trigger zone configuration on the sled-agents
Expand All @@ -464,7 +485,10 @@ mod test {
s2.verify_and_clear();

// Now, disable the target and make sure that we _don't_ invoke the sled
// agent. It's enough to just not set expectations.
// agent. It's enough to just not set expectations on
// match_put_omicron_zones().
mock_server_ignore_spurious_http_requests(&mut s1);
mock_server_ignore_spurious_http_requests(&mut s2);
blueprint.1.internal_dns_version =
blueprint.1.internal_dns_version.next();
blueprint.0.enabled = false;
Expand All @@ -486,19 +510,15 @@ mod test {
blueprint.0.enabled = true;
blueprint_tx.send(Some(Arc::new(blueprint))).unwrap();
s1.expect(
Expectation::matching(request::method_path(
"PUT",
"/omicron-zones",
))
.respond_with(status_code(204)),
Expectation::matching(match_put_omicron_zones())
.respond_with(status_code(204)),
);
s2.expect(
Expectation::matching(request::method_path(
"PUT",
"/omicron-zones",
))
.respond_with(status_code(500)),
Expectation::matching(match_put_omicron_zones())
.respond_with(status_code(500)),
);
mock_server_ignore_spurious_http_requests(&mut s1);
mock_server_ignore_spurious_http_requests(&mut s2);

#[derive(Deserialize)]
struct ErrorResult {
Expand Down

0 comments on commit d0d953f

Please sign in to comment.