From cc74a95c36a1f64c171260abb44d456ec7e8eab5 Mon Sep 17 00:00:00 2001 From: Laura Abbott Date: Fri, 30 Jan 2026 22:42:10 +0000 Subject: [PATCH] make `set_target_release_and_old_repo` the default behavior Previously, we had `set_target_release` and `set_target_release_and_old_repo` for reconfigurator-cli because there were concerns about the behavior of the system if the old repo was set during the middle of an update. This has since been fixed and the behavior of `set_target_release_and_old_repo` is what we should use everywhere --- nexus/reconfigurator/planning/src/system.rs | 27 ------------------- .../tests/integration_tests/planner.rs | 10 +++---- 2 files changed, 5 insertions(+), 32 deletions(-) diff --git a/nexus/reconfigurator/planning/src/system.rs b/nexus/reconfigurator/planning/src/system.rs index af35339dede..61a88668399 100644 --- a/nexus/reconfigurator/planning/src/system.rs +++ b/nexus/reconfigurator/planning/src/system.rs @@ -869,33 +869,6 @@ impl SystemDescription { pub fn set_target_release( &mut self, description: TargetReleaseDescription, - ) -> &mut Self { - // Create a new TufRepoPolicy by bumping the generation. - let new_repo = TufRepoPolicy { - target_release_generation: self - .tuf_repo - .target_release_generation - .next(), - description, - }; - - let _old_repo = self.set_tuf_repo_inner(new_repo); - - // It's tempting to consider setting old_repo to the current tuf_repo, - // but that requires the invariant that old_repo is always the current - // target release and that an update isn't currently in progress. See - // https://github.com/oxidecomputer/omicron/issues/8056 for some - // discussion. - // - // We provide a method to set the old repo explicitly with these - // assumptions in mind: `set_target_release_and_old_repo`. - - self - } - - pub fn set_target_release_and_old_repo( - &mut self, - description: TargetReleaseDescription, ) -> &mut Self { let new_repo = TufRepoPolicy { target_release_generation: self diff --git a/nexus/reconfigurator/planning/tests/integration_tests/planner.rs b/nexus/reconfigurator/planning/tests/integration_tests/planner.rs index e3c4006a1b8..366e220c04f 100644 --- a/nexus/reconfigurator/planning/tests/integration_tests/planner.rs +++ b/nexus/reconfigurator/planning/tests/integration_tests/planner.rs @@ -3158,7 +3158,7 @@ fn test_update_crucible_pantry_before_nexus() { artifacts, }); sim.change_description("set new target release", |desc| { - desc.set_target_release_and_old_repo(description); + desc.set_target_release(description); Ok(()) }) .unwrap(); @@ -3530,7 +3530,7 @@ fn test_update_cockroach() { artifacts, }); sim.change_description("set new target release", |desc| { - desc.set_target_release_and_old_repo(description); + desc.set_target_release(description); Ok(()) }) .unwrap(); @@ -3900,7 +3900,7 @@ fn test_update_boundary_ntp() { artifacts, }); sim.change_description("set new target release", |desc| { - desc.set_target_release_and_old_repo(description); + desc.set_target_release(description); Ok(()) }) .unwrap(); @@ -4290,7 +4290,7 @@ fn test_update_internal_dns() { artifacts, }); sim.change_description("set new target release", |desc| { - desc.set_target_release_and_old_repo(description); + desc.set_target_release(description); Ok(()) }) .unwrap(); @@ -4539,7 +4539,7 @@ fn test_update_all_zones() { }); sim.change_description("set new target release", |desc| { - desc.set_target_release_and_old_repo(description); + desc.set_target_release(description); Ok(()) }) .unwrap();