Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[reconfigurator] clickhouse-admin endpoints to generate ClickHouse config files #6476

Merged
merged 14 commits into from
Sep 5, 2024
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions clickhouse-admin/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ license = "MPL-2.0"
workspace = true

[dependencies]
clickhouse-admin-types.workspace = true
dropshot.workspace = true
omicron-common.workspace = true
omicron-uuid-kinds.workspace = true
Expand Down
45 changes: 32 additions & 13 deletions clickhouse-admin/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,46 @@
// 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 dropshot::{HttpError, HttpResponseOk, RequestContext};
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::SocketAddrV6;
use serde::Deserialize;

#[derive(Debug, Deserialize, JsonSchema)]
pub struct GenerationNum {
/// A unique identifier for the configuration generation.
pub generation: Generation,
}

#[dropshot::api_description]
pub trait ClickhouseAdminApi {
type Context;

/// Retrieve the address the ClickHouse server or keeper node is listening on
/// Generate a ClickHouse configuration file for a server node on a specified
/// directory.
#[endpoint {
method = GET,
path = "/node/address",
method = POST,
path = "/node/server/generate-config/{generation}",
}]
async fn clickhouse_address(
async fn generate_server_config(
rqctx: RequestContext<Self::Context>,
) -> Result<HttpResponseOk<ClickhouseAddress>, HttpError>;
}
path: Path<GenerationNum>,
body: TypedBody<ServerSettings>,
) -> Result<HttpResponseCreated<ReplicaConfig>, HttpError>;

#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub struct ClickhouseAddress {
pub clickhouse_address: SocketAddrV6,
/// Generate a ClickHouse configuration file for a keeper node on a specified
/// directory.
#[endpoint {
method = POST,
path = "/node/keeper/generate-config/{generation}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few notes on the API structure here:

  • We should be using REST style APIs with nouns, not verbs, similar to the rest of our APIs. I suggest /keeper/config.
  • These APIs are also idempotent and should be PUTs.
  • The generation number should be part of the payload, and not a query parameter. We typically do this by wrapping the inner type in a container type with generation number.
  • The /node prefix seems somewhat superfluous. If we need a sharable endpoint for something like stats we can add it and name it precisely without a prefix.

}]
async fn generate_keeper_config(
rqctx: RequestContext<Self::Context>,
path: Path<GenerationNum>,
body: TypedBody<KeeperSettings>,
) -> Result<HttpResponseCreated<KeeperConfig>, HttpError>;
andrewjstone marked this conversation as resolved.
Show resolved Hide resolved
}
12 changes: 3 additions & 9 deletions clickhouse-admin/src/bin/clickhouse-admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,8 @@ use std::net::{SocketAddr, SocketAddrV6};
enum Args {
/// Start the ClickHouse admin server
Run {
// TODO: This address is solely for testing now. We should remove it
// once we have more endpoints up and running.
/// Socket address for a running clickhouse server or keeper instance
#[clap(long, short = 'a', action)]
clickhouse_address: SocketAddrV6,

/// Address on which this server should run
#[clap(long, short = 'H', action)]
#[clap(long, short = 'a', action)]
http_address: SocketAddrV6,

/// Path to the server configuration file
Expand All @@ -47,12 +41,12 @@ async fn main_impl() -> Result<(), CmdError> {
let args = Args::parse();

match args {
Args::Run { clickhouse_address, http_address, config } => {
Args::Run { http_address, config } => {
let mut config = Config::from_file(&config)
.map_err(|err| CmdError::Failure(anyhow!(err)))?;
config.dropshot.bind_address = SocketAddr::V6(http_address);

let clickward = Clickward::new(clickhouse_address);
let clickward = Clickward::new();

let server =
omicron_clickhouse_admin::start_server(clickward, config)
Expand Down
37 changes: 25 additions & 12 deletions clickhouse-admin/src/clickward.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@
// 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::ClickhouseAddress;
use clickhouse_admin_types::config::{KeeperConfig, ReplicaConfig};
use clickhouse_admin_types::{KeeperSettings, ServerSettings};
use dropshot::HttpError;
use slog_error_chain::{InlineErrorChain, SlogInlineError};
use std::io;
use std::net::SocketAddrV6;

#[derive(Debug, thiserror::Error, SlogInlineError)]
pub enum ClickwardError {
#[error("clickward failure")]
Failure {
#[source]
err: io::Error,
err: anyhow::Error,
},
}

Expand All @@ -34,18 +33,32 @@ impl From<ClickwardError> for HttpError {
}

#[derive(Debug)]
pub struct Clickward {
clickhouse_address: SocketAddrV6,
}
pub struct Clickward {}

impl Clickward {
pub fn new(clickhouse_address: SocketAddrV6) -> Self {
Self { clickhouse_address }
pub fn new() -> Self {
Self {}
}

pub fn generate_server_config(
&self,
settings: ServerSettings,
) -> Result<ReplicaConfig, ClickwardError> {
let replica_config = settings
.generate_xml_file()
.map_err(|e| ClickwardError::Failure { err: e })?;

Ok(replica_config)
}

pub fn clickhouse_address(
pub fn generate_keeper_config(
&self,
) -> Result<ClickhouseAddress, ClickwardError> {
Ok(ClickhouseAddress { clickhouse_address: self.clickhouse_address })
settings: KeeperSettings,
) -> Result<KeeperConfig, ClickwardError> {
let keeper_config = settings
.generate_xml_file()
.map_err(|e| ClickwardError::Failure { err: e })?;

Ok(keeper_config)
}
}
34 changes: 27 additions & 7 deletions clickhouse-admin/src/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

use crate::context::ServerContext;
use clickhouse_admin_api::*;
use dropshot::HttpError;
use dropshot::HttpResponseOk;
use dropshot::RequestContext;
use clickhouse_admin_types::config::{KeeperConfig, ReplicaConfig};
use clickhouse_admin_types::{KeeperSettings, ServerSettings};
use dropshot::{
HttpError, HttpResponseCreated, Path, RequestContext, TypedBody,
};
use std::sync::Arc;

type ClickhouseApiDescription = dropshot::ApiDescription<Arc<ServerContext>>;
Expand All @@ -21,11 +23,29 @@ enum ClickhouseAdminImpl {}
impl ClickhouseAdminApi for ClickhouseAdminImpl {
type Context = Arc<ServerContext>;

async fn clickhouse_address(
async fn generate_server_config(
rqctx: RequestContext<Self::Context>,
) -> Result<HttpResponseOk<ClickhouseAddress>, HttpError> {
path: Path<GenerationNum>,
body: TypedBody<ServerSettings>,
) -> Result<HttpResponseCreated<ReplicaConfig>, HttpError> {
let ctx = rqctx.context();
let output = ctx.clickward().clickhouse_address()?;
Ok(HttpResponseOk(output))
let server_settings = body.into_inner();
let output = ctx.clickward().generate_server_config(server_settings)?;
// TODO(https://github.com/oxidecomputer/omicron/issues/5999): Do something with the generation number
println!("{path:?}");
Ok(HttpResponseCreated(output))
}

async fn generate_keeper_config(
rqctx: RequestContext<Self::Context>,
path: Path<GenerationNum>,
body: TypedBody<KeeperSettings>,
) -> Result<HttpResponseCreated<KeeperConfig>, HttpError> {
let ctx = rqctx.context();
let keeper_settings = body.into_inner();
let output = ctx.clickward().generate_keeper_config(keeper_settings)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the returned HttpError look like on conversion here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ curl -X put http://[::1]:8888/keeper/config -H "Content-Type: application/json" -d '{"generation": 3, "settings": {"node_id": 1, "raft_servers": [{"id": 1, "host": {"ipv6": "ff::01"}}, {"id": 2, "host":{"ipv4": "127.0.0.1"}}, {"id": 3, "host":{"domain_name": "hi.there"}}], "config_dir": "./bob/bob", "datastore_path": "./", "listen_addr": "::1" }}' | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   439  100   175  100   264  67281    99k --:--:-- --:--:-- --:--:--  214k
{
  "request_id": "f79d5187-6ed8-4b55-ab1f-fb1b5aa010c6",
  "error_code": "Internal",
  "message": "clickward XML generation failure: No such file or directory (os error 2)"
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. That looks pretty good. I'm assuming Internal means 500, which is probably correct here. I'm not an expert in http response codes 😄.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// TODO(https://github.com/oxidecomputer/omicron/issues/5999): Do something with the generation number
println!("{path:?}");
andrewjstone marked this conversation as resolved.
Show resolved Hide resolved
Ok(HttpResponseCreated(output))
}
}
Loading
Loading