Simplify keeper api

oxidecomputer · Sep 2, 2024 · 3f48362 · 3f48362
1 parent 112bdc3
commit 3f48362
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 59 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/clickhouse-admin/api/Cargo.toml b/clickhouse-admin/api/Cargo.toml
@@ -8,7 +8,6 @@ license = "MPL-2.0"
 workspace = true
 
 [dependencies]
-camino.workspace = true
 clickhouse-admin-types.workspace = true
 dropshot.workspace = true
 omicron-common.workspace = true

diff --git a/clickhouse-admin/api/src/lib.rs b/clickhouse-admin/api/src/lib.rs
@@ -2,18 +2,14 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use camino::Utf8PathBuf;
-use clickhouse_admin_types::config::{
-    path_schema, KeeperConfig, RaftServerSettings, ReplicaConfig,
-};
-use clickhouse_admin_types::{ServerSettings, KeeperId};
+use clickhouse_admin_types::config::{KeeperConfig, ReplicaConfig};
+use clickhouse_admin_types::{KeeperSettings, ServerSettings};
 use dropshot::{
     HttpError, HttpResponseCreated, Path, RequestContext, TypedBody,
 };
 use omicron_common::api::external::Generation;
 use schemars::JsonSchema;
-use serde::{Deserialize, Serialize};
-use std::net::Ipv6Addr;
+use serde::Deserialize;
 
 #[derive(Debug, Deserialize, JsonSchema)]
 pub struct GenerationNum {
@@ -49,15 +45,3 @@ pub trait ClickhouseAdminApi {
         body: TypedBody<KeeperSettings>,
     ) -> Result<HttpResponseCreated<KeeperConfig>, HttpError>;
 }
-
-#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
-#[serde(rename_all = "snake_case")]
-pub struct KeeperSettings {
-    #[schemars(schema_with = "path_schema")]
-    pub config_dir: Utf8PathBuf,
-    #[schemars(schema_with = "path_schema")]
-    pub datastore_path: Utf8PathBuf,
-    pub listen_addr: Ipv6Addr,
-    pub node_id: KeeperId,
-    pub keepers: Vec<RaftServerSettings>,
-}
diff --git a/clickhouse-admin/src/clickward.rs b/clickhouse-admin/src/clickward.rs
@@ -2,9 +2,8 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use clickhouse_admin_api::KeeperSettings;
 use clickhouse_admin_types::config::{KeeperConfig, ReplicaConfig};
-use clickhouse_admin_types::{ClickhouseKeeperConfig, ServerSettings};
+use clickhouse_admin_types::{KeeperSettings, ServerSettings};
 use dropshot::HttpError;
 use slog_error_chain::{InlineErrorChain, SlogInlineError};
 
@@ -44,7 +43,7 @@ impl Clickward {
     pub fn generate_server_config(
         &self,
         settings: ServerSettings,
-     ) -> Result<ReplicaConfig, ClickwardError> {
+    ) -> Result<ReplicaConfig, ClickwardError> {
         let replica_config = settings
             .generate_xml_file()
             .map_err(|e| ClickwardError::Failure { err: e })?;
@@ -56,15 +55,7 @@ impl Clickward {
         &self,
         settings: KeeperSettings,
     ) -> Result<KeeperConfig, ClickwardError> {
-        let config = ClickhouseKeeperConfig::new(
-            settings.config_dir,
-            settings.node_id,
-            settings.keepers,
-            settings.datastore_path,
-            settings.listen_addr,
-        );
-
-        let keeper_config = config
+        let keeper_config = settings
             .generate_xml_file()
             .map_err(|e| ClickwardError::Failure { err: e })?;
 

diff --git a/clickhouse-admin/src/http_entrypoints.rs b/clickhouse-admin/src/http_entrypoints.rs
@@ -5,7 +5,7 @@
 use crate::context::ServerContext;
 use clickhouse_admin_api::*;
 use clickhouse_admin_types::config::{KeeperConfig, ReplicaConfig};
-use clickhouse_admin_types::ServerSettings;
+use clickhouse_admin_types::{KeeperSettings, ServerSettings};
 use dropshot::{
     HttpError, HttpResponseCreated, Path, RequestContext, TypedBody,
 };

diff --git a/clickhouse-admin/types/src/lib.rs b/clickhouse-admin/types/src/lib.rs
@@ -126,41 +126,43 @@ impl ServerSettings {
 
 #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
 #[serde(rename_all = "snake_case")]
-pub struct ClickhouseKeeperConfig {
+pub struct KeeperSettings {
     #[schemars(schema_with = "path_schema")]
     pub config_dir: Utf8PathBuf,
-    pub id: KeeperId,
-    pub raft_servers: Vec<RaftServerConfig>,
+    pub node_id: KeeperId,
+    pub raft_servers: Vec<RaftServerSettings>,
     #[schemars(schema_with = "path_schema")]
     pub datastore_path: Utf8PathBuf,
     pub listen_addr: Ipv6Addr,
 }
 
-impl ClickhouseKeeperConfig {
+impl KeeperSettings {
     pub fn new(
         config_dir: Utf8PathBuf,
-        id: KeeperId,
+        node_id: KeeperId,
         raft_servers: Vec<RaftServerSettings>,
         datastore_path: Utf8PathBuf,
         listen_addr: Ipv6Addr,
     ) -> Self {
-        let raft_servers = raft_servers
-            .iter()
-            .map(|settings| RaftServerConfig::new(settings.clone()))
-            .collect();
-
-        Self { config_dir, id, raft_servers, datastore_path, listen_addr }
+        Self { config_dir, node_id, raft_servers, datastore_path, listen_addr }
     }
 
     /// Generate a configuration file for a keeper node
     pub fn generate_xml_file(&self) -> Result<KeeperConfig> {
         let logger =
             LogConfig::new(self.datastore_path.clone(), NodeType::Keeper);
-        let raft_config = RaftServers::new(self.raft_servers.clone());
+
+        let raft_servers = self
+            .raft_servers
+            .iter()
+            .map(|settings| RaftServerConfig::new(settings.clone()))
+            .collect();
+        let raft_config = RaftServers::new(raft_servers);
+
         let config = KeeperConfig::new(
             logger,
             self.listen_addr,
-            self.id,
+            self.node_id,
             self.datastore_path.clone(),
             raft_config,
         );
@@ -186,8 +188,8 @@ mod tests {
     use camino_tempfile::Builder;
 
     use crate::{
-        ClickhouseHost, ClickhouseKeeperConfig, ServerSettings,
-        KeeperId, RaftServerSettings, ServerId,
+        ClickhouseHost, KeeperId, KeeperSettings, RaftServerSettings, ServerId,
+        ServerSettings,
     };
 
     #[test]
@@ -217,7 +219,7 @@ mod tests {
             },
         ];
 
-        let config = ClickhouseKeeperConfig::new(
+        let config = KeeperSettings::new(
             Utf8PathBuf::from(config_dir.path()),
             KeeperId(1),
             keepers,

diff --git a/openapi/clickhouse-admin.json b/openapi/clickhouse-admin.json
@@ -282,26 +282,26 @@
             "type": "string",
             "format": "Utf8PathBuf"
           },
-          "keepers": {
-            "type": "array",
-            "items": {
-              "$ref": "#/components/schemas/RaftServerSettings"
-            }
+          "id": {
+            "$ref": "#/components/schemas/KeeperId"
           },
           "listen_addr": {
             "type": "string",
             "format": "ipv6"
           },
-          "node_id": {
-            "$ref": "#/components/schemas/KeeperId"
+          "raft_servers": {
+            "type": "array",
+            "items": {
+              "$ref": "#/components/schemas/RaftServerSettings"
+            }
           }
         },
         "required": [
           "config_dir",
           "datastore_path",
-          "keepers",
+          "id",
           "listen_addr",
-          "node_id"
+          "raft_servers"
         ]
       },
       "LogConfig": {