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.

Sign up for GitHub

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

[nexus] Make 'dataset' columns for IP address optional #6055

Merged
merged 5 commits into from
Jul 18, 2024
Merged

[nexus] Make 'dataset' columns for IP address optional #6055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8880e02
[nexus] Make 'dataset' columns for IP address optional
smklein Jul 11, 2024
d8ca014
Merge branch 'main' into dataset-address-optional
smklein Jul 12, 2024
0b939de
Fix merge conflict
smklein Jul 12, 2024
340a20b
Merge branch 'main' into dataset-address-optional
smklein Jul 18, 2024
5122118
Add constraint
smklein Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions dev-tools/omdb/src/bin/omdb/db.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading