Skip to content

Commit cdf91f2

Browse files
smkleinsmklein
authored
Unify services and datasets, external and internal DNS are datasets too (#3390)
## Prelude This combines: - #3022 - #3025 - #3026 Into one PR. Trying to split them out resulted in intermediate states that were not worthwhile debugging - I'd rather invest the energy into fixing this "more-desirable end-state". ## What does this PR do - (Internal and External) DNS services are explicitly backed by datasets, making it possible to "delete and re-initialize" arbitrary zones without data loss. This is necessary for correct behavior in cold boot. - Merges the concept of "datasets" into "services". - Previously, "datasets" were considered services managing data, and "services" were considered services *without datasets* - the two were totally disjoint. This was a bit arbitrary, and made it more complicated to conform sleds during service management. - As one example of an inconsistency, "requesting services" was vectorized ("ask for all services that should be on the sled"), "requesting datasets" was not (it was an ask-for-one-at-a-time API). - Now, "services" are universal, and they may optionally manage datasets. This unifies pathways a bit, and simplifies the API. Fixes #3018 Part of #2978
1 parent fdf300e commit cdf91f2

File tree

21 files changed

+570
-621
lines changed

21 files changed

+570
-621
lines changed

‎Cargo.lock‎

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

‎common/src/sql/dbinit.sql‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,9 @@ CREATE INDEX ON omicron.public.switch (
186186
*/
187187

188188
CREATE TYPE omicron.public.service_kind AS ENUM (
189+
'clickhouse',
190+
'cockroach',
191+
'crucible',
189192
'crucible_pantry',
190193
'dendrite',
191194
'external_dns',
@@ -385,7 +388,9 @@ CREATE TABLE omicron.public.Zpool (
385388
CREATE TYPE omicron.public.dataset_kind AS ENUM (
386389
'crucible',
387390
'cockroach',
388-
'clickhouse'
391+
'clickhouse',
392+
'external_dns',
393+
'internal_dns'
389394
);
390395

391396
/*

‎illumos-utils/Cargo.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ opte-ioctl.workspace = true
3434

3535
[dev-dependencies]
3636
mockall.workspace = true
37+
regress.workspace = true
38+
serde_json.workspace = true
3739
toml.workspace = true
3840

3941
[features]

‎illumos-utils/src/zpool.rs‎

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,45 @@ pub enum ZpoolKind {
284284
///
285285
/// This expects that the format will be: `ox{i,p}_<UUID>` - we parse the prefix
286286
/// when reading the structure, and validate that the UUID can be utilized.
287-
#[derive(Clone, Debug, Hash, PartialEq, Eq, JsonSchema)]
287+
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
288288
pub struct ZpoolName {
289289
id: Uuid,
290290
kind: ZpoolKind,
291291
}
292292

293+
const ZPOOL_NAME_REGEX: &str = r"^ox[ip]_[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$";
294+
295+
/// Custom JsonSchema implementation to encode the constraints on Name.
296+
impl JsonSchema for ZpoolName {
297+
fn schema_name() -> String {
298+
"ZpoolName".to_string()
299+
}
300+
fn json_schema(
301+
_: &mut schemars::gen::SchemaGenerator,
302+
) -> schemars::schema::Schema {
303+
schemars::schema::SchemaObject {
304+
metadata: Some(Box::new(schemars::schema::Metadata {
305+
title: Some(
306+
"The name of a Zpool".to_string(),
307+
),
308+
description: Some(
309+
"Zpool names are of the format ox{i,p}_<UUID>. They are either \
310+
Internal or External, and should be unique"
311+
.to_string(),
312+
),
313+
..Default::default()
314+
})),
315+
instance_type: Some(schemars::schema::InstanceType::String.into()),
316+
string: Some(Box::new(schemars::schema::StringValidation {
317+
pattern: Some(ZPOOL_NAME_REGEX.to_owned()),
318+
..Default::default()
319+
})),
320+
..Default::default()
321+
}
322+
.into()
323+
}
324+
}
325+
293326
impl ZpoolName {
294327
pub fn new_internal(id: Uuid) -> Self {
295328
Self { id, kind: ZpoolKind::Internal }
@@ -374,6 +407,70 @@ impl fmt::Display for ZpoolName {
374407
mod test {
375408
use super::*;
376409

410+
#[test]
411+
fn test_zpool_name_regex() {
412+
let valid = [
413+
"oxi_d462a7f7-b628-40fe-80ff-4e4189e2d62b",
414+
"oxp_d462a7f7-b628-40fe-80ff-4e4189e2d62b",
415+
];
416+
417+
let invalid = [
418+
"",
419+
// Whitespace
420+
" oxp_d462a7f7-b628-40fe-80ff-4e4189e2d62b",
421+
"oxp_d462a7f7-b628-40fe-80ff-4e4189e2d62b ",
422+
// Case sensitivity
423+
"oxp_D462A7F7-b628-40fe-80ff-4e4189e2d62b",
424+
// Bad prefix
425+
"ox_d462a7f7-b628-40fe-80ff-4e4189e2d62b",
426+
"oxa_d462a7f7-b628-40fe-80ff-4e4189e2d62b",
427+
"oxi-d462a7f7-b628-40fe-80ff-4e4189e2d62b",
428+
"oxp-d462a7f7-b628-40fe-80ff-4e4189e2d62b",
429+
// Missing Prefix
430+
"d462a7f7-b628-40fe-80ff-4e4189e2d62b",
431+
// Bad UUIDs (Not following UUIDv4 format)
432+
"oxi_d462a7f7-b628-30fe-80ff-4e4189e2d62b",
433+
"oxi_d462a7f7-b628-40fe-c0ff-4e4189e2d62b",
434+
];
435+
436+
let r = regress::Regex::new(ZPOOL_NAME_REGEX)
437+
.expect("validation regex is valid");
438+
for input in valid {
439+
let m = r
440+
.find(input)
441+
.unwrap_or_else(|| panic!("input {input} did not match regex"));
442+
assert_eq!(m.start(), 0, "input {input} did not match start");
443+
assert_eq!(m.end(), input.len(), "input {input} did not match end");
444+
}
445+
446+
for input in invalid {
447+
assert!(
448+
r.find(input).is_none(),
449+
"invalid input {input} should not match validation regex"
450+
);
451+
}
452+
}
453+
454+
#[test]
455+
fn test_parse_zpool_name_json() {
456+
#[derive(Serialize, Deserialize, JsonSchema)]
457+
struct TestDataset {
458+
pool_name: ZpoolName,
459+
}
460+
461+
// Confirm that we can convert from a JSON string to a a ZpoolName
462+
let json_string =
463+
r#"{"pool_name":"oxi_d462a7f7-b628-40fe-80ff-4e4189e2d62b"}"#;
464+
let dataset: TestDataset = serde_json::from_str(json_string)
465+
.expect("Could not parse ZpoolName from Json Object");
466+
assert!(matches!(dataset.pool_name.kind, ZpoolKind::Internal));
467+
468+
// Confirm we can go the other way (ZpoolName to JSON string) too.
469+
let j = serde_json::to_string(&dataset)
470+
.expect("Cannot convert back to JSON string");
471+
assert_eq!(j, json_string);
472+
}
473+
377474
fn toml_string(s: &str) -> String {
378475
format!("zpool_name = \"{}\"", s)
379476
}

‎nexus/db-model/src/dataset_kind.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ impl_enum_type!(
1919
Crucible => b"crucible"
2020
Cockroach => b"cockroach"
2121
Clickhouse => b"clickhouse"
22+
ExternalDns => b"external_dns"
23+
InternalDns => b"internal_dns"
2224
);
2325

2426
impl From<internal_api::params::DatasetKind> for DatasetKind {
@@ -33,6 +35,12 @@ impl From<internal_api::params::DatasetKind> for DatasetKind {
3335
internal_api::params::DatasetKind::Clickhouse => {
3436
DatasetKind::Clickhouse
3537
}
38+
internal_api::params::DatasetKind::ExternalDns => {
39+
DatasetKind::ExternalDns
40+
}
41+
internal_api::params::DatasetKind::InternalDns => {
42+
DatasetKind::InternalDns
43+
}
3644
}
3745
}
3846
}

‎nexus/db-model/src/service_kind.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ impl_enum_type!(
1717
pub enum ServiceKind;
1818

1919
// Enum values
20+
Clickhouse => b"clickhouse"
21+
Cockroach => b"cockroach"
22+
Crucible => b"crucible"
2023
CruciblePantry => b"crucible_pantry"
2124
Dendrite => b"dendrite"
2225
ExternalDns => b"external_dns"
@@ -48,6 +51,15 @@ impl From<ServiceUsingCertificate> for ServiceKind {
4851
impl From<internal_api::params::ServiceKind> for ServiceKind {
4952
fn from(k: internal_api::params::ServiceKind) -> Self {
5053
match k {
54+
internal_api::params::ServiceKind::Clickhouse => {
55+
ServiceKind::Clickhouse
56+
}
57+
internal_api::params::ServiceKind::Cockroach => {
58+
ServiceKind::Cockroach
59+
}
60+
internal_api::params::ServiceKind::Crucible => {
61+
ServiceKind::Crucible
62+
}
5163
internal_api::params::ServiceKind::ExternalDns { .. } => {
5264
ServiceKind::ExternalDns
5365
}

‎nexus/types/src/internal_api/params.rs‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use std::fmt;
1717
use std::net::IpAddr;
1818
use std::net::SocketAddr;
1919
use std::net::SocketAddrV6;
20-
use std::str::FromStr;
2120
use uuid::Uuid;
2221

2322
/// Describes the role of the sled within the rack.
@@ -134,6 +133,8 @@ pub enum DatasetKind {
134133
Crucible,
135134
Cockroach,
136135
Clickhouse,
136+
ExternalDns,
137+
InternalDns,
137138
}
138139

139140
impl fmt::Display for DatasetKind {
@@ -143,27 +144,13 @@ impl fmt::Display for DatasetKind {
143144
Crucible => "crucible",
144145
Cockroach => "cockroach",
145146
Clickhouse => "clickhouse",
147+
ExternalDns => "external_dns",
148+
InternalDns => "internal_dns",
146149
};
147150
write!(f, "{}", s)
148151
}
149152
}
150153

151-
impl FromStr for DatasetKind {
152-
type Err = omicron_common::api::external::Error;
153-
154-
fn from_str(s: &str) -> Result<Self, Self::Err> {
155-
use DatasetKind::*;
156-
match s {
157-
"crucible" => Ok(Crucible),
158-
"cockroach" => Ok(Cockroach),
159-
"clickhouse" => Ok(Clickhouse),
160-
_ => Err(Self::Err::InternalError {
161-
internal_message: format!("Unknown dataset kind: {}", s),
162-
}),
163-
}
164-
}
165-
}
166-
167154
/// Describes a dataset within a pool.
168155
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
169156
pub struct DatasetPutRequest {
@@ -188,13 +175,16 @@ pub struct ServiceNic {
188175
#[derive(Debug, Serialize, Deserialize, JsonSchema, Clone, PartialEq, Eq)]
189176
#[serde(rename_all = "snake_case", tag = "type", content = "content")]
190177
pub enum ServiceKind {
178+
Clickhouse,
179+
Cockroach,
180+
Crucible,
181+
CruciblePantry,
191182
ExternalDns { external_address: IpAddr, nic: ServiceNic },
192183
InternalDns,
193184
Nexus { external_address: IpAddr, nic: ServiceNic },
194185
Oximeter,
195186
Dendrite,
196187
Tfport,
197-
CruciblePantry,
198188
BoundaryNtp { snat: SourceNatConfig, nic: ServiceNic },
199189
InternalNtp,
200190
}
@@ -203,6 +193,9 @@ impl fmt::Display for ServiceKind {
203193
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
204194
use ServiceKind::*;
205195
let s = match self {
196+
Clickhouse => "clickhouse",
197+
Cockroach => "cockroach",
198+
Crucible => "crucible",
206199
ExternalDns { .. } => "external_dns",
207200
InternalDns => "internal_dns",
208201
Nexus { .. } => "nexus",

‎openapi/nexus-internal.json‎

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -921,7 +921,9 @@
921921
"enum": [
922922
"crucible",
923923
"cockroach",
924-
"clickhouse"
924+
"clickhouse",
925+
"external_dns",
926+
"internal_dns"
925927
]
926928
},
927929
"DatasetPutRequest": {
@@ -2775,6 +2777,62 @@
27752777
"ServiceKind": {
27762778
"description": "Describes the purpose of the service.",
27772779
"oneOf": [
2780+
{
2781+
"type": "object",
2782+
"properties": {
2783+
"type": {
2784+
"type": "string",
2785+
"enum": [
2786+
"clickhouse"
2787+
]
2788+
}
2789+
},
2790+
"required": [
2791+
"type"
2792+
]
2793+
},
2794+
{
2795+
"type": "object",
2796+
"properties": {
2797+
"type": {
2798+
"type": "string",
2799+
"enum": [
2800+
"cockroach"
2801+
]
2802+
}
2803+
},
2804+
"required": [
2805+
"type"
2806+
]
2807+
},
2808+
{
2809+
"type": "object",
2810+
"properties": {
2811+
"type": {
2812+
"type": "string",
2813+
"enum": [
2814+
"crucible"
2815+
]
2816+
}
2817+
},
2818+
"required": [
2819+
"type"
2820+
]
2821+
},
2822+
{
2823+
"type": "object",
2824+
"properties": {
2825+
"type": {
2826+
"type": "string",
2827+
"enum": [
2828+
"crucible_pantry"
2829+
]
2830+
}
2831+
},
2832+
"required": [
2833+
"type"
2834+
]
2835+
},
27782836
{
27792837
"type": "object",
27802838
"properties": {
@@ -2893,20 +2951,6 @@
28932951
"type"
28942952
]
28952953
},
2896-
{
2897-
"type": "object",
2898-
"properties": {
2899-
"type": {
2900-
"type": "string",
2901-
"enum": [
2902-
"crucible_pantry"
2903-
]
2904-
}
2905-
},
2906-
"required": [
2907-
"type"
2908-
]
2909-
},
29102954
{
29112955
"type": "object",
29122956
"properties": {

0 commit comments

Comments
 (0)