Skip to content

Commit

Permalink
Addresses for propolis instances at SLED_PREFIX + 0xFFFF (#4777)
Browse files Browse the repository at this point in the history
Rather than allocating instance IPs starting at `SLED_PREFIX` +
`RSS_RESERVED_ADDRESSES` + 1 where the `1` is the sled-agent allocated
address of the GZ, we begin allocation from a larger block:
`SLED_PREFIX` + `CP_SERVICES_RESERVED_ADDRESSES`. This gives us more
room for nexus to allocate control plane services.

Implements #4765
  • Loading branch information
andrewjstone authored Jan 10, 2024
1 parent 4aebef0 commit 0c7be4c
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 45 deletions.
3 changes: 3 additions & 0 deletions common/src/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,9 @@ const GZ_ADDRESS_INDEX: usize = 2;
/// The maximum number of addresses per sled reserved for RSS.
pub const RSS_RESERVED_ADDRESSES: u16 = 32;

// The maximum number of addresses per sled reserved for control plane services.
pub const CP_SERVICES_RESERVED_ADDRESSES: u16 = 0xFFFF;

/// Wraps an [`Ipv6Network`] with a compile-time prefix length.
#[derive(Debug, Clone, Copy, JsonSchema, Serialize, Hash, PartialEq, Eq)]
#[schemars(rename = "Ipv6Subnet")]
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use omicron_common::api::external::SemverVersion;
///
/// This should be updated whenever the schema is changed. For more details,
/// refer to: schema/crdb/README.adoc
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(23, 0, 1);
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(24, 0, 0);

table! {
disk (id) {
Expand Down
6 changes: 4 additions & 2 deletions nexus/db-model/src/sled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub struct Sled {
pub ip: ipv6::Ipv6Addr,
pub port: SqlU16,

/// The last IP address provided to an Oxide service on this sled
/// The last IP address provided to a propolis instance on this sled
pub last_used_address: ipv6::Ipv6Addr,

provision_state: SledProvisionState,
Expand Down Expand Up @@ -183,7 +183,9 @@ impl SledUpdate {
pub fn into_insertable(self) -> Sled {
let last_used_address = {
let mut segments = self.ip().segments();
segments[7] += omicron_common::address::RSS_RESERVED_ADDRESSES;
// We allocate the entire last segment to control plane services
segments[7] =
omicron_common::address::CP_SERVICES_RESERVED_ADDRESSES;
ipv6::Ipv6Addr::from(Ipv6Addr::from(segments))
};
Sled {
Expand Down
38 changes: 5 additions & 33 deletions nexus/db-queries/src/db/datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ impl DataStore {
self.pool_connection_unauthorized().await
}

/// Return the next available IPv6 address for an Oxide service running on
/// the provided sled.
/// Return the next available IPv6 address for a propolis instance running
/// on the provided sled.
pub async fn next_ipv6_address(
&self,
opctx: &OpContext,
Expand Down Expand Up @@ -1286,7 +1286,6 @@ mod test {
// Test sled-specific IPv6 address allocation
#[tokio::test]
async fn test_sled_ipv6_address_allocation() {
use omicron_common::address::RSS_RESERVED_ADDRESSES as STATIC_IPV6_ADDRESS_OFFSET;
use std::net::Ipv6Addr;

let logctx = dev::test_setup_log("test_sled_ipv6_address_allocation");
Expand Down Expand Up @@ -1322,41 +1321,14 @@ mod test {
datastore.sled_upsert(sled2).await.unwrap();

let ip = datastore.next_ipv6_address(&opctx, sled1_id).await.unwrap();
let expected_ip = Ipv6Addr::new(
0xfd00,
0x1de,
0,
0,
0,
0,
0,
2 + STATIC_IPV6_ADDRESS_OFFSET,
);
let expected_ip = Ipv6Addr::new(0xfd00, 0x1de, 0, 0, 0, 0, 1, 0);
assert_eq!(ip, expected_ip);
let ip = datastore.next_ipv6_address(&opctx, sled1_id).await.unwrap();
let expected_ip = Ipv6Addr::new(
0xfd00,
0x1de,
0,
0,
0,
0,
0,
3 + STATIC_IPV6_ADDRESS_OFFSET,
);
let expected_ip = Ipv6Addr::new(0xfd00, 0x1de, 0, 0, 0, 0, 1, 1);
assert_eq!(ip, expected_ip);

let ip = datastore.next_ipv6_address(&opctx, sled2_id).await.unwrap();
let expected_ip = Ipv6Addr::new(
0xfd00,
0x1df,
0,
0,
0,
0,
0,
2 + STATIC_IPV6_ADDRESS_OFFSET,
);
let expected_ip = Ipv6Addr::new(0xfd00, 0x1df, 0, 0, 0, 0, 1, 0);
assert_eq!(ip, expected_ip);

let _ = db.cleanup().await;
Expand Down
6 changes: 3 additions & 3 deletions nexus/src/app/sagas/instance_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ pub async fn destroy_vmm_record(
Ok(())
}

/// Allocates a new IPv6 address for a service that will run on the supplied
/// sled.
pub(super) async fn allocate_sled_ipv6(
/// Allocates a new IPv6 address for a propolis instance that will run on the
/// supplied sled.
pub(super) async fn allocate_vmm_ipv6(
opctx: &OpContext,
datastore: &DataStore,
sled_uuid: Uuid,
Expand Down
4 changes: 2 additions & 2 deletions nexus/src/app/sagas/instance_migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::app::instance::{
InstanceStateChangeError, InstanceStateChangeRequest,
};
use crate::app::sagas::{
declare_saga_actions, instance_common::allocate_sled_ipv6,
declare_saga_actions, instance_common::allocate_vmm_ipv6,
};
use crate::external_api::params;
use nexus_db_queries::db::{identity::Resource, lookup::LookupPath};
Expand Down Expand Up @@ -181,7 +181,7 @@ async fn sim_allocate_propolis_ip(
&sagactx,
&params.serialized_authn,
);
allocate_sled_ipv6(
allocate_vmm_ipv6(
&opctx,
sagactx.user_data().datastore(),
params.migrate_params.dst_sled_id,
Expand Down
4 changes: 2 additions & 2 deletions nexus/src/app/sagas/instance_start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use std::net::Ipv6Addr;

use super::{
instance_common::allocate_sled_ipv6, NexusActionContext, NexusSaga,
instance_common::allocate_vmm_ipv6, NexusActionContext, NexusSaga,
SagaInitError, ACTION_GENERATE_ID,
};
use crate::app::instance::InstanceStateChangeError;
Expand Down Expand Up @@ -159,7 +159,7 @@ async fn sis_alloc_propolis_ip(
&params.serialized_authn,
);
let sled_uuid = sagactx.lookup::<Uuid>("sled_id")?;
allocate_sled_ipv6(&opctx, sagactx.user_data().datastore(), sled_uuid).await
allocate_vmm_ipv6(&opctx, sagactx.user_data().datastore(), sled_uuid).await
}

async fn sis_create_vmm_record(
Expand Down
72 changes: 72 additions & 0 deletions nexus/tests/integration_tests/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use pretty_assertions::{assert_eq, assert_ne};
use similar_asserts;
use slog::Logger;
use std::collections::{BTreeMap, BTreeSet};
use std::net::IpAddr;
use std::path::PathBuf;
use tokio::time::timeout;
use tokio::time::Duration;
Expand Down Expand Up @@ -192,6 +193,7 @@ enum AnySqlType {
String(String),
TextArray(Vec<String>),
Uuid(Uuid),
Inet(IpAddr),
// TODO: This isn't exhaustive, feel free to add more.
//
// These should only be necessary for rows where the database schema changes also choose to
Expand Down Expand Up @@ -234,6 +236,12 @@ impl From<Uuid> for AnySqlType {
}
}

impl From<IpAddr> for AnySqlType {
fn from(value: IpAddr) -> Self {
Self::Inet(value)
}
}

impl AnySqlType {
fn as_str(&self) -> &str {
match self {
Expand Down Expand Up @@ -279,6 +287,9 @@ impl<'a> tokio_postgres::types::FromSql<'a> for AnySqlType {
ty, raw,
)?));
}
if IpAddr::accepts(ty) {
return Ok(AnySqlType::Inet(IpAddr::from_sql(ty, raw)?));
}

use tokio_postgres::types::Kind;
match ty.kind() {
Expand Down Expand Up @@ -941,6 +952,13 @@ const POOL1: Uuid = Uuid::from_u128(0x11116001_5c3d_4647_83b0_8f3515da7be1);
const POOL2: Uuid = Uuid::from_u128(0x22226001_5c3d_4647_83b0_8f3515da7be1);
const POOL3: Uuid = Uuid::from_u128(0x33336001_5c3d_4647_83b0_8f3515da7be1);

// "513D" -> "Sled"
const SLED1: Uuid = Uuid::from_u128(0x1111513d_5c3d_4647_83b0_8f3515da7be1);
const SLED2: Uuid = Uuid::from_u128(0x2222513d_5c3d_4647_83b0_8f3515da7be1);

// "7AC4" -> "Rack"
const RACK1: Uuid = Uuid::from_u128(0x11117ac4_5c3d_4647_83b0_8f3515da7be1);

fn before_23_0_0(client: &Client) -> BoxFuture<'_, ()> {
Box::pin(async move {
// Create two silos
Expand Down Expand Up @@ -1024,6 +1042,56 @@ fn after_23_0_0(client: &Client) -> BoxFuture<'_, ()> {
})
}

fn before_24_0_0(client: &Client) -> BoxFuture<'_, ()> {
// IP addresses were pulled off dogfood sled 16
Box::pin(async move {
// Create two sleds
client
.batch_execute(&format!(
"INSERT INTO sled
(id, time_created, time_modified, time_deleted, rcgen, rack_id,
is_scrimlet, serial_number, part_number, revision,
usable_hardware_threads, usable_physical_ram, reservoir_size, ip,
port, last_used_address, provision_state) VALUES
('{SLED1}', now(), now(), NULL, 1, '{RACK1}', true, 'abcd', 'defg',
'1', 64, 12345678, 77, 'fd00:1122:3344:104::1', 12345,
'fd00:1122:3344:104::1ac', 'provisionable'),
('{SLED2}', now(), now(), NULL, 1, '{RACK1}', false, 'zzzz', 'xxxx',
'2', 64, 12345678, 77,'fd00:1122:3344:107::1', 12345,
'fd00:1122:3344:107::d4', 'provisionable');
"
))
.await
.expect("Failed to create sleds");
})
}

fn after_24_0_0(client: &Client) -> BoxFuture<'_, ()> {
Box::pin(async {
// Confirm that the IP Addresses have the last 2 bytes changed to `0xFFFF`
let rows = client
.query("SELECT last_used_address FROM sled ORDER BY id", &[])
.await
.expect("Failed to sled last_used_address");
let last_used_addresses = process_rows(&rows);

let expected_addr_1: IpAddr =
"fd00:1122:3344:104::ffff".parse().unwrap();
let expected_addr_2: IpAddr =
"fd00:1122:3344:107::ffff".parse().unwrap();

assert_eq!(
last_used_addresses[0].values,
vec![ColumnValue::new("last_used_address", expected_addr_1)]
);
assert_eq!(
last_used_addresses[1].values,
vec![ColumnValue::new("last_used_address", expected_addr_2)]
);
})
}

// Lazily initializes all migration checks. The combination of Rust function
// pointers and async makes defining a static table fairly painful, so we're
// using lazy initialization instead.
Expand All @@ -1037,6 +1105,10 @@ fn get_migration_checks() -> BTreeMap<SemverVersion, DataMigrationFns> {
SemverVersion(semver::Version::parse("23.0.0").unwrap()),
DataMigrationFns { before: Some(before_23_0_0), after: after_23_0_0 },
);
map.insert(
SemverVersion(semver::Version::parse("24.0.0").unwrap()),
DataMigrationFns { before: Some(before_24_0_0), after: after_24_0_0 },
);

map
}
Expand Down
3 changes: 3 additions & 0 deletions schema/crdb/24.0.0/up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
UPDATE omicron.public.sled
SET last_used_address = (netmask(set_masklen(ip, 64)) & ip) + 0xFFFF
WHERE time_deleted is null;
4 changes: 2 additions & 2 deletions schema/crdb/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.sled (
ip INET NOT NULL,
port INT4 CHECK (port BETWEEN 0 AND 65535) NOT NULL,

/* The last address allocated to an Oxide service on this sled. */
/* The last address allocated to a propolis instance on this sled. */
last_used_address INET NOT NULL,

/* The state of whether resources should be provisioned onto the sled */
Expand Down Expand Up @@ -3258,7 +3258,7 @@ INSERT INTO omicron.public.db_metadata (
version,
target_version
) VALUES
( TRUE, NOW(), NOW(), '23.0.1', NULL)
( TRUE, NOW(), NOW(), '24.0.0', NULL)
ON CONFLICT DO NOTHING;

COMMIT;

0 comments on commit 0c7be4c

Please sign in to comment.