Skip to content

Commit d9a6a8b

Browse files
authored
add end-to-end test that new blueprint on a fresh system is a noop (#7323)
1 parent 667832c commit d9a6a8b

File tree

6 files changed

+174
-1
lines changed

6 files changed

+174
-1
lines changed

Cargo.lock

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

end-to-end-tests/Cargo.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ bytes.workspace = true
1515
chrono.workspace = true
1616
http.workspace = true
1717
futures.workspace = true
18+
internal-dns-resolver.workspace = true
19+
internal-dns-types.workspace = true
20+
nexus-client.workspace = true
1821
omicron-sled-agent.workspace = true
1922
omicron-test-utils.workspace = true
2023
oxide-client.workspace = true
@@ -25,6 +28,9 @@ russh-keys = "0.45.0"
2528
serde.workspace = true
2629
serde_json.workspace = true
2730
sled-agent-types.workspace = true
31+
slog.workspace = true
32+
slog-error-chain.workspace = true
33+
thiserror.workspace = true
2834
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
2935
toml.workspace = true
3036
hickory-resolver.workspace = true

end-to-end-tests/src/helpers/ctx.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl Context {
7373
}
7474
}
7575

76-
fn rss_config() -> Result<RackInitializeRequest> {
76+
pub fn rss_config() -> Result<RackInitializeRequest> {
7777
let path = "/opt/oxide/sled-agent/pkg/config-rss.toml";
7878
let content =
7979
std::fs::read_to_string(&path).unwrap_or(RSS_CONFIG_STR.to_string());

end-to-end-tests/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ pub mod helpers;
22

33
mod instance_launch;
44
mod no_spoof;
5+
mod noop_blueprint;
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
//! Test that generating a new blueprint on a freshly-installed system will not
2+
//! change anything.
3+
4+
#![cfg(test)]
5+
6+
use internal_dns_resolver::Resolver;
7+
use internal_dns_types::names::ServiceName;
8+
use nexus_client::Client as NexusClient;
9+
use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError};
10+
use omicron_test_utils::dev::test_setup_log;
11+
use slog::{debug, info};
12+
use slog_error_chain::InlineErrorChain;
13+
use std::time::Duration;
14+
use thiserror::Error;
15+
16+
/// Test that generating a new blueprint on a freshly-installed system will not
17+
/// change anything.
18+
///
19+
/// If this test fails, there's probably a bug somewhere. Maybe the initial
20+
/// system blueprint is incorrect or incomplete or maybe the planner is doing
21+
/// the wrong thing after initial setup.
22+
#[tokio::test]
23+
async fn new_blueprint_noop() {
24+
// In order to check anything with blueprints, we need to reach the Nexus
25+
// internal API. This in turn requires finding the internal DNS servers.
26+
let rss_config =
27+
crate::helpers::ctx::rss_config().expect("loading RSS config");
28+
let rack_subnet = &rss_config.rack_network_config.rack_subnet;
29+
println!("rack subnet: {}", rack_subnet);
30+
let logctx = test_setup_log("new_blueprint_noop");
31+
let resolver =
32+
Resolver::new_from_ip(logctx.log.clone(), rack_subnet.addr())
33+
.expect("creating internal DNS resolver");
34+
35+
// Wait up to 5 minutes to get a working Nexus client.
36+
let nexus_client = wait_for_condition(
37+
|| async {
38+
match make_nexus_client(&resolver, &logctx.log).await {
39+
Ok(nexus_client) => Ok(nexus_client),
40+
Err(e) => {
41+
debug!(
42+
&logctx.log,
43+
"obtaining a working Nexus client failed";
44+
InlineErrorChain::new(&e),
45+
);
46+
Err(CondCheckError::<()>::NotYet)
47+
}
48+
}
49+
},
50+
&Duration::from_millis(500),
51+
&Duration::from_secs(300),
52+
)
53+
.await
54+
.expect("timed out waiting to obtain a working Nexus client");
55+
println!("Nexus is running and has a target blueprint");
56+
57+
// Now generate a new blueprint.
58+
let new_blueprint = nexus_client
59+
.blueprint_regenerate()
60+
.await
61+
.expect("failed to generate new blueprint")
62+
.into_inner();
63+
println!("new blueprint generated: {}", new_blueprint.id);
64+
let parent_blueprint_id = new_blueprint
65+
.parent_blueprint_id
66+
.expect("generated blueprint always has a parent");
67+
println!("parent blueprint id: {}", parent_blueprint_id);
68+
69+
// Fetch its parent.
70+
let parent_blueprint = nexus_client
71+
.blueprint_view(&parent_blueprint_id)
72+
.await
73+
.expect("failed to fetch parent blueprint")
74+
.into_inner();
75+
76+
let diff = new_blueprint.diff_since_blueprint(&parent_blueprint);
77+
println!("new blueprint: {}", new_blueprint.id);
78+
println!("differences:");
79+
println!("{}", diff.display());
80+
81+
if diff.has_changes() {
82+
panic!(
83+
"unexpected changes between initial blueprint and \
84+
newly-generated one (see above)"
85+
);
86+
}
87+
88+
logctx.cleanup_successful();
89+
}
90+
91+
/// Error returned by [`make_nexus_client()`].
92+
#[derive(Debug, Error)]
93+
enum MakeNexusError {
94+
#[error("looking up Nexus IP in internal DNS")]
95+
Resolve(#[from] internal_dns_resolver::ResolveError),
96+
#[error("making request to Nexus")]
97+
Request(#[from] nexus_client::Error<nexus_client::types::Error>),
98+
}
99+
100+
/// Make one attempt to look up the IP of Nexus in internal DNS and make an HTTP
101+
/// request to its internal API to fetch its current target blueprint.
102+
///
103+
/// If this succeeds, Nexus is ready for the rest of this test to proceed.
104+
///
105+
/// Returns a client for this Nexus.
106+
async fn make_nexus_client(
107+
resolver: &Resolver,
108+
log: &slog::Logger,
109+
) -> Result<NexusClient, MakeNexusError> {
110+
debug!(log, "doing DNS lookup for Nexus");
111+
let nexus_ip = resolver.lookup_socket_v6(ServiceName::Nexus).await?;
112+
let url = format!("http://{}", nexus_ip);
113+
debug!(log, "found Nexus IP"; "nexus_ip" => %nexus_ip, "url" => &url);
114+
115+
let client = NexusClient::new(&url, log.clone());
116+
117+
// Once this call succeeds, Nexus is ready for us to proceed.
118+
let blueprint_response = client.blueprint_target_view().await?.into_inner();
119+
info!(log, "found target blueprint (Nexus is ready)";
120+
"target_blueprint" => ?blueprint_response
121+
);
122+
123+
Ok(client)
124+
}

nexus/types/src/deployment/blueprint_diff.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use super::{
1414
zone_sort_key, Blueprint, ClickhouseClusterConfig,
1515
CockroachDbPreserveDowngrade, DiffBeforeClickhouseClusterConfig,
1616
};
17+
use diffus::Diffable;
1718
use nexus_sled_agent_shared::inventory::ZoneKind;
1819
use omicron_common::api::external::Generation;
1920
use omicron_common::disk::DiskIdentity;
@@ -946,6 +947,41 @@ impl BlueprintDiff {
946947
pub fn display(&self) -> BlueprintDiffDisplay<'_> {
947948
BlueprintDiffDisplay::new(self)
948949
}
950+
951+
/// Returns whether the diff reflects any changes or if the blueprints are
952+
/// equivalent.
953+
pub fn has_changes(&self) -> bool {
954+
// Any changes to physical disks, datasets, or zones would be reflected
955+
// in `self.sleds_modified`, `self.sleds_added`, or
956+
// `self.sleds_removed`.
957+
if !self.sleds_modified.is_empty()
958+
|| !self.sleds_added.is_empty()
959+
|| !self.sleds_removed.is_empty()
960+
{
961+
return true;
962+
}
963+
964+
// The clickhouse cluster config has changed if:
965+
// - there was one before and now there isn't
966+
// - there wasn't one before and now there is
967+
// - there's one both before and after and their generation has changed
968+
match (
969+
&self.before_clickhouse_cluster_config,
970+
&self.after_clickhouse_cluster_config,
971+
) {
972+
(DiffBeforeClickhouseClusterConfig::Blueprint(None), None) => false,
973+
(DiffBeforeClickhouseClusterConfig::Blueprint(None), Some(_)) => {
974+
true
975+
}
976+
(DiffBeforeClickhouseClusterConfig::Blueprint(Some(_)), None) => {
977+
true
978+
}
979+
(
980+
DiffBeforeClickhouseClusterConfig::Blueprint(Some(before)),
981+
Some(after),
982+
) => before.diff(&after).is_change(),
983+
}
984+
}
949985
}
950986

951987
/// A printable representation of `ClickhouseClusterConfig` diff tables where

0 commit comments

Comments
 (0)