Skip to content

Commit 27cb16e

Browse files
committed
Add queries to allocate external IPv6 addresses
- Rewrite the `NextExternalIp` query to allow IPv6 address allocations. This uses queries more like the existing "next-item" queries based on a self-join, where we're joining the existing address with those addresses plus-one, and taking the first free one. - Handle a few more failure-cases from the new query to ensure we detect address exhaustion, reallocation, and so on. All the existing tests continue to pass. - Add expectorate / explain "tests" for the new queries - Add a few new tests, some specifically for IPv6 address allocations and the details / corner cases of the new query structure - Closes #9245 - Closes #1468 - Closes #1371
1 parent a65cda6 commit 27cb16e

File tree

11 files changed

+1411
-509
lines changed

11 files changed

+1411
-509
lines changed

nexus/db-model/src/external_ip.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ impl IncompleteExternalIp {
373373
pool_id,
374374
project_id: Some(project_id),
375375
explicit_ip: Some(explicit_ip.into()),
376-
explicit_port_range: None,
376+
explicit_port_range: Some((0, u16::MAX.into())),
377377
state: kind.initial_state(),
378378
}
379379
}
@@ -398,7 +398,7 @@ impl IncompleteExternalIp {
398398

399399
(
400400
IpKind::Floating,
401-
None,
401+
Some((0, u16::MAX.into())),
402402
Some(name),
403403
Some(zone_kind.report_str().to_string()),
404404
state,

nexus/db-model/src/schema_versions.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
1616
///
1717
/// This must be updated when you change the database schema. Refer to
1818
/// schema/crdb/README.adoc in the root of this repository for details.
19-
pub const SCHEMA_VERSION: Version = Version::new(201, 0, 0);
19+
pub const SCHEMA_VERSION: Version = Version::new(202, 0, 0);
2020

2121
/// List of all past database schema versions, in *reverse* order
2222
///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
2828
// | leaving the first copy as an example for the next person.
2929
// v
3030
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
31+
KnownVersion::new(202, "add-ip-to-external-ip-index"),
3132
KnownVersion::new(201, "scim-client-bearer-token"),
3233
KnownVersion::new(200, "dual-stack-network-interfaces"),
3334
KnownVersion::new(199, "multicast-pool-support"),

nexus/db-queries/src/db/datastore/external_ip.rs

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -243,41 +243,69 @@ impl DataStore {
243243
conn: &async_bb8_diesel::Connection<DbConnection>,
244244
data: IncompleteExternalIp,
245245
) -> Result<ExternalIp, TransactionError<Error>> {
246-
use diesel::result::DatabaseErrorKind::UniqueViolation;
247246
// Name needs to be cloned out here (if present) to give users a
248247
// sensible error message on name collision.
249248
let name = data.name().clone();
250249
let explicit_ip = data.explicit_ip().is_some();
251250
NextExternalIp::new(data).get_result_async(conn).await.map_err(|e| {
251+
use diesel::result::DatabaseErrorKind::NotNullViolation;
252+
use diesel::result::DatabaseErrorKind::UniqueViolation;
252253
use diesel::result::Error::DatabaseError;
253254
use diesel::result::Error::NotFound;
255+
let emit_err_msg = |explicit_ip: bool,
256+
msg: &str|
257+
-> TransactionError<Error> {
258+
if explicit_ip {
259+
TransactionError::CustomError(Error::invalid_request(
260+
"Requested external IP address not available",
261+
))
262+
} else {
263+
TransactionError::CustomError(Error::insufficient_capacity(
264+
"No external IP addresses available",
265+
msg,
266+
))
267+
}
268+
};
254269
match e {
255-
NotFound => {
256-
if explicit_ip {
270+
DatabaseError(NotNullViolation, ref info)
271+
if info.message().contains("in column \"ip\"") =>
272+
{
273+
emit_err_msg(
274+
explicit_ip,
275+
"NextExternalIp::new tried to insert NULL ip",
276+
)
277+
}
278+
NotFound => emit_err_msg(
279+
explicit_ip,
280+
"NextExternalIp::new returned NotFound",
281+
),
282+
DatabaseError(UniqueViolation, ref info) => {
283+
// Attempt to re-use same IP address.
284+
if info.constraint_name() == Some("external_ip_unique") {
257285
TransactionError::CustomError(Error::invalid_request(
258286
"Requested external IP address not available",
259287
))
288+
// Floating IP: name conflict
289+
} else if info
290+
.constraint_name()
291+
.map(|name| name.starts_with("lookup_floating_"))
292+
.unwrap_or(false)
293+
{
294+
TransactionError::CustomError(public_error_from_diesel(
295+
e,
296+
ErrorHandler::Conflict(
297+
ResourceType::FloatingIp,
298+
name.as_ref()
299+
.map(|m| m.as_str())
300+
.unwrap_or_default(),
301+
),
302+
))
260303
} else {
261304
TransactionError::CustomError(
262-
Error::insufficient_capacity(
263-
"No external IP addresses available",
264-
"NextExternalIp::new returned NotFound",
265-
),
305+
crate::db::queries::external_ip::from_diesel(e),
266306
)
267307
}
268308
}
269-
// Floating IP: name conflict
270-
DatabaseError(UniqueViolation, ..) if name.is_some() => {
271-
TransactionError::CustomError(public_error_from_diesel(
272-
e,
273-
ErrorHandler::Conflict(
274-
ResourceType::FloatingIp,
275-
name.as_ref()
276-
.map(|m| m.as_str())
277-
.unwrap_or_default(),
278-
),
279-
))
280-
}
281309
_ => {
282310
if retryable(&e) {
283311
return TransactionError::Database(e);

0 commit comments

Comments
 (0)