diff --git a/common/src/address.rs b/common/src/address.rs index 152fb9319e..3a470c189b 100644 --- a/common/src/address.rs +++ b/common/src/address.rs @@ -44,8 +44,6 @@ pub const DNS_PORT: u16 = 53; pub const DNS_HTTP_PORT: u16 = 5353; pub const SLED_AGENT_PORT: u16 = 12345; -/// The port propolis-server listens on inside the propolis zone. -pub const PROPOLIS_PORT: u16 = 12400; pub const COCKROACH_PORT: u16 = 32221; pub const CRUCIBLE_PORT: u16 = 32345; pub const CLICKHOUSE_PORT: u16 = 8123; diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index d8344c2258..55d3e9b43f 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -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(37, 0, 1); +pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(38, 0, 0); table! { disk (id) { @@ -380,6 +380,7 @@ table! { instance_id -> Uuid, sled_id -> Uuid, propolis_ip -> Inet, + propolis_port -> Int4, state -> crate::InstanceStateEnum, time_state_updated -> Timestamptz, state_generation -> Int8, diff --git a/nexus/db-model/src/vmm.rs b/nexus/db-model/src/vmm.rs index fe1158d5bb..708fe034a6 100644 --- a/nexus/db-model/src/vmm.rs +++ b/nexus/db-model/src/vmm.rs @@ -43,6 +43,9 @@ pub struct Vmm { /// The IP address at which this VMM is serving the Propolis server API. pub propolis_ip: ipnetwork::IpNetwork, + /// The socket port on which this VMM is serving the Propolis server API. + pub propolis_port: i32, + /// Runtime state for the VMM. #[diesel(embed)] pub runtime: VmmRuntimeState, @@ -61,6 +64,7 @@ impl Vmm { instance_id: Uuid, sled_id: Uuid, propolis_ip: ipnetwork::IpNetwork, + propolis_port: u16, initial_state: VmmInitialState, ) -> Self { use omicron_common::api::external::InstanceState as ApiInstanceState; @@ -78,6 +82,7 @@ impl Vmm { instance_id, sled_id, propolis_ip, + propolis_port: propolis_port as i32, runtime: VmmRuntimeState { state: InstanceState::new(api_state), time_state_updated: now, diff --git a/nexus/db-queries/src/db/datastore/vmm.rs b/nexus/db-queries/src/db/datastore/vmm.rs index 18afde84f0..b9bfd7697e 100644 --- a/nexus/db-queries/src/db/datastore/vmm.rs +++ b/nexus/db-queries/src/db/datastore/vmm.rs @@ -23,6 +23,7 @@ use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; +use std::net::SocketAddr; use uuid::Uuid; impl DataStore { @@ -134,7 +135,7 @@ impl DataStore { Ok(updated) } - /// Forcibly overwrites the Propolis IP in the supplied VMM's record with + /// Forcibly overwrites the Propolis IP/Port in the supplied VMM's record with /// the supplied Propolis IP. /// /// This is used in tests to overwrite the IP for a VMM that is backed by a @@ -142,15 +143,20 @@ impl DataStore { /// allocated by the instance start procedure. (Unfortunately, this can't be /// marked #[cfg(test)] because the integration tests require this /// functionality.) - pub async fn vmm_overwrite_ip_for_test( + pub async fn vmm_overwrite_addr_for_test( &self, opctx: &OpContext, vmm_id: &Uuid, - new_ip: ipnetwork::IpNetwork, + new_addr: SocketAddr, ) -> UpdateResult { + let new_ip = ipnetwork::IpNetwork::from(new_addr.ip()); + let new_port = new_addr.port(); let vmm = diesel::update(dsl::vmm) .filter(dsl::id.eq(*vmm_id)) - .set(dsl::propolis_ip.eq(new_ip)) + .set(( + dsl::propolis_ip.eq(new_ip), + dsl::propolis_port.eq(new_port as i32), + )) .returning(Vmm::as_returning()) .get_result_async(&*self.pool_connection_authorized(opctx).await?) .await diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 4b52b597ba..8f8f567b7f 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -29,7 +29,6 @@ use nexus_db_queries::db::identity::Resource; use nexus_db_queries::db::lookup; use nexus_db_queries::db::lookup::LookupPath; use nexus_types::external_api::views; -use omicron_common::address::PROPOLIS_PORT; use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::external::ByteCount; use omicron_common::api::external::CreateResult; @@ -1223,7 +1222,7 @@ impl super::Nexus { propolis_id: *propolis_id, propolis_addr: SocketAddr::new( initial_vmm.propolis_ip.ip(), - PROPOLIS_PORT, + initial_vmm.propolis_port as u16, ) .to_string(), }, @@ -1762,7 +1761,7 @@ impl super::Nexus { | InstanceState::Rebooting | InstanceState::Migrating | InstanceState::Repairing => { - Ok(SocketAddr::new(vmm.propolis_ip.ip(), PROPOLIS_PORT)) + Ok(SocketAddr::new(vmm.propolis_ip.ip(), vmm.propolis_port as u16)) } InstanceState::Creating | InstanceState::Starting diff --git a/nexus/src/app/sagas/instance_common.rs b/nexus/src/app/sagas/instance_common.rs index 445abd5daf..e915c026dd 100644 --- a/nexus/src/app/sagas/instance_common.rs +++ b/nexus/src/app/sagas/instance_common.rs @@ -24,6 +24,9 @@ use uuid::Uuid; use super::NexusActionContext; +/// The port propolis-server listens on inside the propolis zone. +const DEFAULT_PROPOLIS_PORT: u16 = 12400; + /// Reserves resources for a new VMM whose instance has `ncpus` guest logical /// processors and `guest_memory` bytes of guest RAM. The selected sled is /// random within the set of sleds allowed by the supplied `constraints`. @@ -98,6 +101,7 @@ pub async fn create_and_insert_vmm_record( instance_id, sled_id, IpAddr::V6(propolis_ip).into(), + DEFAULT_PROPOLIS_PORT, initial_state, ); diff --git a/nexus/src/app/sagas/instance_migrate.rs b/nexus/src/app/sagas/instance_migrate.rs index ff3ff66e78..d5b2da2e4e 100644 --- a/nexus/src/app/sagas/instance_migrate.rs +++ b/nexus/src/app/sagas/instance_migrate.rs @@ -12,7 +12,6 @@ use crate::app::sagas::{ use crate::external_api::params; use nexus_db_queries::db::{identity::Resource, lookup::LookupPath}; use nexus_db_queries::{authn, authz, db}; -use omicron_common::address::PROPOLIS_PORT; use serde::Deserialize; use serde::Serialize; use sled_agent_client::types::{ @@ -424,8 +423,10 @@ async fn sim_instance_migrate( let db_instance = sagactx.lookup::("set_migration_ids")?; - let src_vmm_addr = - SocketAddr::new(params.src_vmm.propolis_ip.ip(), PROPOLIS_PORT); + let src_vmm_addr = SocketAddr::new( + params.src_vmm.propolis_ip.ip(), + params.src_vmm.propolis_port as u16, + ); let src_propolis_id = db_instance.runtime().propolis_id.unwrap(); let dst_vmm = sagactx.lookup::("dst_vmm_record")?; diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 09f91a2288..36cf12fe0f 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -3762,7 +3762,7 @@ async fn test_instance_serial(cptestctx: &ControlPlaneTestContext) { let nexus = &apictx.nexus; let instance_name = "kris-picks"; - cptestctx + let propolis_addr = cptestctx .sled_agent .sled_agent .start_local_mock_propolis_server(&cptestctx.logctx.log) @@ -3819,12 +3819,12 @@ async fn test_instance_serial(cptestctx: &ControlPlaneTestContext) { .runtime() .propolis_id .expect("running instance should have vmm"); - let localhost = std::net::IpAddr::V6(std::net::Ipv6Addr::LOCALHOST); let updated_vmm = datastore - .vmm_overwrite_ip_for_test(&opctx, &propolis_id, localhost.into()) + .vmm_overwrite_addr_for_test(&opctx, &propolis_id, propolis_addr) .await .unwrap(); - assert_eq!(updated_vmm.propolis_ip.ip(), localhost); + assert_eq!(updated_vmm.propolis_ip.ip(), propolis_addr.ip()); + assert_eq!(updated_vmm.propolis_port as u16, propolis_addr.port()); // Query serial output history endpoint // This is the first line of output generated by the mock propolis-server. diff --git a/schema/crdb/38.0.0/up.sql b/schema/crdb/38.0.0/up.sql new file mode 100644 index 0000000000..725a9e2c90 --- /dev/null +++ b/schema/crdb/38.0.0/up.sql @@ -0,0 +1,11 @@ +-- 12400 was the hardcoded PROPOLIS_PORT prior to this addition; default to +-- that for all existing instances when creating the column. +-- in production, this value will always end up still being 12400. +-- however, nexus' testability in scenarios where each "propolis-server" API +-- isn't necessarily served from an illumos zone with its own IP address, but +-- rather in a sled-agent-sim thread during a cargo nextest run? useful to +-- allow some dynamic range there to avoid flakes caused by port collisions. +ALTER TABLE omicron.public.vmm +ADD COLUMN IF NOT EXISTS propolis_port INT4 NOT NULL + CHECK (propolis_port BETWEEN 0 AND 65535) + DEFAULT 12400; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 837be42c35..40a6fd463f 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -3310,7 +3310,8 @@ CREATE TABLE IF NOT EXISTS omicron.public.vmm ( time_state_updated TIMESTAMPTZ NOT NULL, state_generation INT NOT NULL, sled_id UUID NOT NULL, - propolis_ip INET NOT NULL + propolis_ip INET NOT NULL, + propolis_port INT4 NOT NULL CHECK (propolis_port BETWEEN 0 AND 65535) DEFAULT 12400 ); /* @@ -3551,7 +3552,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - ( TRUE, NOW(), NOW(), '37.0.1', NULL) + ( TRUE, NOW(), NOW(), '38.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/sled-agent/src/sim/sled_agent.rs b/sled-agent/src/sim/sled_agent.rs index d4c92fca51..27a06e4617 100644 --- a/sled-agent/src/sim/sled_agent.rs +++ b/sled-agent/src/sim/sled_agent.rs @@ -27,7 +27,6 @@ use illumos_utils::opte::params::{ DeleteVirtualNetworkInterfaceHost, SetVirtualNetworkInterfaceHost, }; use nexus_client::types::PhysicalDiskKind; -use omicron_common::address::PROPOLIS_PORT; use omicron_common::api::external::{ ByteCount, DiskState, Error, Generation, ResourceType, }; @@ -684,14 +683,15 @@ impl SledAgent { } /// Used for integration tests that require a component to talk to a - /// mocked propolis-server API. - // TODO: fix schemas so propolis-server's port isn't hardcoded in nexus - // such that we can run more than one of these. - // (this is only needed by test_instance_serial at present) + /// mocked propolis-server API. Returns the socket on which the dropshot + /// service is listening, which *must* be patched into Nexus with + /// `nexus_db_queries::db::datastore::vmm_overwrite_addr_for_test` after + /// the instance creation saga if functionality touching propolis-server + /// is to be tested (e.g. serial console connection). pub async fn start_local_mock_propolis_server( &self, log: &Logger, - ) -> Result<(), Error> { + ) -> Result { let mut mock_lock = self.mock_propolis.lock().await; if mock_lock.is_some() { return Err(Error::ObjectAlreadyExists { @@ -700,7 +700,7 @@ impl SledAgent { }); } let propolis_bind_address = - SocketAddr::new(Ipv6Addr::LOCALHOST.into(), PROPOLIS_PORT); + SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 0); let dropshot_config = dropshot::ConfigDropshot { bind_address: propolis_bind_address, ..Default::default() @@ -721,12 +721,10 @@ impl SledAgent { Error::unavail(&format!("initializing propolis-server: {}", error)) })? .start(); - let client = propolis_client::Client::new(&format!( - "http://{}", - srv.local_addr() - )); + let addr = srv.local_addr(); + let client = propolis_client::Client::new(&format!("http://{}", addr)); *mock_lock = Some((srv, client)); - Ok(()) + Ok(addr) } pub fn inventory(&self, addr: SocketAddr) -> anyhow::Result {