[clickhouse-admin] Modify endpoints to avoid redundancy (#7002)

Now that clickhouse-admin has been separated into two APIs, some endpoint names that started with `/keeper/` have become redundant. This commit fixes that.
oxidecomputer · Nov 7, 2024 · d8f231e · d8f231e
1 parent 8d73079
commit d8f231e
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 60 deletions.
diff --git a/clickhouse-admin/api/src/lib.rs b/clickhouse-admin/api/src/lib.rs
@@ -35,7 +35,7 @@ pub trait ClickhouseAdminKeeperApi {
         method = PUT,
         path = "/config",
     }]
-    async fn generate_config(
+    async fn generate_config_and_enable_svc(
         rqctx: RequestContext<Self::Context>,
         body: TypedBody<KeeperConfigurableSettings>,
     ) -> Result<HttpResponseCreated<KeeperConfig>, HttpError>;
@@ -45,7 +45,7 @@ pub trait ClickhouseAdminKeeperApi {
     /// and logs for consistency and recovery.
     #[endpoint {
         method = GET,
-        path = "/keeper/lgif",
+        path = "/4lw-lgif",
     }]
     async fn lgif(
         rqctx: RequestContext<Self::Context>,
@@ -55,7 +55,7 @@ pub trait ClickhouseAdminKeeperApi {
     /// contains last committed cluster configuration.
     #[endpoint {
         method = GET,
-        path = "/keeper/raft-config",
+        path = "/raft-config",
     }]
     async fn raft_config(
         rqctx: RequestContext<Self::Context>,
@@ -64,7 +64,7 @@ pub trait ClickhouseAdminKeeperApi {
     /// Retrieve configuration information from a keeper node.
     #[endpoint {
         method = GET,
-        path = "/keeper/conf",
+        path = "/4lw-conf",
     }]
     async fn keeper_conf(
         rqctx: RequestContext<Self::Context>,
@@ -73,7 +73,7 @@ pub trait ClickhouseAdminKeeperApi {
     /// Retrieve cluster membership information from a keeper node.
     #[endpoint {
         method = GET,
-        path = "/keeper/cluster-membership",
+        path = "/cluster-membership",
     }]
     async fn keeper_cluster_membership(
         rqctx: RequestContext<Self::Context>,
@@ -101,7 +101,7 @@ pub trait ClickhouseAdminServerApi {
         method = PUT,
         path = "/config"
     }]
-    async fn generate_config(
+    async fn generate_config_and_enable_svc(
         rqctx: RequestContext<Self::Context>,
         body: TypedBody<ServerConfigurableSettings>,
     ) -> Result<HttpResponseCreated<ReplicaConfig>, HttpError>;

diff --git a/clickhouse-admin/src/http_entrypoints.rs b/clickhouse-admin/src/http_entrypoints.rs
@@ -32,7 +32,7 @@ enum ClickhouseAdminServerImpl {}
 impl ClickhouseAdminServerApi for ClickhouseAdminServerImpl {
     type Context = Arc<ServerContext>;
 
-    async fn generate_config(
+    async fn generate_config_and_enable_svc(
         rqctx: RequestContext<Self::Context>,
         body: TypedBody<ServerConfigurableSettings>,
     ) -> Result<HttpResponseCreated<ReplicaConfig>, HttpError> {
@@ -62,7 +62,7 @@ enum ClickhouseAdminKeeperImpl {}
 impl ClickhouseAdminKeeperApi for ClickhouseAdminKeeperImpl {
     type Context = Arc<ServerContext>;
 
-    async fn generate_config(
+    async fn generate_config_and_enable_svc(
         rqctx: RequestContext<Self::Context>,
         body: TypedBody<KeeperConfigurableSettings>,
     ) -> Result<HttpResponseCreated<KeeperConfig>, HttpError> {

diff --git a/nexus/reconfigurator/execution/src/clickhouse.rs b/nexus/reconfigurator/execution/src/clickhouse.rs
@@ -93,18 +93,22 @@ pub(crate) async fn deploy_nodes(
         let log = log.new(slog::o!("admin_url" => admin_url.clone()));
         futs.push(Either::Left(async move {
             let client = ClickhouseKeeperClient::new(&admin_url, log.clone());
-            client.generate_config(&config).await.map(|_| ()).map_err(|e| {
-                anyhow!(
-                    concat!(
+            client
+                .generate_config_and_enable_svc(&config)
+                .await
+                .map(|_| ())
+                .map_err(|e| {
+                    anyhow!(
+                        concat!(
                         "failed to send config for clickhouse keeper ",
                         "with id {} to clickhouse-admin-keeper; admin_url = {}",
                         "error = {}"
                     ),
-                    config.settings.id,
-                    admin_url,
-                    e
-                )
-            })
+                        config.settings.id,
+                        admin_url,
+                        e
+                    )
+                })
         }));
     }
     for config in server_configs {
@@ -118,18 +122,22 @@ pub(crate) async fn deploy_nodes(
         let log = opctx.log.new(slog::o!("admin_url" => admin_url.clone()));
         futs.push(Either::Right(async move {
             let client = ClickhouseServerClient::new(&admin_url, log.clone());
-            client.generate_config(&config).await.map(|_| ()).map_err(|e| {
-                anyhow!(
-                    concat!(
+            client
+                .generate_config_and_enable_svc(&config)
+                .await
+                .map(|_| ())
+                .map_err(|e| {
+                    anyhow!(
+                        concat!(
                         "failed to send config for clickhouse server ",
                         "with id {} to clickhouse-admin-server; admin_url = {}",
                         "error = {}"
                     ),
-                    config.settings.id,
-                    admin_url,
-                    e
-                )
-            })
+                        config.settings.id,
+                        admin_url,
+                        e
+                    )
+                })
         }));
     }
 

diff --git a/openapi/clickhouse-admin-keeper.json b/openapi/clickhouse-admin-keeper.json
@@ -10,28 +10,17 @@
     "version": "0.0.1"
   },
   "paths": {
-    "/config": {
-      "put": {
-        "summary": "Generate a ClickHouse configuration file for a keeper node on a specified",
-        "description": "directory and enable the SMF service if not currently enabled.\n\nNote that we cannot start the keeper service until there is an initial configuration set via this endpoint.",
-        "operationId": "generate_config",
-        "requestBody": {
-          "content": {
-            "application/json": {
-              "schema": {
-                "$ref": "#/components/schemas/KeeperConfigurableSettings"
-              }
-            }
-          },
-          "required": true
-        },
+    "/4lw-conf": {
+      "get": {
+        "summary": "Retrieve configuration information from a keeper node.",
+        "operationId": "keeper_conf",
         "responses": {
-          "201": {
-            "description": "successful creation",
+          "200": {
+            "description": "successful operation",
             "content": {
               "application/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/KeeperConfig"
+                  "$ref": "#/components/schemas/KeeperConf"
                 }
               }
             }
@@ -45,17 +34,18 @@
         }
       }
     },
-    "/keeper/cluster-membership": {
+    "/4lw-lgif": {
       "get": {
-        "summary": "Retrieve cluster membership information from a keeper node.",
-        "operationId": "keeper_cluster_membership",
+        "summary": "Retrieve a logically grouped information file from a keeper node.",
+        "description": "This information is used internally by ZooKeeper to manage snapshots and logs for consistency and recovery.",
+        "operationId": "lgif",
         "responses": {
           "200": {
             "description": "successful operation",
             "content": {
               "application/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/ClickhouseKeeperClusterMembership"
+                  "$ref": "#/components/schemas/Lgif"
                 }
               }
             }
@@ -69,17 +59,17 @@
         }
       }
     },
-    "/keeper/conf": {
+    "/cluster-membership": {
       "get": {
-        "summary": "Retrieve configuration information from a keeper node.",
-        "operationId": "keeper_conf",
+        "summary": "Retrieve cluster membership information from a keeper node.",
+        "operationId": "keeper_cluster_membership",
         "responses": {
           "200": {
             "description": "successful operation",
             "content": {
               "application/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/KeeperConf"
+                  "$ref": "#/components/schemas/ClickhouseKeeperClusterMembership"
                 }
               }
             }
@@ -93,18 +83,28 @@
         }
       }
     },
-    "/keeper/lgif": {
-      "get": {
-        "summary": "Retrieve a logically grouped information file from a keeper node.",
-        "description": "This information is used internally by ZooKeeper to manage snapshots and logs for consistency and recovery.",
-        "operationId": "lgif",
+    "/config": {
+      "put": {
+        "summary": "Generate a ClickHouse configuration file for a keeper node on a specified",
+        "description": "directory and enable the SMF service if not currently enabled.\n\nNote that we cannot start the keeper service until there is an initial configuration set via this endpoint.",
+        "operationId": "generate_config_and_enable_svc",
+        "requestBody": {
+          "content": {
+            "application/json": {
+              "schema": {
+                "$ref": "#/components/schemas/KeeperConfigurableSettings"
+              }
+            }
+          },
+          "required": true
+        },
         "responses": {
-          "200": {
-            "description": "successful operation",
+          "201": {
+            "description": "successful creation",
             "content": {
               "application/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/Lgif"
+                  "$ref": "#/components/schemas/KeeperConfig"
                 }
               }
             }
@@ -118,7 +118,7 @@
         }
       }
     },
-    "/keeper/raft-config": {
+    "/raft-config": {
       "get": {
         "summary": "Retrieve information from ClickHouse virtual node /keeper/config which",
         "description": "contains last committed cluster configuration.",

diff --git a/openapi/clickhouse-admin-server.json b/openapi/clickhouse-admin-server.json
@@ -14,7 +14,7 @@
       "put": {
         "summary": "Generate a ClickHouse configuration file for a server node on a specified",
         "description": "directory and enable the SMF service.",
-        "operationId": "generate_config",
+        "operationId": "generate_config_and_enable_svc",
         "requestBody": {
           "content": {
             "application/json": {