Skip to content

Commit

Permalink
[nexus] Make 'dataset' columns for IP address optional (#6055)
Browse files Browse the repository at this point in the history 
This is part of an effort to make datasets usable without an explicit
service managing them (e.g., in the context of Support Bundles).

Related to #6042
Fixes #2000
  • Loading branch information
@smklein
smklein authored Jul 18, 2024
1 parent c5ed4de commit c04e1ac
Show file tree
Hide file tree
Showing 20 changed files with 170 additions and 90 deletions.
24 changes: 17 additions & 7 deletions dev-tools/omdb/src/bin/omdb/db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2879,7 +2879,12 @@ async fn cmd_db_validate_region_snapshots(
use crucible_agent_client::types::State;
use crucible_agent_client::Client as CrucibleAgentClient;

let url = format!("http://{}", dataset.address());
let Some(dataset_addr) = dataset.address() else {
eprintln!("Dataset {} missing an IP address", dataset.id());
continue;
};

let url = format!("http://{}", dataset_addr);
let client = CrucibleAgentClient::new(&url);

let actual_region_snapshots = client
Expand Down Expand Up @@ -2940,7 +2945,7 @@ async fn cmd_db_validate_region_snapshots(
dataset_id: region_snapshot.dataset_id,
region_id: region_snapshot.region_id,
snapshot_id: region_snapshot.snapshot_id,
dataset_addr: dataset.address(),
dataset_addr,
error: String::from(
"region snapshot was deleted, please remove its record",
),
Expand All @@ -2955,7 +2960,7 @@ async fn cmd_db_validate_region_snapshots(
dataset_id: region_snapshot.dataset_id,
region_id: region_snapshot.region_id,
snapshot_id: region_snapshot.snapshot_id,
dataset_addr: dataset.address(),
dataset_addr,
error: String::from(
"NEXUS BUG: region snapshot was deleted, but the higher level snapshot was not!",
),
Expand Down Expand Up @@ -2984,7 +2989,7 @@ async fn cmd_db_validate_region_snapshots(
dataset_id: region_snapshot.dataset_id,
region_id: region_snapshot.region_id,
snapshot_id: region_snapshot.snapshot_id,
dataset_addr: dataset.address(),
dataset_addr,
error: format!(
"AGENT BUG: region snapshot was deleted but has a running snapshot in state {:?}!",
running_snapshot.state,
Expand Down Expand Up @@ -3034,7 +3039,12 @@ async fn cmd_db_validate_region_snapshots(
use crucible_agent_client::types::State;
use crucible_agent_client::Client as CrucibleAgentClient;

let url = format!("http://{}", dataset.address());
let Some(dataset_addr) = dataset.address() else {
eprintln!("Dataset {} missing an IP address", dataset.id());
continue;
};

let url = format!("http://{}", dataset_addr);
let client = CrucibleAgentClient::new(&url);

let actual_region_snapshots = client
Expand All @@ -3052,7 +3062,7 @@ async fn cmd_db_validate_region_snapshots(
dataset_id: dataset.id(),
region_id: region.id(),
snapshot_id,
dataset_addr: dataset.address(),
dataset_addr,
error: String::from(
"Nexus does not know about this snapshot!",
),
Expand All @@ -3077,7 +3087,7 @@ async fn cmd_db_validate_region_snapshots(
dataset_id: dataset.id(),
region_id: region.id(),
snapshot_id,
dataset_addr: dataset.address(),
dataset_addr,
error: String::from(
"Nexus does not know about this running snapshot!"
),
Expand Down
18 changes: 9 additions & 9 deletions nexus/db-model/src/dataset.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ pub struct Dataset {

pub pool_id: Uuid,

ip: ipv6::Ipv6Addr,
port: SqlU16,
ip: Option<ipv6::Ipv6Addr>,
port: Option<SqlU16>,

pub kind: DatasetKind,
pub size_used: Option<i64>,
Expand All @@ -47,7 +47,7 @@ impl Dataset {
pub fn new(
id: Uuid,
pool_id: Uuid,
addr: SocketAddrV6,
addr: Option<SocketAddrV6>,
kind: DatasetKind,
) -> Self {
let size_used = match kind {
Expand All @@ -59,19 +59,19 @@ impl Dataset {
time_deleted: None,
rcgen: Generation::new(),
pool_id,
ip: addr.ip().into(),
port: addr.port().into(),
ip: addr.map(|addr| addr.ip().into()),
port: addr.map(|addr| addr.port().into()),
kind,
size_used,
}
}

pub fn address(&self) -> SocketAddrV6 {
self.address_with_port(self.port.into())
pub fn address(&self) -> Option<SocketAddrV6> {
self.address_with_port(self.port?.into())
}

pub fn address_with_port(&self, port: u16) -> SocketAddrV6 {
SocketAddrV6::new(Ipv6Addr::from(self.ip), port, 0, 0)
pub fn address_with_port(&self, port: u16) -> Option<SocketAddrV6> {
Some(SocketAddrV6::new(Ipv6Addr::from(self.ip?), port, 0, 0))
}
}

Expand Down
4 changes: 2 additions & 2 deletions nexus/db-model/src/schema.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1022,8 +1022,8 @@ table! {

pool_id -> Uuid,

ip -> Inet,
port -> Int4,
ip -> Nullable<Inet>,
port -> Nullable<Int4>,

kind -> crate::DatasetKindEnum,
size_used -> Nullable<Int8>,
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-model/src/schema_versions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::collections::BTreeMap;
///
/// This must be updated when you change the database schema. Refer to
/// schema/crdb/README.adoc in the root of this repository for details.
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(82, 0, 0);
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(83, 0, 0);

/// List of all past database schema versions, in *reverse* order
///
Expand All @@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
// | leaving the first copy as an example for the next person.
// v
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
KnownVersion::new(83, "dataset-address-optional"),
KnownVersion::new(82, "region-port"),
KnownVersion::new(81, "add-nullable-filesystem-pool"),
KnownVersion::new(80, "add-instance-id-to-migrations"),
Expand Down
8 changes: 4 additions & 4 deletions nexus/db-queries/src/db/datastore/dataset.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ mod test {
.dataset_insert_if_not_exists(Dataset::new(
Uuid::new_v4(),
zpool_id,
"[::1]:0".parse().unwrap(),
Some("[::1]:0".parse().unwrap()),
DatasetKind::Crucible,
))
.await
Expand Down Expand Up @@ -323,7 +323,7 @@ mod test {
.dataset_insert_if_not_exists(Dataset::new(
dataset1.id(),
zpool_id,
"[::1]:12345".parse().unwrap(),
Some("[::1]:12345".parse().unwrap()),
DatasetKind::Cockroach,
))
.await
Expand All @@ -339,7 +339,7 @@ mod test {
.dataset_upsert(Dataset::new(
Uuid::new_v4(),
zpool_id,
"[::1]:0".parse().unwrap(),
Some("[::1]:0".parse().unwrap()),
DatasetKind::Cockroach,
))
.await
Expand Down Expand Up @@ -371,7 +371,7 @@ mod test {
.dataset_insert_if_not_exists(Dataset::new(
dataset1.id(),
zpool_id,
"[::1]:12345".parse().unwrap(),
Some("[::1]:12345".parse().unwrap()),
DatasetKind::Cockroach,
))
.await
Expand Down
12 changes: 8 additions & 4 deletions nexus/db-queries/src/db/datastore/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,8 @@ mod test {
.collect()
.await;

let bogus_addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 8080, 0, 0);
let bogus_addr =
Some(SocketAddrV6::new(Ipv6Addr::LOCALHOST, 8080, 0, 0));

let datasets = stream::iter(zpools)
.map(|zpool| {
Expand Down Expand Up @@ -1266,7 +1267,8 @@ mod test {
.collect()
.await;

let bogus_addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 8080, 0, 0);
let bogus_addr =
Some(SocketAddrV6::new(Ipv6Addr::LOCALHOST, 8080, 0, 0));

// 1 dataset per zpool
stream::iter(zpool_ids.clone())
Expand Down Expand Up @@ -1365,7 +1367,8 @@ mod test {
.collect()
.await;

let bogus_addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 8080, 0, 0);
let bogus_addr =
Some(SocketAddrV6::new(Ipv6Addr::LOCALHOST, 8080, 0, 0));

// 1 dataset per zpool
stream::iter(zpool_ids)
Expand Down Expand Up @@ -1444,7 +1447,8 @@ mod test {
physical_disk_id,
)
.await;
let bogus_addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 8080, 0, 0);
let bogus_addr =
Some(SocketAddrV6::new(Ipv6Addr::LOCALHOST, 8080, 0, 0));
let dataset = Dataset::new(
Uuid::new_v4(),
zpool_id,
Expand Down
8 changes: 7 additions & 1 deletion nexus/db-queries/src/db/datastore/region.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,13 @@ impl DataStore {

let dataset = self.dataset_get(region.dataset_id()).await?;

Ok(Some(SocketAddrV6::new(*dataset.address().ip(), port, 0, 0)))
let Some(address) = dataset.address() else {
return Err(Error::internal_error(
"Dataset for Crucible region does know IP address",
));
};

Ok(Some(SocketAddrV6::new(*address.ip(), port, 0, 0)))
}

pub async fn regions_missing_ports(
Expand Down
14 changes: 8 additions & 6 deletions nexus/db-queries/src/db/datastore/volume.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1164,12 +1164,14 @@ impl DataStore {

let mut targets: Vec<SocketAddrV6> = vec![];

find_matching_rw_regions_in_volume(
&vcr,
dataset.address().ip(),
&mut targets,
)
.map_err(|e| Error::internal_error(&e.to_string()))?;
let Some(address) = dataset.address() else {
return Err(Error::internal_error(
"Crucible Dataset missing IP address",
));
};

find_matching_rw_regions_in_volume(&vcr, address.ip(), &mut targets)
.map_err(|e| Error::internal_error(&e.to_string()))?;

Ok(targets)
}
Expand Down
2 changes: 1 addition & 1 deletion nexus/reconfigurator/execution/src/datasets.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub(crate) async fn ensure_dataset_records_exist(
let dataset = Dataset::new(
id.into_untyped_uuid(),
pool_id.into_untyped_uuid(),
address,
Some(address),
kind.into(),
);
let maybe_inserted = datastore
Expand Down
41 changes: 24 additions & 17 deletions nexus/src/app/background/tasks/lookup_region_port.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,26 +91,33 @@ impl BackgroundTask for LookupRegionPort {
}
};

let returned_region = match get_region_from_agent(
&dataset.address(),
region.id(),
)
.await
{
Ok(returned_region) => returned_region,
let Some(dataset_addr) = dataset.address() else {
let s = format!(
"Missing dataset address for dataset: {dataset_id}"
);
error!(log, "{s}");
status.errors.push(s);
continue;
};

Err(e) => {
let s = format!(
"could not get region {} from agent: {e}",
region.id(),
);
let returned_region =
match get_region_from_agent(&dataset_addr, region.id())
.await
{
Ok(returned_region) => returned_region,

error!(log, "{s}");
status.errors.push(s);
Err(e) => {
let s = format!(
"could not get region {} from agent: {e}",
region.id(),
);

continue;
}
};
error!(log, "{s}");
status.errors.push(s);

continue;
}
};

match self
.datastore
Expand Down
26 changes: 16 additions & 10 deletions nexus/src/app/crucible.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,17 @@ impl super::Nexus {
fn crucible_agent_client_for_dataset(
&self,
dataset: &db::model::Dataset,
) -> CrucibleAgentClient {
CrucibleAgentClient::new_with_client(
&format!("http://{}", dataset.address()),
) -> Result<CrucibleAgentClient, Error> {
let Some(addr) = dataset.address() else {
return Err(Error::internal_error(
"Missing crucible dataset address",
));
};

Ok(CrucibleAgentClient::new_with_client(
&format!("http://{}", addr),
self.reqwest_client.clone(),
)
))
}

/// Return if the Crucible agent is expected to be there and answer Nexus:
Expand Down Expand Up @@ -147,7 +153,7 @@ impl super::Nexus {
dataset: &db::model::Dataset,
region: &db::model::Region,
) -> Result<Region, Error> {
let client = self.crucible_agent_client_for_dataset(dataset);
let client = self.crucible_agent_client_for_dataset(dataset)?;
let dataset_id = dataset.id();

let Ok(extent_count) = u32::try_from(region.extent_count()) else {
Expand Down Expand Up @@ -261,7 +267,7 @@ impl super::Nexus {
dataset: &db::model::Dataset,
region_id: Uuid,
) -> Result<Option<Region>, Error> {
let client = self.crucible_agent_client_for_dataset(dataset);
let client = self.crucible_agent_client_for_dataset(dataset)?;
let dataset_id = dataset.id();

let result = ProgenitorOperationRetry::new(
Expand Down Expand Up @@ -303,7 +309,7 @@ impl super::Nexus {
dataset: &db::model::Dataset,
region_id: Uuid,
) -> Result<GetSnapshotResponse, Error> {
let client = self.crucible_agent_client_for_dataset(dataset);
let client = self.crucible_agent_client_for_dataset(dataset)?;
let dataset_id = dataset.id();

let result = ProgenitorOperationRetry::new(
Expand Down Expand Up @@ -343,7 +349,7 @@ impl super::Nexus {
dataset: &db::model::Dataset,
region_id: Uuid,
) -> Result<(), Error> {
let client = self.crucible_agent_client_for_dataset(dataset);
let client = self.crucible_agent_client_for_dataset(dataset)?;
let dataset_id = dataset.id();

let result = ProgenitorOperationRetry::new(
Expand Down Expand Up @@ -386,7 +392,7 @@ impl super::Nexus {
region_id: Uuid,
snapshot_id: Uuid,
) -> Result<(), Error> {
let client = self.crucible_agent_client_for_dataset(dataset);
let client = self.crucible_agent_client_for_dataset(dataset)?;
let dataset_id = dataset.id();

let result = ProgenitorOperationRetry::new(
Expand Down Expand Up @@ -435,7 +441,7 @@ impl super::Nexus {
region_id: Uuid,
snapshot_id: Uuid,
) -> Result<(), Error> {
let client = self.crucible_agent_client_for_dataset(dataset);
let client = self.crucible_agent_client_for_dataset(dataset)?;
let dataset_id = dataset.id();

let result = ProgenitorOperationRetry::new(
Expand Down
Loading

0 comments on commit c04e1ac

Please sign in to comment.