Skip to content

Commit 55ba0c6

Browse files
authored
[2/n] [reconfigurator-cli] move integration tests with Nexus into another crate (#7893)
Decouple the reconfigurator-cli tests from this more expensive Nexus integration test. The integration test is quite valuable to keep around, but in my experience it fails much less often than the blueprint snapshot-based tests, and having to build all of Nexus to run snapshot tests is bothersome. Depends on: * #7892
1 parent acdcbd9 commit 55ba0c6

File tree

10 files changed

+380
-259
lines changed

10 files changed

+380
-259
lines changed

Cargo.lock

+30-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ members = [
8888
"nexus/mgs-updates",
8989
"nexus/networking",
9090
"nexus/reconfigurator/blippy",
91+
"nexus/reconfigurator/cli-integration-tests",
9192
"nexus/reconfigurator/execution",
9293
"nexus/reconfigurator/planning",
9394
"nexus/reconfigurator/preparation",
@@ -230,6 +231,7 @@ default-members = [
230231
"nexus/mgs-updates",
231232
"nexus/networking",
232233
"nexus/reconfigurator/blippy",
234+
"nexus/reconfigurator/cli-integration-tests",
233235
"nexus/reconfigurator/execution",
234236
"nexus/reconfigurator/planning",
235237
"nexus/reconfigurator/preparation",
@@ -593,6 +595,7 @@ range-requests = { path = "range-requests" }
593595
ratatui = "0.29.0"
594596
rayon = "1.10"
595597
rcgen = "0.12.1"
598+
reconfigurator-cli = { path = "dev-tools/reconfigurator-cli" }
596599
reedline = "0.38.0"
597600
ref-cast = "1.0"
598601
regex = "1.11.1"

dev-tools/reconfigurator-cli/Cargo.toml

+3-6
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,11 @@ uuid.workspace = true
4444
omicron-workspace-hack.workspace = true
4545

4646
[dev-dependencies]
47+
# To keep test iterations fast, please avoid a dependency on omicron-nexus or
48+
# its database code here. Instead, put those tests inside
49+
# nexus/reconfigurator/cli-integration-tests.
4750
camino-tempfile.workspace = true
4851
expectorate.workspace = true
49-
nexus-client.workspace = true
50-
nexus-db-queries.workspace = true
51-
nexus-reconfigurator-preparation.workspace = true
52-
nexus-test-utils.workspace = true
53-
nexus-test-utils-macros.workspace = true
54-
omicron-nexus.workspace = true
5552
omicron-test-utils.workspace = true
5653
serde.workspace = true
5754
subprocess.workspace = true

dev-tools/reconfigurator-cli/tests/test_basic.rs

-247
Original file line numberDiff line numberDiff line change
@@ -2,38 +2,16 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

5-
use anyhow::Context;
65
use camino::Utf8Path;
76
use expectorate::assert_contents;
8-
use nexus_db_queries::authn;
9-
use nexus_db_queries::authz;
10-
use nexus_db_queries::context::OpContext;
11-
use nexus_test_utils::SLED_AGENT_UUID;
12-
use nexus_test_utils::resource_helpers::DiskTest;
13-
use nexus_test_utils_macros::nexus_test;
14-
use nexus_types::deployment::Blueprint;
15-
use nexus_types::deployment::SledFilter;
16-
use nexus_types::deployment::UnstableReconfiguratorState;
17-
use omicron_common::api::external::Error;
18-
use omicron_test_utils::dev::poll::CondCheckError;
19-
use omicron_test_utils::dev::poll::wait_for_condition;
207
use omicron_test_utils::dev::test_cmds::EXIT_SUCCESS;
218
use omicron_test_utils::dev::test_cmds::Redactor;
229
use omicron_test_utils::dev::test_cmds::assert_exit_code;
2310
use omicron_test_utils::dev::test_cmds::path_to_executable;
2411
use omicron_test_utils::dev::test_cmds::run_command;
25-
use omicron_uuid_kinds::GenericUuid;
26-
use omicron_uuid_kinds::SledUuid;
27-
use slog::debug;
28-
use std::io::BufReader;
29-
use std::io::BufWriter;
3012
use std::path::PathBuf;
31-
use std::sync::Arc;
32-
use std::time::Duration;
3313
use subprocess::Exec;
3414
use subprocess::ExitStatus;
35-
use swrite::SWrite;
36-
use swrite::swriteln;
3715

3816
fn path_to_cli() -> PathBuf {
3917
path_to_executable(env!("CARGO_BIN_EXE_reconfigurator-cli"))
@@ -125,228 +103,3 @@ fn test_set_zone_images() {
125103
assert_contents("tests/output/cmd-set-zone-images-stdout", &stdout_text);
126104
assert_contents("tests/output/cmd-set-zone-images-stderr", &stderr_text);
127105
}
128-
129-
type ControlPlaneTestContext =
130-
nexus_test_utils::ControlPlaneTestContext<omicron_nexus::Server>;
131-
132-
// Tests a round trip of blueprint editing: start with the blueprint that's
133-
// present in a running system, fetch it with the rest of the reconfigurator
134-
// state, load it into reconfigurator-cli, edit it, save that to a file, then
135-
// import it back.
136-
#[nexus_test]
137-
async fn test_blueprint_edit(cptestctx: &ControlPlaneTestContext) {
138-
let nexus = &cptestctx.server.server_context().nexus;
139-
let datastore = nexus.datastore();
140-
141-
let log = &cptestctx.logctx.log;
142-
let opctx = OpContext::for_background(
143-
log.clone(),
144-
Arc::new(authz::Authz::new(log)),
145-
authn::Context::internal_api(),
146-
datastore.clone(),
147-
);
148-
149-
// Setup
150-
//
151-
// For all the disks our blueprint says each sled should have, actually
152-
// insert them into the DB. This is working around nexus-test-utils's setup
153-
// being a little scattershot and spread out; tests are supposed to do their
154-
// own disk setup.
155-
let (_blueprint_target, initial_blueprint) = datastore
156-
.blueprint_target_get_current_full(&opctx)
157-
.await
158-
.expect("failed to read current target blueprint");
159-
let mut disk_test = DiskTest::new(&cptestctx).await;
160-
disk_test.add_blueprint_disks(&initial_blueprint).await;
161-
162-
let tmpdir = camino_tempfile::tempdir().expect("failed to create tmpdir");
163-
// Save the path and prevent the temporary directory from being cleaned up
164-
// automatically. We want to be preserve the contents if this test fails.
165-
let tmpdir_path = tmpdir.into_path();
166-
let saved_state1_path = tmpdir_path.join("reconfigurator-state1.json");
167-
let saved_state2_path = tmpdir_path.join("reconfigurator-state2.json");
168-
let script1_path = tmpdir_path.join("cmds1");
169-
let script2_path = tmpdir_path.join("cmds2");
170-
let new_blueprint_path = tmpdir_path.join("new_blueprint.json");
171-
172-
println!("temporary directory: {}", tmpdir_path);
173-
174-
// Wait until Nexus has successfully completed an inventory collection.
175-
// We don't need it directly but we want it to be present in the saved
176-
// reconfigurator state.
177-
let collection = wait_for_condition(
178-
|| async {
179-
let result =
180-
datastore.inventory_get_latest_collection(&opctx).await;
181-
let log_result = match &result {
182-
Ok(Some(_)) => Ok("found"),
183-
Ok(None) => Ok("not found"),
184-
Err(error) => Err(error),
185-
};
186-
debug!(
187-
log,
188-
"attempt to fetch latest inventory collection";
189-
"result" => ?log_result,
190-
);
191-
192-
match result {
193-
Ok(None) => Err(CondCheckError::NotYet),
194-
Ok(Some(c)) => Ok(c),
195-
Err(Error::ServiceUnavailable { .. }) => {
196-
Err(CondCheckError::NotYet)
197-
}
198-
Err(error) => Err(CondCheckError::Failed(error)),
199-
}
200-
},
201-
&Duration::from_millis(50),
202-
&Duration::from_secs(30),
203-
)
204-
.await
205-
.expect("took too long to find first inventory collection");
206-
207-
// Assemble state that we can load into reconfigurator-cli.
208-
let state1 = nexus_reconfigurator_preparation::reconfigurator_state_load(
209-
&opctx, datastore,
210-
)
211-
.await
212-
.expect("failed to assemble reconfigurator state");
213-
214-
// Smoke check the initial state.
215-
let sled_id: SledUuid = SLED_AGENT_UUID.parse().unwrap();
216-
state1
217-
.planning_input
218-
.sled_lookup(SledFilter::Commissioned, sled_id)
219-
.expect("state1 has initial sled");
220-
assert!(!state1.planning_input.service_ip_pool_ranges().is_empty());
221-
assert!(!state1.silo_names.is_empty());
222-
assert!(!state1.external_dns_zone_names.is_empty());
223-
// We waited for the first inventory collection already.
224-
assert!(state1.collections.iter().any(|c| c.id == collection.id));
225-
assert!(!state1.collections.is_empty());
226-
// Test suite setup establishes the initial blueprint.
227-
assert!(!state1.blueprints.is_empty());
228-
// Setup requires that internal and external DNS be configured so we should
229-
// have at least the current DNS generations here.
230-
assert!(!state1.internal_dns.is_empty());
231-
assert!(!state1.external_dns.is_empty());
232-
233-
// unwrap: we checked above that this list was non-empty.
234-
let blueprint = state1.blueprints.first().unwrap();
235-
236-
// Write a reconfigurator-cli script to load the file, edit the
237-
// blueprint, and save the entire state to a new file.
238-
let mut s = String::new();
239-
swriteln!(s, "load {} {}", saved_state1_path, collection.id);
240-
swriteln!(s, "blueprint-edit {} add-nexus {}", blueprint.id, sled_id);
241-
swriteln!(s, "save {}", saved_state2_path);
242-
std::fs::write(&script1_path, &s)
243-
.with_context(|| format!("write {}", &script1_path))
244-
.unwrap();
245-
246-
// Run this reconfigurator-cli invocation.
247-
write_json(&saved_state1_path, &state1).unwrap();
248-
let exec = Exec::cmd(path_to_cli()).arg(&script1_path);
249-
let (exit_status, _, stderr_text) = run_command(exec);
250-
assert_exit_code(exit_status, EXIT_SUCCESS, &stderr_text);
251-
252-
// Load the new file and find the new blueprint name.
253-
let state2: UnstableReconfiguratorState =
254-
read_json(&saved_state2_path).unwrap();
255-
assert_eq!(state2.blueprints.len(), state1.blueprints.len() + 1);
256-
let new_blueprint = state2.blueprints.into_iter().rev().next().unwrap();
257-
assert_ne!(new_blueprint.id, blueprint.id);
258-
259-
// While we're at it, smoke check the new blueprint.
260-
assert_eq!(new_blueprint.parent_blueprint_id, Some(blueprint.id));
261-
assert_eq!(new_blueprint.creator, "reconfigurator-cli");
262-
263-
// Now run reconfigurator-cli again just to save the new blueprint. This is
264-
// a little unfortunate but it's hard to avoid if we want to test that
265-
// blueprint-save works.
266-
let mut s = String::new();
267-
swriteln!(s, "load {} {}", saved_state2_path, collection.id);
268-
swriteln!(s, "blueprint-save {} {}", new_blueprint.id, new_blueprint_path);
269-
std::fs::write(&script2_path, &s)
270-
.with_context(|| format!("write {}", &script2_path))
271-
.unwrap();
272-
let exec = Exec::cmd(path_to_cli()).arg(&script2_path);
273-
let (exit_status, _, stderr_text) = run_command(exec);
274-
assert_exit_code(exit_status, EXIT_SUCCESS, &stderr_text);
275-
276-
// Load the blueprint we just wrote.
277-
let new_blueprint2: Blueprint = read_json(&new_blueprint_path).unwrap();
278-
assert_eq!(new_blueprint, new_blueprint2);
279-
280-
// Import the new blueprint.
281-
let nexus_internal_url =
282-
format!("http://{}/", cptestctx.internal_client.bind_address);
283-
let nexus_client =
284-
nexus_client::Client::new(&nexus_internal_url, log.clone());
285-
nexus_client
286-
.blueprint_import(&new_blueprint)
287-
.await
288-
.expect("failed to import new blueprint");
289-
290-
let found_blueprint = nexus_client
291-
.blueprint_view(new_blueprint.id.as_untyped_uuid())
292-
.await
293-
.expect("failed to find imported blueprint in Nexus")
294-
.into_inner();
295-
assert_eq!(found_blueprint, new_blueprint2);
296-
297-
// Set the blueprint as the (disabled) target.
298-
nexus_client
299-
.blueprint_target_set(&nexus_client::types::BlueprintTargetSet {
300-
target_id: new_blueprint.id,
301-
enabled: false,
302-
})
303-
.await
304-
.context("setting target blueprint")
305-
.unwrap();
306-
307-
// Read that back.
308-
let target = nexus_client
309-
.blueprint_target_view()
310-
.await
311-
.context("fetching target blueprint")
312-
.unwrap();
313-
assert_eq!(target.target_id, new_blueprint.id);
314-
315-
// Now clean up the temporary directory.
316-
for path in [
317-
saved_state1_path,
318-
saved_state2_path,
319-
script1_path,
320-
script2_path,
321-
new_blueprint_path,
322-
] {
323-
std::fs::remove_file(&path)
324-
.with_context(|| format!("remove {}", path))
325-
.unwrap();
326-
}
327-
328-
std::fs::remove_dir(&tmpdir_path)
329-
.with_context(|| format!("remove {}", tmpdir_path))
330-
.unwrap();
331-
}
332-
333-
fn read_json<T: for<'a> serde::Deserialize<'a>>(
334-
path: &Utf8Path,
335-
) -> Result<T, anyhow::Error> {
336-
let file = std::fs::File::open(path)
337-
.with_context(|| format!("open {:?}", path))?;
338-
let bufread = BufReader::new(file);
339-
serde_json::from_reader(bufread).with_context(|| format!("read {:?}", path))
340-
}
341-
342-
fn write_json<T: serde::Serialize>(
343-
path: &Utf8Path,
344-
obj: &T,
345-
) -> Result<(), anyhow::Error> {
346-
let file = std::fs::File::create(path)
347-
.with_context(|| format!("create {:?}", path))?;
348-
let bufwrite = BufWriter::new(file);
349-
serde_json::to_writer_pretty(bufwrite, obj)
350-
.with_context(|| format!("write {:?}", path))?;
351-
Ok(())
352-
}

0 commit comments

Comments
 (0)