From 97e3c59b3c36e765469a2c9be8cbd3f7e67aa8da Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 26 Jun 2025 05:41:12 +0000 Subject: [PATCH 1/2] clippy Created using spr 1.3.6-beta.1 --- dev-tools/reconfigurator-cli/src/lib.rs | 2 +- .../tests/input/cmds-mupdate-update-flow.txt | 2 ++ .../tests/output/cmds-mupdate-update-flow-stdout | 16 +++++++++++----- nexus/reconfigurator/planning/src/planner.rs | 13 +++++++------ 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/dev-tools/reconfigurator-cli/src/lib.rs b/dev-tools/reconfigurator-cli/src/lib.rs index 1026c77981..192fda91a1 100644 --- a/dev-tools/reconfigurator-cli/src/lib.rs +++ b/dev-tools/reconfigurator-cli/src/lib.rs @@ -1019,7 +1019,7 @@ fn cmd_sled_set_mupdate_override( let desc = match args.mupdate_override_id { MupdateOverrideUuidOpt::Set(id) => format!("set to {id}"), - MupdateOverrideUuidOpt::Unset => format!("unset"), + MupdateOverrideUuidOpt::Unset => "unset".to_owned(), }; sim.commit_and_bump( diff --git a/dev-tools/reconfigurator-cli/tests/input/cmds-mupdate-update-flow.txt b/dev-tools/reconfigurator-cli/tests/input/cmds-mupdate-update-flow.txt index 95ac475914..944e4ca386 100644 --- a/dev-tools/reconfigurator-cli/tests/input/cmds-mupdate-update-flow.txt +++ b/dev-tools/reconfigurator-cli/tests/input/cmds-mupdate-update-flow.txt @@ -32,3 +32,5 @@ blueprint-plan latest 61f451b3-2121-4ed6-91c7-a550054f6c21 # TODO: we do not yet reset the install dataset image back to # the desired artifact version -- we should do that in the future. blueprint-diff 58d5e830-0884-47d8-a7cd-b2b3751adeb4 latest + +# TODO: set target release diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout index 3151562f69..c6cd3bd68e 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout @@ -24,7 +24,7 @@ set sled 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c mupdate override: set to 6123eac1- generated inventory collection eb0796d5-ab8a-4f7b-a884-b4aeacb8ab51 from configured sleds > blueprint-plan latest eb0796d5-ab8a-4f7b-a884-b4aeacb8ab51 -INFO blueprint mupdate override updated to match inventory, sled_id: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c, new_bp_override: 6123eac1-ec5b-42ba-b73f-9845105a9971, prev_bp_override: None, zones: +INFO blueprint mupdate override updated to match inventory, phase: do_plan_mupdate_override, sled_id: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c, new_bp_override: 6123eac1-ec5b-42ba-b73f-9845105a9971, prev_bp_override: None, zones: - zone 353b3b65-20f7-48c3-88f7-495bd5d31545 (Clickhouse) left unchanged, image source: install dataset - zone 466a9f29-62bf-4e63-924a-b9efdb86afec (Nexus) updated from artifact: version 1.2.3 to install dataset - zone 62620961-fc4a-481e-968b-f5acbac0dc63 (InternalNtp) left unchanged, image source: install dataset @@ -33,8 +33,8 @@ INFO blueprint mupdate override updated to match inventory, sled_id: 2b8f0cb3-02 - zone ad6a3a03-8d0f-4504-99a4-cbf73d69b973 (CruciblePantry) left unchanged, image source: install dataset - zone bd354eef-d8a6-4165-9124-283fb5e46d77 (Crucible) left unchanged, image source: install dataset -INFO updating target release minimum generation based on new set-override actions, current_generation: 1, new_generation: 2 -INFO not ready to add or update new zones yet, reasons: sleds have remove mupdate override set in blueprint: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c; current target release generation (1) is lower than minimum required by blueprint (2) +INFO updating target release minimum generation based on new set-override actions, phase: do_plan_mupdate_override, current_generation: 1, new_generation: 2 +INFO not ready to add or update new zones yet, phase: do_plan_mupdate_override, reasons: sleds have remove mupdate override set in blueprint: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c; current target release generation (1) is lower than minimum required by blueprint (2) INFO will ensure cockroachdb setting, setting: cluster.preserve_downgrade_option, value: DoNotModify generated blueprint 58d5e830-0884-47d8-a7cd-b2b3751adeb4 based on parent blueprint 8da82a8e-bf97-4fbd-8ddd-9f6462732cf1 @@ -232,12 +232,16 @@ set sled 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c mupdate override: unset generated inventory collection 61f451b3-2121-4ed6-91c7-a550054f6c21 from configured sleds > blueprint-plan latest 61f451b3-2121-4ed6-91c7-a550054f6c21 -INFO inventory override no longer exists, blueprint override cleared, sled_id: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c, prev_bp_override: 6123eac1-ec5b-42ba-b73f-9845105a9971 -INFO not ready to add or update new zones yet, reasons: current target release generation (1) is lower than minimum required by blueprint (2) +INFO inventory override no longer exists, blueprint override cleared, phase: do_plan_mupdate_override, sled_id: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c, prev_bp_override: 6123eac1-ec5b-42ba-b73f-9845105a9971 +INFO not ready to add or update new zones yet, phase: do_plan_mupdate_override, reasons: current target release generation (1) is lower than minimum required by blueprint (2) INFO will ensure cockroachdb setting, setting: cluster.preserve_downgrade_option, value: DoNotModify generated blueprint af934083-59b5-4bf6-8966-6fb5292c29e1 based on parent blueprint 58d5e830-0884-47d8-a7cd-b2b3751adeb4 +> # Diff the blueprints. This diff should show the "remove mupdate +> # override" line going away. +> # TODO: we do not yet reset the install dataset image back to +> # the desired artifact version -- we should do that in the future. > blueprint-diff 58d5e830-0884-47d8-a7cd-b2b3751adeb4 latest from: blueprint 58d5e830-0884-47d8-a7cd-b2b3751adeb4 to: blueprint af934083-59b5-4bf6-8966-6fb5292c29e1 @@ -414,3 +418,5 @@ external DNS: + +> # TODO: set target release diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index 928cdd0591..b83f7a97d8 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -1138,11 +1138,12 @@ impl<'a> Planner<'a> { // For each sled, compare what's in the inventory to what's in the // blueprint. let mut actions_by_sled = BTreeMap::new(); + let log = self.log.new(o!("phase" => "do_plan_mupdate_override")); // We use the list of in-service sleds here -- we don't want to alter // expunged or decommissioned sleds. for sled_id in self.input.all_sled_ids(SledFilter::InService) { - let log = self.log.new(o!("sled_id" => sled_id.to_string())); + let log = log.new(o!("sled_id" => sled_id.to_string())); let Some(inv_sled) = self.inventory.sled_agents.get(&sled_id) else { warn!(log, "no inventory found for commissioned sled"); @@ -1212,7 +1213,7 @@ impl<'a> Planner<'a> { if current == new { // No change needed. info!( - self.log, + log, "would have updated target release minimum generation, but \ it was already set to the desired value, so no change was \ needed"; @@ -1221,7 +1222,7 @@ impl<'a> Planner<'a> { } else { if current < new { info!( - self.log, + log, "updating target release minimum generation based on \ new set-override actions"; "current_generation" => %current, @@ -1236,7 +1237,7 @@ impl<'a> Planner<'a> { // // In this case we warn but set the value. warn!( - self.log, + log, "target release minimum generation was set to current, \ but we are trying to set it to an older generation -- \ this is unexpected and may indicate a problem with the \ @@ -1301,7 +1302,7 @@ impl<'a> Planner<'a> { { reasons.push(format!( "current target release generation ({}) is lower than \ - minimum required by blueprint ({})", + minimum required by blueprint ({})", self.input.tuf_repo().target_release_generation, self.blueprint.target_release_minimum_generation(), )); @@ -1310,7 +1311,7 @@ impl<'a> Planner<'a> { if !reasons.is_empty() { let reasons = reasons.join("; "); info!( - self.log, + log, "not ready to add or update new zones yet"; "reasons" => reasons, ); From e506a8441a729a6aa3c69d02746c867dc31882b7 Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 10 Jul 2025 03:30:56 +0000 Subject: [PATCH 2/2] update comment Created using spr 1.3.6-beta.1 --- nexus/reconfigurator/planning/src/planner.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index c4e4244dd0..ede9fd694f 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -1565,15 +1565,15 @@ impl<'a> Planner<'a> { // don't want to add zones on *any* sled. // // This might seem overly conservative (why block zone additions on - // *any* sled currently recovering from a MUPdate?), but is probably - // correct for the medium term: we want to minimize the number of - // different versions of services running at any time. + // *all* sleds if *any* are currently recovering from a MUPdate?), + // but is probably correct for the medium term: we want to minimize + // the number of different versions of services running at any time. // // There's some potential to relax this in the future (e.g. by // matching up the zone manifest with the target release to compute // the number of versions running at a given time), but that's a - // non-trivial optimization that we should probably defer until we see - // its necessity. + // non-trivial optimization that we should probably defer until we + // see its necessity. // // What does "any sleds" mean in this context? We don't need to care // about decommissioned or expunged sleds, so we consider in-service