From 8f51ea1034d174de666c6ee8444e9439b522f2f7 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 21 Mar 2022 10:23:06 -0400 Subject: [PATCH 1/7] Allocate static Ipv6 addresses for propolis zones Add ability for Nexus to hand static Ipv6 addresses for use by propolis zones. Allocate the address as part of the instance create saga, and the instance migrate saga. Send to sled-agent as part of the instance ensure request so it can attach to the control vnic. TODO left in internal api endpoints, are they required anymore? TODO instance destroy should free IP? or through sled agent? TODO associate IP with instance? --- common/src/sql/dbinit.sql | 7 ++ nexus/src/db/datastore.rs | 132 ++++++++++++++++++++- nexus/src/db/model.rs | 10 +- nexus/src/db/schema.rs | 6 + nexus/src/internal_api/http_entrypoints.rs | 40 ++++++- nexus/src/internal_api/params.rs | 14 ++- nexus/src/nexus.rs | 19 ++- nexus/src/sagas.rs | 61 ++++++++++ openapi/nexus-internal.json | 77 ++++++++++++ openapi/sled-agent.json | 6 + sled-agent/src/http_entrypoints.rs | 1 + sled-agent/src/instance.rs | 17 +++ sled-agent/src/instance_manager.rs | 13 +- sled-agent/src/mocks/mod.rs | 9 +- sled-agent/src/params.rs | 2 + sled-agent/src/sled_agent.rs | 3 +- 16 files changed, 399 insertions(+), 18 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index bb4028ec2fa..736a15ca16e 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -912,6 +912,13 @@ CREATE TABLE omicron.public.role_assignment_builtin ( PRIMARY KEY(user_builtin_id, resource_type, resource_id, role_name) ); +/* + * Store addresses allocated for internal services (like propolis zones) + */ +CREATE TABLE omicron.public.static_v6_address ( + address INET PRIMARY KEY NOT NULL +); + /*******************************************************************/ /* diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 8aabb87ea50..c24e299dc90 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -54,6 +54,7 @@ use omicron_common::api::external::{ use omicron_common::api::internal::nexus::UpdateArtifact; use omicron_common::bail_unless; use std::convert::{TryFrom, TryInto}; +use std::net::IpAddr; use std::sync::Arc; use uuid::Uuid; @@ -66,9 +67,9 @@ use crate::db::{ Name, NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, ProducerEndpoint, Project, ProjectUpdate, Region, RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, RouterRouteUpdate, - Sled, UpdateArtifactKind, UpdateAvailableArtifact, UserBuiltin, Volume, - Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, VpcSubnet, - VpcSubnetUpdate, VpcUpdate, Zpool, + Sled, StaticV6Address, UpdateArtifactKind, UpdateAvailableArtifact, + UserBuiltin, Volume, Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, + VpcSubnet, VpcSubnetUpdate, VpcUpdate, Zpool, }, pagination::paginated, pagination::paginated_multicolumn, @@ -3409,6 +3410,61 @@ impl DataStore { )) }) } + + pub async fn allocate_static_v6_address( + &self, + ) -> Result { + use db::schema::static_v6_address::dsl; + + self.pool() + .transaction(move |conn| { + // Grab the next sequential address + // XXX should this use push_next_available_ip_subquery? + // refactor? + let new_address: Vec = diesel::sql_query("select static_v6_address.address + 1 as address from static_v6_address where not exists (select 1 from static_v6_address tmp where tmp.address = static_v6_address.address + 1) limit 1").load(conn)?; + + let new_address = if new_address.len() != 1 { + // This means there are no addresses in the table! + // XXX normally this should come from something else, hard + // code here. + StaticV6Address { + address: "fd00:1234::101".parse().unwrap(), + } + } else { + new_address[0].clone() + }; + + let new_address = diesel::insert_into(dsl::static_v6_address) + .values(new_address) + .returning(StaticV6Address::as_returning()) + .get_result(conn)?; + + return Ok(new_address.address.ip()); + }) + .await + .map_err(|e: async_bb8_diesel::PoolError| { + Error::internal_error(&format!("Transaction error: {}", e)) + }) + } + + pub async fn free_static_v6_address( + &self, + address: IpAddr, + ) -> UpdateResult<()> { + use db::schema::static_v6_address::dsl; + + diesel::delete(dsl::static_v6_address) + .filter(dsl::address.eq(ipnetwork::IpNetwork::from(address))) + .execute_async(self.pool()) + .await + .map(|_| ()) + .map_err(|e| { + Error::internal_error(&format!( + "error freeing static v6 address: {:?}", + e + )) + }) + } } /// Constructs a DataStore for use in test suites that has preloaded the @@ -3460,7 +3516,7 @@ mod test { }; use omicron_test_utils::dev; use std::collections::HashSet; - use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use std::sync::Arc; use uuid::Uuid; @@ -3877,4 +3933,72 @@ mod test { let _ = db.cleanup().await; } + + // Test static v6 address allocation + #[tokio::test] + async fn test_static_v6_address_allocation() -> Result<(), Error> { + let logctx = dev::test_setup_log("test_static_v6_address_allocation"); + let mut db = test_setup_database(&logctx.log).await; + let cfg = db::Config { url: db.pg_config().clone() }; + let pool = db::Pool::new(&cfg); + let datastore = DataStore::new(Arc::new(pool)); + + // db contents: [] + let address1 = + datastore.allocate_static_v6_address().await?; + // db contents: [101] + assert_eq!(address1, "fd00:1234::101".parse::().unwrap()); + + // db contents: [101] + let address2 = + datastore.allocate_static_v6_address().await?; + // db contents: [101, 102] + assert_eq!(address2, "fd00:1234::102".parse::().unwrap()); + + // db contents: [101, 102] + let address3 = + datastore.allocate_static_v6_address().await?; + // db contents: [101, 102, 103] + assert_eq!(address3, "fd00:1234::103".parse::().unwrap()); + + // db contents: [101, 102, 103] + datastore.free_static_v6_address(address1).await?; + // db contents: [102, 103] + + // db contents: [102, 103] + let address4 = + datastore.allocate_static_v6_address().await?; + // db contents: [102, 103, 104] + assert_eq!(address4, "fd00:1234::104".parse::().unwrap()); + + // db contents: [102, 103, 104] + datastore.free_static_v6_address(address4).await?; + // db contents: [102, 103] + + // db contents: [102, 103] + datastore.free_static_v6_address(address2).await?; + // db contents: [103] + + // db contents: [103] + let address5 = + datastore.allocate_static_v6_address().await?; + // db contents: [103, 104] + assert_eq!(address5, "fd00:1234::104".parse::().unwrap()); + + // db contents: [103, 104] + let address6 = + datastore.allocate_static_v6_address().await?; + // db contents: [103, 104, 105] + assert_eq!(address6, "fd00:1234::105".parse::().unwrap()); + + // db contents: [103, 104, 105] + let address7 = + datastore.allocate_static_v6_address().await?; + // db contents: [103, 104, 105, 106] + assert_eq!(address7, "fd00:1234::106".parse::().unwrap()); + + let _ = db.cleanup().await; + + Ok(()) + } } diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index ebc837db711..9505543e896 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -10,8 +10,8 @@ use crate::db::schema::{ console_session, dataset, disk, instance, metric_producer, network_interface, organization, oximeter, project, rack, region, role_assignment_builtin, role_builtin, router_route, sled, snapshot, - update_available_artifact, user_builtin, volume, vpc, vpc_firewall_rule, - vpc_router, vpc_subnet, zpool, + static_v6_address, update_available_artifact, user_builtin, volume, vpc, + vpc_firewall_rule, vpc_router, vpc_subnet, zpool, }; use crate::defaults; use crate::external_api::params; @@ -2355,6 +2355,12 @@ pub struct UpdateAvailableArtifact { pub target_length: i64, } +#[derive(Queryable, QueryableByName, Insertable, Clone, Debug, Selectable, AsChangeset)] +#[table_name = "static_v6_address"] +pub struct StaticV6Address { + pub address: IpNetwork, +} + #[cfg(test)] mod tests { use super::Uuid; diff --git a/nexus/src/db/schema.rs b/nexus/src/db/schema.rs index 1cab17dd90d..05f91630666 100644 --- a/nexus/src/db/schema.rs +++ b/nexus/src/db/schema.rs @@ -373,6 +373,12 @@ table! { } } +table! { + static_v6_address (address) { + address -> Inet, + } +} + allow_tables_to_appear_in_same_query!( dataset, disk, diff --git a/nexus/src/internal_api/http_entrypoints.rs b/nexus/src/internal_api/http_entrypoints.rs index 4df80916d77..d0c98895c5f 100644 --- a/nexus/src/internal_api/http_entrypoints.rs +++ b/nexus/src/internal_api/http_entrypoints.rs @@ -7,7 +7,8 @@ use crate::context::OpContext; use crate::ServerContext; use super::params::{ - DatasetPutRequest, DatasetPutResponse, OximeterInfo, SledAgentStartupInfo, + AllocateStaticV6AddressResponse, DatasetPutRequest, DatasetPutResponse, + FreeStaticV6AddressParams, OximeterInfo, SledAgentStartupInfo, ZpoolPutRequest, ZpoolPutResponse, }; use dropshot::endpoint; @@ -45,6 +46,8 @@ pub fn internal_api() -> NexusApiDescription { api.register(cpapi_collectors_post)?; api.register(cpapi_metrics_collect)?; api.register(cpapi_artifact_download)?; + api.register(allocate_static_v6_address)?; + api.register(free_static_v6_address)?; Ok(()) } @@ -279,3 +282,38 @@ async fn cpapi_artifact_download( Ok(Response::builder().status(StatusCode::OK).body(body.into())?) } + +/// Endpoint used by Sled Agents to allocate static V6 addresses +#[endpoint { + method = POST, + path = "/static_address/v6", +}] +async fn allocate_static_v6_address( + request_context: Arc>>, +) -> Result, HttpError> { + let context = request_context.context(); + let nexus = &context.nexus; + + Ok(HttpResponseOk(AllocateStaticV6AddressResponse { + address: nexus.allocate_static_v6_address().await?, + })) +} + +/// Endpoint used by Sled Agents to free static V6 addresses +#[endpoint { + method = DELETE, + path = "/static_address/v6/{address}", +}] +async fn free_static_v6_address( + request_context: Arc>>, + path_params: Path, +) -> Result, HttpError> { + let context = request_context.context(); + let nexus = &context.nexus; + + nexus + .free_static_v6_address(path_params.into_inner().address) + .await?; + + Ok(HttpResponseOk(())) +} diff --git a/nexus/src/internal_api/params.rs b/nexus/src/internal_api/params.rs index 96f078b9c6d..a2fde8c7a67 100644 --- a/nexus/src/internal_api/params.rs +++ b/nexus/src/internal_api/params.rs @@ -7,7 +7,7 @@ use omicron_common::api::external::ByteCount; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::fmt; -use std::net::SocketAddr; +use std::net::{IpAddr, SocketAddr}; use std::str::FromStr; use uuid::Uuid; @@ -105,3 +105,15 @@ pub struct OximeterInfo { /// The address on which this oximeter instance listens for requests pub address: SocketAddr, } + +/// Response when allocating a static v6 address +#[derive(Debug, Clone, Copy, JsonSchema, Serialize, Deserialize)] +pub struct AllocateStaticV6AddressResponse { + pub address: IpAddr, +} + +/// Static Ipv6 address to free +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct FreeStaticV6AddressParams { + pub address: IpAddr, +} diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 5d76015800e..d627f20d549 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -69,7 +69,7 @@ use sled_agent_client::types::InstanceStateRequested; use sled_agent_client::Client as SledAgentClient; use slog::Logger; use std::convert::{TryFrom, TryInto}; -use std::net::SocketAddr; +use std::net::{IpAddr, SocketAddr}; use std::num::NonZeroU32; use std::path::Path; use std::sync::Arc; @@ -1263,6 +1263,10 @@ impl Nexus { initial: instance_hardware, target: requested, migrate: None, + // XXX where does this come from? we need to associate the + // control ip with the instance, but the static_v6_address + // table only has one column (the address) + allocated_control_ip: "TODO".parse().unwrap(), }, ) .await @@ -3069,6 +3073,19 @@ impl Nexus { })?; Ok(body) } + + pub async fn allocate_static_v6_address(&self) -> Result { + self.db_datastore + .allocate_static_v6_address() + .await + } + + pub async fn free_static_v6_address( + &self, + address: IpAddr, + ) -> Result<(), Error> { + self.db_datastore.free_static_v6_address(address).await + } } fn generate_session_token() -> String { diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index 01961863918..29eb5332bdd 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -52,6 +52,7 @@ use steno::SagaTemplateBuilder; use steno::SagaTemplateGeneric; use steno::SagaType; use uuid::Uuid; +use std::net::IpAddr; // We'll need a richer mechanism for registering sagas, but this works for now. pub const SAGA_INSTANCE_CREATE_NAME: &'static str = "instance-create"; @@ -190,6 +191,15 @@ pub fn saga_instance_create() -> SagaTemplate { new_action_noop_undo(sic_create_network_interfaces), ); + template_builder.append( + "allocated_control_ip", + "AllocateIpv6ForPropolisControl", + ActionFunc::new_action( + sic_allocate_v6_address, + sic_allocate_v6_address_undo, + ) + ); + template_builder.append( "instance_ensure", "InstanceEnsure", @@ -514,6 +524,42 @@ async fn sic_create_instance_record( Ok(instance.runtime().clone().into()) } +// XXX can't reuse this function? + +async fn sic_allocate_v6_address( + sagactx: ActionContext, +) -> Result { + let osagactx = sagactx.user_data(); + let nexus = osagactx.nexus(); + Ok(nexus.allocate_static_v6_address().await.map_err(ActionError::action_failed)?) +} + +async fn sic_allocate_v6_address_undo( + sagactx: ActionContext, +) -> Result<(), anyhow::Error> { + let osagactx = sagactx.user_data(); + let nexus = osagactx.nexus(); + let ipv6 = sagactx.lookup::("allocated_ipv6")?; + Ok(nexus.free_static_v6_address(ipv6).await?) +} + +async fn sim_allocate_v6_address( + sagactx: ActionContext, +) -> Result { + let osagactx = sagactx.user_data(); + let nexus = osagactx.nexus(); + Ok(nexus.allocate_static_v6_address().await.map_err(ActionError::action_failed)?) +} + +async fn sim_allocate_v6_address_undo( + sagactx: ActionContext, +) -> Result<(), anyhow::Error> { + let osagactx = sagactx.user_data(); + let nexus = osagactx.nexus(); + let ipv6 = sagactx.lookup::("allocated_ipv6")?; + Ok(nexus.free_static_v6_address(ipv6).await?) +} + async fn sic_instance_ensure( sagactx: ActionContext, ) -> Result<(), ActionError> { @@ -538,6 +584,8 @@ async fn sic_instance_ensure( .await .map_err(ActionError::action_failed)?; + let allocated_control_ip = sagactx.lookup::("allocated_control_ip")?; + // Ask the sled agent to begin the state change. Then update the database // to reflect the new intermediate state. If this update is not the newest // one, that's fine. That might just mean the sled agent beat us to it. @@ -548,6 +596,7 @@ async fn sic_instance_ensure( initial: initial_hardware, target: runtime_params, migrate: None, + allocated_control_ip: allocated_control_ip.to_string(), }, ) .await @@ -601,6 +650,15 @@ pub fn saga_instance_migrate() -> SagaTemplate { new_action_noop_undo(sim_migrate_prep), ); + template_builder.append( + "allocated_control_ip", + "AllocateIpv6ForPropolisControl", + ActionFunc::new_action( + sim_allocate_v6_address, + sim_allocate_v6_address_undo, + ) + ); + template_builder.append( "instance_migrate", "InstanceMigrate", @@ -691,6 +749,8 @@ async fn sim_instance_migrate( .await .map_err(ActionError::action_failed)?; + let allocated_control_ip = sagactx.lookup::("allocated_control_ip")?; + let new_runtime_state: InstanceRuntimeState = dst_sa .instance_put( &instance_id, @@ -701,6 +761,7 @@ async fn sim_instance_migrate( src_propolis_addr: src_propolis_addr.to_string(), src_propolis_uuid, }), + allocated_control_ip: allocated_control_ip.to_string(), }, ) .await diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index 4cfadbdfc74..c37c1eadd50 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -323,6 +323,70 @@ } } }, + "/static_address/v6": { + "post": { + "summary": "Endpoint used by Sled Agents to allocate static V6 addresses", + "operationId": "allocate_static_v6_address", + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AllocateStaticV6AddressResponse" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/static_address/v6/{address}": { + "delete": { + "summary": "Endpoint used by Sled Agents to free static V6 addresses", + "operationId": "free_static_v6_address", + "parameters": [ + { + "in": "path", + "name": "address", + "required": true, + "schema": { + "type": "string", + "format": "ip" + }, + "style": "simple" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "title": "Null", + "type": "string", + "enum": [ + null + ] + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/zpools/{zpool_id}/dataset/{dataset_id}": { "put": { "summary": "Report that a dataset within a pool has come online.", @@ -394,6 +458,19 @@ } }, "schemas": { + "AllocateStaticV6AddressResponse": { + "description": "Response when allocating a static v6 address", + "type": "object", + "properties": { + "address": { + "type": "string", + "format": "ip" + } + }, + "required": [ + "address" + ] + }, "BinRangedouble": { "description": "A type storing a range over `T`.\n\nThis type supports ranges similar to the `RangeTo`, `Range` and `RangeFrom` types in the standard library. Those cover `(..end)`, `(start..end)`, and `(start..)` respectively.", "oneOf": [ diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index 2b5a3bc1b95..bdd7e88efdc 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -559,6 +559,11 @@ "description": "Sent to a sled agent to establish the runtime state of an Instance", "type": "object", "properties": { + "allocated_control_ip": { + "description": "Allocated address for propolis's control vnic", + "type": "string", + "format": "ip" + }, "initial": { "description": "Last runtime state of the Instance known to Nexus (used if the agent has never seen this Instance before).", "allOf": [ @@ -586,6 +591,7 @@ } }, "required": [ + "allocated_control_ip", "initial", "target" ] diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs index 11f6d9d2eb3..7cdc53f5c46 100644 --- a/sled-agent/src/http_entrypoints.rs +++ b/sled-agent/src/http_entrypoints.rs @@ -100,6 +100,7 @@ async fn instance_put( body_args.initial, body_args.target, body_args.migrate, + body_args.allocated_control_ip, ) .await .map_err(Error::from)?, diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index 6215138fb88..105d14b9655 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -188,6 +188,9 @@ struct InstanceInner { // Connection to Nexus nexus_client: Arc, + + // Allocated IP for control traffic + allocated_control_ip: std::net::IpAddr, } impl InstanceInner { @@ -360,6 +363,7 @@ mockall::mock! { initial: InstanceHardware, vlan: Option, nexus_client: Arc, + allocated_control_ip: std::net::IpAddr, ) -> Result; pub async fn start( &self, @@ -397,6 +401,7 @@ impl Instance { initial: InstanceHardware, vlan: Option, nexus_client: Arc, + allocated_control_ip: std::net::IpAddr, ) -> Result { info!(log, "Instance::new w/initial HW: {:?}", initial); let instance = InstanceInner { @@ -421,6 +426,7 @@ impl Instance { state: InstanceStates::new(initial.runtime), running_state: None, nexus_client, + allocated_control_ip, }; let inner = Arc::new(Mutex::new(instance)); @@ -476,6 +482,14 @@ impl Instance { let network = running_zone.ensure_address(AddressRequest::Dhcp).await?; info!(inner.log, "Created address {} for zone: {}", network, zname); + let v6 = running_zone + .ensure_address(AddressRequest::new_static( + inner.allocated_control_ip, + None, // XXX what should this be? + )) + .await?; + info!(inner.log, "Created v6 address {} for zone: {}", v6, zname); + // Run Propolis in the Zone. let server_addr = SocketAddr::new(network.ip(), PROPOLIS_PORT); @@ -558,6 +572,8 @@ impl Instance { async fn stop(&self) -> Result<(), Error> { let mut inner = self.inner.lock().await; + // XXX how to free the static V6 address? where do I store it? + let zname = propolis_zone_name(inner.propolis_id()); warn!(inner.log, "Halting and removing zone: {}", zname); Zones::halt_and_remove(&inner.log, &zname).unwrap(); @@ -711,6 +727,7 @@ mod test { new_initial_instance(), None, Arc::new(nexus_client), + "127.0.0.1".parse().unwrap(), ) .unwrap(); diff --git a/sled-agent/src/instance_manager.rs b/sled-agent/src/instance_manager.rs index d8b7e946c84..6e287719c9d 100644 --- a/sled-agent/src/instance_manager.rs +++ b/sled-agent/src/instance_manager.rs @@ -76,6 +76,7 @@ impl InstanceManager { initial_hardware: InstanceHardware, target: InstanceRuntimeStateRequested, migrate: Option, + allocated_control_ip: std::net::IpAddr, ) -> Result { info!( &self.inner.log, @@ -120,6 +121,7 @@ impl InstanceManager { initial_hardware, self.inner.vlan, self.inner.nexus_client.clone(), + allocated_control_ip, )?; let instance_clone = instance.clone(); let old_instance = instances @@ -271,7 +273,7 @@ mod test { let ticket = Arc::new(std::sync::Mutex::new(None)); let ticket_clone = ticket.clone(); let instance_new_ctx = MockInstance::new_context(); - instance_new_ctx.expect().return_once(move |_, _, _, _, _, _| { + instance_new_ctx.expect().return_once(move |_, _, _, _, _, _, _| { let mut inst = MockInstance::default(); inst.expect_clone().return_once(move || { let mut inst = MockInstance::default(); @@ -300,6 +302,7 @@ mod test { migration_params: None, }, None, + "127.0.0.1".parse().unwrap(), ) .await .unwrap(); @@ -335,7 +338,7 @@ mod test { let ticket_clone = ticket.clone(); let instance_new_ctx = MockInstance::new_context(); let mut seq = mockall::Sequence::new(); - instance_new_ctx.expect().return_once(move |_, _, _, _, _, _| { + instance_new_ctx.expect().return_once(move |_, _, _, _, _, _, _| { let mut inst = MockInstance::default(); // First call to ensure (start + transition). inst.expect_clone().times(1).in_sequence(&mut seq).return_once( @@ -377,11 +380,11 @@ mod test { }; // Creates instance, start + transition. - im.ensure(id, rt.clone(), target.clone(), None).await.unwrap(); + im.ensure(id, rt.clone(), target.clone(), None, "127.0.0.1".parse().unwrap()).await.unwrap(); // Transition only. - im.ensure(id, rt.clone(), target.clone(), None).await.unwrap(); + im.ensure(id, rt.clone(), target.clone(), None, "127.0.0.1".parse().unwrap()).await.unwrap(); // Transition only. - im.ensure(id, rt, target, None).await.unwrap(); + im.ensure(id, rt, target, None, "127.0.0.1".parse().unwrap()).await.unwrap(); assert_eq!(im.inner.instances.lock().unwrap().len(), 1); ticket.lock().unwrap().take(); diff --git a/sled-agent/src/mocks/mod.rs b/sled-agent/src/mocks/mod.rs index 4c877e0864c..f3b0ab2ec5e 100644 --- a/sled-agent/src/mocks/mod.rs +++ b/sled-agent/src/mocks/mod.rs @@ -6,9 +6,9 @@ use mockall::mock; use nexus_client::types::{ - DatasetPutRequest, DatasetPutResponse, DiskRuntimeState, - InstanceRuntimeState, SledAgentStartupInfo, UpdateArtifactKind, - ZpoolPutRequest, ZpoolPutResponse, + AllocateStaticV6AddressResponse, DatasetPutRequest, DatasetPutResponse, + DiskRuntimeState, InstanceRuntimeState, SledAgentStartupInfo, + UpdateArtifactKind, ZpoolPutRequest, ZpoolPutResponse, }; use reqwest::Response; use slog::Logger; @@ -57,5 +57,8 @@ mock! { dataset_id: &Uuid, info: &DatasetPutRequest, ) -> Result; + pub async fn allocate_static_v_6_address( + &self, + ) -> Result; } } diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index c691d90eac0..09b2eb702bb 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -62,6 +62,8 @@ pub struct InstanceEnsureBody { pub target: InstanceRuntimeStateRequested, /// If we're migrating this instance, the details needed to drive the migration pub migrate: Option, + /// Allocated address for propolis's control vnic + pub allocated_control_ip: std::net::IpAddr, } #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index 99bb25ec6d1..29fbaf6fa94 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -181,9 +181,10 @@ impl SledAgent { initial: InstanceHardware, target: InstanceRuntimeStateRequested, migrate: Option, + allocated_control_ip: std::net::IpAddr, ) -> Result { self.instances - .ensure(instance_id, initial, target, migrate) + .ensure(instance_id, initial, target, migrate, allocated_control_ip) .await .map_err(|e| Error::Instance(e)) } From 9fa10bb1e67a542ff2b031eb8ae5a218c8a1a4c7 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 18:54:24 -0400 Subject: [PATCH 2/7] more to resolve --- nexus/src/db/datastore.rs | 1 + sled-agent/src/instance.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index c24e299dc90..8acd502e6ec 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -3453,6 +3453,7 @@ impl DataStore { ) -> UpdateResult<()> { use db::schema::static_v6_address::dsl; + // XXX what if the address isn't in the table? diesel::delete(dsl::static_v6_address) .filter(dsl::address.eq(ipnetwork::IpNetwork::from(address))) .execute_async(self.pool()) diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index 105d14b9655..96f7c349c4e 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -572,7 +572,7 @@ impl Instance { async fn stop(&self) -> Result<(), Error> { let mut inner = self.inner.lock().await; - // XXX how to free the static V6 address? where do I store it? + // XXX free address through nexus internal client? let zname = propolis_zone_name(inner.propolis_id()); warn!(inner.log, "Halting and removing zone: {}", zname); From 8aa57735500af543cad37fa508a29b91465377e8 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 21:41:47 -0400 Subject: [PATCH 3/7] associate allocated IPs with ids --- common/src/sql/dbinit.sql | 3 +- nexus/src/db/datastore.rs | 64 ++++++++++++++---- nexus/src/db/model.rs | 11 +++- nexus/src/db/schema.rs | 1 + nexus/src/internal_api/http_entrypoints.rs | 40 +---------- nexus/src/nexus.rs | 17 ++--- nexus/src/sagas.rs | 24 +++++-- openapi/nexus-internal.json | 77 ---------------------- sled-agent/src/instance_manager.rs | 24 ++++++- sled-agent/src/mocks/mod.rs | 9 +-- 10 files changed, 117 insertions(+), 153 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 736a15ca16e..203b9d8d451 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -916,7 +916,8 @@ CREATE TABLE omicron.public.role_assignment_builtin ( * Store addresses allocated for internal services (like propolis zones) */ CREATE TABLE omicron.public.static_v6_address ( - address INET PRIMARY KEY NOT NULL + address INET PRIMARY KEY NOT NULL, + associated_id UUID NOT NULL ); /*******************************************************************/ diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 8acd502e6ec..1d7e0ea7120 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -3413,6 +3413,7 @@ impl DataStore { pub async fn allocate_static_v6_address( &self, + associated_id: Uuid, ) -> Result { use db::schema::static_v6_address::dsl; @@ -3420,14 +3421,24 @@ impl DataStore { .transaction(move |conn| { // Grab the next sequential address // XXX should this use push_next_available_ip_subquery? - // refactor? - let new_address: Vec = diesel::sql_query("select static_v6_address.address + 1 as address from static_v6_address where not exists (select 1 from static_v6_address tmp where tmp.address = static_v6_address.address + 1) limit 1").load(conn)?; + // XXX refactor push_next_available_ip_subquery? + // XXX this is a full table scan + let query = vec![ + "select".to_string(), + format!("'{}'::UUID", associated_id), + "as associated_id, static_v6_address.address + 1 as address".to_string(), + "from static_v6_address where not exists".to_string(), + "(select 1 from static_v6_address tmp where tmp.address = static_v6_address.address + 1)".to_string(), + "limit 1".to_string(), + ]; + let new_address: Vec = diesel::sql_query(query.join(" ")).load(conn)?; let new_address = if new_address.len() != 1 { // This means there are no addresses in the table! - // XXX normally this should come from something else, hard - // code here. + // XXX normally this base address should come from something + // else, hard code here. StaticV6Address { + associated_id, address: "fd00:1234::101".parse().unwrap(), } } else { @@ -3447,6 +3458,26 @@ impl DataStore { }) } + pub async fn static_v6_address_fetch( + &self, + associated_id: Uuid, + ) -> LookupResult { + use db::schema::static_v6_address::dsl; + + dsl::static_v6_address + .filter(dsl::associated_id.eq(associated_id)) + .select(StaticV6Address::as_select()) + .first_async(self.pool()) + .await + .map(|x| x.address.ip()) + .map_err(|e| { + Error::internal_error(&format!( + "error fetching static v6 address for {:?}: {:?}", + associated_id, e + )) + }) + } + pub async fn free_static_v6_address( &self, address: IpAddr, @@ -3946,19 +3977,19 @@ mod test { // db contents: [] let address1 = - datastore.allocate_static_v6_address().await?; + datastore.allocate_static_v6_address(Uuid::new_v4()).await?; // db contents: [101] assert_eq!(address1, "fd00:1234::101".parse::().unwrap()); // db contents: [101] let address2 = - datastore.allocate_static_v6_address().await?; + datastore.allocate_static_v6_address(Uuid::new_v4()).await?; // db contents: [101, 102] assert_eq!(address2, "fd00:1234::102".parse::().unwrap()); // db contents: [101, 102] let address3 = - datastore.allocate_static_v6_address().await?; + datastore.allocate_static_v6_address(Uuid::new_v4()).await?; // db contents: [101, 102, 103] assert_eq!(address3, "fd00:1234::103".parse::().unwrap()); @@ -3968,7 +3999,7 @@ mod test { // db contents: [102, 103] let address4 = - datastore.allocate_static_v6_address().await?; + datastore.allocate_static_v6_address(Uuid::new_v4()).await?; // db contents: [102, 103, 104] assert_eq!(address4, "fd00:1234::104".parse::().unwrap()); @@ -3982,22 +4013,33 @@ mod test { // db contents: [103] let address5 = - datastore.allocate_static_v6_address().await?; + datastore.allocate_static_v6_address(Uuid::new_v4()).await?; // db contents: [103, 104] assert_eq!(address5, "fd00:1234::104".parse::().unwrap()); // db contents: [103, 104] let address6 = - datastore.allocate_static_v6_address().await?; + datastore.allocate_static_v6_address(Uuid::new_v4()).await?; // db contents: [103, 104, 105] assert_eq!(address6, "fd00:1234::105".parse::().unwrap()); // db contents: [103, 104, 105] let address7 = - datastore.allocate_static_v6_address().await?; + datastore.allocate_static_v6_address(Uuid::new_v4()).await?; // db contents: [103, 104, 105, 106] assert_eq!(address7, "fd00:1234::106".parse::().unwrap()); + // get associated address + let id = Uuid::new_v4(); + let address = datastore.allocate_static_v6_address(id.clone()).await?; + assert_eq!(address, datastore.static_v6_address_fetch(id).await?); + + // assert that getting the address of an unknown ID returns an error + assert!(datastore + .static_v6_address_fetch(Uuid::new_v4()) + .await + .is_err()); + let _ = db.cleanup().await; Ok(()) diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 9505543e896..a9a29272eaa 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -2355,10 +2355,19 @@ pub struct UpdateAvailableArtifact { pub target_length: i64, } -#[derive(Queryable, QueryableByName, Insertable, Clone, Debug, Selectable, AsChangeset)] +#[derive( + Queryable, + QueryableByName, + Insertable, + Clone, + Debug, + Selectable, + AsChangeset, +)] #[table_name = "static_v6_address"] pub struct StaticV6Address { pub address: IpNetwork, + pub associated_id: Uuid, } #[cfg(test)] diff --git a/nexus/src/db/schema.rs b/nexus/src/db/schema.rs index 05f91630666..152f4b4fbbd 100644 --- a/nexus/src/db/schema.rs +++ b/nexus/src/db/schema.rs @@ -376,6 +376,7 @@ table! { table! { static_v6_address (address) { address -> Inet, + associated_id -> Uuid, } } diff --git a/nexus/src/internal_api/http_entrypoints.rs b/nexus/src/internal_api/http_entrypoints.rs index d0c98895c5f..4df80916d77 100644 --- a/nexus/src/internal_api/http_entrypoints.rs +++ b/nexus/src/internal_api/http_entrypoints.rs @@ -7,8 +7,7 @@ use crate::context::OpContext; use crate::ServerContext; use super::params::{ - AllocateStaticV6AddressResponse, DatasetPutRequest, DatasetPutResponse, - FreeStaticV6AddressParams, OximeterInfo, SledAgentStartupInfo, + DatasetPutRequest, DatasetPutResponse, OximeterInfo, SledAgentStartupInfo, ZpoolPutRequest, ZpoolPutResponse, }; use dropshot::endpoint; @@ -46,8 +45,6 @@ pub fn internal_api() -> NexusApiDescription { api.register(cpapi_collectors_post)?; api.register(cpapi_metrics_collect)?; api.register(cpapi_artifact_download)?; - api.register(allocate_static_v6_address)?; - api.register(free_static_v6_address)?; Ok(()) } @@ -282,38 +279,3 @@ async fn cpapi_artifact_download( Ok(Response::builder().status(StatusCode::OK).body(body.into())?) } - -/// Endpoint used by Sled Agents to allocate static V6 addresses -#[endpoint { - method = POST, - path = "/static_address/v6", -}] -async fn allocate_static_v6_address( - request_context: Arc>>, -) -> Result, HttpError> { - let context = request_context.context(); - let nexus = &context.nexus; - - Ok(HttpResponseOk(AllocateStaticV6AddressResponse { - address: nexus.allocate_static_v6_address().await?, - })) -} - -/// Endpoint used by Sled Agents to free static V6 addresses -#[endpoint { - method = DELETE, - path = "/static_address/v6/{address}", -}] -async fn free_static_v6_address( - request_context: Arc>>, - path_params: Path, -) -> Result, HttpError> { - let context = request_context.context(); - let nexus = &context.nexus; - - nexus - .free_static_v6_address(path_params.into_inner().address) - .await?; - - Ok(HttpResponseOk(())) -} diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index d627f20d549..a986581f403 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -1256,6 +1256,9 @@ impl Nexus { nics: vec![], }; + let allocated_control_ip = + self.db_datastore.static_v6_address_fetch(db_instance.id()).await?; + let new_runtime = sa .instance_put( &db_instance.id(), @@ -1263,10 +1266,7 @@ impl Nexus { initial: instance_hardware, target: requested, migrate: None, - // XXX where does this come from? we need to associate the - // control ip with the instance, but the static_v6_address - // table only has one column (the address) - allocated_control_ip: "TODO".parse().unwrap(), + allocated_control_ip: allocated_control_ip.to_string(), }, ) .await @@ -3074,10 +3074,11 @@ impl Nexus { Ok(body) } - pub async fn allocate_static_v6_address(&self) -> Result { - self.db_datastore - .allocate_static_v6_address() - .await + pub async fn allocate_static_v6_address( + &self, + associated_id: Uuid, + ) -> Result { + self.db_datastore.allocate_static_v6_address(associated_id).await } pub async fn free_static_v6_address( diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index 29eb5332bdd..ae93d08badd 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -42,6 +42,7 @@ use slog::warn; use slog::Logger; use std::collections::BTreeMap; use std::convert::{TryFrom, TryInto}; +use std::net::IpAddr; use std::sync::Arc; use steno::new_action_noop_undo; use steno::ActionContext; @@ -52,7 +53,6 @@ use steno::SagaTemplateBuilder; use steno::SagaTemplateGeneric; use steno::SagaType; use uuid::Uuid; -use std::net::IpAddr; // We'll need a richer mechanism for registering sagas, but this works for now. pub const SAGA_INSTANCE_CREATE_NAME: &'static str = "instance-create"; @@ -197,7 +197,7 @@ pub fn saga_instance_create() -> SagaTemplate { ActionFunc::new_action( sic_allocate_v6_address, sic_allocate_v6_address_undo, - ) + ), ); template_builder.append( @@ -531,7 +531,11 @@ async fn sic_allocate_v6_address( ) -> Result { let osagactx = sagactx.user_data(); let nexus = osagactx.nexus(); - Ok(nexus.allocate_static_v6_address().await.map_err(ActionError::action_failed)?) + let instance_id = sagactx.lookup::("instance_id")?; + Ok(nexus + .allocate_static_v6_address(instance_id) + .await + .map_err(ActionError::action_failed)?) } async fn sic_allocate_v6_address_undo( @@ -548,7 +552,11 @@ async fn sim_allocate_v6_address( ) -> Result { let osagactx = sagactx.user_data(); let nexus = osagactx.nexus(); - Ok(nexus.allocate_static_v6_address().await.map_err(ActionError::action_failed)?) + let instance_id = sagactx.lookup::("instance_id")?; + Ok(nexus + .allocate_static_v6_address(instance_id) + .await + .map_err(ActionError::action_failed)?) } async fn sim_allocate_v6_address_undo( @@ -584,7 +592,8 @@ async fn sic_instance_ensure( .await .map_err(ActionError::action_failed)?; - let allocated_control_ip = sagactx.lookup::("allocated_control_ip")?; + let allocated_control_ip = + sagactx.lookup::("allocated_control_ip")?; // Ask the sled agent to begin the state change. Then update the database // to reflect the new intermediate state. If this update is not the newest @@ -656,7 +665,7 @@ pub fn saga_instance_migrate() -> SagaTemplate { ActionFunc::new_action( sim_allocate_v6_address, sim_allocate_v6_address_undo, - ) + ), ); template_builder.append( @@ -749,7 +758,8 @@ async fn sim_instance_migrate( .await .map_err(ActionError::action_failed)?; - let allocated_control_ip = sagactx.lookup::("allocated_control_ip")?; + let allocated_control_ip = + sagactx.lookup::("allocated_control_ip")?; let new_runtime_state: InstanceRuntimeState = dst_sa .instance_put( diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index c37c1eadd50..4cfadbdfc74 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -323,70 +323,6 @@ } } }, - "/static_address/v6": { - "post": { - "summary": "Endpoint used by Sled Agents to allocate static V6 addresses", - "operationId": "allocate_static_v6_address", - "responses": { - "200": { - "description": "successful operation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/AllocateStaticV6AddressResponse" - } - } - } - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } - } - }, - "/static_address/v6/{address}": { - "delete": { - "summary": "Endpoint used by Sled Agents to free static V6 addresses", - "operationId": "free_static_v6_address", - "parameters": [ - { - "in": "path", - "name": "address", - "required": true, - "schema": { - "type": "string", - "format": "ip" - }, - "style": "simple" - } - ], - "responses": { - "200": { - "description": "successful operation", - "content": { - "application/json": { - "schema": { - "title": "Null", - "type": "string", - "enum": [ - null - ] - } - } - } - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } - } - }, "/zpools/{zpool_id}/dataset/{dataset_id}": { "put": { "summary": "Report that a dataset within a pool has come online.", @@ -458,19 +394,6 @@ } }, "schemas": { - "AllocateStaticV6AddressResponse": { - "description": "Response when allocating a static v6 address", - "type": "object", - "properties": { - "address": { - "type": "string", - "format": "ip" - } - }, - "required": [ - "address" - ] - }, "BinRangedouble": { "description": "A type storing a range over `T`.\n\nThis type supports ranges similar to the `RangeTo`, `Range` and `RangeFrom` types in the standard library. Those cover `(..end)`, `(start..end)`, and `(start..)` respectively.", "oneOf": [ diff --git a/sled-agent/src/instance_manager.rs b/sled-agent/src/instance_manager.rs index 6e287719c9d..af7dc99bd43 100644 --- a/sled-agent/src/instance_manager.rs +++ b/sled-agent/src/instance_manager.rs @@ -380,11 +380,29 @@ mod test { }; // Creates instance, start + transition. - im.ensure(id, rt.clone(), target.clone(), None, "127.0.0.1".parse().unwrap()).await.unwrap(); + im.ensure( + id, + rt.clone(), + target.clone(), + None, + "127.0.0.1".parse().unwrap(), + ) + .await + .unwrap(); // Transition only. - im.ensure(id, rt.clone(), target.clone(), None, "127.0.0.1".parse().unwrap()).await.unwrap(); + im.ensure( + id, + rt.clone(), + target.clone(), + None, + "127.0.0.1".parse().unwrap(), + ) + .await + .unwrap(); // Transition only. - im.ensure(id, rt, target, None, "127.0.0.1".parse().unwrap()).await.unwrap(); + im.ensure(id, rt, target, None, "127.0.0.1".parse().unwrap()) + .await + .unwrap(); assert_eq!(im.inner.instances.lock().unwrap().len(), 1); ticket.lock().unwrap().take(); diff --git a/sled-agent/src/mocks/mod.rs b/sled-agent/src/mocks/mod.rs index f3b0ab2ec5e..4c877e0864c 100644 --- a/sled-agent/src/mocks/mod.rs +++ b/sled-agent/src/mocks/mod.rs @@ -6,9 +6,9 @@ use mockall::mock; use nexus_client::types::{ - AllocateStaticV6AddressResponse, DatasetPutRequest, DatasetPutResponse, - DiskRuntimeState, InstanceRuntimeState, SledAgentStartupInfo, - UpdateArtifactKind, ZpoolPutRequest, ZpoolPutResponse, + DatasetPutRequest, DatasetPutResponse, DiskRuntimeState, + InstanceRuntimeState, SledAgentStartupInfo, UpdateArtifactKind, + ZpoolPutRequest, ZpoolPutResponse, }; use reqwest::Response; use slog::Logger; @@ -57,8 +57,5 @@ mock! { dataset_id: &Uuid, info: &DatasetPutRequest, ) -> Result; - pub async fn allocate_static_v_6_address( - &self, - ) -> Result; } } From 32dfa2d33d74ac37b9804f7cccf082bf4864b2e1 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 21:57:28 -0400 Subject: [PATCH 4/7] more code to write --- nexus/src/db/datastore.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 1d7e0ea7120..4602ffb9ee1 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -1300,6 +1300,16 @@ impl DataStore { let failed = DbInstanceState::new(ApiInstanceState::Failed); let instance_id = authz_instance.id(); + + // XXX delete allocated ip + // XXX what if the address isn't in the table? + // XXX do in transaction (saga?) + // + //use db::schema::static_v6_address::dsl as address_dsl + //diesel::delete(address_dsl::static_v6_address) + // .filter(address_dsl::associated_id.eq(instance_id)) + // .execute(conn)?; + let result = diesel::update(dsl::instance) .filter(dsl::time_deleted.is_null()) .filter(dsl::id.eq(instance_id)) @@ -1314,6 +1324,7 @@ impl DataStore { ErrorHandler::NotFoundByResource(authz_instance), ) })?; + match result.status { UpdateStatus::Updated => Ok(()), UpdateStatus::NotUpdatedButExists => { From d332d4b83729a7f78f8155eb7d81ad4d5268ce9d Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 24 Mar 2022 21:59:17 -0400 Subject: [PATCH 5/7] clarify or remove things to resolve --- nexus/src/sagas.rs | 2 +- sled-agent/src/instance.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index ae93d08badd..c53199ed7fc 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -524,7 +524,7 @@ async fn sic_create_instance_record( Ok(instance.runtime().clone().into()) } -// XXX can't reuse this function? +// XXX can't reuse this function because ActionContext impl changes? async fn sic_allocate_v6_address( sagactx: ActionContext, diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index 96f7c349c4e..58f8dfeefd1 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -485,7 +485,7 @@ impl Instance { let v6 = running_zone .ensure_address(AddressRequest::new_static( inner.allocated_control_ip, - None, // XXX what should this be? + None, )) .await?; info!(inner.log, "Created v6 address {} for zone: {}", v6, zname); @@ -572,8 +572,6 @@ impl Instance { async fn stop(&self) -> Result<(), Error> { let mut inner = self.inner.lock().await; - // XXX free address through nexus internal client? - let zname = propolis_zone_name(inner.propolis_id()); warn!(inner.log, "Halting and removing zone: {}", zname); Zones::halt_and_remove(&inner.log, &zname).unwrap(); From 0eb7b14267a7b5398043ae5922e256f4e026a236 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 28 Mar 2022 10:20:21 -0400 Subject: [PATCH 6/7] post-merge fix --- nexus/src/db/datastore.rs | 11 ++++++----- nexus/src/db/model.rs | 2 +- nexus/src/sagas.rs | 4 ---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index e3d32be90ce..c4a42ba0cdb 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -66,11 +66,12 @@ use crate::db::{ ConsoleSession, Dataset, DatasetKind, Disk, DiskRuntimeState, Generation, IncompleteNetworkInterface, Instance, InstanceRuntimeState, Name, NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, - ProducerEndpoint, Project, ProjectUpdate, Region, RoleAssignmentBuiltin, - RoleBuiltin, RouterRoute, RouterRouteUpdate, Silo, SiloUser, Sled, - StaticV6Address, UpdateArtifactKind, UpdateAvailableArtifact, - UserBuiltin, Volume, Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, - VpcSubnet, VpcSubnetUpdate, VpcUpdate, Zpool, + ProducerEndpoint, Project, ProjectUpdate, Region, + RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, RouterRouteUpdate, + Silo, SiloUser, Sled, StaticV6Address, UpdateArtifactKind, + UpdateAvailableArtifact, UserBuiltin, Volume, Vpc, VpcFirewallRule, + VpcRouter, VpcRouterUpdate, VpcSubnet, VpcSubnetUpdate, VpcUpdate, + Zpool, }, pagination::paginated, pagination::paginated_multicolumn, diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 75357f3f19b..eb19a541a62 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -10,7 +10,7 @@ use crate::db::schema::{ console_session, dataset, disk, instance, metric_producer, network_interface, organization, oximeter, project, rack, region, role_assignment_builtin, role_builtin, router_route, silo, silo_user, sled, - static_v6_address, snapshot, update_available_artifact, user_builtin, + snapshot, static_v6_address, update_available_artifact, user_builtin, volume, vpc, vpc_firewall_rule, vpc_router, vpc_subnet, zpool, }; use crate::defaults; diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index cef3ff4ebc5..51dc472356f 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -806,9 +806,6 @@ async fn sic_instance_ensure( let instance_name = sagactx.lookup::("instance_name")?; let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); - let allocated_control_ip = - sagactx.lookup::("allocated_control_ip")?; - let authz_project = osagactx .datastore() .project_lookup_by_id(params.project_id) @@ -828,7 +825,6 @@ async fn sic_instance_ensure( &authz_instance, &instance, runtime_params, - allocated_control_ip: allocated_control_ip.to_string(), ) .await .map_err(ActionError::action_failed)?; From 9ad6b3d7b89e4104839eaee347fc0787e42675b3 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 28 Mar 2022 16:15:35 -0400 Subject: [PATCH 7/7] correct saga node name --- nexus/src/sagas.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index 51dc472356f..aa78cdc1a39 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -767,7 +767,7 @@ async fn sic_allocate_v6_address_undo( ) -> Result<(), anyhow::Error> { let osagactx = sagactx.user_data(); let nexus = osagactx.nexus(); - let ipv6 = sagactx.lookup::("allocated_ipv6")?; + let ipv6 = sagactx.lookup::("allocated_control_ip")?; Ok(nexus.free_static_v6_address(ipv6).await?) } @@ -788,7 +788,7 @@ async fn sim_allocate_v6_address_undo( ) -> Result<(), anyhow::Error> { let osagactx = sagactx.user_data(); let nexus = osagactx.nexus(); - let ipv6 = sagactx.lookup::("allocated_ipv6")?; + let ipv6 = sagactx.lookup::("allocated_control_ip")?; Ok(nexus.free_static_v6_address(ipv6).await?) }