From 969de795dff2ddb0d9997dfb4487219a59a7394f Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 5 May 2023 10:39:07 -0400 Subject: [PATCH 01/12] Unify services and datasets. Will rely on https://github.com/oxidecomputer/omicron/issues/2614 --- openapi/sled-agent.json | 114 +++-------------- sled-agent/src/http_entrypoints.rs | 22 +--- sled-agent/src/params.rs | 86 ++++--------- sled-agent/src/rack_setup/plan/service.rs | 133 ++++++++++++-------- sled-agent/src/rack_setup/service.rs | 142 +++++++--------------- sled-agent/src/services.rs | 111 ++--------------- sled-agent/src/sled_agent.rs | 80 ++++-------- 7 files changed, 205 insertions(+), 483 deletions(-) diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index ccb0eae4c67..d8510a33d65 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -54,32 +54,6 @@ } } }, - "/filesystem": { - "put": { - "operationId": "filesystems_put", - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/DatasetEnsureBody" - } - } - }, - "required": true - }, - "responses": { - "204": { - "description": "resource updated" - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } - } - }, "/instances/{instance_id}": { "put": { "operationId": "instance_register", @@ -608,48 +582,6 @@ "target" ] }, - "DatasetEnsureBody": { - "description": "Used to request that the Sled initialize multiple datasets.", - "type": "object", - "properties": { - "datasets": { - "type": "array", - "items": { - "$ref": "#/components/schemas/DatasetEnsureRequest" - } - } - }, - "required": [ - "datasets" - ] - }, - "DatasetEnsureRequest": { - "description": "Used to request a new dataset kind exists within a zpool.\n\nMany dataset types are associated with services that will be instantiated when the dataset is detected.", - "type": "object", - "properties": { - "address": { - "type": "string" - }, - "dataset_name": { - "$ref": "#/components/schemas/DatasetName" - }, - "gz_address": { - "nullable": true, - "default": null, - "type": "string", - "format": "ipv6" - }, - "id": { - "type": "string", - "format": "uuid" - } - }, - "required": [ - "address", - "dataset_name", - "id" - ] - }, "DatasetKind": { "description": "The type of a dataset, and an auxiliary information necessary to successfully launch a zone managing the associated data.", "oneOf": [ @@ -698,22 +630,6 @@ { "type": "object", "properties": { - "dns_address": { - "description": "The address at which the external DNS server is reachable.", - "type": "string" - }, - "http_address": { - "description": "The address at which the external DNS server API is reachable.", - "type": "string" - }, - "nic": { - "description": "The service vNIC providing external connectivity using OPTE.", - "allOf": [ - { - "$ref": "#/components/schemas/NetworkInterface" - } - ] - }, "type": { "type": "string", "enum": [ @@ -722,21 +638,12 @@ } }, "required": [ - "dns_address", - "http_address", - "nic", "type" ] }, { "type": "object", "properties": { - "dns_address": { - "type": "string" - }, - "http_address": { - "type": "string" - }, "type": { "type": "string", "enum": [ @@ -745,8 +652,6 @@ } }, "required": [ - "dns_address", - "http_address", "type" ] } @@ -767,6 +672,23 @@ "pool_name" ] }, + "DatasetRequest": { + "description": "Describes a request to provision a specific dataset", + "type": "object", + "properties": { + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "$ref": "#/components/schemas/DatasetName" + } + }, + "required": [ + "id", + "name" + ] + }, "DiskEnsureBody": { "description": "Sent from to a sled agent to establish the runtime state of a Disk", "type": "object", @@ -2085,7 +2007,7 @@ "default": null, "allOf": [ { - "$ref": "#/components/schemas/DatasetName" + "$ref": "#/components/schemas/DatasetRequest" } ] }, diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs index e38b41d9685..779c96ff0f8 100644 --- a/sled-agent/src/http_entrypoints.rs +++ b/sled-agent/src/http_entrypoints.rs @@ -5,10 +5,9 @@ //! HTTP entrypoint functions for the sled agent's exposed API use crate::params::{ - DatasetEnsureBody, DiskEnsureBody, InstanceEnsureBody, - InstancePutMigrationIdsBody, InstancePutStateBody, - InstancePutStateResponse, InstanceUnregisterResponse, ServiceEnsureBody, - SledRole, TimeSync, VpcFirewallRulesEnsureBody, Zpool, + DiskEnsureBody, InstanceEnsureBody, InstancePutMigrationIdsBody, + InstancePutStateBody, InstancePutStateResponse, InstanceUnregisterResponse, + ServiceEnsureBody, SledRole, TimeSync, VpcFirewallRulesEnsureBody, Zpool, }; use dropshot::{ endpoint, ApiDescription, HttpError, HttpResponseOk, @@ -31,7 +30,6 @@ type SledApiDescription = ApiDescription; pub fn api() -> SledApiDescription { fn register_endpoints(api: &mut SledApiDescription) -> Result<(), String> { api.register(disk_put)?; - api.register(filesystems_put)?; api.register(instance_issue_disk_snapshot_request)?; api.register(instance_put_migration_ids)?; api.register(instance_put_state)?; @@ -92,20 +90,6 @@ async fn sled_role_get( Ok(HttpResponseOk(sa.get_role().await)) } -#[endpoint { - method = PUT, - path = "/filesystem", -}] -async fn filesystems_put( - rqctx: RequestContext, - body: TypedBody, -) -> Result { - let sa = rqctx.context(); - let body_args = body.into_inner(); - sa.filesystems_ensure(body_args).await.map_err(|e| Error::from(e))?; - Ok(HttpResponseUpdatedNoContent()) -} - /// Path parameters for Instance requests (sled agent API) #[derive(Deserialize, JsonSchema)] struct InstancePathParam { diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 08c9f18cc25..cf9c60ef065 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -216,50 +216,8 @@ pub enum DatasetKind { CockroachDb, Crucible, Clickhouse, - ExternalDns { - /// The address at which the external DNS server API is reachable. - http_address: SocketAddrV6, - /// The address at which the external DNS server is reachable. - dns_address: SocketAddr, - /// The service vNIC providing external connectivity using OPTE. - nic: NetworkInterface, - }, - InternalDns { - http_address: SocketAddrV6, - dns_address: SocketAddrV6, - }, -} - -impl DatasetKind { - /// Returns the type of the zone which manages this dataset. - pub fn zone_type(&self) -> ZoneType { - match *self { - DatasetKind::CockroachDb => ZoneType::CockroachDb, - DatasetKind::Crucible => ZoneType::Crucible, - DatasetKind::Clickhouse => ZoneType::Clickhouse, - DatasetKind::ExternalDns { .. } => ZoneType::ExternalDns, - DatasetKind::InternalDns { .. } => ZoneType::InternalDns, - } - } - - /// Returns the service type which runs in the zone managing this dataset. - /// - /// NOTE: This interface is only viable because datasets run a single - /// service in their zone. If that precondition is no longer true, this - /// interface should be re-visited. - pub fn service_type(&self) -> ServiceType { - match self.clone() { - DatasetKind::CockroachDb => ServiceType::CockroachDb, - DatasetKind::Crucible => ServiceType::Crucible, - DatasetKind::Clickhouse => ServiceType::Clickhouse, - DatasetKind::ExternalDns { http_address, dns_address, nic } => { - ServiceType::ExternalDns { http_address, dns_address, nic } - } - DatasetKind::InternalDns { http_address, dns_address } => { - ServiceType::InternalDns { http_address, dns_address } - } - } - } + ExternalDns, + InternalDns, } impl From for sled_agent_client::types::DatasetKind { @@ -269,17 +227,8 @@ impl From for sled_agent_client::types::DatasetKind { CockroachDb => Self::CockroachDb, Crucible => Self::Crucible, Clickhouse => Self::Clickhouse, - ExternalDns { http_address, dns_address, nic } => { - Self::ExternalDns { - http_address: http_address.to_string(), - dns_address: dns_address.to_string(), - nic: nic.into(), - } - } - InternalDns { http_address, dns_address } => Self::InternalDns { - http_address: http_address.to_string(), - dns_address: dns_address.to_string(), - }, + ExternalDns => Self::ExternalDns, + InternalDns => Self::InternalDns, } } } @@ -288,11 +237,11 @@ impl From for nexus_client::types::DatasetKind { fn from(k: DatasetKind) -> Self { use DatasetKind::*; match k { - CockroachDb { .. } => Self::Cockroach, + CockroachDb => Self::Cockroach, Crucible => Self::Crucible, Clickhouse => Self::Clickhouse, - ExternalDns { .. } => Self::ExternalDns, - InternalDns { .. } => Self::InternalDns, + ExternalDns => Self::ExternalDns, + InternalDns => Self::InternalDns, } } } @@ -335,6 +284,7 @@ pub struct DatasetEnsureRequest { pub gz_address: Option, } +/* impl From for sled_agent_client::types::DatasetEnsureRequest { @@ -347,6 +297,7 @@ impl From } } } +*/ /// Describes service-specific parameters. #[derive( @@ -576,6 +527,21 @@ impl std::fmt::Display for ZoneType { } } +/// Describes a request to provision a specific dataset +#[derive( + Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash, +)] +pub struct DatasetRequest { + pub id: Uuid, + pub name: crate::storage::dataset::DatasetName, +} + +impl From for sled_agent_client::types::DatasetRequest { + fn from(d: DatasetRequest) -> Self { + Self { id: d.id, name: d.name.into() } + } +} + /// Describes a request to create a zone running one or more services. #[derive( Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash, @@ -589,7 +555,7 @@ pub struct ServiceZoneRequest { pub addresses: Vec, // Datasets which should be managed by this service. #[serde(default)] - pub dataset: Option, + pub dataset: Option, // The addresses in the global zone which should be created, if necessary // to route to the service. // @@ -614,7 +580,7 @@ impl ServiceZoneRequest { // The name of a unique identifier for the zone, if one is necessary. pub fn zone_name_unique_identifier(&self) -> Option { - self.dataset.as_ref().map(|d| d.pool().to_string()) + self.dataset.as_ref().map(|d| d.name.pool().to_string()) } } diff --git a/sled-agent/src/rack_setup/plan/service.rs b/sled-agent/src/rack_setup/plan/service.rs index b127d560ad8..71311b795a1 100644 --- a/sled-agent/src/rack_setup/plan/service.rs +++ b/sled-agent/src/rack_setup/plan/service.rs @@ -7,8 +7,8 @@ use crate::bootstrap::params::SledAgentRequest; use crate::ledger::{Ledger, Ledgerable}; use crate::params::{ - DatasetEnsureRequest, ServiceType, ServiceZoneRequest, ServiceZoneService, - ZoneType, + DatasetKind, DatasetRequest, ServiceType, ServiceZoneRequest, + ServiceZoneService, ZoneType, }; use crate::rack_setup::config::SetupServiceConfig as Config; use crate::storage::dataset::DatasetName; @@ -93,10 +93,6 @@ pub enum PlanError { #[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq)] pub struct SledRequest { - /// Datasets to be created. - #[serde(default, rename = "dataset")] - pub datasets: Vec, - /// Services to be instantiated. #[serde(default, rename = "service")] pub services: Vec, @@ -302,11 +298,21 @@ impl Plan { .unwrap(); let (nic, external_ip) = svc_port_builder.next_dns(id, &mut services_ip_pool)?; - request.datasets.push(DatasetEnsureRequest { + request.services.push(ServiceZoneRequest { id, - dataset_name: DatasetName::new( - u2_zpools[0].clone(), - crate::params::DatasetKind::ExternalDns { + zone_type: ZoneType::ExternalDns, + addresses: vec![*http_address.ip()], + dataset: Some(DatasetRequest { + id: Uuid::new_v4(), + name: DatasetName::new( + u2_zpools[0].clone(), + DatasetKind::ExternalDns, + ), + }), + gz_addresses: vec![], + services: vec![ServiceZoneService { + id: Uuid::new_v4(), + details: ServiceType::ExternalDns { http_address: SocketAddrV6::new( internal_ip, http_port, @@ -316,9 +322,7 @@ impl Plan { dns_address: SocketAddr::new(external_ip, dns_port), nic, }, - ), - address: http_address, - gz_address: None, + }], }); } @@ -385,19 +389,26 @@ impl Plan { let id = Uuid::new_v4(); let ip = addr_alloc.next().expect("Not enough addrs"); let port = omicron_common::address::COCKROACH_PORT; - let address = SocketAddrV6::new(ip, port, 0, 0); let zone = dns_builder.host_zone(id, ip).unwrap(); dns_builder .service_backend_zone(ServiceName::Cockroach, &zone, port) .unwrap(); - request.datasets.push(DatasetEnsureRequest { + request.services.push(ServiceZoneRequest { id, - dataset_name: DatasetName::new( - u2_zpools[0].clone(), - crate::params::DatasetKind::CockroachDb, - ), - address, - gz_address: None, + zone_type: ZoneType::CockroachDb, + addresses: vec![ip], + dataset: Some(DatasetRequest { + id, + name: DatasetName::new( + u2_zpools[0].clone(), + crate::params::DatasetKind::CockroachDb, + ), + }), + gz_addresses: vec![], + services: vec![ServiceZoneService { + id, + details: ServiceType::CockroachDb, + }], }); } @@ -406,19 +417,26 @@ impl Plan { let id = Uuid::new_v4(); let ip = addr_alloc.next().expect("Not enough addrs"); let port = omicron_common::address::CLICKHOUSE_PORT; - let address = SocketAddrV6::new(ip, port, 0, 0); let zone = dns_builder.host_zone(id, ip).unwrap(); dns_builder .service_backend_zone(ServiceName::Clickhouse, &zone, port) .unwrap(); - request.datasets.push(DatasetEnsureRequest { + request.services.push(ServiceZoneRequest { id, - dataset_name: DatasetName::new( - u2_zpools[0].clone(), - crate::params::DatasetKind::Clickhouse, - ), - address, - gz_address: None, + zone_type: ZoneType::Clickhouse, + addresses: vec![ip], + dataset: Some(DatasetRequest { + id, + name: DatasetName::new( + u2_zpools[0].clone(), + crate::params::DatasetKind::Clickhouse, + ), + }), + gz_addresses: vec![], + services: vec![ServiceZoneService { + id, + details: ServiceType::Clickhouse, + }], }); } @@ -428,7 +446,6 @@ impl Plan { for pool in &u2_zpools { let ip = addr_alloc.next().expect("Not enough addrs"); let port = omicron_common::address::CRUCIBLE_PORT; - let address = SocketAddrV6::new(ip, port, 0, 0); let id = Uuid::new_v4(); let zone = dns_builder.host_zone(id, ip).unwrap(); dns_builder @@ -439,14 +456,22 @@ impl Plan { ) .unwrap(); - request.datasets.push(DatasetEnsureRequest { + request.services.push(ServiceZoneRequest { id, - dataset_name: DatasetName::new( - pool.clone(), - crate::params::DatasetKind::Crucible, - ), - address, - gz_address: None, + zone_type: ZoneType::Crucible, + addresses: vec![ip], + dataset: Some(DatasetRequest { + id, + name: DatasetName::new( + pool.clone(), + crate::params::DatasetKind::Crucible, + ), + }), + gz_addresses: vec![], + services: vec![ServiceZoneService { + id, + details: ServiceType::Crucible, + }], }); } @@ -454,11 +479,9 @@ impl Plan { // responsibility of being internal DNS servers. if idx < dns_subnets.len() { let dns_subnet = &dns_subnets[idx]; - let dns_ip = dns_subnet.dns_address().ip(); - let http_address = - SocketAddrV6::new(dns_ip, DNS_HTTP_PORT, 0, 0); + let ip = dns_subnet.dns_address().ip(); let id = Uuid::new_v4(); - let zone = dns_builder.host_zone(id, dns_ip).unwrap(); + let zone = dns_builder.host_zone(id, ip).unwrap(); dns_builder .service_backend_zone( ServiceName::InternalDns, @@ -466,24 +489,30 @@ impl Plan { DNS_HTTP_PORT, ) .unwrap(); - request.datasets.push(DatasetEnsureRequest { + request.services.push(ServiceZoneRequest { id, - dataset_name: DatasetName::new( - u2_zpools[0].clone(), - crate::params::DatasetKind::InternalDns { + zone_type: ZoneType::InternalDns, + addresses: vec![ip], + dataset: Some(DatasetRequest { + id, + name: DatasetName::new( + u2_zpools[0].clone(), + crate::params::DatasetKind::InternalDns, + ), + }), + gz_addresses: vec![dns_subnet.gz_address().ip()], + services: vec![ServiceZoneService { + id, + details: ServiceType::InternalDns { http_address: SocketAddrV6::new( - dns_ip, + ip, DNS_HTTP_PORT, 0, 0, ), - dns_address: SocketAddrV6::new( - dns_ip, DNS_PORT, 0, 0, - ), + dns_address: SocketAddrV6::new(ip, DNS_PORT, 0, 0), }, - ), - address: http_address, - gz_address: Some(dns_subnet.gz_address().ip()), + }], }); } diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 98bd0d4f1fc..60264735e28 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -62,8 +62,8 @@ use crate::bootstrap::rss_handle::BootstrapAgentHandle; use crate::ledger::{Ledger, Ledgerable}; use crate::nexus::d2n_params; use crate::params::{ - AutonomousServiceOnlyError, DatasetEnsureRequest, DatasetKind, ServiceType, - ServiceZoneRequest, TimeSync, ZoneType, + AutonomousServiceOnlyError, DatasetKind, ServiceType, ServiceZoneRequest, + ServiceZoneService, TimeSync, ZoneType, }; use crate::rack_setup::plan::service::{ Plan as ServicePlan, PlanError as ServicePlanError, @@ -80,8 +80,9 @@ use nexus_client::{ types as NexusTypes, Client as NexusClient, Error as NexusError, }; use omicron_common::address::{ - get_sled_address, CRUCIBLE_PANTRY_PORT, DENDRITE_PORT, NEXUS_INTERNAL_PORT, - NTP_PORT, OXIMETER_PORT, + get_sled_address, CLICKHOUSE_PORT, COCKROACH_PORT, CRUCIBLE_PANTRY_PORT, + CRUCIBLE_PORT, DENDRITE_PORT, DNS_HTTP_PORT, NEXUS_INTERNAL_PORT, NTP_PORT, + OXIMETER_PORT, }; use omicron_common::backoff::{ retry_notify, retry_policy_internal_service_aggressive, BackoffError, @@ -243,51 +244,6 @@ impl ServiceInner { ServiceInner { log } } - async fn initialize_datasets( - &self, - sled_address: SocketAddrV6, - datasets: &Vec, - ) -> Result<(), SetupServiceError> { - let dur = std::time::Duration::from_secs(60); - - let client = reqwest::ClientBuilder::new() - .connect_timeout(dur) - .timeout(dur) - .build() - .map_err(SetupServiceError::HttpClient)?; - let client = SledAgentClient::new_with_client( - &format!("http://{}", sled_address), - client, - self.log.new(o!("SledAgentClient" => sled_address.to_string())), - ); - - let datasets = - datasets.iter().map(|d| d.clone().into()).collect::>(); - - info!(self.log, "sending dataset requests..."); - let filesystem_put = || async { - info!(self.log, "creating new filesystems: {:?}", datasets); - client - .filesystems_put(&SledAgentTypes::DatasetEnsureBody { - datasets: datasets.clone(), - }) - .await - .map_err(BackoffError::transient)?; - Ok::<(), BackoffError>>(()) - }; - let log_failure = |error, _| { - warn!(self.log, "failed to create filesystem"; "error" => ?error); - }; - retry_notify( - retry_policy_internal_service_aggressive(), - filesystem_put, - log_failure, - ) - .await?; - - Ok(()) - } - async fn initialize_services( &self, sled_address: SocketAddrV6, @@ -346,22 +302,19 @@ impl ServiceInner { // Start up the internal DNS services futures::future::join_all(service_plan.services.iter().map( |(sled_address, services_request)| async move { - let datasets: Vec<_> = services_request - .datasets + let services: Vec<_> = services_request + .services .iter() - .filter_map(|dataset| { - if matches!( - dataset.dataset_name.dataset(), - DatasetKind::InternalDns { .. } - ) { - Some(dataset.clone()) + .filter_map(|service| { + if matches!(service.zone_type, ZoneType::InternalDns,) { + Some(service.clone()) } else { None } }) .collect(); - if !datasets.is_empty() { - self.initialize_datasets(*sled_address, &datasets).await?; + if !services.is_empty() { + self.initialize_services(*sled_address, &services).await?; } Ok(()) }, @@ -378,11 +331,16 @@ impl ServiceInner { |(_, services_request)| { // iterate services for this sled let dns_addrs: Vec = services_request - .datasets + .services .iter() - .filter_map(|dataset| { - match dataset.dataset_name.dataset() { - DatasetKind::InternalDns { http_address, .. } => Some(http_address.clone()), + .filter_map(|service| { + match &service.services[0] { + ServiceZoneService { + details: ServiceType::InternalDns { http_address, .. }, + .. + } => { + Some(http_address.clone()) + }, _ => None, } }) @@ -704,15 +662,31 @@ impl ServiceInner { } } - for dataset in service_request.datasets.iter() { - datasets.push(NexusTypes::DatasetCreateRequest { - zpool_id: dataset.dataset_name.pool().id(), - dataset_id: dataset.id, - request: NexusTypes::DatasetPutRequest { - address: dataset.address.to_string(), - kind: dataset.dataset_name.dataset().clone().into(), - }, - }) + for service in service_request.services.iter() { + if let Some(dataset) = &service.dataset { + let port = match dataset.name.dataset() { + DatasetKind::CockroachDb => COCKROACH_PORT, + DatasetKind::Clickhouse => CLICKHOUSE_PORT, + DatasetKind::Crucible => CRUCIBLE_PORT, + DatasetKind::ExternalDns => DNS_HTTP_PORT, + DatasetKind::InternalDns => DNS_HTTP_PORT, + }; + + datasets.push(NexusTypes::DatasetCreateRequest { + zpool_id: dataset.name.pool().id(), + dataset_id: dataset.id, + request: NexusTypes::DatasetPutRequest { + address: SocketAddrV6::new( + service.addresses[0], + port, + 0, + 0, + ) + .to_string(), + kind: dataset.name.dataset().clone().into(), + }, + }) + } } } let internal_services_ip_pool_ranges = config @@ -1003,31 +977,9 @@ impl ServiceInner { // Wait until time is synchronized on all sleds before proceeding. self.wait_for_timesync(&sled_addresses).await?; - // Issue the dataset initialization requests to all sleds. - futures::future::join_all(service_plan.services.iter().map( - |(sled_address, services_request)| async move { - self.initialize_datasets( - *sled_address, - &services_request.datasets, - ) - .await?; - Ok(()) - }, - )) - .await - .into_iter() - .collect::>()?; - - info!(self.log, "Finished setting up agents and datasets"); + info!(self.log, "Finished setting up Internal DNS and NTP"); // Issue service initialization requests. - // - // NOTE: This must happen *after* the dataset initialization, - // to ensure that CockroachDB has been initialized before Nexus - // starts. - // - // If Nexus was more resilient to concurrent initialization - // of CRDB, this requirement could be relaxed. futures::future::join_all(service_plan.services.iter().map( |(sled_address, services_request)| async move { // With the current implementation of "initialize_services", diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index c70cbd38997..93ea0e7abe7 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -209,7 +209,6 @@ impl Config { // The filename of the ledger, within the provided directory. const SERVICES_LEDGER_FILENAME: &str = "services.toml"; -const STORAGE_SERVICES_LEDGER_FILENAME: &str = "storage-services.toml"; // A wrapper around `ZoneRequest`, which allows it to be serialized // to a toml file. @@ -298,8 +297,6 @@ pub struct ServiceManagerInner { switch_zone_maghemite_links: Vec, // Zones representing running services zones: Mutex>, - // Zones representing services which own datasets - dataset_zones: Mutex>, underlay_vnic_allocator: VnicAllocator, underlay_vnic: EtherstubVnic, bootstrap_vnic_allocator: VnicAllocator, @@ -371,7 +368,6 @@ impl ServiceManager { sidecar_revision, switch_zone_maghemite_links, zones: Mutex::new(vec![]), - dataset_zones: Mutex::new(vec![]), underlay_vnic_allocator: VnicAllocator::new( "Service", underlay_etherstub, @@ -415,22 +411,7 @@ impl ServiceManager { .collect() } - async fn all_storage_service_ledgers(&self) -> Vec { - if let Some(dir) = self.inner.ledger_directory_override.get() { - return vec![dir.join(STORAGE_SERVICES_LEDGER_FILENAME)]; - } - - self.inner - .storage - .resources() - .all_m2_mountpoints(sled_hardware::disk::CONFIG_DATASET) - .await - .into_iter() - .map(|p| p.join(STORAGE_SERVICES_LEDGER_FILENAME)) - .collect() - } - - pub async fn load_non_storage_services(&self) -> Result<(), Error> { + pub async fn load_services(&self) -> Result<(), Error> { let log = &self.inner.log; let mut existing_zones = self.inner.zones.lock().await; let Some(ledger) = Ledger::::new( @@ -506,25 +487,6 @@ impl ServiceManager { Ok(()) } - pub async fn load_storage_services(&self) -> Result<(), Error> { - let log = &self.inner.log; - let mut existing_zones = self.inner.dataset_zones.lock().await; - let Some(ledger) = Ledger::::new( - log, - self.all_storage_service_ledgers().await, - ) - .await else { - return Ok(()); - }; - let services = ledger.data(); - self.initialize_services_locked( - &mut existing_zones, - &services.requests, - ) - .await?; - Ok(()) - } - /// Loads services from the services manager, and returns once all requested /// services have been started. pub async fn sled_agent_started( @@ -550,15 +512,11 @@ impl ServiceManager { .map_err(|_| "already set".to_string()) .expect("Sled Agent should only start once"); - self.load_non_storage_services().await?; // TODO(https://github.com/oxidecomputer/omicron/issues/2973): // These will fail if the disks aren't attached. // Should we have a retry loop here? Kinda like we have with the switch // / NTP zone? - // - // NOTE: We could totally do the same thing with - // "load_non_storage_services". - self.load_storage_services().await?; + self.load_services().await?; Ok(()) } @@ -922,7 +880,7 @@ impl ServiceManager { .zone .dataset .iter() - .map(|d| zone::Dataset { name: d.full() }) + .map(|d| zone::Dataset { name: d.name.full() }) .collect::>(); let devices: Vec = device_names @@ -1098,16 +1056,18 @@ impl ServiceManager { let listen_addr = &request.zone.addresses[0].to_string(); let listen_port = &CRUCIBLE_PORT.to_string(); - let dataset = request + let dataset_name = request .zone .dataset .as_ref() + .map(|d| d.name.full()) + .clone() .expect("Crucible requires dataset"); let uuid = &Uuid::new_v4().to_string(); let config = PropertyGroupBuilder::new("config") .add_property("datalink", "astring", datalink) .add_property("gateway", "astring", gateway) - .add_property("dataset", "astring", &dataset.full()) + .add_property("dataset", "astring", &dataset_name) .add_property("listen_addr", "astring", listen_addr) .add_property("listen_port", "astring", listen_port) .add_property("uuid", "astring", uuid) @@ -1855,63 +1815,6 @@ impl ServiceManager { Ok(()) } - /// Ensures that a storage zone be initialized. - /// - /// These services will be instantiated by this function, and will be - /// recorded to a local file to ensure they start automatically on next - /// boot. - pub async fn ensure_storage_service( - &self, - request: ServiceZoneRequest, - ) -> Result<(), Error> { - let log = &self.inner.log; - let mut existing_zones = self.inner.dataset_zones.lock().await; - - // Read the existing set of services from the ledger. - let service_paths = self.all_storage_service_ledgers().await; - let mut ledger = - match Ledger::::new(log, service_paths.clone()) - .await - { - Some(ledger) => ledger, - None => Ledger::::new_with( - log, - service_paths.clone(), - AllZoneRequests::default(), - ), - }; - let ledger_zone_requests = ledger.data_mut(); - - if !ledger_zone_requests - .requests - .iter() - .any(|zone_request| zone_request.zone.id == request.id) - { - // If this is a new request, provision a zone filesystem on the same - // disk as the dataset. - let dataset = request - .dataset - .as_ref() - .expect("Storage services should have a dataset"); - let root = dataset - .pool() - .dataset_mountpoint(sled_hardware::disk::ZONE_DATASET); - ledger_zone_requests - .requests - .push(ZoneRequest { zone: request, root }); - } - - self.initialize_services_locked( - &mut existing_zones, - &ledger_zone_requests.requests, - ) - .await?; - - ledger.commit().await?; - - Ok(()) - } - pub fn boottime_rewrite(&self, zones: &Vec) { if self .inner diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index 8ae7c146581..91f6a14d0ec 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -9,10 +9,10 @@ use crate::config::Config; use crate::instance_manager::InstanceManager; use crate::nexus::{LazyNexusClient, NexusRequestQueue}; use crate::params::{ - DatasetEnsureBody, DiskStateRequested, InstanceHardware, - InstanceMigrationSourceParams, InstancePutStateResponse, - InstanceStateRequested, InstanceUnregisterResponse, ServiceEnsureBody, - ServiceZoneService, SledRole, TimeSync, VpcFirewallRule, Zpool, + DiskStateRequested, InstanceHardware, InstanceMigrationSourceParams, + InstancePutStateResponse, InstanceStateRequested, + InstanceUnregisterResponse, ServiceEnsureBody, SledRole, TimeSync, + VpcFirewallRule, Zpool, }; use crate::services::{self, ServiceManager}; use crate::storage_manager::{self, StorageManager}; @@ -491,6 +491,25 @@ impl SledAgent { &self, requested_services: ServiceEnsureBody, ) -> Result<(), Error> { + let datasets: Vec<_> = requested_services + .services + .iter() + .filter_map(|service| service.dataset.clone()) + .collect(); + + // TODO: + // - If these are the set of filesystems, we should also consider + // removing the ones which are not listed here. + // - It's probably worth sending a bulk request to the storage system, + // rather than requesting individual datasets. + for dataset in &datasets { + // First, ensure the dataset exists + self.inner + .storage + .upsert_filesystem(dataset.id, dataset.name.clone()) + .await?; + } + self.inner.services.ensure_all_services(requested_services).await?; Ok(()) } @@ -510,59 +529,6 @@ impl SledAgent { } } - /// Ensures that all filesystem type exists within the zpool. - pub async fn filesystems_ensure( - &self, - requested_datasets: DatasetEnsureBody, - ) -> Result<(), Error> { - // TODO: - // - If these are the set of filesystems, we should also consider - // removing the ones which are not listed here. - // - It's probably worth sending a bulk request to the storage system, - // rather than requesting individual datasets. - for dataset in &requested_datasets.datasets { - let dataset_id = dataset.id; - - // First, ensure the dataset exists - self.inner - .storage - .upsert_filesystem(dataset_id, dataset.dataset_name.clone()) - .await?; - } - - for dataset in &requested_datasets.datasets { - let dataset_id = dataset.id; - let address = dataset.address; - let gz_address = dataset.gz_address; - - // NOTE: We use the "dataset_id" as the "service_id" here. - // - // Since datasets are tightly coupled with their own services - e.g., - // from the perspective of Nexus, provisioning a dataset implies the - // sled should start a service - this is ID re-use is reasonable. - // - // If Nexus ever wants sleds to provision datasets independently of - // launching services, this ID type overlap should be reconsidered. - let service_type = dataset.dataset_name.dataset().service_type(); - let services = vec![ServiceZoneService { - id: dataset_id, - details: service_type, - }]; - - // Next, ensure a zone exists to manage storage for that dataset - let request = crate::params::ServiceZoneRequest { - id: dataset_id, - zone_type: dataset.dataset_name.dataset().zone_type(), - addresses: vec![*address.ip()], - dataset: Some(dataset.dataset_name.clone()), - gz_addresses: gz_address.into_iter().collect(), - services, - }; - self.inner.services.ensure_storage_service(request).await?; - } - Ok(()) - } - /// Idempotently ensures that a given instance is registered with this sled, /// i.e., that it can be addressed by future calls to /// [`instance_ensure_state`]. From 71b26dc02cf94946aee6ac9c532c14120643c2e8 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 5 May 2023 11:46:09 -0400 Subject: [PATCH 02/12] Delete more code --- sled-agent/src/params.rs | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index cf9c60ef065..314a059681f 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -260,45 +260,6 @@ impl std::fmt::Display for DatasetKind { } } -/// Used to request that the Sled initialize multiple datasets. -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)] -pub struct DatasetEnsureBody { - pub datasets: Vec, -} - -/// Used to request a new dataset kind exists within a zpool. -/// -/// Many dataset types are associated with services that will be -/// instantiated when the dataset is detected. -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)] -pub struct DatasetEnsureRequest { - // The UUID of the dataset, as well as the service using it directly. - pub id: Uuid, - // The name (and UUID) of the Zpool which we are inserting into. - pub dataset_name: crate::storage::dataset::DatasetName, - // The address on which the zone will listen for requests. - pub address: SocketAddrV6, - // The addresses in the global zone which should be created, if necessary - // to route to the service. - #[serde(default)] - pub gz_address: Option, -} - -/* -impl From - for sled_agent_client::types::DatasetEnsureRequest -{ - fn from(p: DatasetEnsureRequest) -> Self { - Self { - id: p.id, - dataset_name: p.dataset_name.into(), - address: p.address.to_string(), - gz_address: p.gz_address, - } - } -} -*/ - /// Describes service-specific parameters. #[derive( Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash, From 1a43229f9e1f5d4d3df182a095d73df620cf355c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Sun, 18 Jun 2023 11:42:07 -0700 Subject: [PATCH 03/12] Clarify the CTE around region allocation (#3374) Some minor docs improvements, as I dug through this codebase recently: - I wanted to clarify, "Does this CTE only provision Crucible datasets to U.2s, and not M.2s?". The answer to this question is: "yes, but implicitly decided when we provision the datasets themselves". Added a comment to clarify this during region selection. - Additionally, when referencing a virtual table from an earlier `WITH candidate_zpools AS ...` arm of the CTE, make that reference explicit. This is a no-op change, but it clarifies the dependencies within the CTE. --- nexus/db-queries/src/db/queries/region_allocation.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/nexus/db-queries/src/db/queries/region_allocation.rs b/nexus/db-queries/src/db/queries/region_allocation.rs index 7ec16a70e9c..4c76689cffc 100644 --- a/nexus/db-queries/src/db/queries/region_allocation.rs +++ b/nexus/db-queries/src/db/queries/region_allocation.rs @@ -74,6 +74,11 @@ impl OldRegions { } /// A subquery to find datasets which could be used for provisioning regions. +/// +/// We only consider datasets which are already allocated as "Crucible". +/// This implicitly distinguishes between "M.2s" and "U.2s" -- Nexus needs to +/// determine during dataset provisioning which devices should be considered for +/// usage as Crucible storage. #[derive(Subquery, QueryId)] #[subquery(name = candidate_datasets)] struct CandidateDatasets { @@ -214,13 +219,14 @@ struct OldPoolUsage { } impl OldPoolUsage { - fn new() -> Self { + fn new(candidate_zpools: &CandidateZpools) -> Self { use crate::db::schema::dataset::dsl as dataset_dsl; Self { query: Box::new( dataset_dsl::dataset .inner_join( - candidate_zpools::dsl::candidate_zpools + candidate_zpools + .query_source() .on(dataset_dsl::pool_id .eq(candidate_zpools::dsl::id)), ) @@ -473,7 +479,7 @@ impl RegionAllocate { extent_count, ); let proposed_changes = ProposedChanges::new(&candidate_regions); - let old_pool_usage = OldPoolUsage::new(); + let old_pool_usage = OldPoolUsage::new(&candidate_zpools); let zpool_size_delta = ZpoolSizeDelta::new(&proposed_changes); let proposed_datasets_fit = ProposedDatasetsFit::new(&old_pool_usage, &zpool_size_delta); From 92290ea67c5298da4786bdfd321b1a4735621771 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 18 Jun 2023 20:16:26 -0700 Subject: [PATCH 04/12] Bump progenitor from `7377251` to `dcdc044` (#3379) --- Cargo.lock | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 04f83f440be..c83ca51ef6a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5854,7 +5854,7 @@ dependencies = [ [[package]] name = "progenitor" version = "0.3.0" -source = "git+https://github.com/oxidecomputer/progenitor?branch=main#7377251307456cddd90d67d2235819212e2a92e3" +source = "git+https://github.com/oxidecomputer/progenitor?branch=main#dcdc0446693f461057251ed08c245c3e67df5986" dependencies = [ "progenitor-client", "progenitor-impl", @@ -5865,7 +5865,7 @@ dependencies = [ [[package]] name = "progenitor-client" version = "0.3.0" -source = "git+https://github.com/oxidecomputer/progenitor?branch=main#7377251307456cddd90d67d2235819212e2a92e3" +source = "git+https://github.com/oxidecomputer/progenitor?branch=main#dcdc0446693f461057251ed08c245c3e67df5986" dependencies = [ "bytes", "futures-core", @@ -5879,7 +5879,7 @@ dependencies = [ [[package]] name = "progenitor-impl" version = "0.3.0" -source = "git+https://github.com/oxidecomputer/progenitor?branch=main#7377251307456cddd90d67d2235819212e2a92e3" +source = "git+https://github.com/oxidecomputer/progenitor?branch=main#dcdc0446693f461057251ed08c245c3e67df5986" dependencies = [ "getopts", "heck 0.4.1", @@ -5901,7 +5901,7 @@ dependencies = [ [[package]] name = "progenitor-macro" version = "0.3.0" -source = "git+https://github.com/oxidecomputer/progenitor?branch=main#7377251307456cddd90d67d2235819212e2a92e3" +source = "git+https://github.com/oxidecomputer/progenitor?branch=main#dcdc0446693f461057251ed08c245c3e67df5986" dependencies = [ "openapiv3", "proc-macro2", @@ -8613,7 +8613,7 @@ checksum = "497961ef93d974e23eb6f433eb5fe1b7930b659f06d12dec6fc44a8f554c0bba" [[package]] name = "typify" version = "0.0.13" -source = "git+https://github.com/oxidecomputer/typify#f53fbe86fb6e9d6cc6b92d7d380abf711b0989f6" +source = "git+https://github.com/oxidecomputer/typify#ab78614353f242f60dba979f9da3207a0039d05d" dependencies = [ "typify-impl", "typify-macro", @@ -8622,7 +8622,7 @@ dependencies = [ [[package]] name = "typify-impl" version = "0.0.13" -source = "git+https://github.com/oxidecomputer/typify#f53fbe86fb6e9d6cc6b92d7d380abf711b0989f6" +source = "git+https://github.com/oxidecomputer/typify#ab78614353f242f60dba979f9da3207a0039d05d" dependencies = [ "heck 0.4.1", "log", @@ -8639,7 +8639,7 @@ dependencies = [ [[package]] name = "typify-macro" version = "0.0.13" -source = "git+https://github.com/oxidecomputer/typify#f53fbe86fb6e9d6cc6b92d7d380abf711b0989f6" +source = "git+https://github.com/oxidecomputer/typify#ab78614353f242f60dba979f9da3207a0039d05d" dependencies = [ "proc-macro2", "quote", From df079e648a244de73e41a882785362909b94da17 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 18 Jun 2023 20:16:38 -0700 Subject: [PATCH 05/12] Bump clap from 4.3.3 to 4.3.4 (#3381) --- Cargo.lock | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c83ca51ef6a..d55afa014bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -950,9 +950,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.3.3" +version = "4.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca8f255e4b8027970e78db75e78831229c9815fdbfa67eb1a1b777a62e24b4a0" +checksum = "80672091db20273a15cf9fdd4e47ed43b5091ec9841bf4c6145c9dfbbcae09ed" dependencies = [ "clap_builder", "clap_derive 4.3.2", @@ -961,9 +961,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.3.3" +version = "4.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "acd4f3c17c83b0ba34ffbc4f8bbd74f079413f747f84a6f89292f138057e36ab" +checksum = "c1458a1df40e1e2afebb7ab60ce55c1fa8f431146205aa5f4887e0b111c27636" dependencies = [ "anstream", "anstyle", @@ -1176,7 +1176,7 @@ dependencies = [ "anes", "cast", "ciborium", - "clap 4.3.3", + "clap 4.3.4", "criterion-plot", "futures", "is-terminal", @@ -1872,7 +1872,7 @@ dependencies = [ "anyhow", "camino", "chrono", - "clap 4.3.3", + "clap 4.3.4", "dns-service-client 0.1.0", "dropshot", "expectorate", @@ -2550,7 +2550,7 @@ name = "gateway-cli" version = "0.1.0" dependencies = [ "anyhow", - "clap 4.3.3", + "clap 4.3.4", "futures", "gateway-client", "gateway-messages", @@ -3244,7 +3244,7 @@ dependencies = [ "buf-list", "bytes", "camino", - "clap 4.3.3", + "clap 4.3.4", "ddm-admin-client", "display-error-chain", "futures", @@ -3306,7 +3306,7 @@ dependencies = [ "anyhow", "async-trait", "camino", - "clap 4.3.3", + "clap 4.3.4", "dropshot", "expectorate", "hyper", @@ -4483,7 +4483,7 @@ name = "omicron-deploy" version = "0.1.0" dependencies = [ "anyhow", - "clap 4.3.3", + "clap 4.3.4", "crossbeam", "omicron-package", "omicron-sled-agent", @@ -4502,7 +4502,7 @@ dependencies = [ "anyhow", "camino", "camino-tempfile", - "clap 4.3.3", + "clap 4.3.4", "dropshot", "expectorate", "futures", @@ -4533,7 +4533,7 @@ dependencies = [ "anyhow", "async-trait", "ciborium", - "clap 4.3.3", + "clap 4.3.4", "crucible-smf", "dropshot", "expectorate", @@ -4582,7 +4582,7 @@ dependencies = [ "bb8", "camino", "chrono", - "clap 4.3.3", + "clap 4.3.4", "cookie", "criterion", "crucible-agent-client", @@ -4680,7 +4680,7 @@ name = "omicron-package" version = "0.1.0" dependencies = [ "anyhow", - "clap 4.3.3", + "clap 4.3.4", "expectorate", "futures", "hex", @@ -4755,7 +4755,7 @@ dependencies = [ "camino-tempfile", "cfg-if 1.0.0", "chrono", - "clap 4.3.3", + "clap 4.3.4", "crucible-agent-client", "crucible-client-types", "ddm-admin-client", @@ -5121,7 +5121,7 @@ dependencies = [ name = "oximeter-collector" version = "0.1.0" dependencies = [ - "clap 4.3.3", + "clap 4.3.4", "dropshot", "expectorate", "futures", @@ -5153,7 +5153,7 @@ dependencies = [ "async-trait", "bytes", "chrono", - "clap 4.3.3", + "clap 4.3.4", "dropshot", "itertools", "omicron-test-utils", @@ -5982,7 +5982,7 @@ dependencies = [ "bytes", "cfg-if 1.0.0", "chrono", - "clap 4.3.3", + "clap 4.3.4", "const_format", "dropshot", "enum-iterator", @@ -7578,7 +7578,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-trait", - "clap 4.3.3", + "clap 4.3.4", "dropshot", "futures", "gateway-messages", @@ -8496,7 +8496,7 @@ dependencies = [ "assert_cmd", "camino", "chrono", - "clap 4.3.3", + "clap 4.3.4", "console", "datatest-stable", "fs-err", @@ -9130,7 +9130,7 @@ dependencies = [ "buf-list", "camino", "ciborium", - "clap 4.3.3", + "clap 4.3.4", "crossterm 0.26.1", "futures", "hex", @@ -9192,7 +9192,7 @@ dependencies = [ "bytes", "camino", "ciborium", - "clap 4.3.3", + "clap 4.3.4", "crossterm 0.26.1", "reedline", "serde", @@ -9216,7 +9216,7 @@ dependencies = [ "bytes", "camino", "camino-tempfile", - "clap 4.3.3", + "clap 4.3.4", "debug-ignore", "display-error-chain", "dropshot", From a2e09d9f41e31e134b7ed8d6d9494a82c787c39f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 18 Jun 2023 20:16:57 -0700 Subject: [PATCH 06/12] Bump rustls from 0.21.1 to 0.21.2 (#3380) --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d55afa014bc..9ce1c98c65e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6694,9 +6694,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.21.1" +version = "0.21.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c911ba11bc8433e811ce56fde130ccf32f5127cab0e0194e9c68c5a5b671791e" +checksum = "e32ca28af694bc1bbf399c33a516dbdf1c90090b8ab23c2bc24f834aa2247f5f" dependencies = [ "log", "ring", diff --git a/Cargo.toml b/Cargo.toml index dae0b7537af..759734999c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -273,7 +273,7 @@ reqwest = { version = "0.11", default-features = false } ring = "0.16" rpassword = "7.2.0" rustfmt-wrapper = "0.2" -rustls = "0.21.1" +rustls = "0.21.2" samael = { git = "https://github.com/njaremko/samael", features = ["xmlsec"], branch = "master" } schemars = "0.8.12" secrecy = "0.8.0" From 33be0b1744a9ce8a1cab80128f9d1ed97b9f434f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 18 Jun 2023 20:17:23 -0700 Subject: [PATCH 07/12] Bump sha2 from 0.10.6 to 0.10.7 (#3377) --- Cargo.lock | 52 ++++++++++++++++++++++++++-------------------------- Cargo.toml | 2 +- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ce1c98c65e..2d11bc563a4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -442,7 +442,7 @@ checksum = "3806a8db60cf56efee531616a34a6aaa9a114d6da2add861b0fa4a188881b2c7" dependencies = [ "blowfish", "pbkdf2", - "sha2 0.10.6", + "sha2 0.10.7", ] [[package]] @@ -583,7 +583,7 @@ version = "0.10.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" dependencies = [ - "digest 0.10.6", + "digest 0.10.7", ] [[package]] @@ -1786,9 +1786,9 @@ dependencies = [ [[package]] name = "digest" -version = "0.10.6" +version = "0.10.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8168378f4e5023e7218c89c891c0fd8ecdb5e5e4f18cb78f38cf245dd021e76f" +checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer 0.10.4", "const-oid", @@ -2092,7 +2092,7 @@ checksum = "968405c8fdc9b3bf4df0a6638858cc0b52462836ab6b1c87377785dd09cf1c0b" dependencies = [ "base16ct", "crypto-bigint", - "digest 0.10.6", + "digest 0.10.7", "ff", "generic-array", "group", @@ -2892,7 +2892,7 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c49c37c09c17a53d937dfbb742eb3a961d65a994e6bcdcf37e7399d0cc8ab5e" dependencies = [ - "digest 0.10.6", + "digest 0.10.7", ] [[package]] @@ -3730,7 +3730,7 @@ dependencies = [ "pem-rfc7468", "rsa", "serde", - "sha2 0.10.6", + "sha2 0.10.7", "thiserror", "x509-cert", "zerocopy 0.6.1", @@ -3784,7 +3784,7 @@ version = "0.10.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6365506850d44bff6e2fbcb5176cf63650e48bd45ef2fe2665ae1570e0f4b9ca" dependencies = [ - "digest 0.10.6", + "digest 0.10.7", ] [[package]] @@ -5409,10 +5409,10 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "83a0692ec44e4cf1ef28ca317f14f8f07da2d95ec3fa01f86e4467b725e60917" dependencies = [ - "digest 0.10.6", + "digest 0.10.7", "hmac", "password-hash 0.4.2", - "sha2 0.10.6", + "sha2 0.10.7", ] [[package]] @@ -5486,7 +5486,7 @@ checksum = "5e3b284b1f13a20dc5ebc90aff59a51b8d7137c221131b52a7260c08cbc1cc80" dependencies = [ "once_cell", "pest", - "sha2 0.10.6", + "sha2 0.10.7", ] [[package]] @@ -5690,7 +5690,7 @@ dependencies = [ "md-5", "memchr", "rand 0.8.5", - "sha2 0.10.6", + "sha2 0.10.7", "stringprep", ] @@ -6505,7 +6505,7 @@ checksum = "72f1471dbb4be5de45050e8ef7040625298ccb9efe941419ac2697088715925f" dependencies = [ "byteorder", "const-oid", - "digest 0.10.6", + "digest 0.10.7", "num-bigint-dig", "num-integer", "num-iter", @@ -6513,7 +6513,7 @@ dependencies = [ "pkcs1", "pkcs8", "rand_core 0.6.4", - "sha2 0.10.6", + "sha2 0.10.7", "signature 2.1.0", "subtle", "zeroize", @@ -6543,7 +6543,7 @@ dependencies = [ "chacha20", "ctr", "curve25519-dalek", - "digest 0.10.6", + "digest 0.10.7", "flate2", "futures", "generic-array", @@ -6557,7 +6557,7 @@ dependencies = [ "russh-cryptovec", "russh-keys", "sha1", - "sha2 0.10.6", + "sha2 0.10.7", "subtle", "thiserror", "tokio", @@ -6602,7 +6602,7 @@ dependencies = [ "rand_core 0.5.1", "russh-cryptovec", "serde", - "sha2 0.10.6", + "sha2 0.10.7", "thiserror", "tokio", "tokio-stream", @@ -7181,7 +7181,7 @@ checksum = "f5058ada175748e33390e40e872bd0fe59a19f265d0158daa551c5a88a76009c" dependencies = [ "cfg-if 1.0.0", "cpufeatures", - "digest 0.10.6", + "digest 0.10.7", ] [[package]] @@ -7192,7 +7192,7 @@ checksum = "f04293dc80c3993519f2d7f6f511707ee7094fe0c6d3406feb330cdb3540eba3" dependencies = [ "cfg-if 1.0.0", "cpufeatures", - "digest 0.10.6", + "digest 0.10.7", ] [[package]] @@ -7210,13 +7210,13 @@ dependencies = [ [[package]] name = "sha2" -version = "0.10.6" +version = "0.10.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82e6b795fe2e3b1e845bafcb27aa35405c4d47cdfc92af5fc8d3002f76cebdc0" +checksum = "479fb9d862239e610720565ca91403019f2f00410f1864c5aa7479b950a76ed8" dependencies = [ "cfg-if 1.0.0", "cpufeatures", - "digest 0.10.6", + "digest 0.10.7", ] [[package]] @@ -7225,7 +7225,7 @@ version = "0.10.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "75872d278a8f37ef87fa0ddbda7802605cb18344497949862c0d4dcb291eba60" dependencies = [ - "digest 0.10.6", + "digest 0.10.7", "keccak", ] @@ -7295,7 +7295,7 @@ version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5e1788eed21689f9cf370582dfc467ef36ed9c707f073528ddafa8d83e3b8500" dependencies = [ - "digest 0.10.6", + "digest 0.10.7", "rand_core 0.6.4", ] @@ -8534,7 +8534,7 @@ dependencies = [ "serde", "serde_json", "serde_path_to_error", - "sha2 0.10.6", + "sha2 0.10.7", "slog", "tar", "toml 0.7.4", @@ -9245,7 +9245,7 @@ dependencies = [ "schemars", "serde", "serde_json", - "sha2 0.10.6", + "sha2 0.10.7", "sled-hardware", "slog", "slog-dtrace", diff --git a/Cargo.toml b/Cargo.toml index 759734999c0..9b80d64dde5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -287,7 +287,7 @@ serde_tokenstream = "0.2" serde_urlencoded = "0.7.1" serde_with = "2.3.3" serial_test = "0.10" -sha2 = "0.10.6" +sha2 = "0.10.7" sha3 = "0.10.8" shell-words = "1.1.0" signal-hook = "0.3" From 6642158b5786e39ded08fb01c0cd748ebd128409 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 18 Jun 2023 22:45:06 -0700 Subject: [PATCH 08/12] Bump serde_json from 1.0.96 to 1.0.97 (#3375) Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.96 to 1.0.97.
Release notes

Sourced from serde_json's releases.

v1.0.97

  • Add io_error_kind() method to serde_json::Error: fn io_error_kind(&self) -> Option<std::io::ErrorKind> (#1026)
Commits
  • a0ddb25 Release 1.0.97
  • 8b681ff Merge pull request #1026 from dtolnay/errorkind
  • 9308d97 Add Error::io_error_kind
  • 136b773 Merge pull request #1025 from dtolnay/io
  • 207a57b Standardize on "I/O" instead of "IO"
  • 6fda385 Merge pull request 1021 from ndmitchell/patch-2
  • 009a53b Switch to using null
  • 931ee23 Show error details during miri setup in CI
  • 0d7b0d3 Add an example of producing a Null in a json! literal
  • See full diff in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=serde_json&package-manager=cargo&previous-version=1.0.96&new-version=1.0.97)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2d11bc563a4..1a4c7e9fcd6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7025,9 +7025,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.96" +version = "1.0.97" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "057d394a50403bcac12672b2b18fb387ab6d289d957dab67dd201875391e52f1" +checksum = "bdf3bf93142acad5821c99197022e170842cdbc1c30482b98750c688c640842a" dependencies = [ "itoa", "ryu", diff --git a/Cargo.toml b/Cargo.toml index 9b80d64dde5..e0d699ce8e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -281,7 +281,7 @@ semver = { version = "1.0.17", features = ["std", "serde"] } serde = { version = "1.0", default-features = false, features = [ "derive" ] } serde_derive = "1.0" serde_human_bytes = { git = "http://github.com/oxidecomputer/serde_human_bytes" } -serde_json = "1.0.96" +serde_json = "1.0.97" serde_path_to_error = "0.1.11" serde_tokenstream = "0.2" serde_urlencoded = "0.7.1" From 9eb7bbb5ff50f0b093adae6bebb6f88b87e2faac Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Tue, 20 Jun 2023 12:52:04 -0700 Subject: [PATCH 09/12] [ci] tuf-repo: more curl flags fun (#3384) ci: use --netrc for downloading hubris artifacts as well An actions scope is needed to download artifacts from the Github API. Pass `--netrc` to use the buildomat provided API key as we do for the other curl invocations. ci: tuf-repo: harden curl invocations Some useful flags for curl in a scripted context: -f fail on server errors -s silent mode -S show error on failure -L follow redirects --- .github/buildomat/jobs/tuf-repo.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/buildomat/jobs/tuf-repo.sh b/.github/buildomat/jobs/tuf-repo.sh index 60c844cea06..fb5457c0a25 100644 --- a/.github/buildomat/jobs/tuf-repo.sh +++ b/.github/buildomat/jobs/tuf-repo.sh @@ -137,15 +137,15 @@ done # Fetch SP images from oxidecomputer/hubris GHA artifacts. HUBRIS_VERSION="1.0.0-alpha+git${HUBRIS_COMMIT:0:11}" -run_id=$(curl --netrc "https://api.github.com/repos/oxidecomputer/hubris/actions/runs?head_sha=$HUBRIS_COMMIT" \ +run_id=$(curl --netrc -fsS "https://api.github.com/repos/oxidecomputer/hubris/actions/runs?head_sha=$HUBRIS_COMMIT" \ | /opt/ooce/bin/jq -r '.workflow_runs[] | select(.path == ".github/workflows/dist.yml") | .id') -artifacts=$(curl --netrc "https://api.github.com/repos/oxidecomputer/hubris/actions/runs/$run_id/artifacts") +artifacts=$(curl --netrc -fsS "https://api.github.com/repos/oxidecomputer/hubris/actions/runs/$run_id/artifacts") for noun in gimlet-c psc-b sidecar-b; do tufaceous_kind=${noun%-?} tufaceous_kind=${tufaceous_kind//sidecar/switch}_sp job_name=dist-ubuntu-latest-$noun url=$(/opt/ooce/bin/jq --arg name "$job_name" -r '.artifacts[] | select(.name == $name) | .archive_download_url' <<<"$artifacts") - curl -L -o /work/$job_name.zip "$url" + curl --netrc -fsSL -o /work/$job_name.zip "$url" cat >>/work/manifest.toml < Date: Tue, 20 Jun 2023 12:53:30 -0700 Subject: [PATCH 10/12] Update dendrite to 3857dac (#3357) Updated dendrite to commit 3857dac. --------- Co-authored-by: reflector[bot] <130185838+reflector[bot]@users.noreply.github.com> --- package-manifest.toml | 12 ++++++------ tools/dendrite_openapi_version | 4 ++-- tools/dendrite_stub_checksums | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/package-manifest.toml b/package-manifest.toml index 8826c18db1c..4a06e6d973c 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -300,8 +300,8 @@ only_for_targets.image = "standard" # 2. Copy dendrite.tar.gz from dendrite/out to omicron/out source.type = "prebuilt" source.repo = "dendrite" -source.commit = "9e3764239515a7bb3f09c922cfb88b1be3dade77" -source.sha256 = "13d8ff8374ec0b5d9b681b83aef8a2d4f0aed15d0ad92dc5b04a43d850196309" +source.commit = "3857dac89bf16851df170db2fe3797cec3c1b711" +source.sha256 = "e42742c6d253f99ef280341b6c2c3ab8658dec0595a2ebf65b6f6618e4d34b6a" output.type = "zone" output.intermediate_only = true @@ -325,8 +325,8 @@ only_for_targets.image = "standard" # 2. Copy the output zone image from dendrite/out to omicron/out source.type = "prebuilt" source.repo = "dendrite" -source.commit = "9e3764239515a7bb3f09c922cfb88b1be3dade77" -source.sha256 = "c90dedceec367f5fc8f2c585fda4be1804f59124c067dfc3632cfb7a938b2b4f" +source.commit = "3857dac89bf16851df170db2fe3797cec3c1b711" +source.sha256 = "3a4238cda6c408133c2f4699f2f6575701dc8551199b83b3b3dbd8f0d0b1a512" output.type = "zone" output.intermediate_only = true @@ -343,8 +343,8 @@ only_for_targets.image = "standard" # 2. Copy dendrite.tar.gz from dendrite/out to omicron/out/dendrite-softnpu.tar.gz source.type = "prebuilt" source.repo = "dendrite" -source.commit = "9e3764239515a7bb3f09c922cfb88b1be3dade77" -source.sha256 = "60633b939e9492c6e5302d9ea4c373ffa7d652be1ca7b1f43524f4da07cf14c7" +source.commit = "3857dac89bf16851df170db2fe3797cec3c1b711" +source.sha256 = "dcaab63d5bc1a786eb0bd2868a14a0e72ede879867fc49c0c368861c9e4f9f1f" output.type = "zone" output.intermediate_only = true diff --git a/tools/dendrite_openapi_version b/tools/dendrite_openapi_version index 83845c8add8..57dc977b9b0 100644 --- a/tools/dendrite_openapi_version +++ b/tools/dendrite_openapi_version @@ -1,2 +1,2 @@ -COMMIT="9e3764239515a7bb3f09c922cfb88b1be3dade77" -SHA2="885f05cb273d22a1481f693552cee25265992520a7ab029937d66b4dc50f5038" +COMMIT="3857dac89bf16851df170db2fe3797cec3c1b711" +SHA2="c687851c097dfba4f2006fafd5043e4507c1a162fce8289ecc02098157356608" diff --git a/tools/dendrite_stub_checksums b/tools/dendrite_stub_checksums index 7cdd0c8a969..e9f56d58ae6 100644 --- a/tools/dendrite_stub_checksums +++ b/tools/dendrite_stub_checksums @@ -1,3 +1,3 @@ -CIDL_SHA256_ILLUMOS="13d8ff8374ec0b5d9b681b83aef8a2d4f0aed15d0ad92dc5b04a43d850196309" -CIDL_SHA256_LINUX_DPD="ceecf310f7ca84660423259f3d9bca778e91c81a03f99cc6cf2bb40cd631598f" -CIDL_SHA256_LINUX_SWADM="af3a5d12b17eb353a513a3e1c06505b20b279ae7f26fef94152d5af06757a3d4" +CIDL_SHA256_ILLUMOS="e42742c6d253f99ef280341b6c2c3ab8658dec0595a2ebf65b6f6618e4d34b6a" +CIDL_SHA256_LINUX_DPD="14e8bfacb9367abb868901306224ab80c10c1e567484b79bf35637e9e603c5f3" +CIDL_SHA256_LINUX_SWADM="6d1b9e59e2fe0436b15f0280d26c1327a34c6fb1445ad69eca80fd112dd9a2dc" From 9b85b6190cf58e7860a04471f506fbaecabd219f Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 20 Jun 2023 17:14:36 -0400 Subject: [PATCH 11/12] [wicketd] Add periodic check for sled agent bootstrap addrs (#3327) RSS wants to know the bootstrap addresses of the sleds to include; this PR teaches wicketd how to find them. It also updates the wicket TOML config introduced in #3326 to include the sled's bootstrap address (if known) in a comment in the sled list; I'm unsure if this is something we'll want to keep once customers are running RSS, but it seems handy for debugging for now. Builds on #3326. --- Cargo.lock | 1 + openapi/wicketd.json | 78 +++++++++--- sled-hardware/src/lib.rs | 11 +- wicket/src/rack_setup/config_toml.rs | 58 ++++++--- wicketd-client/src/lib.rs | 1 + wicketd/Cargo.toml | 1 + wicketd/src/bootstrap_addrs.rs | 175 +++++++++++++++++++++++++++ wicketd/src/context.rs | 2 + wicketd/src/http_entrypoints.rs | 70 ++++++++++- wicketd/src/lib.rs | 5 + wicketd/src/rss_config.rs | 28 +++-- 11 files changed, 379 insertions(+), 51 deletions(-) create mode 100644 wicketd/src/bootstrap_addrs.rs diff --git a/Cargo.lock b/Cargo.lock index 1a4c7e9fcd6..1965ee06425 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9217,6 +9217,7 @@ dependencies = [ "camino", "camino-tempfile", "clap 4.3.4", + "ddm-admin-client", "debug-ignore", "display-error-chain", "dropshot", diff --git a/openapi/wicketd.json b/openapi/wicketd.json index a0bdafc1a8e..86067d803bf 100644 --- a/openapi/wicketd.json +++ b/openapi/wicketd.json @@ -107,6 +107,30 @@ } } }, + "/bootstrap-sleds": { + "get": { + "summary": "Get wicketd's current view of all sleds visible on the bootstrap network.", + "operationId": "get_bootstrap_sleds", + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BootstrapSledIps" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/clear-update-state/{type}/{slot}": { "post": { "summary": "Resets update state for a sled.", @@ -664,26 +688,52 @@ "BootstrapSledDescription": { "type": "object", "properties": { - "id": { - "$ref": "#/components/schemas/SpIdentifier" + "baseboard": { + "$ref": "#/components/schemas/Baseboard" }, - "model": { - "type": "string" + "bootstrap_ip": { + "nullable": true, + "description": "The sled's bootstrap address, if the host is on and we've discovered it on the bootstrap network.", + "type": "string", + "format": "ipv6" }, - "revision": { - "type": "integer", - "format": "uint32", - "minimum": 0 + "id": { + "$ref": "#/components/schemas/SpIdentifier" + } + }, + "required": [ + "baseboard", + "id" + ] + }, + "BootstrapSledIp": { + "type": "object", + "properties": { + "baseboard": { + "$ref": "#/components/schemas/Baseboard" }, - "serial_number": { - "type": "string" + "ip": { + "type": "string", + "format": "ipv6" } }, "required": [ - "id", - "model", - "revision", - "serial_number" + "baseboard", + "ip" + ] + }, + "BootstrapSledIps": { + "type": "object", + "properties": { + "sleds": { + "type": "array", + "items": { + "$ref": "#/components/schemas/BootstrapSledIp" + } + } + }, + "required": [ + "sleds" ] }, "CertificateUploadResponse": { diff --git a/sled-hardware/src/lib.rs b/sled-hardware/src/lib.rs index 60cb83ca7c1..4de5e94aa58 100644 --- a/sled-hardware/src/lib.rs +++ b/sled-hardware/src/lib.rs @@ -74,7 +74,16 @@ pub enum SledMode { /// Describes properties that should uniquely identify a Gimlet. #[derive( - Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, + Clone, + Debug, + PartialOrd, + Ord, + PartialEq, + Eq, + Hash, + Serialize, + Deserialize, + JsonSchema, )] #[serde(tag = "type", rename_all = "snake_case")] pub enum Baseboard { diff --git a/wicket/src/rack_setup/config_toml.rs b/wicket/src/rack_setup/config_toml.rs index fbd55e7051d..992b0f254cf 100644 --- a/wicket/src/rack_setup/config_toml.rs +++ b/wicket/src/rack_setup/config_toml.rs @@ -106,6 +106,29 @@ fn format_multiline_array(array: &mut Array) { } fn build_sleds_array(sleds: &[BootstrapSledDescription]) -> Array { + // Helper function to build the comment attached to a given sled. + fn sled_comment(sled: &BootstrapSledDescription, end: &str) -> String { + use wicketd_client::types::Baseboard; + let ip = sled + .bootstrap_ip + .map(|ip| Cow::from(format!("{ip}"))) + .unwrap_or_else(|| Cow::from("IP address UNKNOWN")); + match &sled.baseboard { + Baseboard::Gimlet { identifier, model, revision } => { + format!( + " # {identifier} (model {model} revision {revision}, {ip})\ + {end}" + ) + } + Baseboard::Unknown => { + format!(" # UNKNOWN SLED ({ip}){end}") + } + Baseboard::Pc { identifier, model } => { + format!(" # NON-GIMLET {identifier} (model {model}, {ip}){end}") + } + } + } + let mut array = Array::new(); let mut prev: Option<&BootstrapSledDescription> = None; @@ -121,13 +144,7 @@ fn build_sleds_array(sleds: &[BootstrapSledDescription]) -> Array { // We have to attach the comment for each sled on the _next_ item in the // array, so here we set our prefix to be the previous item's details. if let Some(prev) = prev { - decor.set_prefix(format!( - " # {serial} (model {model}, revision {rev}){sep}", - serial = prev.serial_number, - model = prev.model, - rev = prev.revision, - sep = ARRAY_SEP, - )); + decor.set_prefix(sled_comment(prev, ARRAY_SEP)); } else { decor.set_prefix(ARRAY_SEP); } @@ -139,12 +156,7 @@ fn build_sleds_array(sleds: &[BootstrapSledDescription]) -> Array { // Because we attach comments to previous items, we also need to add a // comment to the last element. if let Some(prev) = prev { - array.set_trailing(format!( - " # {serial} (model {model}, revision {rev})\n", - serial = prev.serial_number, - model = prev.model, - rev = prev.revision, - )); + array.set_trailing(sled_comment(prev, "\n")); array.set_trailing_comma(true); } @@ -189,7 +201,9 @@ fn populate_network_table( mod tests { use super::*; use omicron_common::api::internal::shared::RackNetworkConfig as InternalRackNetworkConfig; + use std::net::Ipv6Addr; use wicket_common::rack_setup::PutRssUserConfigInsensitive; + use wicketd_client::types::Baseboard; use wicketd_client::types::PortFec; use wicketd_client::types::PortSpeed; use wicketd_client::types::SpIdentifier; @@ -258,15 +272,21 @@ mod tests { bootstrap_sleds: vec![ BootstrapSledDescription { id: SpIdentifier { slot: 1, type_: SpType::Sled }, - model: "model1".into(), - revision: 3, - serial_number: "serial 1 2 3".into(), + baseboard: Baseboard::Gimlet { + model: "model1".into(), + revision: 3, + identifier: "serial 1 2 3".into(), + }, + bootstrap_ip: None, }, BootstrapSledDescription { id: SpIdentifier { slot: 5, type_: SpType::Sled }, - model: "model2".into(), - revision: 5, - serial_number: "serial 4 5 6".into(), + baseboard: Baseboard::Gimlet { + model: "model2".into(), + revision: 5, + identifier: "serial 4 5 6".into(), + }, + bootstrap_ip: Some(Ipv6Addr::LOCALHOST), }, ], dns_servers: vec!["1.1.1.1".into(), "2.2.2.2".into()], diff --git a/wicketd-client/src/lib.rs b/wicketd-client/src/lib.rs index 672da3fcde8..1f130a19151 100644 --- a/wicketd-client/src/lib.rs +++ b/wicketd-client/src/lib.rs @@ -37,6 +37,7 @@ progenitor::generate_api!( Ipv4Range = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, Ipv6Range = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, IpRange = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, + Baseboard = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, BootstrapSledDescription = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, RackNetworkConfig = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, CurrentRssUserConfigInsensitive = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, diff --git a/wicketd/Cargo.toml b/wicketd/Cargo.toml index f6b16e64135..fbdb3d9944a 100644 --- a/wicketd/Cargo.toml +++ b/wicketd/Cargo.toml @@ -36,6 +36,7 @@ toml.workspace = true uuid.workspace = true bootstrap-agent-client.workspace = true +ddm-admin-client.workspace = true gateway-client.workspace = true installinator-artifactd.workspace = true installinator-common.workspace = true diff --git a/wicketd/src/bootstrap_addrs.rs b/wicketd/src/bootstrap_addrs.rs new file mode 100644 index 00000000000..f6936fd3326 --- /dev/null +++ b/wicketd/src/bootstrap_addrs.rs @@ -0,0 +1,175 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use ddm_admin_client::Client as DdmAdminClient; +use futures::stream::FuturesUnordered; +use sled_hardware::underlay::BootstrapInterface; +use sled_hardware::Baseboard; +use slog::warn; +use slog::Logger; +use std::collections::BTreeMap; +use std::net::Ipv6Addr; +use std::sync::Arc; +use std::sync::Mutex; +use std::time::Duration; +use tokio::task::JoinHandle; +use tokio_stream::StreamExt; + +pub(crate) struct BootstrapPeers { + // We use a standard mutex here, not a tokio mutex, even though this is + // shared with a tokio task. We only keep it locked long enough to insert a + // new entry or clone it. + sleds: Arc>>, + inner_task: JoinHandle<()>, +} + +impl Drop for BootstrapPeers { + fn drop(&mut self) { + self.inner_task.abort(); + } +} + +#[allow(dead_code)] // TODO REMOVE +impl BootstrapPeers { + pub(crate) fn new(log: &Logger) -> Self { + let log = log.new(slog::o!("component" => "BootstrapPeers")); + let sleds = Arc::default(); + let inner_task = tokio::spawn(scan_for_peers(Arc::clone(&sleds), log)); + Self { sleds, inner_task } + } + + pub(crate) fn sleds(&self) -> BTreeMap { + self.sleds.lock().unwrap().clone() + } +} + +async fn scan_for_peers( + sleds: Arc>>, + log: Logger, +) { + // How frequently do we attempt to refresh the set of peers? This does not + // count the time it takes to do each refresh, which is potentially + // significant if any prefixes reported by DDM are not running responsive + // sled-agents, since we'll wait for them to time out. + const SLEEP_BETWEEN_REFRESH: Duration = Duration::from_secs(30); + + let ddm_client = make_ddm_admin_client(&log).await; + + // We only share `sleds` with the `BootstrapPeers` that created us, and it + // only ever reads the current value: we are the only one that changes it. + // We keep the previous version we set it to so if the set of addresses + // remains unchanged (which we expect nearly all the time), we don't bother + // locking and copying the new set in. + let mut prev_sleds = None; + loop { + // Ask mg-ddm for a list of bootstrap address prefixes. + let addrs = possible_sled_agent_addrs(&ddm_client, &log).await; + + // Query the sled-agent on each prefix for its baseboard, dropping any + // addresses that fail to return. + let mut addrs_to_sleds = addrs + .map(|ip| { + let log = &log; + async move { + let client = bootstrap_agent_client::Client::new( + &format!("http://[{ip}]"), + log.clone(), + ); + let result = client.baseboard_get().await; + + (ip, result) + } + }) + .collect::>(); + + let mut all_sleds = BTreeMap::new(); + while let Some((ip, result)) = addrs_to_sleds.next().await { + match result { + Ok(baseboard) => { + // Convert from progenitor type back to `sled-hardware` + // type. + let baseboard = match baseboard.into_inner() { + bootstrap_agent_client::types::Baseboard::Gimlet { + identifier, + model, + revision, + } => Baseboard::new_gimlet(identifier, model, revision), + bootstrap_agent_client::types::Baseboard::Unknown => { + Baseboard::unknown() + } + bootstrap_agent_client::types::Baseboard::Pc { + identifier, + model, + } => Baseboard::new_pc(identifier, model), + }; + + all_sleds.insert(baseboard, ip); + } + Err(err) => { + warn!( + log, "Failed to get baseboard for {ip}"; + "err" => #%err, + ); + } + } + } + + // Did our set of peers change? If so, update both `sleds` (shared with + // our parent `BootstrapPeers`) and `prev_sleds` (our local cache). + if Some(&all_sleds) != prev_sleds.as_ref() { + *sleds.lock().unwrap() = all_sleds.clone(); + prev_sleds = Some(all_sleds); + } + + tokio::time::sleep(SLEEP_BETWEEN_REFRESH).await; + } +} + +async fn possible_sled_agent_addrs( + ddm_client: &DdmAdminClient, + log: &Logger, +) -> impl Iterator { + // TODO: Should we use `backoff` here instead of a loop/sleep? We're talking + // to a service's admin interface on localhost within our own switch zone, + // and we're only asking for its current state. Backoff should be + // unnecessary, I think? + const RETRY: Duration = Duration::from_secs(5); + + loop { + match ddm_client + .derive_bootstrap_addrs_from_prefixes(&[ + BootstrapInterface::GlobalZone, + ]) + .await + { + Ok(addrs) => return addrs, + Err(err) => { + warn!( + log, "Failed to get prefixes from ddm"; + "err" => #%err, + ); + tokio::time::sleep(RETRY).await; + } + } + } +} + +async fn make_ddm_admin_client(log: &Logger) -> DdmAdminClient { + const DDM_CONSTRUCT_RETRY: Duration = Duration::from_secs(1); + + // We don't really expect this to fail ever, so just keep retrying + // indefinitely if it does. + loop { + match DdmAdminClient::localhost(log) { + Ok(client) => return client, + Err(err) => { + warn!( + log, "Failed to construct DdmAdminClient"; + "err" => #%err, + ); + tokio::time::sleep(DDM_CONSTRUCT_RETRY).await; + } + } + } +} diff --git a/wicketd/src/context.rs b/wicketd/src/context.rs index 1142ee91444..a2075d051bf 100644 --- a/wicketd/src/context.rs +++ b/wicketd/src/context.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use std::sync::Mutex; +use crate::bootstrap_addrs::BootstrapPeers; use crate::rss_config::CurrentRssConfig; use crate::update_tracker::UpdateTracker; use crate::MgsHandle; @@ -16,6 +17,7 @@ use sled_hardware::Baseboard; pub struct ServerContext { pub mgs_handle: MgsHandle, pub mgs_client: gateway_client::Client, + pub(crate) bootstrap_peers: BootstrapPeers, pub(crate) update_tracker: Arc, pub(crate) baseboard: Option, pub(crate) rss_config: Mutex, diff --git a/wicketd/src/http_entrypoints.rs b/wicketd/src/http_entrypoints.rs index 6d77ed15c89..f2c00197bf9 100644 --- a/wicketd/src/http_entrypoints.rs +++ b/wicketd/src/http_entrypoints.rs @@ -32,6 +32,7 @@ use serde::Serialize; use sled_hardware::Baseboard; use std::collections::BTreeMap; use std::collections::BTreeSet; +use std::net::Ipv6Addr; use std::time::Duration; use uuid::Uuid; use wicket_common::rack_setup::PutRssUserConfigInsensitive; @@ -46,6 +47,7 @@ pub fn api() -> WicketdApiDescription { fn register_endpoints( api: &mut WicketdApiDescription, ) -> Result<(), String> { + api.register(get_bootstrap_sleds)?; api.register(get_rss_config)?; api.register(put_rss_config)?; api.register(put_rss_config_recovery_user_password_hash)?; @@ -71,6 +73,57 @@ pub fn api() -> WicketdApiDescription { api } +#[derive( + Clone, + Debug, + Serialize, + Deserialize, + JsonSchema, + PartialEq, + Eq, + PartialOrd, + Ord, +)] +pub struct BootstrapSledIp { + pub baseboard: Baseboard, + pub ip: Ipv6Addr, +} + +#[derive( + Clone, + Debug, + Serialize, + Deserialize, + JsonSchema, + PartialEq, + Eq, + PartialOrd, + Ord, +)] +pub struct BootstrapSledIps { + pub sleds: Vec, +} + +/// Get wicketd's current view of all sleds visible on the bootstrap network. +#[endpoint { + method = GET, + path = "/bootstrap-sleds" +}] +async fn get_bootstrap_sleds( + rqctx: RequestContext, +) -> Result, HttpError> { + let ctx = rqctx.context(); + + let sleds = ctx + .bootstrap_peers + .sleds() + .into_iter() + .map(|(baseboard, ip)| BootstrapSledIp { baseboard, ip }) + .collect(); + + Ok(HttpResponseOk(BootstrapSledIps { sleds })) +} + #[derive( Clone, Debug, @@ -84,9 +137,10 @@ pub fn api() -> WicketdApiDescription { )] pub struct BootstrapSledDescription { pub id: SpIdentifier, - pub serial_number: String, - pub model: String, - pub revision: u32, + pub baseboard: Baseboard, + /// The sled's bootstrap address, if the host is on and we've discovered it + /// on the bootstrap network. + pub bootstrap_ip: Option, } // This is the subset of `RackInitializeRequest` that the user fills in in clear @@ -152,7 +206,10 @@ async fn get_rss_config( let inventory = inventory_or_unavail(&ctx.mgs_handle).await?; let mut config = ctx.rss_config.lock().unwrap(); - config.populate_available_bootstrap_sleds_from_inventory(&inventory); + config.populate_available_bootstrap_sleds_from_inventory( + &inventory, + &ctx.bootstrap_peers, + ); Ok(HttpResponseOk((&*config).into())) } @@ -176,7 +233,10 @@ async fn put_rss_config( let inventory = inventory_or_unavail(&ctx.mgs_handle).await?; let mut config = ctx.rss_config.lock().unwrap(); - config.populate_available_bootstrap_sleds_from_inventory(&inventory); + config.populate_available_bootstrap_sleds_from_inventory( + &inventory, + &ctx.bootstrap_peers, + ); config .update(body.into_inner(), ctx.baseboard.as_ref()) .map_err(|err| HttpError::for_bad_request(None, err))?; diff --git a/wicketd/src/lib.rs b/wicketd/src/lib.rs index 61c729c8393..52c750f3412 100644 --- a/wicketd/src/lib.rs +++ b/wicketd/src/lib.rs @@ -3,6 +3,7 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. mod artifacts; +mod bootstrap_addrs; mod config; mod context; mod http_entrypoints; @@ -14,6 +15,7 @@ mod update_tracker; use anyhow::{anyhow, Result}; use artifacts::{WicketdArtifactServer, WicketdArtifactStore}; +use bootstrap_addrs::BootstrapPeers; pub use config::Config; pub(crate) use context::ServerContext; pub use installinator_progress::{IprUpdateTracker, RunningUpdateState}; @@ -97,6 +99,8 @@ impl Server { ipr_update_tracker.clone(), )); + let bootstrap_peers = BootstrapPeers::new(&log); + let wicketd_server = { let log = log.new(o!("component" => "dropshot (wicketd)")); let mgs_client = make_mgs_client(log.clone(), args.mgs_address); @@ -106,6 +110,7 @@ impl Server { ServerContext { mgs_handle, mgs_client, + bootstrap_peers, update_tracker: update_tracker.clone(), baseboard: args.baseboard, rss_config: Default::default(), diff --git a/wicketd/src/rss_config.rs b/wicketd/src/rss_config.rs index dfe766f57f5..4c0b057f133 100644 --- a/wicketd/src/rss_config.rs +++ b/wicketd/src/rss_config.rs @@ -4,6 +4,7 @@ //! Support for user-provided RSS configuration options. +use crate::bootstrap_addrs::BootstrapPeers; use crate::http_entrypoints::BootstrapSledDescription; use crate::http_entrypoints::CertificateUploadResponse; use crate::http_entrypoints::CurrentRssUserConfig; @@ -51,7 +52,10 @@ impl CurrentRssConfig { pub(crate) fn populate_available_bootstrap_sleds_from_inventory( &mut self, inventory: &RackV1Inventory, + bootstrap_peers: &BootstrapPeers, ) { + let bootstrap_sleds = bootstrap_peers.sleds(); + self.inventory = inventory .sps .iter() @@ -60,11 +64,16 @@ impl CurrentRssConfig { return None; } let state = sp.state.as_ref()?; + let baseboard = Baseboard::new_gimlet( + state.serial_number.clone(), + state.model.clone(), + state.revision.into(), + ); + let bootstrap_ip = bootstrap_sleds.get(&baseboard).copied(); Some(BootstrapSledDescription { id: sp.id, - serial_number: state.serial_number.clone(), - model: state.model.clone(), - revision: state.revision, + baseboard, + bootstrap_ip, }) }) .collect(); @@ -148,17 +157,12 @@ impl CurrentRssConfig { // First, confirm we have ourself in the inventory _and_ the user didn't // remove us from the list. - if let Some(Baseboard::Gimlet { identifier, model, revision }) = - our_baseboard - { + if let Some(our_baseboard @ Baseboard::Gimlet { .. }) = our_baseboard { let our_slot = self .inventory .iter() .find_map(|sled| { - if &sled.serial_number == identifier - && &sled.model == model - && i64::from(sled.revision) == *revision - { + if sled.baseboard == *our_baseboard { Some(sled.id.slot) } else { None @@ -167,13 +171,13 @@ impl CurrentRssConfig { .ok_or_else(|| { format!( "Inventory is missing the scrimlet where wicketd is \ - running ({identifier}, model {model} rev {revision})", + running ({our_baseboard:?})", ) })?; if !value.bootstrap_sleds.contains(&our_slot) { return Err(format!( "Cannot remove the scrimlet where wicketd is running \ - (sled {our_slot}: {identifier}, model {model} rev {revision}) \ + (sled {our_slot}: {our_baseboard:?}) \ from bootstrap_sleds" )); } From d54654bc22ce6b64b23cff9158f94ec93119d988 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 21 Jun 2023 10:41:16 -0700 Subject: [PATCH 12/12] Avoid passing ExternalDns as service and dataset, zpool serde simplification --- Cargo.lock | 1 + illumos-utils/Cargo.toml | 1 + illumos-utils/src/zpool.rs | 83 +++-------------------- openapi/sled-agent.json | 2 +- sled-agent/src/params.rs | 4 -- sled-agent/src/rack_setup/plan/service.rs | 2 + 6 files changed, 13 insertions(+), 80 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1965ee06425..e7ac41acc71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3174,6 +3174,7 @@ dependencies = [ "oxide-vpc", "schemars", "serde", + "serde_json", "slog", "smf", "thiserror", diff --git a/illumos-utils/Cargo.toml b/illumos-utils/Cargo.toml index 2ad76d579f3..79b7d20e4b4 100644 --- a/illumos-utils/Cargo.toml +++ b/illumos-utils/Cargo.toml @@ -34,6 +34,7 @@ opte-ioctl.workspace = true [dev-dependencies] mockall.workspace = true +serde_json.workspace = true toml.workspace = true [features] diff --git a/illumos-utils/src/zpool.rs b/illumos-utils/src/zpool.rs index 2560e68eba9..b6dbc33232b 100644 --- a/illumos-utils/src/zpool.rs +++ b/illumos-utils/src/zpool.rs @@ -7,7 +7,7 @@ use crate::{execute, ExecutionError, PFEXEC}; use camino::{Utf8Path, Utf8PathBuf}; use schemars::JsonSchema; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Serialize}; use std::fmt; use std::str::FromStr; use uuid::Uuid; @@ -271,7 +271,7 @@ impl Zpool { } } -#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, JsonSchema)] +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, JsonSchema, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum ZpoolKind { // This zpool is used for external storage (u.2) @@ -284,7 +284,7 @@ pub enum ZpoolKind { /// /// This expects that the format will be: `ox{i,p}_` - we parse the prefix /// when reading the structure, and validate that the UUID can be utilized. -#[derive(Clone, Debug, Hash, PartialEq, Eq, JsonSchema)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, JsonSchema, Serialize, Deserialize)] pub struct ZpoolName { id: Uuid, kind: ZpoolKind, @@ -323,25 +323,6 @@ impl ZpoolName { } } -impl<'de> Deserialize<'de> for ZpoolName { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let s = String::deserialize(deserializer)?; - ZpoolName::from_str(&s).map_err(serde::de::Error::custom) - } -} - -impl Serialize for ZpoolName { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_str(&self.to_string()) - } -} - impl FromStr for ZpoolName { type Err = String; @@ -374,60 +355,12 @@ impl fmt::Display for ZpoolName { mod test { use super::*; - fn toml_string(s: &str) -> String { - format!("zpool_name = \"{}\"", s) - } - - fn parse_name(s: &str) -> Result { - toml_string(s) - .parse::() - .expect("Cannot parse as TOML value") - .get("zpool_name") - .expect("Missing key") - .clone() - .try_into::() - } - - #[test] - fn test_parse_external_zpool_name() { - let uuid: Uuid = - "d462a7f7-b628-40fe-80ff-4e4189e2d62b".parse().unwrap(); - let good_name = format!("{}{}", ZPOOL_EXTERNAL_PREFIX, uuid); - - let name = parse_name(&good_name).expect("Cannot parse as ZpoolName"); - assert_eq!(uuid, name.id()); - assert_eq!(ZpoolKind::External, name.kind()); - } - #[test] - fn test_parse_internal_zpool_name() { - let uuid: Uuid = - "d462a7f7-b628-40fe-80ff-4e4189e2d62b".parse().unwrap(); - let good_name = format!("{}{}", ZPOOL_INTERNAL_PREFIX, uuid); - - let name = parse_name(&good_name).expect("Cannot parse as ZpoolName"); - assert_eq!(uuid, name.id()); - assert_eq!(ZpoolKind::Internal, name.kind()); - } - - #[test] - fn test_parse_bad_zpool_names() { - let bad_names = vec![ - // Nonsense string - "this string is GARBAGE", - // Missing prefix - "d462a7f7-b628-40fe-80ff-4e4189e2d62b", - // Underscores - "oxp_d462a7f7_b628_40fe_80ff_4e4189e2d62b", - ]; - - for bad_name in &bad_names { - assert!( - parse_name(&bad_name).is_err(), - "Parsing {} should fail", - bad_name - ); - } + fn test_parse_zpool_name_json() { + let _zpool_name: ZpoolName = serde_json::from_str(r#"{ + "id": "d462a7f7-b628-40fe-80ff-4e4189e2d62b", + "kind": "external" + }"#).expect("Could not parse ZpoolName from Json Object"); } #[test] diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index f7e89787499..47e62c03cd0 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -1799,7 +1799,7 @@ "pattern": "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$" }, "ServiceEnsureBody": { - "description": "Used to request that the Sled initialize multiple services.\n\nThis may be used to record that certain sleds are responsible for launching services which may not be associated with a dataset, such as Nexus.", + "description": "Used to request that the Sled initialize multiple services.", "type": "object", "properties": { "services": { diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 868d293caa8..836dec3b0e4 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -665,10 +665,6 @@ impl TryFrom } /// Used to request that the Sled initialize multiple services. -/// -/// This may be used to record that certain sleds are responsible for -/// launching services which may not be associated with a dataset, such -/// as Nexus. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)] pub struct ServiceEnsureBody { pub services: Vec, diff --git a/sled-agent/src/rack_setup/plan/service.rs b/sled-agent/src/rack_setup/plan/service.rs index f102d25c3a9..f8002126540 100644 --- a/sled-agent/src/rack_setup/plan/service.rs +++ b/sled-agent/src/rack_setup/plan/service.rs @@ -312,6 +312,7 @@ impl Plan { address: http_address, gz_address: None, }); + /* request.services.push(ServiceZoneRequest { id, zone_type: ZoneType::ExternalDns, @@ -332,6 +333,7 @@ impl Plan { }, }], }); + */ } // The first enumerated sleds get assigned the responsibility