Address comments

oximeter/collector/src/agent.rs

@@ -396,15 +396,16 @@ impl OximeterAgent {
         let db_address = if let Some(address) = db_config.address {
             address
         } else if replicated {
-            SocketAddr::new(
-                resolver.lookup_ip(ServiceName::ClickhouseServer).await?,
-                CLICKHOUSE_PORT,
-            )
+            let mut address = resolver
+                .lookup_socket_v6(ServiceName::ClickhouseServer)
+                .await?;
+            address.set_port(CLICKHOUSE_PORT);
+            SocketAddr::V6(address)
         } else {
-            SocketAddr::new(
-                resolver.lookup_ip(ServiceName::Clickhouse).await?,
-                CLICKHOUSE_PORT,
-            )
+            let mut address =
+                resolver.lookup_socket_v6(ServiceName::Clickhouse).await?;
+            address.set_port(CLICKHOUSE_PORT);
+            SocketAddr::V6(address)
         };
 
         // Determine the version of the database.

oximeter/collector/src/bin/oximeter.rs

@@ -51,9 +51,9 @@ enum Args {
         address: SocketAddrV6,
 
         // TODO (https://github.com/oxidecomputer/omicron/issues/4148): This flag
-        // should be removed once single node functionality is removed.
+        // should be removed if single node functionality is removed.
         /// Is `oximeter` connecting to a replicated ClickHouse cluster
-        #[clap(short, long, num_args = 0, action)]
+        #[clap(short, long)]
         replicated: bool,
     },
 
@@ -105,12 +105,6 @@ enum Args {
         /// The log-level.
         #[arg(long, default_value_t = Level::Info, value_parser = parse_log_level)]
         log_level: Level,
-
-        // TODO (https://github.com/oxidecomputer/omicron/issues/4148): This flag
-        // should be removed once single node functionality is removed.
-        /// Is `oximeter` connecting to a replicated ClickHouse cluster
-        #[clap(short, long, num_args = 0, action)]
-        replicated: bool,
     },
 
     /// Print the fake Nexus's standalone API.
@@ -142,20 +136,13 @@ async fn do_run() -> Result<(), CmdError> {
                 .context("Failed to create oximeter")
                 .map_err(CmdError::Failure)
         }
-        Args::Standalone {
-            id,
-            address,
-            nexus,
-            clickhouse,
-            log_level,
-            replicated,
-        } => {
+        Args::Standalone { id, address, nexus, clickhouse, log_level } => {
             // Start the standalone Nexus server, for registration of both the
             // collector and producers.
             let nexus_server = StandaloneNexus::new(nexus.into(), log_level)
                 .context("Failed to create nexus")
                 .map_err(CmdError::Failure)?;
-            let args = OximeterArguments { id, address, replicated };
+            let args = OximeterArguments { id, address, replicated: false };
             Oximeter::new_standalone(
                 nexus_server.log(),
                 &args,

package-manifest.toml

@@ -148,6 +148,7 @@ output.type = "zone"
 output.intermediate_only = true
 
 [package.clickhouse]
+# This service runs a single-node ClickHouse server.
 service_name = "clickhouse"
 only_for_targets.image = "standard"
 source.type = "composite"
@@ -176,6 +177,9 @@ output.intermediate_only = true
 setup_hint = "Run `cargo xtask download clickhouse` to download the necessary binaries"
 
 [package.clickhouse_server]
+# This service runs a server for a replicated ClickHouse cluster.
+# It is complimentary to the clickhouse_keeper service.
+# One cannot be run without the other.
 service_name = "clickhouse_server"
 only_for_targets.image = "standard"
 source.type = "composite"
@@ -203,6 +207,9 @@ output.intermediate_only = true
 setup_hint = "Run `cargo xtask download clickhouse` to download the necessary binaries"
 
 [package.clickhouse_keeper]
+# This service runs a keeper for a replicated ClickHouse cluster.
+# It is complimentary to the clickhouse_server service.
+# One cannot be run without the other.
 service_name = "clickhouse_keeper"
 only_for_targets.image = "standard"
 source.type = "composite"

package/src/bin/omicron-package.rs

@@ -277,9 +277,7 @@ async fn do_target(
                 machine.clone(),
                 switch.clone(),
                 rack_topology.clone(),
-                clickhouse_topology
-                    .clone()
-                    .expect("clickhouse topology should never be NONE"),
+                clickhouse_topology.clone(),
             )?;
 
             let path = get_single_target(&target_dir, name).await?;

package/src/lib.rs

@@ -82,7 +82,7 @@ pub enum TargetCommand {
         ///
         /// Replicated cluster configuration is an experimental feature to be
         /// used only for testing.
-        clickhouse_topology: Option<crate::target::ClickhouseTopology>,
+        clickhouse_topology: crate::target::ClickhouseTopology,
     },
     /// List all existing targets
     List,

package/src/target.rs

@@ -62,7 +62,7 @@ pub enum RackTopology {
     SingleSled,
 }
 
-/// Topology of the sleds within the rack.
+/// Topology of the ClickHouse installation within the rack.
 #[derive(Clone, Debug, strum::EnumString, strum::Display, ValueEnum)]
 #[strum(serialize_all = "kebab-case")]
 #[clap(rename_all = "kebab-case")]

sled-agent/src/rack_setup/plan/service.rs

@@ -649,8 +649,8 @@ impl Plan {
             };
             let id = OmicronZoneUuid::new_v4();
             let ip = sled.addr_alloc.next().expect("Not enough addrs");
-            // TODO: Should this be a different port in case we want to have single node
-            // and replicated running side by side for some weird reason?
+            // TODO: This may need to be a different port if/when to have single node
+            // and replicated running side by side as per stage 1 of RFD 468.
             let port = omicron_common::address::CLICKHOUSE_PORT;
             let address = SocketAddrV6::new(ip, port, 0, 0);
             dns_builder

sled-agent/src/services.rs

@@ -1628,7 +1628,7 @@ impl ServiceManager {
                 };
 
                 let listen_addr = *underlay_address;
-                let listen_port = &CLICKHOUSE_PORT.to_string();
+                let listen_port = CLICKHOUSE_PORT.to_string();
 
                 let nw_setup_service = Self::zone_network_setup_install(
                     Some(&info.underlay_address),