PR feedback

Author: jgallagher

dev-tools/reconfigurator-cli/src/main.rs

@@ -783,13 +783,14 @@ fn cmd_blueprint_edit(
 
     let mut new_blueprint = builder.build();
 
-    // We want to preserve the CockroachDB cluster settings from the parent
-    // blueprint.
+    // Normally `builder.build()` would construct the cockroach fingerprint
+    // based on what we read from CRDB and put into the planning input, but
+    // since we don't have a CRDB we had to make something up for our planning
+    // input's CRDB fingerprint. In the absense of a better alternative, we'll
+    // just copy our parent's CRDB fingerprint and carry it forward.
     new_blueprint
         .cockroachdb_fingerprint
         .clone_from(&blueprint.cockroachdb_fingerprint);
-    new_blueprint.cockroachdb_setting_preserve_downgrade =
-        blueprint.cockroachdb_setting_preserve_downgrade;
 
     let rv = format!(
         "blueprint {} created from blueprint {}: {}",

nexus/reconfigurator/planning/src/blueprint_builder/builder.rs

@@ -969,9 +969,6 @@ impl<'a> BlueprintBuilder<'a> {
         let sled_zones = self.zones.change_sled_zones(sled_id);
 
         // Find the internal NTP zone and expunge it.
-        //
-        // TODO-cleanup Is there ever a case where we might want to do some kind
-        // of graceful shutdown of an internal NTP zone? Seems unlikely...
         let mut internal_ntp_zone_id_iter = sled_zones
             .iter_zones(BlueprintZoneFilter::ShouldBeRunning)
             .filter_map(|config| {