From 3f483629f93a4d7c69e953a6a69ba360a22b16b7 Mon Sep 17 00:00:00 2001 From: karencfv Date: Mon, 2 Sep 2024 18:32:55 +1200 Subject: [PATCH] Simplify keeper api --- Cargo.lock | 1 - clickhouse-admin/api/Cargo.toml | 1 - clickhouse-admin/api/src/lib.rs | 22 +++------------ clickhouse-admin/src/clickward.rs | 15 +++-------- clickhouse-admin/src/http_entrypoints.rs | 2 +- clickhouse-admin/types/src/lib.rs | 34 +++++++++++++----------- openapi/clickhouse-admin.json | 18 ++++++------- 7 files changed, 34 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c25eda9d99..8666963ed8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1119,7 +1119,6 @@ checksum = "98cc8fbded0c607b7ba9dd60cd98df59af97e84d24e49c8557331cfc26d301ce" name = "clickhouse-admin-api" version = "0.1.0" dependencies = [ - "camino", "clickhouse-admin-types", "dropshot 0.10.2-dev", "omicron-common", diff --git a/clickhouse-admin/api/Cargo.toml b/clickhouse-admin/api/Cargo.toml index e3bb96d001..a5d1dca202 100644 --- 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 index a5f1ff8924..8d7eadf73b 100644 --- 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, ) -> Result, 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, -} diff --git a/clickhouse-admin/src/clickward.rs b/clickhouse-admin/src/clickward.rs index e2ba5f3524..d73fde94b8 100644 --- 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 { + ) -> Result { let replica_config = settings .generate_xml_file() .map_err(|e| ClickwardError::Failure { err: e })?; @@ -56,15 +55,7 @@ impl Clickward { &self, settings: KeeperSettings, ) -> Result { - 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 index f266c4e8e5..782b2b4790 100644 --- 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 index ac52280480..d5e401c66a 100644 --- 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, + pub node_id: KeeperId, + pub raft_servers: Vec, #[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, 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 { 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 index 0b407e00f2..c1342cbfec 100644 --- 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": {