From bf1f74a0d7e00c35b960e335a5070cfc6c980b17 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 17 Oct 2023 16:15:44 +0000 Subject: [PATCH 01/11] Add type conversions for EarlyNetworkConfig upgrade This commit adds deserialization and conversion support for v0 and v1 of EarlyNetworkConfig types. Future commits will add support for writing the v1 version back to the bootstore ledger if necessary. Unit tests for conversion also coming soon. --- common/src/api/internal/shared.rs | 62 ++++++++++++++- openapi/bootstrap-agent.json | 6 +- openapi/sled-agent.json | 29 +++++-- schema/rss-sled-plan.json | 6 +- sled-agent/src/bootstrap/early_networking.rs | 81 ++++++++++++++++++-- sled-agent/src/rack_setup/service.rs | 12 ++- sled-agent/src/sim/http_entrypoints.rs | 23 +++--- sled-agent/src/sled_agent.rs | 2 +- 8 files changed, 187 insertions(+), 34 deletions(-) diff --git a/common/src/api/internal/shared.rs b/common/src/api/internal/shared.rs index 1300a8d5ff..1d631bec7b 100644 --- a/common/src/api/internal/shared.rs +++ b/common/src/api/internal/shared.rs @@ -68,9 +68,17 @@ pub struct SourceNatConfig { pub last_port: u16, } +// We alias [`RackNetworkConfig`] to the current version of the protocol, so +// that we can convert between versions as necessary. +pub type RackNetworkConfig = RackNetworkConfigV1; + /// Initial network configuration +/// +/// TODO(AJS): It's unclear if this should be serde renamed to `RackNetworkConfig` +/// I *think* it's useful to have the version be explicit in the name, but I'm open +/// to discussion. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] -pub struct RackNetworkConfig { +pub struct RackNetworkConfigV1 { // TODO: #3591 Consider making infra-ip ranges implicit for uplinks /// First ip address to be used for configuring network infrastructure pub infra_ip_first: Ipv4Addr, @@ -82,6 +90,41 @@ pub struct RackNetworkConfig { pub bgp: Vec, } +/// Deprecated, use `RackNetworkConfig` instead. Cannot actually deprecate due to +/// +/// +/// Our first version of `RackNetworkConfig`. If this exists in the bootstore, we +/// upgrade out of it into `RackNetworkConfigV1` or later versions if possible. +/// Initial network configuration +/// +// TODO(AJS): Should this actually exist in the OpenAPI spec or should we just +// hide the old version inside sled-agent/src/bootstrap/early_networking.rs +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] +pub struct RackNetworkConfigV0 { + // TODO: #3591 Consider making infra-ip ranges implicit for uplinks + /// First ip address to be used for configuring network infrastructure + pub infra_ip_first: Ipv4Addr, + /// Last ip address to be used for configuring network infrastructure + pub infra_ip_last: Ipv4Addr, + /// Uplinks for connecting the rack to external networks + pub uplinks: Vec, +} + +impl From for RackNetworkConfigV1 { + fn from(value: RackNetworkConfigV0) -> Self { + RackNetworkConfigV1 { + infra_ip_first: value.infra_ip_first, + infra_ip_last: value.infra_ip_last, + ports: value + .uplinks + .into_iter() + .map(|uplink| PortConfigV1::from(uplink)) + .collect(), + bgp: vec![], + } + } +} + #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] pub struct BgpConfig { /// The autonomous system number for the BGP configuration. @@ -126,6 +169,23 @@ pub struct PortConfigV1 { pub bgp_peers: Vec, } +impl From for PortConfigV1 { + fn from(value: UplinkConfig) -> Self { + PortConfigV1 { + routes: vec![RouteConfig { + destination: "0.0.0.0/0".parse().unwrap(), + nexthop: value.gateway_ip.into(), + }], + addresses: vec![value.uplink_cidr.into()], + switch: value.switch, + port: value.uplink_port, + uplink_port_speed: value.uplink_port_speed, + uplink_port_fec: value.uplink_port_fec, + bgp_peers: vec![], + } + } +} + /// Deprecated, use PortConfigV1 instead. Cannot actually deprecate due to /// #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] diff --git a/openapi/bootstrap-agent.json b/openapi/bootstrap-agent.json index 91b8ae9130..1e70bf3920 100644 --- a/openapi/bootstrap-agent.json +++ b/openapi/bootstrap-agent.json @@ -626,7 +626,7 @@ "description": "Initial rack network configuration", "allOf": [ { - "$ref": "#/components/schemas/RackNetworkConfig" + "$ref": "#/components/schemas/RackNetworkConfigV1" } ] }, @@ -663,8 +663,8 @@ "recovery_silo" ] }, - "RackNetworkConfig": { - "description": "Initial network configuration", + "RackNetworkConfigV1": { + "description": "Initial network configuration\n\nTODO(AJS): It's unclear if this should be serde renamed to `RackNetworkConfig` I *think* it's useful to have the version be explicit in the name, but I'm open to discussion.", "type": "object", "properties": { "bgp": { diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index 0dbf1f6fb4..e7ce138877 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -1692,14 +1692,33 @@ ] }, "EarlyNetworkConfig": { - "description": "Network configuration required to bring up the control plane\n\nThe fields in this structure are those from [`super::params::RackInitializeRequest`] necessary for use beyond RSS. This is just for the initial rack configuration and cold boot purposes. Updates will come from Nexus in the future.", + "description": "Network configuration required to bring up the control plane\n\nThe fields in this structure are those from [`super::params::RackInitializeRequest`] necessary for use beyond RSS. This is just for the initial rack configuration and cold boot purposes. Updates come from Nexus.", "type": "object", "properties": { + "body": { + "$ref": "#/components/schemas/EarlyNetworkConfigBody" + }, "generation": { "type": "integer", "format": "uint64", "minimum": 0 }, + "schema_version": { + "type": "integer", + "format": "uint32", + "minimum": 0 + } + }, + "required": [ + "body", + "generation", + "schema_version" + ] + }, + "EarlyNetworkConfigBody": { + "description": "This is the actual configuration of EarlyNetworking.\n\nWe nest it below the \"header\" of `generation` and `schema_version` so that we can perform partial deserialization of `EarlyNetworkConfig` to only read the header and defer deserialization of the body once we know the schema version. This is possible via the use of [`serde_json::value::RawValue`] in future (post-v1) deserialization paths.", + "type": "object", + "properties": { "ntp_servers": { "description": "The external NTP server addresses.", "type": "array", @@ -1709,10 +1728,9 @@ }, "rack_network_config": { "nullable": true, - "description": "A copy of the initial rack network configuration when we are in generation `1`.", "allOf": [ { - "$ref": "#/components/schemas/RackNetworkConfig" + "$ref": "#/components/schemas/RackNetworkConfigV1" } ] }, @@ -1722,7 +1740,6 @@ } }, "required": [ - "generation", "ntp_servers", "rack_subnet" ] @@ -2566,8 +2583,8 @@ "minItems": 2, "maxItems": 2 }, - "RackNetworkConfig": { - "description": "Initial network configuration", + "RackNetworkConfigV1": { + "description": "Initial network configuration\n\nTODO(AJS): It's unclear if this should be serde renamed to `RackNetworkConfig` I *think* it's useful to have the version be explicit in the name, but I'm open to discussion.", "type": "object", "properties": { "bgp": { diff --git a/schema/rss-sled-plan.json b/schema/rss-sled-plan.json index d91f2080c1..0cac13a8d9 100644 --- a/schema/rss-sled-plan.json +++ b/schema/rss-sled-plan.json @@ -472,7 +472,7 @@ "description": "Initial rack network configuration", "anyOf": [ { - "$ref": "#/definitions/RackNetworkConfig" + "$ref": "#/definitions/RackNetworkConfigV1" }, { "type": "null" @@ -503,8 +503,8 @@ } } }, - "RackNetworkConfig": { - "description": "Initial network configuration", + "RackNetworkConfigV1": { + "description": "Initial network configuration\n\nTODO(AJS): It's unclear if this should be serde renamed to `RackNetworkConfig` I *think* it's useful to have the version be explicit in the name, but I'm open to discussion.", "type": "object", "required": [ "bgp", diff --git a/sled-agent/src/bootstrap/early_networking.rs b/sled-agent/src/bootstrap/early_networking.rs index 7bb2506dc3..6efcc51b17 100644 --- a/sled-agent/src/bootstrap/early_networking.rs +++ b/sled-agent/src/bootstrap/early_networking.rs @@ -20,7 +20,8 @@ use ipnetwork::IpNetwork; use omicron_common::address::{Ipv6Subnet, AZ_PREFIX, MGS_PORT}; use omicron_common::address::{DDMD_PORT, DENDRITE_PORT}; use omicron_common::api::internal::shared::{ - PortConfigV1, PortFec, PortSpeed, RackNetworkConfig, SwitchLocation, + PortConfigV1, PortFec, PortSpeed, RackNetworkConfig, RackNetworkConfigV0, + RackNetworkConfigV1, SwitchLocation, }; use omicron_common::backoff::{ retry_notify, retry_policy_local, BackoffError, ExponentialBackoff, @@ -568,25 +569,67 @@ fn retry_policy_switch_mapping() -> ExponentialBackoff { /// The fields in this structure are those from /// [`super::params::RackInitializeRequest`] necessary for use beyond RSS. This /// is just for the initial rack configuration and cold boot purposes. Updates -/// will come from Nexus in the future. +/// come from Nexus. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct EarlyNetworkConfig { - // The version of data. + // The current generation number of data as stored in CRDB. + // The initial generation is set during RSS time and then only mutated + // by Nexus. pub generation: u64, + // Which version of the data structure do we have. This is to help with + // deserialization and conversion in future updates. + pub schema_version: u32, + + // The actual configuration details + pub body: EarlyNetworkConfigBody, +} + +/// This is the actual configuration of EarlyNetworking. +/// +/// We nest it below the "header" of `generation` and `schema_version` so that +/// we can perform partial deserialization of `EarlyNetworkConfig` to only read +/// the header and defer deserialization of the body once we know the schema +/// version. This is possible via the use of [`serde_json::value::RawValue`] in +/// future (post-v1) deserialization paths. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct EarlyNetworkConfigBody { pub rack_subnet: Ipv6Addr, /// The external NTP server addresses. pub ntp_servers: Vec, - /// A copy of the initial rack network configuration when we are in - /// generation `1`. + // Rack network configuration as delivered from RSS or Nexus pub rack_network_config: Option, } +// The first production version of the EarlyNetworkConfig. +// +// If this version is in the bootstore than we need to convert it to +// `EarlyNetworkConfig`. We intend to convert this data on +// +// Once we do this for all customers that have initialized racks with the old version we +// can go ahead and remove this type and its conversion code altogether. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +struct EarlyNetworkConfigV0 { + // The current generation number of data as stored in CRDB. + // The initial generation is set during RSS time and then only mutated + // by Nexus. + pub generation: u64, + + pub rack_subnet: Ipv6Addr, + + /// The external NTP server addresses. + pub ntp_servers: Vec, + + // Rack network configuration as delivered from RSS and only existing at + // generation 1 + pub rack_network_config: Option, +} + impl EarlyNetworkConfig { pub fn az_subnet(&self) -> Ipv6Subnet { - Ipv6Subnet::::new(self.rack_subnet) + Ipv6Subnet::::new(self.body.rack_subnet) } } @@ -603,13 +646,37 @@ impl From for bootstore::NetworkConfig { } } +// Note: This currently only converts between v0 and v1 or deserializes v1 of +// `EarlyNetworkConfig`. impl TryFrom for EarlyNetworkConfig { type Error = serde_json::Error; fn try_from( value: bootstore::NetworkConfig, ) -> std::result::Result { - serde_json::from_slice(&value.blob) + // Try to deserialize the latest version of the data structure (v1). If + // that succeeds we are done. + if let Ok(val) = + serde_json::from_slice::(&value.blob) + { + return Ok(val); + } + + // We don't have the latest version. Try to deserialize v0 and then + // convert it to the latest version. + let v0 = serde_json::from_slice::(&value.blob)?; + + Ok(EarlyNetworkConfig { + generation: v0.generation, + schema_version: 1, + body: EarlyNetworkConfigBody { + rack_subnet: v0.rack_subnet, + ntp_servers: v0.ntp_servers, + rack_network_config: v0 + .rack_network_config + .map(|v0_config| RackNetworkConfigV1::from(v0_config)), + }, + }) } } diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 212a554c47..7b15ad1171 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -57,7 +57,8 @@ use super::config::SetupServiceConfig as Config; use crate::bootstrap::config::BOOTSTRAP_AGENT_HTTP_PORT; use crate::bootstrap::early_networking::{ - EarlyNetworkConfig, EarlyNetworkSetup, EarlyNetworkSetupError, + EarlyNetworkConfig, EarlyNetworkConfigBody, EarlyNetworkSetup, + EarlyNetworkSetupError, }; use crate::bootstrap::params::BootstrapAddressDiscovery; use crate::bootstrap::params::StartSledAgentRequest; @@ -895,9 +896,12 @@ impl ServiceInner { // from the bootstore". let early_network_config = EarlyNetworkConfig { generation: 1, - rack_subnet: config.rack_subnet, - ntp_servers: config.ntp_servers.clone(), - rack_network_config: config.rack_network_config.clone(), + schema_version: 1, + body: EarlyNetworkConfigBody { + rack_subnet: config.rack_subnet, + ntp_servers: config.ntp_servers.clone(), + rack_network_config: config.rack_network_config.clone(), + }, }; info!(self.log, "Writing Rack Network Configuration to bootstore"); //NOTE(ry) this is where the early network config gets saved. diff --git a/sled-agent/src/sim/http_entrypoints.rs b/sled-agent/src/sim/http_entrypoints.rs index fdbaa84cc9..797cb3535f 100644 --- a/sled-agent/src/sim/http_entrypoints.rs +++ b/sled-agent/src/sim/http_entrypoints.rs @@ -4,7 +4,9 @@ //! HTTP entrypoint functions for the sled agent's exposed API -use crate::bootstrap::early_networking::EarlyNetworkConfig; +use crate::bootstrap::early_networking::{ + EarlyNetworkConfig, EarlyNetworkConfigBody, +}; use crate::params::{ DiskEnsureBody, InstanceEnsureBody, InstancePutMigrationIdsBody, InstancePutStateBody, InstancePutStateResponse, InstanceUnregisterResponse, @@ -355,14 +357,17 @@ async fn read_network_bootstore_config( ) -> Result, HttpError> { let config = EarlyNetworkConfig { generation: 0, - rack_subnet: Ipv6Addr::UNSPECIFIED, - ntp_servers: Vec::new(), - rack_network_config: Some(RackNetworkConfig { - infra_ip_first: Ipv4Addr::UNSPECIFIED, - infra_ip_last: Ipv4Addr::UNSPECIFIED, - ports: Vec::new(), - bgp: Vec::new(), - }), + schema_version: 1, + body: EarlyNetworkConfigBody { + rack_subnet: Ipv6Addr::UNSPECIFIED, + ntp_servers: Vec::new(), + rack_network_config: Some(RackNetworkConfig { + infra_ip_first: Ipv4Addr::UNSPECIFIED, + infra_ip_last: Ipv4Addr::UNSPECIFIED, + ports: Vec::new(), + bgp: Vec::new(), + }), + }, }; Ok(HttpResponseOk(config)) } diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index d8d43d1895..52513f081d 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -412,7 +412,7 @@ impl SledAgent { EarlyNetworkConfig::try_from(serialized_config) .map_err(|err| BackoffError::transient(err.to_string()))?; - Ok(early_network_config.rack_network_config) + Ok(early_network_config.body.rack_network_config) }; let rack_network_config: Option = retry_notify::<_, String, _, _, _, _>( From a992afac9c5cb9622da4b8368a91d810cad58cc9 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 17 Oct 2023 18:27:36 +0000 Subject: [PATCH 02/11] review cleanup --- common/src/api/internal/shared.rs | 39 ----------------- openapi/bootstrap-agent.json | 2 +- openapi/sled-agent.json | 2 +- schema/rss-sled-plan.json | 2 +- sled-agent/src/bootstrap/early_networking.rs | 46 +++++++++++++++++--- 5 files changed, 42 insertions(+), 49 deletions(-) diff --git a/common/src/api/internal/shared.rs b/common/src/api/internal/shared.rs index 1d631bec7b..d475ec63d3 100644 --- a/common/src/api/internal/shared.rs +++ b/common/src/api/internal/shared.rs @@ -73,10 +73,6 @@ pub struct SourceNatConfig { pub type RackNetworkConfig = RackNetworkConfigV1; /// Initial network configuration -/// -/// TODO(AJS): It's unclear if this should be serde renamed to `RackNetworkConfig` -/// I *think* it's useful to have the version be explicit in the name, but I'm open -/// to discussion. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] pub struct RackNetworkConfigV1 { // TODO: #3591 Consider making infra-ip ranges implicit for uplinks @@ -90,41 +86,6 @@ pub struct RackNetworkConfigV1 { pub bgp: Vec, } -/// Deprecated, use `RackNetworkConfig` instead. Cannot actually deprecate due to -/// -/// -/// Our first version of `RackNetworkConfig`. If this exists in the bootstore, we -/// upgrade out of it into `RackNetworkConfigV1` or later versions if possible. -/// Initial network configuration -/// -// TODO(AJS): Should this actually exist in the OpenAPI spec or should we just -// hide the old version inside sled-agent/src/bootstrap/early_networking.rs -#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] -pub struct RackNetworkConfigV0 { - // TODO: #3591 Consider making infra-ip ranges implicit for uplinks - /// First ip address to be used for configuring network infrastructure - pub infra_ip_first: Ipv4Addr, - /// Last ip address to be used for configuring network infrastructure - pub infra_ip_last: Ipv4Addr, - /// Uplinks for connecting the rack to external networks - pub uplinks: Vec, -} - -impl From for RackNetworkConfigV1 { - fn from(value: RackNetworkConfigV0) -> Self { - RackNetworkConfigV1 { - infra_ip_first: value.infra_ip_first, - infra_ip_last: value.infra_ip_last, - ports: value - .uplinks - .into_iter() - .map(|uplink| PortConfigV1::from(uplink)) - .collect(), - bgp: vec![], - } - } -} - #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] pub struct BgpConfig { /// The autonomous system number for the BGP configuration. diff --git a/openapi/bootstrap-agent.json b/openapi/bootstrap-agent.json index 1e70bf3920..8bfdbeae2a 100644 --- a/openapi/bootstrap-agent.json +++ b/openapi/bootstrap-agent.json @@ -664,7 +664,7 @@ ] }, "RackNetworkConfigV1": { - "description": "Initial network configuration\n\nTODO(AJS): It's unclear if this should be serde renamed to `RackNetworkConfig` I *think* it's useful to have the version be explicit in the name, but I'm open to discussion.", + "description": "Initial network configuration", "type": "object", "properties": { "bgp": { diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index e7ce138877..d22dff4362 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -2584,7 +2584,7 @@ "maxItems": 2 }, "RackNetworkConfigV1": { - "description": "Initial network configuration\n\nTODO(AJS): It's unclear if this should be serde renamed to `RackNetworkConfig` I *think* it's useful to have the version be explicit in the name, but I'm open to discussion.", + "description": "Initial network configuration", "type": "object", "properties": { "bgp": { diff --git a/schema/rss-sled-plan.json b/schema/rss-sled-plan.json index 0cac13a8d9..2d35c2a355 100644 --- a/schema/rss-sled-plan.json +++ b/schema/rss-sled-plan.json @@ -504,7 +504,7 @@ } }, "RackNetworkConfigV1": { - "description": "Initial network configuration\n\nTODO(AJS): It's unclear if this should be serde renamed to `RackNetworkConfig` I *think* it's useful to have the version be explicit in the name, but I'm open to discussion.", + "description": "Initial network configuration", "type": "object", "required": [ "bgp", diff --git a/sled-agent/src/bootstrap/early_networking.rs b/sled-agent/src/bootstrap/early_networking.rs index 6efcc51b17..949e4d3082 100644 --- a/sled-agent/src/bootstrap/early_networking.rs +++ b/sled-agent/src/bootstrap/early_networking.rs @@ -20,8 +20,8 @@ use ipnetwork::IpNetwork; use omicron_common::address::{Ipv6Subnet, AZ_PREFIX, MGS_PORT}; use omicron_common::address::{DDMD_PORT, DENDRITE_PORT}; use omicron_common::api::internal::shared::{ - PortConfigV1, PortFec, PortSpeed, RackNetworkConfig, RackNetworkConfigV0, - RackNetworkConfigV1, SwitchLocation, + PortConfigV1, PortFec, PortSpeed, RackNetworkConfig, RackNetworkConfigV1, + SwitchLocation, UplinkConfig, }; use omicron_common::backoff::{ retry_notify, retry_policy_local, BackoffError, ExponentialBackoff, @@ -31,7 +31,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use slog::Logger; use std::collections::{HashMap, HashSet}; -use std::net::{IpAddr, Ipv6Addr, SocketAddrV6}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddrV6}; use std::time::{Duration, Instant}; use thiserror::Error; @@ -603,13 +603,14 @@ pub struct EarlyNetworkConfigBody { pub rack_network_config: Option, } -// The first production version of the EarlyNetworkConfig. +// The first production version of the `EarlyNetworkConfig`. // // If this version is in the bootstore than we need to convert it to -// `EarlyNetworkConfig`. We intend to convert this data on +// `EarlyNetworkConfigV1`. // -// Once we do this for all customers that have initialized racks with the old version we -// can go ahead and remove this type and its conversion code altogether. +// Once we do this for all customers that have initialized racks with the +// old version we can go ahead and remove this type and its conversion code +// altogether. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] struct EarlyNetworkConfigV0 { // The current generation number of data as stored in CRDB. @@ -680,6 +681,37 @@ impl TryFrom for EarlyNetworkConfig { } } +/// Deprecated, use `RackNetworkConfig` instead. Cannot actually deprecate due to +/// +/// +/// Our first version of `RackNetworkConfig`. If this exists in the bootstore, we +/// upgrade out of it into `RackNetworkConfigV1` or later versions if possible. +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] +pub struct RackNetworkConfigV0 { + // TODO: #3591 Consider making infra-ip ranges implicit for uplinks + /// First ip address to be used for configuring network infrastructure + pub infra_ip_first: Ipv4Addr, + /// Last ip address to be used for configuring network infrastructure + pub infra_ip_last: Ipv4Addr, + /// Uplinks for connecting the rack to external networks + pub uplinks: Vec, +} + +impl From for RackNetworkConfigV1 { + fn from(value: RackNetworkConfigV0) -> Self { + RackNetworkConfigV1 { + infra_ip_first: value.infra_ip_first, + infra_ip_last: value.infra_ip_last, + ports: value + .uplinks + .into_iter() + .map(|uplink| PortConfigV1::from(uplink)) + .collect(), + bgp: vec![], + } + } +} + // The following two conversion functions translate the speed and fec types used // in the internal API to the types used in the dpd-client API. The conversion // is done here, rather than with "impl From" at the definition, to avoid a From 8e56948f983e0060e785f86cbf0900c7154d65ce Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 17 Oct 2023 19:35:45 +0000 Subject: [PATCH 03/11] Type fixes for nexus --- nexus/src/app/rack.rs | 28 +++++++++++-------- .../app/sagas/switch_port_settings_apply.rs | 2 +- nexus/src/app/switch_port.rs | 2 +- sled-agent/src/bootstrap/early_networking.rs | 19 +++++++++++++ 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index 16535416fb..7efc9674be 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -43,9 +43,10 @@ use omicron_common::api::external::Name; use omicron_common::api::external::NameOrId; use omicron_common::api::external::SwitchLocation; use omicron_common::api::internal::shared::ExternalPortDiscovery; +use sled_agent_client::types::EarlyNetworkConfigBody; use sled_agent_client::types::{ BgpConfig, BgpPeerConfig, EarlyNetworkConfig, PortConfigV1, - RackNetworkConfig, RouteConfig as SledRouteConfig, + RackNetworkConfigV1, RouteConfig as SledRouteConfig, }; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -596,7 +597,7 @@ impl super::Nexus { .into_inner(); rack.rack_subnet = - result.rack_network_config.map(|x| x.rack_subnet.into()); + result.body.rack_network_config.map(|x| x.rack_subnet.into()); self.datastore().rack_insert(opctx, &rack).await?; @@ -696,16 +697,19 @@ impl super::Nexus { let result = EarlyNetworkConfig { generation: 0, - ntp_servers: Vec::new(), //TODO - rack_network_config: Some(RackNetworkConfig { - rack_subnet: subnet, - //TODO(ry) you are here. We need to remove these too. They are - // inconsistent with a generic set of addresses on ports. - infra_ip_first: Ipv4Addr::UNSPECIFIED, - infra_ip_last: Ipv4Addr::UNSPECIFIED, - ports, - bgp, - }), + schema_version: 1, + body: EarlyNetworkConfigBody { + ntp_servers: Vec::new(), //TODO + rack_network_config: Some(RackNetworkConfigV1 { + rack_subnet: subnet, + //TODO(ry) you are here. We need to remove these too. They are + // inconsistent with a generic set of addresses on ports. + infra_ip_first: Ipv4Addr::UNSPECIFIED, + infra_ip_last: Ipv4Addr::UNSPECIFIED, + ports, + bgp, + }), + }, }; Ok(result) diff --git a/nexus/src/app/sagas/switch_port_settings_apply.rs b/nexus/src/app/sagas/switch_port_settings_apply.rs index 543ba77a51..9475d33687 100644 --- a/nexus/src/app/sagas/switch_port_settings_apply.rs +++ b/nexus/src/app/sagas/switch_port_settings_apply.rs @@ -952,7 +952,7 @@ pub(crate) fn apply_bootstore_update( ) -> Result { let mut change = BootstoreNetworkPortChange::default(); - let rack_net_config = match &mut config.rack_network_config { + let rack_net_config = match &mut config.body.rack_network_config { Some(cfg) => cfg, None => { return Err(ActionError::action_failed( diff --git a/nexus/src/app/switch_port.rs b/nexus/src/app/switch_port.rs index 4d67389ad8..03b874727b 100644 --- a/nexus/src/app/switch_port.rs +++ b/nexus/src/app/switch_port.rs @@ -25,7 +25,7 @@ use omicron_common::api::external::{ use sled_agent_client::types::BgpConfig; use sled_agent_client::types::BgpPeerConfig; use sled_agent_client::types::{ - EarlyNetworkConfig, PortConfigV1, RackNetworkConfig, RouteConfig, + EarlyNetworkConfig, PortConfigV1, RackNetworkConfigV1, RouteConfig, }; use std::sync::Arc; use uuid::Uuid; diff --git a/sled-agent/src/bootstrap/early_networking.rs b/sled-agent/src/bootstrap/early_networking.rs index 21922a7540..df6bbfab67 100644 --- a/sled-agent/src/bootstrap/early_networking.rs +++ b/sled-agent/src/bootstrap/early_networking.rs @@ -738,3 +738,22 @@ fn convert_fec(fec: &PortFec) -> dpd_client::types::PortFec { PortFec::Rs => dpd_client::types::PortFec::Rs, } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn serialized_early_network_config_v0_to_v1_conversion() { + let v0 = EarlyNetworkConfigV0 { + generation: 1, + rack_subnet: Ipv6Addr::UNSPECIFIED, + ntp_servers: Vec::new(), + rack_network_config: Some(RackNetworkConfigV0 { + infra_ip_first: Ipv4Addr::UNSPECIFIED, + infra_ip_last: Ipv4Addr::UNSPECIFIED, + uplinks: vec![], + }), + }; + } +} From 1dd0dc137f5a8de268f297bc390d8d52c9dbac97 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 17 Oct 2023 20:01:24 +0000 Subject: [PATCH 04/11] Add a test for deserialization/conversion --- sled-agent/src/bootstrap/early_networking.rs | 50 ++++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/sled-agent/src/bootstrap/early_networking.rs b/sled-agent/src/bootstrap/early_networking.rs index df6bbfab67..2741ceb409 100644 --- a/sled-agent/src/bootstrap/early_networking.rs +++ b/sled-agent/src/bootstrap/early_networking.rs @@ -595,7 +595,7 @@ struct EarlyNetworkConfigV0 { /// [`super::params::RackInitializeRequest`] necessary for use beyond RSS. This /// is just for the initial rack configuration and cold boot purposes. Updates /// come from Nexus. -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)] pub struct EarlyNetworkConfig { // The current generation number of data as stored in CRDB. // The initial generation is set during RSS time and then only mutated @@ -617,7 +617,7 @@ pub struct EarlyNetworkConfig { /// the header and defer deserialization of the body once we know the schema /// version. This is possible via the use of [`serde_json::value::RawValue`] in /// future (post-v1) deserialization paths. -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)] pub struct EarlyNetworkConfigBody { /// The external NTP server addresses. pub ntp_servers: Vec, @@ -742,6 +742,7 @@ fn convert_fec(fec: &PortFec) -> dpd_client::types::PortFec { #[cfg(test)] mod tests { use super::*; + use omicron_common::api::internal::shared::RouteConfig; #[test] fn serialized_early_network_config_v0_to_v1_conversion() { @@ -752,8 +753,51 @@ mod tests { rack_network_config: Some(RackNetworkConfigV0 { infra_ip_first: Ipv4Addr::UNSPECIFIED, infra_ip_last: Ipv4Addr::UNSPECIFIED, - uplinks: vec![], + uplinks: vec![UplinkConfig { + gateway_ip: Ipv4Addr::UNSPECIFIED, + switch: SwitchLocation::Switch0, + uplink_port: "Port0".to_string(), + uplink_port_speed: PortSpeed::Speed100G, + uplink_port_fec: PortFec::None, + uplink_cidr: "192.168.0.1/16".parse().unwrap(), + uplink_vid: None, + }], }), }; + + let v0_serialized = serde_json::to_vec(&v0).unwrap(); + let bootstore_conf = + bootstore::NetworkConfig { generation: 1, blob: v0_serialized }; + + let v1 = EarlyNetworkConfig::try_from(bootstore_conf).unwrap(); + let v0_rack_network_config = v0.rack_network_config.unwrap(); + let uplink = v0_rack_network_config.uplinks[0].clone(); + let expected = EarlyNetworkConfig { + generation: 1, + schema_version: 1, + body: EarlyNetworkConfigBody { + ntp_servers: v0.ntp_servers.clone(), + rack_network_config: Some(RackNetworkConfigV1 { + rack_subnet: Ipv6Network::new(v0.rack_subnet, 56).unwrap(), + infra_ip_first: v0_rack_network_config.infra_ip_first, + infra_ip_last: v0_rack_network_config.infra_ip_last, + ports: vec![PortConfigV1 { + routes: vec![RouteConfig { + destination: "0.0.0.0/0".parse().unwrap(), + nexthop: uplink.gateway_ip.into(), + }], + addresses: vec![uplink.uplink_cidr.into()], + switch: uplink.switch, + port: uplink.uplink_port, + uplink_port_speed: uplink.uplink_port_speed, + uplink_port_fec: uplink.uplink_port_fec, + bgp_peers: vec![], + }], + bgp: vec![], + }), + }, + }; + + assert_eq!(expected, v1); } } From 08dc4c3349df5df62fdd2850907bfc710a009239 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 17 Oct 2023 20:19:53 +0000 Subject: [PATCH 05/11] fix failing builds --- sled-agent/Cargo.toml | 2 +- wicketd/src/rss_config.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sled-agent/Cargo.toml b/sled-agent/Cargo.toml index 82d7411d1a..0bf4dfcb85 100644 --- a/sled-agent/Cargo.toml +++ b/sled-agent/Cargo.toml @@ -56,7 +56,7 @@ reqwest = { workspace = true, features = ["rustls-tls", "stream"] } schemars = { workspace = true, features = [ "chrono", "uuid1" ] } semver.workspace = true serde.workspace = true -serde_json.workspace = true +serde_json = {workspace = true, features = ["raw_value"]} sha3.workspace = true sled-agent-client.workspace = true sled-hardware.workspace = true diff --git a/wicketd/src/rss_config.rs b/wicketd/src/rss_config.rs index 35e9ef383e..a96acc56a0 100644 --- a/wicketd/src/rss_config.rs +++ b/wicketd/src/rss_config.rs @@ -453,7 +453,7 @@ impl From<&'_ CurrentRssConfig> for CurrentRssUserConfig { fn validate_rack_network_config( config: &RackNetworkConfig, -) -> Result { +) -> Result { use bootstrap_agent_client::types::BgpConfig as BaBgpConfig; use bootstrap_agent_client::types::BgpPeerConfig as BaBgpPeerConfig; use bootstrap_agent_client::types::PortConfigV1 as BaPortConfigV1; @@ -496,7 +496,7 @@ fn validate_rack_network_config( } // TODO Add more client side checks on `rack_network_config` contents? - Ok(bootstrap_agent_client::types::RackNetworkConfig { + Ok(bootstrap_agent_client::types::RackNetworkConfigV1 { rack_subnet: config.rack_subnet, infra_ip_first: config.infra_ip_first, infra_ip_last: config.infra_ip_last, From 09309f47ec6b24dea348aa00a2420d43bdedfe4d Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Tue, 17 Oct 2023 22:03:17 +0000 Subject: [PATCH 06/11] fix tests --- openapi/nexus-internal.json | 4 ++-- openapi/wicketd.json | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index 47fb0fb056..f0df2206e1 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -4402,7 +4402,7 @@ "description": "Initial rack network configuration", "allOf": [ { - "$ref": "#/components/schemas/RackNetworkConfig" + "$ref": "#/components/schemas/RackNetworkConfigV1" } ] }, @@ -4433,7 +4433,7 @@ "services" ] }, - "RackNetworkConfig": { + "RackNetworkConfigV1": { "description": "Initial network configuration", "type": "object", "properties": { diff --git a/openapi/wicketd.json b/openapi/wicketd.json index 5a5ee337ff..899b139e22 100644 --- a/openapi/wicketd.json +++ b/openapi/wicketd.json @@ -1054,7 +1054,7 @@ "nullable": true, "allOf": [ { - "$ref": "#/components/schemas/RackNetworkConfig" + "$ref": "#/components/schemas/RackNetworkConfigV1" } ] } @@ -2089,7 +2089,7 @@ } }, "rack_network_config": { - "$ref": "#/components/schemas/RackNetworkConfig" + "$ref": "#/components/schemas/RackNetworkConfigV1" } }, "required": [ @@ -2106,7 +2106,7 @@ "type": "string", "format": "uuid" }, - "RackNetworkConfig": { + "RackNetworkConfigV1": { "description": "Initial network configuration", "type": "object", "properties": { From 252e2a14dfb14568b40385b232747e1779d15439 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Wed, 18 Oct 2023 03:58:44 +0000 Subject: [PATCH 07/11] generated client related fix --- .../preprocessed_configs/config.xml | 41 +++++++++++++++++++ sled-agent/src/rack_setup/service.rs | 2 +- todo-for-sled-agent.md | 9 ++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 dev-tools/omicron-dev/preprocessed_configs/config.xml create mode 100644 todo-for-sled-agent.md diff --git a/dev-tools/omicron-dev/preprocessed_configs/config.xml b/dev-tools/omicron-dev/preprocessed_configs/config.xml new file mode 100644 index 0000000000..9b13f12aea --- /dev/null +++ b/dev-tools/omicron-dev/preprocessed_configs/config.xml @@ -0,0 +1,41 @@ + + + + + trace + true + + + 8123 + 9000 + 9004 + + ./ + + true + + + + + + + ::/0 + + + default + default + 1 + + + + + + + + + + + \ No newline at end of file diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index eef05202be..3d5e799aaa 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -576,7 +576,7 @@ impl ServiceInner { let rack_network_config = match &config.rack_network_config { Some(config) => { - let value = NexusTypes::RackNetworkConfig { + let value = NexusTypes::RackNetworkConfigV1 { rack_subnet: config.rack_subnet, infra_ip_first: config.infra_ip_first, infra_ip_last: config.infra_ip_last, diff --git a/todo-for-sled-agent.md b/todo-for-sled-agent.md new file mode 100644 index 0000000000..0c4dc93885 --- /dev/null +++ b/todo-for-sled-agent.md @@ -0,0 +1,9 @@ +-[ ] Fix tests +-[x] Add a storage monitor to report to nexus and handle dump setup +-[ ] Move sprockets server into LongRunningTasks +-[ ] Add a bunch of log messages +-[ ] Rename sled-agent/src/bootstrap/bootstore.rs to bootstore_setup.rs +-[ ] See if we can make `ServiceManager` part of long-running tasks (will this +require calling all the spawn methods directly in pre-server?) +-[ ] Change the name of `long_running_task_handles` to something else ? +-[ ] Fix installinator to use new code, and any other packages that need it. From 48dc69d342b20b2040b3862dfe2c63b7551b8af1 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Wed, 18 Oct 2023 04:02:00 +0000 Subject: [PATCH 08/11] Revert "generated client related fix" This reverts commit 252e2a14dfb14568b40385b232747e1779d15439. --- .../preprocessed_configs/config.xml | 41 ------------------- sled-agent/src/rack_setup/service.rs | 2 +- todo-for-sled-agent.md | 9 ---- 3 files changed, 1 insertion(+), 51 deletions(-) delete mode 100644 dev-tools/omicron-dev/preprocessed_configs/config.xml delete mode 100644 todo-for-sled-agent.md diff --git a/dev-tools/omicron-dev/preprocessed_configs/config.xml b/dev-tools/omicron-dev/preprocessed_configs/config.xml deleted file mode 100644 index 9b13f12aea..0000000000 --- a/dev-tools/omicron-dev/preprocessed_configs/config.xml +++ /dev/null @@ -1,41 +0,0 @@ - - - - - trace - true - - - 8123 - 9000 - 9004 - - ./ - - true - - - - - - - ::/0 - - - default - default - 1 - - - - - - - - - - - \ No newline at end of file diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 3d5e799aaa..eef05202be 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -576,7 +576,7 @@ impl ServiceInner { let rack_network_config = match &config.rack_network_config { Some(config) => { - let value = NexusTypes::RackNetworkConfigV1 { + let value = NexusTypes::RackNetworkConfig { rack_subnet: config.rack_subnet, infra_ip_first: config.infra_ip_first, infra_ip_last: config.infra_ip_last, diff --git a/todo-for-sled-agent.md b/todo-for-sled-agent.md deleted file mode 100644 index 0c4dc93885..0000000000 --- a/todo-for-sled-agent.md +++ /dev/null @@ -1,9 +0,0 @@ --[ ] Fix tests --[x] Add a storage monitor to report to nexus and handle dump setup --[ ] Move sprockets server into LongRunningTasks --[ ] Add a bunch of log messages --[ ] Rename sled-agent/src/bootstrap/bootstore.rs to bootstore_setup.rs --[ ] See if we can make `ServiceManager` part of long-running tasks (will this -require calling all the spawn methods directly in pre-server?) --[ ] Change the name of `long_running_task_handles` to something else ? --[ ] Fix installinator to use new code, and any other packages that need it. From 90e92ba6de05a4aa5a223b661c99de07b869d91f Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Wed, 18 Oct 2023 04:14:07 +0000 Subject: [PATCH 09/11] Fix type error --- sled-agent/src/rack_setup/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index eef05202be..3d5e799aaa 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -576,7 +576,7 @@ impl ServiceInner { let rack_network_config = match &config.rack_network_config { Some(config) => { - let value = NexusTypes::RackNetworkConfig { + let value = NexusTypes::RackNetworkConfigV1 { rack_subnet: config.rack_subnet, infra_ip_first: config.infra_ip_first, infra_ip_last: config.infra_ip_last, From 72ede1a0684c030e98be42f0fb7ccb229cdd7472 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Wed, 18 Oct 2023 04:47:59 +0000 Subject: [PATCH 10/11] one more fucking fix --- clients/wicketd-client/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/wicketd-client/src/lib.rs b/clients/wicketd-client/src/lib.rs index e4325bdb69..982ec13780 100644 --- a/clients/wicketd-client/src/lib.rs +++ b/clients/wicketd-client/src/lib.rs @@ -42,7 +42,7 @@ progenitor::generate_api!( RackInitId = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, RackResetId = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, RackOperationStatus = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, - RackNetworkConfig = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, + RackNetworkConfigV1 = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, UplinkConfig = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, PortConfigV1 = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, BgpPeerConfig = { derives = [ PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize ] }, From 614804bd4516c67ad0287337102c8d46903eccb0 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Wed, 18 Oct 2023 05:02:11 +0000 Subject: [PATCH 11/11] one more --- wicket/src/rack_setup/config_toml.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/wicket/src/rack_setup/config_toml.rs b/wicket/src/rack_setup/config_toml.rs index 0d5bc9c948..e087c9aa7c 100644 --- a/wicket/src/rack_setup/config_toml.rs +++ b/wicket/src/rack_setup/config_toml.rs @@ -18,7 +18,7 @@ use toml_edit::Value; use wicketd_client::types::BootstrapSledDescription; use wicketd_client::types::CurrentRssUserConfigInsensitive; use wicketd_client::types::IpRange; -use wicketd_client::types::RackNetworkConfig; +use wicketd_client::types::RackNetworkConfigV1; use wicketd_client::types::SpType; static TEMPLATE: &str = include_str!("config_template.toml"); @@ -176,7 +176,7 @@ fn build_sleds_array(sleds: &[BootstrapSledDescription]) -> Array { fn populate_network_table( table: &mut Table, - config: Option<&RackNetworkConfig>, + config: Option<&RackNetworkConfigV1>, ) { // Helper function to serialize enums into their appropriate string // representations. @@ -314,7 +314,7 @@ fn populate_network_table( #[cfg(test)] mod tests { use super::*; - use omicron_common::api::internal::shared::RackNetworkConfig as InternalRackNetworkConfig; + use omicron_common::api::internal::shared::RackNetworkConfigV1 as InternalRackNetworkConfig; use std::net::Ipv6Addr; use wicket_common::rack_setup::PutRssUserConfigInsensitive; use wicketd_client::types::Baseboard; @@ -472,7 +472,7 @@ mod tests { )], external_dns_ips: vec!["10.0.0.1".parse().unwrap()], ntp_servers: vec!["ntp1.com".into(), "ntp2.com".into()], - rack_network_config: Some(RackNetworkConfig { + rack_network_config: Some(RackNetworkConfigV1 { rack_subnet: "fd00:1122:3344:01::/56".parse().unwrap(), infra_ip_first: "172.30.0.1".parse().unwrap(), infra_ip_last: "172.30.0.10".parse().unwrap(),