Skip to content

Commit

Permalink
bgp-config as announce-set owner instead of peers
Browse files Browse the repository at this point in the history
  • Loading branch information
rcgoodfellow committed Oct 14, 2023
1 parent fc9b0d4 commit 9a9ef8d
Show file tree
Hide file tree
Showing 16 changed files with 99 additions and 90 deletions.
3 changes: 0 additions & 3 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2469,9 +2469,6 @@ pub struct SwitchPortBgpPeerConfig {
/// The port settings object this BGP configuration belongs to.
pub port_settings_id: Uuid,

/// The id for the set of prefixes announced in this peer configuration.
pub bgp_announce_set_id: Uuid,

/// The id of the global BGP configuration referenced by this peer
/// configuration.
pub bgp_config_id: Uuid,
Expand Down
11 changes: 8 additions & 3 deletions nexus/db-model/src/bgp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub struct BgpConfig {
#[diesel(embed)]
pub identity: BgpConfigIdentity,
pub asn: SqlU32,
pub bgp_announce_set_id: Uuid,
pub vrf: Option<String>,
}

Expand All @@ -41,8 +42,11 @@ impl Into<external::BgpConfig> for BgpConfig {
}
}

impl From<params::BgpConfigCreate> for BgpConfig {
fn from(c: params::BgpConfigCreate) -> BgpConfig {
impl BgpConfig {
pub fn from_config_create(
c: &params::BgpConfigCreate,
bgp_announce_set_id: Uuid,
) -> BgpConfig {
BgpConfig {
identity: BgpConfigIdentity::new(
Uuid::new_v4(),
Expand All @@ -52,7 +56,8 @@ impl From<params::BgpConfigCreate> for BgpConfig {
},
),
asn: c.asn.into(),
vrf: c.vrf.map(|x| x.to_string()),
bgp_announce_set_id,
vrf: c.vrf.as_ref().map(|x| x.to_string()),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ table! {
table! {
switch_port_settings_bgp_peer_config (port_settings_id, interface_name, addr) {
port_settings_id -> Uuid,
bgp_announce_set_id -> Uuid,
bgp_config_id -> Uuid,
interface_name -> Text,
addr -> Inet,
Expand All @@ -216,6 +215,7 @@ table! {
time_modified -> Timestamptz,
time_deleted -> Nullable<Timestamptz>,
asn -> Int8,
bgp_announce_set_id -> Uuid,
vrf -> Nullable<Text>,
}
}
Expand Down
15 changes: 1 addition & 14 deletions nexus/db-model/src/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,11 +439,6 @@ impl Into<external::SwitchPortRouteConfig> for SwitchPortRouteConfig {
#[diesel(table_name = switch_port_settings_bgp_peer_config)]
pub struct SwitchPortBgpPeerConfig {
pub port_settings_id: Uuid,

//TODO(ry) this should be associated with the BGP configuration
// not an individual peer.
pub bgp_announce_set_id: Uuid,

pub bgp_config_id: Uuid,
pub interface_name: String,
pub addr: IpNetwork,
Expand All @@ -452,26 +447,18 @@ pub struct SwitchPortBgpPeerConfig {
impl SwitchPortBgpPeerConfig {
pub fn new(
port_settings_id: Uuid,
bgp_announce_set_id: Uuid,
bgp_config_id: Uuid,
interface_name: String,
addr: IpNetwork,
) -> Self {
Self {
port_settings_id,
bgp_announce_set_id,
bgp_config_id,
interface_name,
addr,
}
Self { port_settings_id, bgp_config_id, interface_name, addr }
}
}

impl Into<external::SwitchPortBgpPeerConfig> for SwitchPortBgpPeerConfig {
fn into(self) -> external::SwitchPortBgpPeerConfig {
external::SwitchPortBgpPeerConfig {
port_settings_id: self.port_settings_id,
bgp_announce_set_id: self.bgp_announce_set_id,
bgp_config_id: self.bgp_config_id,
interface_name: self.interface_name.clone(),
addr: self.addr.ip(),
Expand Down
70 changes: 44 additions & 26 deletions nexus/db-queries/src/db/datastore/bgp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,45 @@ impl DataStore {
config: &params::BgpConfigCreate,
) -> CreateResult<BgpConfig> {
use db::schema::bgp_config::dsl;
use db::schema::{
bgp_announce_set, bgp_announce_set::dsl as announce_set_dsl,
};
let pool = self.pool_connection_authorized(opctx).await?;

let config: BgpConfig = config.clone().into();

let result = diesel::insert_into(dsl::bgp_config)
.values(config.clone())
.returning(BgpConfig::as_returning())
.get_result_async(&*pool)
.await
.map_err(|e| {
public_error_from_diesel(
e,
ErrorHandler::Conflict(
ResourceType::BgpConfig,
&config.id().to_string(),
),
)
})?;

Ok(result)
/*
#[derive(Debug)]
enum BgpConfigSetError {
AnnounceSetNotFound,
}
type TxnError = TransactionError<BgpConfigSetError>;
*/

pool.transaction_async(|conn| async move {
let id: Uuid = match &config.bgp_announce_set_id {
NameOrId::Name(name) => {
announce_set_dsl::bgp_announce_set
.filter(bgp_announce_set::time_deleted.is_null())
.filter(bgp_announce_set::name.eq(name.to_string()))
.select(bgp_announce_set::id)
.limit(1)
.first_async::<Uuid>(&conn)
.await?
}
NameOrId::Id(id) => *id,
};

let config = BgpConfig::from_config_create(config, id);

let result = diesel::insert_into(dsl::bgp_config)
.values(config.clone())
.returning(BgpConfig::as_returning())
.get_result_async(&conn)
.await?;
Ok(result)
})
.await
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}

pub async fn bgp_config_delete(
Expand Down Expand Up @@ -278,8 +297,8 @@ impl DataStore {
use db::schema::bgp_announce_set::dsl as announce_set_dsl;
use db::schema::bgp_announcement::dsl as bgp_announcement_dsl;

use db::schema::switch_port_settings_bgp_peer_config as sps_bgp_peer_config;
use db::schema::switch_port_settings_bgp_peer_config::dsl as sps_bgp_peer_config_dsl;
use db::schema::bgp_config;
use db::schema::bgp_config::dsl as bgp_config_dsl;

#[derive(Debug)]
enum BgpAnnounceSetDeleteError {
Expand All @@ -303,12 +322,11 @@ impl DataStore {
NameOrId::Id(id) => id,
};

let count =
sps_bgp_peer_config_dsl::switch_port_settings_bgp_peer_config
.filter(sps_bgp_peer_config::bgp_announce_set_id.eq(id))
.count()
.execute_async(&conn)
.await?;
let count = bgp_config_dsl::bgp_config
.filter(bgp_config::bgp_announce_set_id.eq(id))
.count()
.execute_async(&conn)
.await?;

if count > 0 {
return Err(TxnError::CustomError(
Expand Down
10 changes: 4 additions & 6 deletions nexus/db-queries/src/db/datastore/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl DataStore {
) -> CreateResult<SwitchPortSettingsCombinedResult> {
use db::schema::{
address_lot::dsl as address_lot_dsl,
bgp_announce_set::dsl as bgp_announce_set_dsl,
//XXX ANNOUNCE bgp_announce_set::dsl as bgp_announce_set_dsl,
bgp_config::dsl as bgp_config_dsl,
lldp_service_config::dsl as lldp_config_dsl,
switch_port_settings::dsl as port_settings_dsl,
Expand All @@ -158,7 +158,7 @@ impl DataStore {
#[derive(Debug)]
enum SwitchPortSettingsCreateError {
AddressLotNotFound,
BgpAnnounceSetNotFound,
//XXX ANNOUNCE BgpAnnounceSetNotFound,
BgpConfigNotFound,
ReserveBlock(ReserveBlockError),
}
Expand Down Expand Up @@ -298,6 +298,7 @@ impl DataStore {

let mut bgp_peer_config = Vec::new();
for (interface_name, p) in &params.bgp_peers {
/* XXX ANNOUNCE
use db::schema::bgp_announce_set;
let announce_set_id = match &p.bgp_announce_set {
NameOrId::Id(id) => *id,
Expand All @@ -317,6 +318,7 @@ impl DataStore {
})?
}
};
*/

use db::schema::bgp_config;
let bgp_config_id = match &p.bgp_config {
Expand All @@ -340,7 +342,6 @@ impl DataStore {

bgp_peer_config.push(SwitchPortBgpPeerConfig::new(
psid,
announce_set_id,
bgp_config_id,
interface_name.clone(),
p.addr.into(),
Expand Down Expand Up @@ -420,9 +421,6 @@ impl DataStore {
})
.await
.map_err(|e| match e {
TxnError::CustomError(SpsCreateError::BgpAnnounceSetNotFound) => {
Error::invalid_request("BGP announce set not found")
}
TxnError::CustomError(SpsCreateError::AddressLotNotFound) => {
Error::invalid_request("AddressLot not found")
}
Expand Down
4 changes: 2 additions & 2 deletions nexus/src/app/sagas/switch_port_settings_apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ pub(crate) async fn ensure_switch_port_bgp_settings(
.bgp_announce_list(
&opctx,
&params::BgpAnnounceSetSelector {
name_or_id: NameOrId::Id(peer.bgp_announce_set_id),
name_or_id: NameOrId::Id(config.bgp_announce_set_id),
},
)
.await
Expand Down Expand Up @@ -854,7 +854,7 @@ pub(crate) async fn bootstore_update(
.bgp_announce_list(
&opctx,
&params::BgpAnnounceSetSelector {
name_or_id: NameOrId::Id(p.bgp_announce_set_id),
name_or_id: NameOrId::Id(bgp_config.bgp_announce_set_id),
},
)
.await
Expand Down
2 changes: 1 addition & 1 deletion nexus/src/app/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ impl super::Nexus {
.bgp_announce_list(
&opctx,
&params::BgpAnnounceSetSelector {
name_or_id: p.bgp_announce_set_id.into(),
name_or_id: bgp_config.bgp_announce_set_id.into(),
},
)
.await?;
Expand Down
1 change: 1 addition & 0 deletions nexus/tests/integration_tests/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ lazy_static! {
name: "as47".parse().unwrap(),
description: "BGP config for AS47".into(),
},
bgp_announce_set_id: NameOrId::Name("instances".parse().unwrap()),
asn: 47,
vrf: None,
};
Expand Down
37 changes: 19 additions & 18 deletions nexus/tests/integration_tests/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,42 +57,43 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
.await
.unwrap();

// Create BGP config
let bgp_config = BgpConfigCreate {
// Create BGP announce set
let announce_set = BgpAnnounceSetCreate {
identity: IdentityMetadataCreateParams {
name: "as47".parse().unwrap(),
description: "autonomous system 47".into(),
name: "instances".parse().unwrap(),
description: "autonomous system 47 announcements".into(),
},
asn: 47,
vrf: None,
announcement: vec![BgpAnnouncementCreate {
address_lot_block: NameOrId::Name("parkinglot".parse().unwrap()),
network: "1.2.3.0/24".parse().unwrap(),
}],
};

NexusRequest::objects_post(
client,
"/v1/system/networking/bgp",
&bgp_config,
"/v1/system/networking/bgp-announce",
&announce_set,
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap();

// Create BGP announce set
let announce_set = BgpAnnounceSetCreate {
// Create BGP config
let bgp_config = BgpConfigCreate {
identity: IdentityMetadataCreateParams {
name: "instances".parse().unwrap(),
description: "autonomous system 47 announcements".into(),
name: "as47".parse().unwrap(),
description: "autonomous system 47".into(),
},
announcement: vec![BgpAnnouncementCreate {
address_lot_block: NameOrId::Name("parkinglot".parse().unwrap()),
network: "1.2.3.0/24".parse().unwrap(),
}],
bgp_announce_set_id: NameOrId::Name("instances".parse().unwrap()),
asn: 47,
vrf: None,
};

NexusRequest::objects_post(
client,
"/v1/system/networking/bgp-announce",
&announce_set,
"/v1/system/networking/bgp",
&bgp_config,
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
Expand Down
2 changes: 2 additions & 0 deletions nexus/types/src/external_api/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,8 @@ pub struct BgpConfigCreate {
/// The autonomous system number of this BGP configuration.
pub asn: u32,

pub bgp_announce_set_id: NameOrId,

/// Optional virtual routing and forwarding identifier for this BGP
/// configuration.
pub vrf: Option<Name>,
Expand Down
10 changes: 4 additions & 6 deletions openapi/nexus.json
Original file line number Diff line number Diff line change
Expand Up @@ -8309,6 +8309,9 @@
"format": "uint32",
"minimum": 0
},
"bgp_announce_set_id": {
"$ref": "#/components/schemas/NameOrId"
},
"description": {
"type": "string"
},
Expand All @@ -8327,6 +8330,7 @@
},
"required": [
"asn",
"bgp_announce_set_id",
"description",
"name"
]
Expand Down Expand Up @@ -14336,11 +14340,6 @@
"type": "string",
"format": "ip"
},
"bgp_announce_set_id": {
"description": "The id for the set of prefixes announced in this peer configuration.",
"type": "string",
"format": "uuid"
},
"bgp_config_id": {
"description": "The id of the global BGP configuration referenced by this peer configuration.",
"type": "string",
Expand All @@ -14358,7 +14357,6 @@
},
"required": [
"addr",
"bgp_announce_set_id",
"bgp_config_id",
"interface_name",
"port_settings_id"
Expand Down
File renamed without changes.
2 changes: 2 additions & 0 deletions schema/crdb/8.0.0/up2.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE omicron.public.bgp_config ADD COLUMN IF NOT EXISTS bgp_announce_set_id UUID NOT NULL;
ALTER TABLE omicron.public.switch_port_settings_bgp_peer_config DROP COLUMN IF EXISTS bgp_announce_set_id;
Loading

0 comments on commit 9a9ef8d

Please sign in to comment.