Skip to content

Commit 75d391d

Browse files
authored
Proxy cockroach HTTP requests through the admin server (#8377)
- Start a CockroachDB admin server in the integration test environment - Add endpoints to the admin server to proxy to the CockroachDB HTTP server This is an alternative to #8351
1 parent 2627bf8 commit 75d391d

File tree

17 files changed

+315
-11
lines changed

17 files changed

+315
-11
lines changed

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cockroach-admin/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ omicron-common.workspace = true
2323
omicron-uuid-kinds.workspace = true
2424
# See omicron-rpaths for more about the "pq-sys" dependency.
2525
pq-sys = "*"
26+
reqwest.workspace = true
2627
schemars.workspace = true
2728
slog.workspace = true
2829
slog-async.workspace = true

cockroach-admin/api/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ workspace = true
1010
[dependencies]
1111
cockroach-admin-types.workspace = true
1212
dropshot.workspace = true
13+
http.workspace = true
1314
omicron-common.workspace = true
1415
omicron-uuid-kinds.workspace = true
1516
omicron-workspace-hack.workspace = true

cockroach-admin/api/src/lib.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,29 @@ pub trait CockroachAdminApi {
6161
rqctx: RequestContext<Self::Context>,
6262
body: TypedBody<NodeId>,
6363
) -> Result<HttpResponseOk<NodeDecommission>, HttpError>;
64+
65+
/// Proxy to CockroachDB's /_status/vars endpoint
66+
//
67+
// Dropshot isn't happy about the "_status" portion of the path; it fails
68+
// the linter. Instead, I'm adding the prefix "proxy" to make it clear these
69+
// are "intended-to-be-proxied" endpoints, rather than exact matches for the
70+
// CRDB HTTP interface paths.
71+
#[endpoint {
72+
method = GET,
73+
path = "/proxy/status/vars",
74+
}]
75+
async fn status_vars(
76+
rqctx: RequestContext<Self::Context>,
77+
) -> Result<HttpResponseOk<String>, HttpError>;
78+
79+
/// Proxy to CockroachDB's /_status/nodes endpoint
80+
#[endpoint {
81+
method = GET,
82+
path = "/proxy/status/nodes",
83+
}]
84+
async fn status_nodes(
85+
rqctx: RequestContext<Self::Context>,
86+
) -> Result<HttpResponseOk<String>, HttpError>;
6487
}
6588

6689
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]

cockroach-admin/src/bin/cockroach-admin.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ enum Args {
2929
#[clap(long, action)]
3030
cockroach_address: SocketAddrV6,
3131

32+
/// Socket address for a running cockroach HTTP server instance
33+
///
34+
/// Although this may be on an arbitrary address, it should point
35+
/// to the same address as the "cockroach_address" server.
36+
#[clap(long, action)]
37+
cockroach_http_address: SocketAddr,
38+
3239
/// Address on which this server should run
3340
#[clap(long, action)]
3441
http_address: SocketAddrV6,
@@ -57,12 +64,16 @@ async fn main_impl() -> Result<(), CmdError> {
5764
Args::Run {
5865
path_to_cockroach_binary,
5966
cockroach_address,
67+
cockroach_http_address,
6068
http_address,
6169
config_file_path,
6270
zone_id,
6371
} => {
64-
let cockroach_cli =
65-
CockroachCli::new(path_to_cockroach_binary, cockroach_address);
72+
let cockroach_cli = CockroachCli::new(
73+
path_to_cockroach_binary,
74+
cockroach_address,
75+
cockroach_http_address,
76+
);
6677
let mut config = Config::from_file(&config_file_path)
6778
.map_err(|err| CmdError::Failure(anyhow!(err)))?;
6879
config.dropshot.bind_address = SocketAddr::V6(http_address);

cockroach-admin/src/cockroach_cli.rs

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use illumos_utils::output_to_exec_error;
1212
use slog_error_chain::InlineErrorChain;
1313
use slog_error_chain::SlogInlineError;
1414
use std::io;
15-
use std::net::SocketAddrV6;
15+
use std::net::{SocketAddr, SocketAddrV6};
1616
use std::process::Output;
1717
use tokio::process::Command;
1818

@@ -93,20 +93,30 @@ impl From<CockroachCliError> for HttpError {
9393
pub struct CockroachCli {
9494
path_to_cockroach_binary: Utf8PathBuf,
9595
cockroach_address: SocketAddrV6,
96+
cockroach_http_address: SocketAddr,
9697
}
9798

9899
impl CockroachCli {
99100
pub fn new(
100101
path_to_cockroach_binary: Utf8PathBuf,
101102
cockroach_address: SocketAddrV6,
103+
cockroach_http_address: SocketAddr,
102104
) -> Self {
103-
Self { path_to_cockroach_binary, cockroach_address }
105+
Self {
106+
path_to_cockroach_binary,
107+
cockroach_address,
108+
cockroach_http_address,
109+
}
104110
}
105111

106112
pub fn cockroach_address(&self) -> SocketAddrV6 {
107113
self.cockroach_address
108114
}
109115

116+
pub fn cockroach_http_address(&self) -> SocketAddr {
117+
self.cockroach_http_address
118+
}
119+
110120
pub async fn cluster_init(&self) -> Result<(), CockroachCliError> {
111121
self.invoke_cli_raw(
112122
["init"],
@@ -443,7 +453,11 @@ mod tests {
443453
)
444454
.parse()
445455
.expect("valid SocketAddrV6");
446-
let cli = CockroachCli::new("cockroach".into(), cockroach_address);
456+
let cli = CockroachCli::new(
457+
"cockroach".into(),
458+
cockroach_address,
459+
SocketAddr::V6(cockroach_address),
460+
);
447461
let status = cli.node_status().await.expect("got node status");
448462

449463
// We can't check all the fields exactly, but some we know based on the
@@ -505,7 +519,11 @@ mod tests {
505519
)
506520
.parse()
507521
.expect("valid SocketAddrV6");
508-
let cli = CockroachCli::new("cockroach".into(), cockroach_address);
522+
let cli = CockroachCli::new(
523+
"cockroach".into(),
524+
cockroach_address,
525+
SocketAddr::V6(cockroach_address),
526+
);
509527
let result = cli
510528
.invoke_node_decommission("1")
511529
.await
@@ -652,7 +670,11 @@ mod tests {
652670
// Repeat the above test but using our wrapper.
653671
{
654672
let db = UninitializedCockroach::start().await;
655-
let cli = CockroachCli::new("cockroach".into(), db.listen_addr);
673+
let cli = CockroachCli::new(
674+
"cockroach".into(),
675+
db.listen_addr,
676+
SocketAddr::V6(db.listen_addr),
677+
);
656678

657679
cli.cluster_init().await.expect("cluster initialized");
658680
cli.cluster_init().await.expect("cluster still initialized");
@@ -675,7 +697,11 @@ mod tests {
675697
.parse()
676698
.expect("valid SocketAddrV6");
677699

678-
let cli = CockroachCli::new("cockroach".into(), cockroach_address);
700+
let cli = CockroachCli::new(
701+
"cockroach".into(),
702+
cockroach_address,
703+
SocketAddr::V6(cockroach_address),
704+
);
679705
cli.schema_init_impl(DBINIT_RELATIVE_PATH)
680706
.await
681707
.expect("initialized schema");
@@ -692,7 +718,11 @@ mod tests {
692718
let logctx =
693719
dev::test_setup_log("test_cluster_schema_init_interleaved");
694720
let db = UninitializedCockroach::start().await;
695-
let cli = CockroachCli::new("cockroach".into(), db.listen_addr);
721+
let cli = CockroachCli::new(
722+
"cockroach".into(),
723+
db.listen_addr,
724+
SocketAddr::V6(db.listen_addr),
725+
);
696726

697727
// We should be able to initialize the cluster, then install the schema,
698728
// then do both of those things again.
@@ -778,6 +808,7 @@ mod tests {
778808
let cli = CockroachCli::new(
779809
"never-called".into(),
780810
"[::1]:0".parse().unwrap(),
811+
SocketAddr::V6("[::1]:0".parse().unwrap()),
781812
);
782813

783814
match cli.validate_node_decommissionable(

cockroach-admin/src/context.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,11 @@ mod tests {
164164
)
165165
.parse()
166166
.expect("valid SocketAddrV6");
167-
let cli = CockroachCli::new("cockroach".into(), cockroach_address);
167+
let cli = CockroachCli::new(
168+
"cockroach".into(),
169+
cockroach_address,
170+
SocketAddr::V6(cockroach_address),
171+
);
168172
let context = ServerContext::new(
169173
OmicronZoneUuid::new_v4(),
170174
cli,

cockroach-admin/src/http_entrypoints.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,88 @@ impl CockroachAdminApi for CockroachAdminImpl {
7474
);
7575
Ok(HttpResponseOk(decommission_status))
7676
}
77+
78+
async fn status_vars(
79+
rqctx: RequestContext<Self::Context>,
80+
) -> Result<HttpResponseOk<String>, HttpError> {
81+
let ctx = rqctx.context();
82+
let cockroach_http_address =
83+
ctx.cockroach_cli().cockroach_http_address();
84+
let url = format!("http://{}/_status/vars", cockroach_http_address);
85+
86+
let client = reqwest::Client::new();
87+
let response = client.get(&url).send().await.map_err(|e| {
88+
HttpError::for_internal_error(format!(
89+
"Failed to proxy to CockroachDB: {}",
90+
e
91+
))
92+
})?;
93+
94+
if !response.status().is_success() {
95+
let status = response.status();
96+
let body = response.text().await.unwrap_or_else(|_| {
97+
"Failed to read error response".to_string()
98+
});
99+
let status_code = dropshot::ErrorStatusCode::from_status(status)
100+
.unwrap_or(dropshot::ErrorStatusCode::INTERNAL_SERVER_ERROR);
101+
return Err(HttpError {
102+
status_code,
103+
error_code: None,
104+
external_message: body.clone(),
105+
internal_message: body,
106+
headers: None,
107+
});
108+
}
109+
110+
let body = response.text().await.map_err(|e| {
111+
HttpError::for_internal_error(format!(
112+
"Failed to read response body: {}",
113+
e
114+
))
115+
})?;
116+
117+
Ok(HttpResponseOk(body))
118+
}
119+
120+
async fn status_nodes(
121+
rqctx: RequestContext<Self::Context>,
122+
) -> Result<HttpResponseOk<String>, HttpError> {
123+
let ctx = rqctx.context();
124+
let cockroach_http_address =
125+
ctx.cockroach_cli().cockroach_http_address();
126+
let url = format!("http://{}/_status/nodes", cockroach_http_address);
127+
128+
let client = reqwest::Client::new();
129+
let response = client.get(&url).send().await.map_err(|e| {
130+
HttpError::for_internal_error(format!(
131+
"Failed to proxy to CockroachDB: {}",
132+
e
133+
))
134+
})?;
135+
136+
if !response.status().is_success() {
137+
let status = response.status();
138+
let body = response.text().await.unwrap_or_else(|_| {
139+
"Failed to read error response".to_string()
140+
});
141+
let status_code = dropshot::ErrorStatusCode::from_status(status)
142+
.unwrap_or(dropshot::ErrorStatusCode::INTERNAL_SERVER_ERROR);
143+
return Err(HttpError {
144+
status_code,
145+
error_code: None,
146+
external_message: body.clone(),
147+
internal_message: body,
148+
headers: None,
149+
});
150+
}
151+
152+
let body = response.text().await.map_err(|e| {
153+
HttpError::for_internal_error(format!(
154+
"Failed to read response body: {}",
155+
e
156+
))
157+
})?;
158+
159+
Ok(HttpResponseOk(body))
160+
}
77161
}

nexus/test-utils/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ nexus-db-queries = { workspace = true, features = [ "testing" ] }
3434
nexus-sled-agent-shared.workspace = true
3535
nexus-test-interface.workspace = true
3636
nexus-types.workspace = true
37+
omicron-cockroach-admin.workspace = true
3738
omicron-common.workspace = true
3839
omicron-passwords.workspace = true
3940
omicron-sled-agent.workspace = true

nexus/test-utils/src/lib.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ pub struct ControlPlaneTestContext<N> {
166166
pub internal_client: ClientTestContext,
167167
pub server: N,
168168
pub database: dev::db::CockroachInstance,
169+
pub database_admin: omicron_cockroach_admin::Server,
169170
pub clickhouse: dev::clickhouse::ClickHouseDeployment,
170171
pub logctx: LogContext,
171172
pub sled_agents: Vec<ControlPlaneTestContextSledAgent>,
@@ -377,6 +378,7 @@ pub struct ControlPlaneTestContextBuilder<'a, N: NexusServer> {
377378

378379
pub server: Option<N>,
379380
pub database: Option<dev::db::CockroachInstance>,
381+
pub database_admin: Option<omicron_cockroach_admin::Server>,
380382
pub clickhouse: Option<dev::clickhouse::ClickHouseDeployment>,
381383
pub sled_agents: Vec<ControlPlaneTestContextSledAgent>,
382384
pub oximeter: Option<Oximeter>,
@@ -429,6 +431,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
429431
internal_client: None,
430432
server: None,
431433
database: None,
434+
database_admin: None,
432435
clickhouse: None,
433436
sled_agents: vec![],
434437
oximeter: None,
@@ -541,7 +544,28 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
541544
),
542545
image_source: BlueprintZoneImageSource::InstallDataset,
543546
});
547+
let http_address = database.http_addr();
544548
self.database = Some(database);
549+
550+
let cli = omicron_cockroach_admin::CockroachCli::new(
551+
omicron_test_utils::dev::db::COCKROACHDB_BIN.into(),
552+
address,
553+
http_address,
554+
);
555+
let server = omicron_cockroach_admin::start_server(
556+
zone_id,
557+
cli,
558+
omicron_cockroach_admin::Config {
559+
dropshot: dropshot::ConfigDropshot::default(),
560+
log: ConfigLogging::StderrTerminal {
561+
level: ConfigLoggingLevel::Error,
562+
},
563+
},
564+
)
565+
.await
566+
.expect("Failed to start CRDB admin server");
567+
568+
self.database_admin = Some(server);
545569
}
546570

547571
// Start ClickHouse database server.
@@ -1409,6 +1433,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
14091433
techport_client: self.techport_client.unwrap(),
14101434
internal_client: self.internal_client.unwrap(),
14111435
database: self.database.unwrap(),
1436+
database_admin: self.database_admin.unwrap(),
14121437
clickhouse: self.clickhouse.unwrap(),
14131438
sled_agents: self.sled_agents,
14141439
oximeter: self.oximeter.unwrap(),

0 commit comments

Comments
 (0)