Remove HTTP ports from the oximeter_db::Client (#7118)

- Remove the HTTP address argument from the client constructor
- Remove the HTTP resolver from the client constructor
- Remove the HTTP pool from the client
- Rename `native_{address,port}` to just `{address,port}` in
configuration and CLI flags
- Closes #7094
- NOTE: We're still populating the DNS records for the HTTP address, and
relying on the implicit addition of the native TCP address via
`DnsBuilder::service_zone_clickhouse()`. We're also specifying the HTTP
address in the SMF properties of the ClickHouse zone. We don't strictly
need these anymore, but it's a bit more complicated to remove them,
since some deployed systems rely on those records. That can be done in a
follow-up, in which we'll switch from specifying the HTTP to the native
addresses.

clickhouse-admin/src/http_entrypoints.rs

@@ -130,18 +130,13 @@ impl ClickhouseAdminSingleApi for ClickhouseAdminSingleImpl {
     ) -> Result<HttpResponseUpdatedNoContent, HttpError> {
         let log = &rqctx.log;
         let ctx = rqctx.context();
-        let http_address = ctx.clickhouse_cli().listen_address;
-        let native_address =
-            SocketAddrV6::new(*http_address.ip(), CLICKHOUSE_TCP_PORT, 0, 0);
-        let client = OximeterClient::new(
-            http_address.into(),
-            native_address.into(),
-            log,
-        );
+        let ip = ctx.clickhouse_cli().listen_address.ip();
+        let address = SocketAddrV6::new(*ip, CLICKHOUSE_TCP_PORT, 0, 0);
+        let client = OximeterClient::new(address.into(), log);
         debug!(
             log,
             "initializing single-node ClickHouse \
-             at {http_address} to version {OXIMETER_VERSION}"
+             at {address} to version {OXIMETER_VERSION}"
         );
 
         // Database initialization is idempotent, but not concurrency-safe.
@@ -154,7 +149,7 @@ impl ClickhouseAdminSingleApi for ClickhouseAdminSingleImpl {
             .map_err(|e| {
                 HttpError::for_internal_error(format!(
                     "can't initialize single-node ClickHouse \
-                     at {http_address} to version {OXIMETER_VERSION}: {e}",
+                     at {address} to version {OXIMETER_VERSION}: {e}",
                 ))
             })?;
 

dev-tools/ch-dev/src/main.rs

@@ -9,7 +9,7 @@ use clap::{Args, Parser, Subcommand};
 use dropshot::test_util::LogContext;
 use futures::StreamExt;
 use libc::SIGINT;
-use omicron_common::address::{CLICKHOUSE_HTTP_PORT, CLICKHOUSE_TCP_PORT};
+use omicron_common::address::CLICKHOUSE_TCP_PORT;
 use omicron_test_utils::dev::{self, clickhouse::ClickHousePorts};
 use signal_hook_tokio::Signals;
 
@@ -43,14 +43,11 @@ enum ChDevCmd {
 
 #[derive(Clone, Debug, Args)]
 struct ChRunArgs {
-    /// The HTTP port on which the server will listen
-    #[clap(short = 'H', long, default_value_t = CLICKHOUSE_HTTP_PORT, action)]
-    http_port: u16,
     /// The port on which the native protocol server will listen
     #[clap(short, long, default_value_t = CLICKHOUSE_TCP_PORT, action)]
-    native_port: u16,
+    port: u16,
     /// Starts a ClickHouse replicated cluster of 2 replicas and 3 keeper nodes
-    #[clap(long, conflicts_with_all = ["http_port", "native_port"], action)]
+    #[clap(long, conflicts_with_all = ["port"], action)]
     replicated: bool,
 }
 
@@ -65,24 +62,22 @@ impl ChRunArgs {
         if self.replicated {
             start_replicated_cluster(&logctx).await?;
         } else {
-            start_single_node(&logctx, self.http_port, self.native_port)
-                .await?;
+            start_single_node(&logctx, self.port).await?;
         }
         Ok(())
     }
 }
 
 async fn start_single_node(
     logctx: &LogContext,
-    http_port: u16,
     native_port: u16,
 ) -> Result<(), anyhow::Error> {
     // Start a stream listening for SIGINT
     let signals = Signals::new(&[SIGINT]).expect("failed to wait for SIGINT");
     let mut signal_stream = signals.fuse();
 
     // Start the database server process, possibly on a specific port
-    let ports = ClickHousePorts::new(http_port, native_port)?;
+    let ports = ClickHousePorts::new(0, native_port)?;
     let mut deployment =
         dev::clickhouse::ClickHouseDeployment::new_single_node_with_ports(
             logctx, ports,

dev-tools/clickhouse-cluster-dev/src/main.rs

@@ -37,13 +37,11 @@ async fn main() -> Result<()> {
     deployment.deploy().context("failed to deploy")?;
 
     let client1 = Client::new_with_request_timeout(
-        deployment.http_addr(1.into()),
         deployment.native_addr(1.into()),
         &logctx.log,
         request_timeout,
     );
     let client2 = Client::new_with_request_timeout(
-        deployment.http_addr(2.into()),
         deployment.native_addr(2.into()),
         &logctx.log,
         request_timeout,

dev-tools/omdb/src/bin/omdb/oxql.rs

@@ -31,15 +31,6 @@ pub struct OxqlArgs {
     )]
     clickhouse_url: Option<String>,
 
-    /// URL of the ClickHouse server to connect to for the native protcol.
-    #[arg(
-        long,
-        env = "OMDB_CLICKHOUSE_NATIVE_URL",
-        global = true,
-        help_heading = CONNECTION_OPTIONS_HEADING,
-    )]
-    clickhouse_native_url: Option<String>,
-
     /// Print summaries of each SQL query run against the database.
     #[clap(long = "summaries")]
     print_summaries: bool,
@@ -56,7 +47,6 @@ impl OxqlArgs {
         omdb: &Omdb,
         log: &Logger,
     ) -> anyhow::Result<()> {
-        let http_addr = self.resolve_http_addr(omdb, log).await?;
         let native_addr = self.resolve_native_addr(omdb, log).await?;
 
         let opts = ShellOptions {
@@ -65,8 +55,7 @@ impl OxqlArgs {
         };
 
         oxql::shell(
-            http_addr.ip(),
-            http_addr.port(),
+            native_addr.ip(),
             native_addr.port(),
             log.new(slog::o!("component" => "clickhouse-client")),
             opts,
@@ -79,27 +68,12 @@ impl OxqlArgs {
         &self,
         omdb: &Omdb,
         log: &Logger,
-    ) -> anyhow::Result<SocketAddr> {
-        self.resolve_addr(
-            omdb,
-            log,
-            self.clickhouse_native_url.as_deref(),
-            ServiceName::ClickhouseNative,
-        )
-        .await
-    }
-
-    /// Resolve the ClickHouse HTTP URL to a socket address.
-    async fn resolve_http_addr(
-        &self,
-        omdb: &Omdb,
-        log: &Logger,
     ) -> anyhow::Result<SocketAddr> {
         self.resolve_addr(
             omdb,
             log,
             self.clickhouse_url.as_deref(),
-            ServiceName::Clickhouse,
+            ServiceName::ClickhouseNative,
         )
         .await
     }

dev-tools/omdb/tests/usage_errors.out

@@ -790,16 +790,13 @@ Usage: omdb oxql [OPTIONS]
 Options:
       --log-level <LOG_LEVEL>  log level filter [env: LOG_LEVEL=] [default: warn]
       --summaries              Print summaries of each SQL query run against the database
-      --color <COLOR>          Color output [default: auto] [possible values: auto, always, never]
       --elapsed                Print the total elapsed query duration
+      --color <COLOR>          Color output [default: auto] [possible values: auto, always, never]
   -h, --help                   Print help
 
 Connection Options:
       --clickhouse-url <CLICKHOUSE_URL>
           URL of the ClickHouse server to connect to [env: OMDB_CLICKHOUSE_URL=]
-      --clickhouse-native-url <CLICKHOUSE_NATIVE_URL>
-          URL of the ClickHouse server to connect to for the native protcol [env:
-          OMDB_CLICKHOUSE_NATIVE_URL=]
       --dns-server <DNS_SERVER>
           [env: OMDB_DNS_SERVER=]
 
@@ -818,7 +815,7 @@ error: unexpected argument '--summarizes' found
 
   tip: a similar argument exists: '--summaries'
 
-Usage: omdb oxql <--clickhouse-url <CLICKHOUSE_URL>|--clickhouse-native-url <CLICKHOUSE_NATIVE_URL>|--summaries|--elapsed>
+Usage: omdb oxql <--clickhouse-url <CLICKHOUSE_URL>|--summaries|--elapsed>
 
 For more information, try '--help'.
 =============================================

nexus-config/src/nexus_config.rs

@@ -250,12 +250,9 @@ pub struct SchemaConfig {
 /// Optional configuration for the timeseries database.
 #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
 pub struct TimeseriesDbConfig {
-    /// The HTTP address of the ClickHouse server.
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub address: Option<SocketAddr>,
     /// The native TCP address of the ClickHouse server.
     #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub native_address: Option<SocketAddr>,
+    pub address: Option<SocketAddr>,
 }
 
 /// Configuration for the `Dendrite` dataplane daemon.
@@ -779,7 +776,7 @@ mod test {
     use super::*;
 
     use omicron_common::address::{
-        Ipv6Subnet, CLICKHOUSE_HTTP_PORT, CLICKHOUSE_TCP_PORT, RACK_PREFIX,
+        Ipv6Subnet, CLICKHOUSE_TCP_PORT, RACK_PREFIX,
     };
     use omicron_common::api::internal::shared::SwitchLocation;
 
@@ -894,8 +891,7 @@ mod test {
             path = "/nonexistent/path"
             if_exists = "fail"
             [timeseries_db]
-            address = "[::1]:8123"
-            native_address = "[::1]:9000"
+            address = "[::1]:9000"
             [updates]
             trusted_root = "/path/to/root.json"
             [tunables]
@@ -1016,18 +1012,10 @@ mod test {
                     timeseries_db: TimeseriesDbConfig {
                         address: Some(SocketAddr::V6(SocketAddrV6::new(
                             Ipv6Addr::LOCALHOST,
-                            CLICKHOUSE_HTTP_PORT,
+                            CLICKHOUSE_TCP_PORT,
                             0,
                             0,
                         ))),
-                        native_address: Some(SocketAddr::V6(
-                            SocketAddrV6::new(
-                                Ipv6Addr::LOCALHOST,
-                                CLICKHOUSE_TCP_PORT,
-                                0,
-                                0,
-                            )
-                        )),
                     },
                     updates: Some(UpdatesConfig {
                         trusted_root: Utf8PathBuf::from("/path/to/root.json"),
@@ -1180,7 +1168,7 @@ mod test {
             path = "/nonexistent/path"
             if_exists = "fail"
             [timeseries_db]
-            address = "[::1]:8123"
+            address = "[::1]:9000"
             [deployment]
             id = "28b90dc4-c22a-65ba-f49a-f051fe01208f"
             rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f"
@@ -1267,7 +1255,7 @@ mod test {
             path = "/nonexistent/path"
             if_exists = "fail"
             [timeseries_db]
-            address = "[::1]:8123"
+            address = "[::1]:9000"
             [deployment]
             id = "28b90dc4-c22a-65ba-f49a-f051fe01208f"
             rack_id = "38b90dc4-c22a-65ba-f49a-f051fe01208f"
@@ -1319,7 +1307,7 @@ mod test {
             path = "/nonexistent/path"
             if_exists = "fail"
             [timeseries_db]
-            address = "[::1]:8123"
+            address = "[::1]:9000"
             [updates]
             trusted_root = "/path/to/root.json"
             default_base_url = "http://example.invalid/"

nexus/src/app/mod.rs

@@ -24,8 +24,6 @@ use nexus_db_queries::authn;
 use nexus_db_queries::authz;
 use nexus_db_queries::context::OpContext;
 use nexus_db_queries::db;
-use omicron_common::address::CLICKHOUSE_HTTP_PORT;
-use omicron_common::address::CLICKHOUSE_TCP_PORT;
 use omicron_common::address::DENDRITE_PORT;
 use omicron_common::address::MGD_PORT;
 use omicron_common::address::MGS_PORT;
@@ -37,7 +35,6 @@ use sagas::common_storage::make_pantry_connection_pool;
 use sagas::common_storage::PooledPantryClient;
 use slog::Logger;
 use std::collections::HashMap;
-use std::net::SocketAddr;
 use std::net::SocketAddrV6;
 use std::net::{IpAddr, Ipv6Addr};
 use std::sync::Arc;
@@ -412,38 +409,13 @@ impl Nexus {
             .map_err(|e| e.to_string())?;
 
         // Client to the ClickHouse database.
-        // TODO-cleanup: Simplify this when we remove the HTTP client.
-        let timeseries_client = match (
-            &config.pkg.timeseries_db.address,
-            &config.pkg.timeseries_db.native_address,
-        ) {
-            (None, None) => {
-                let http_resolver =
-                    qorb_resolver.for_service(ServiceName::Clickhouse);
+        let timeseries_client = match &config.pkg.timeseries_db.address {
+            None => {
                 let native_resolver =
                     qorb_resolver.for_service(ServiceName::ClickhouseNative);
-                oximeter_db::Client::new_with_pool(
-                    http_resolver,
-                    native_resolver,
-                    &log,
-                )
-            }
-            (maybe_http, maybe_native) => {
-                let (http_address, native_address) =
-                    match (maybe_http, maybe_native) {
-                        (None, None) => unreachable!("handled above"),
-                        (None, Some(native)) => (
-                            SocketAddr::new(native.ip(), CLICKHOUSE_HTTP_PORT),
-                            *native,
-                        ),
-                        (Some(http), None) => (
-                            *http,
-                            SocketAddr::new(http.ip(), CLICKHOUSE_TCP_PORT),
-                        ),
-                        (Some(http), Some(native)) => (*http, *native),
-                    };
-                oximeter_db::Client::new(http_address, native_address, &log)
+                oximeter_db::Client::new_with_pool(native_resolver, &log)
             }
+            Some(address) => oximeter_db::Client::new(*address, &log),
         };
 
         // TODO-cleanup We may want to make the populator a first-class

nexus/test-utils/src/lib.rs

@@ -504,8 +504,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
             .as_mut()
             .expect("Tests expect to set a port of Clickhouse")
             .set_port(http_port);
-        self.config.pkg.timeseries_db.native_address =
-            Some(native_address.into());
+        self.config.pkg.timeseries_db.address = Some(native_address.into());
 
         let pool_name = illumos_utils::zpool::ZpoolName::new_external(zpool_id)
             .to_string()
@@ -623,7 +622,6 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
         let oximeter = start_oximeter(
             log.new(o!("component" => "oximeter")),
             nexus_internal_addr,
-            clickhouse.http_address().port(),
             clickhouse.native_address().port(),
             collector_id,
         )
@@ -1457,16 +1455,11 @@ pub async fn start_sled_agent(
 pub async fn start_oximeter(
     log: Logger,
     nexus_address: SocketAddr,
-    http_port: u16,
     native_port: u16,
     id: Uuid,
 ) -> Result<Oximeter, String> {
     let db = oximeter_collector::DbConfig {
-        address: Some(SocketAddr::new(Ipv6Addr::LOCALHOST.into(), http_port)),
-        native_address: Some(SocketAddr::new(
-            Ipv6Addr::LOCALHOST.into(),
-            native_port,
-        )),
+        address: Some(SocketAddr::new(Ipv6Addr::LOCALHOST.into(), native_port)),
         batch_size: 10,
         batch_interval: 1,
         replicated: false,

nexus/tests/integration_tests/oximeter.rs

@@ -117,13 +117,8 @@ async fn test_oximeter_reregistration() {
         row.get::<&str, chrono::DateTime<chrono::Utc>>("time_modified");
 
     // ClickHouse client for verifying collection.
-    let ch_address = context.clickhouse.http_address().into();
     let native_address = context.clickhouse.native_address().into();
-    let client = oximeter_db::Client::new(
-        ch_address,
-        native_address,
-        &context.logctx.log,
-    );
+    let client = oximeter_db::Client::new(native_address, &context.logctx.log);
     client
         .init_single_node_db()
         .await
@@ -306,7 +301,6 @@ async fn test_oximeter_reregistration() {
     context.oximeter = nexus_test_utils::start_oximeter(
         context.logctx.log.new(o!("component" => "oximeter")),
         context.server.get_http_server_internal_address().await,
-        context.clickhouse.http_address().port(),
         context.clickhouse.native_address().port(),
         oximeter_id,
     )

oximeter/collector/config.toml

@@ -3,7 +3,7 @@
 nexus_address = "[::1]:12221"
 
 [db]
-address = "[::1]:8123"
+address = "[::1]:9000"
 batch_size = 1000
 batch_interval = 5 # In seconds