Return all LLDP configs for each switch port settings object (#8665)

* We allowed multiple LLDP link configs to be created on a single switch
port settings object, but we would only return one when viewing the
object. This has been fixed.

* Note that until we support breakout connections, only one lldp
configuration will be applied. This is not a regression.

Related
---

Resolves #8640

Author: internet-diglett

nexus/db-model/src/schema_versions.rs

@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: Version = Version::new(177, 0, 0);
+pub const SCHEMA_VERSION: Version = Version::new(178, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(178, "change-lldp-management-ip-to-inet"),
         KnownVersion::new(177, "add-host-ereport-part-number"),
         KnownVersion::new(176, "audit-log"),
         KnownVersion::new(175, "inv-host-phase-1-active-slot"),

nexus/db-queries/src/db/datastore/switch_port.rs

@@ -504,7 +504,6 @@ impl DataStore {
                 result.link_lldp = lldp_link_config::dsl::lldp_link_config
                     .filter(lldp_link_config::id.eq_any(lldp_link_ids))
                     .select(LldpLinkConfig::as_select())
-                    .limit(1)
                     .load_async::<LldpLinkConfig>(&conn)
                     .await?;
 
@@ -523,7 +522,6 @@ impl DataStore {
                 let configs = tx_eq_config::dsl::tx_eq_config
                     .filter(tx_eq_config::id.eq_any(tx_eq_ids))
                     .select(TxEqConfig::as_select())
-                    .limit(1)
                     .load_async::<TxEqConfig>(&conn)
                     .await?;
                     result.tx_eq = tx_eq_ids_and_nulls

nexus/tests/integration_tests/switch_port.rs

@@ -112,38 +112,70 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
             description: "just a port".into(),
         });
 
-    let link_name =
+    let link0_name =
         Name::from_str("phy0").expect("phy0 should be a valid name");
 
-    let lldp_params = LldpLinkConfigCreate {
+    let lldp0_params = LldpLinkConfigCreate {
         enabled: true,
         link_name: Some("Link Name".into()),
-        link_description: Some("link_ Dscription".into()),
+        link_description: Some("link description".into()),
         chassis_id: Some("Chassis ID".into()),
         system_name: Some("System Name".into()),
         system_description: Some("System description".into()),
-        management_ip: None,
+        management_ip: Some(
+            "203.0.113.10"
+                .parse()
+                .expect("management_ip should be a valid address"),
+        ),
+    };
+
+    let link1_name =
+        Name::from_str("phy1").expect("phy1 should be a valid name");
+
+    let lldp1_params = LldpLinkConfigCreate {
+        enabled: true,
+        link_name: Some("Link Name 2".into()),
+        link_description: Some("link description".into()),
+        chassis_id: Some("Chassis ID".into()),
+        system_name: Some("System Name".into()),
+        system_description: Some("System description".into()),
+        management_ip: Some(
+            "203.0.113.10"
+                .parse()
+                .expect("management_ip should be a valid address"),
+        ),
     };
 
     // links
     settings.links.push(LinkConfigCreate {
-        link_name: link_name.clone(),
+        link_name: link0_name.clone(),
+        mtu: 4700,
+        lldp: lldp0_params.clone(),
+        fec: Some(LinkFec::None),
+        speed: LinkSpeed::Speed100G,
+        autoneg: false,
+        tx_eq: None,
+    });
+
+    settings.links.push(LinkConfigCreate {
+        link_name: link1_name.clone(),
         mtu: 4700,
-        lldp: lldp_params.clone(),
+        lldp: lldp1_params.clone(),
         fec: Some(LinkFec::None),
         speed: LinkSpeed::Speed100G,
         autoneg: false,
         tx_eq: None,
     });
+
     // interfaces
     settings.interfaces.push(SwitchInterfaceConfigCreate {
-        link_name: link_name.clone(),
+        link_name: link0_name.clone(),
         v6_enabled: true,
         kind: SwitchInterfaceKind::Primary,
     });
     // routes
     settings.routes.push(RouteConfig {
-        link_name: link_name.clone(),
+        link_name: link0_name.clone(),
         routes: vec![Route {
             dst: "1.2.3.0/24".parse().unwrap(),
             gw: "1.2.3.4".parse().unwrap(),
@@ -153,7 +185,7 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
     });
     // addresses
     settings.addresses.push(AddressConfig {
-        link_name: link_name.clone(),
+        link_name: link0_name.clone(),
         addresses: vec![Address {
             address: "203.0.113.10/24".parse().unwrap(),
             vlan_id: None,
@@ -173,7 +205,7 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
     .parsed_body()
     .unwrap();
 
-    assert_eq!(created.links.len(), 1);
+    assert_eq!(created.links.len(), 2);
     assert_eq!(created.routes.len(), 1);
     assert_eq!(created.addresses.len(), 1);
 
@@ -182,7 +214,14 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
     assert_eq!(link0.mtu, 4700);
 
     let lldp0 = link0.lldp_link_config.clone().unwrap();
-    assert_eq!(lldp0, lldp_params);
+    assert_eq!(lldp0, lldp0_params);
+
+    let link1 = &created.links[1];
+    assert_eq!(&link1.link_name.to_string(), "phy1");
+    assert_eq!(link1.mtu, 4700);
+
+    let lldp1 = link1.lldp_link_config.clone().unwrap();
+    assert_eq!(lldp1, lldp1_params);
 
     let ifx0 = &created.interfaces[0];
     assert_eq!(&ifx0.interface_name.to_string(), "phy0");
@@ -208,7 +247,7 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
     .parsed_body()
     .unwrap();
 
-    assert_eq!(roundtrip.links.len(), 1);
+    assert_eq!(roundtrip.links.len(), 2);
     assert_eq!(roundtrip.routes.len(), 1);
     assert_eq!(roundtrip.addresses.len(), 1);
 
@@ -217,7 +256,14 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
     assert_eq!(link0.mtu, 4700);
 
     let lldp0 = link0.lldp_link_config.clone().unwrap();
-    assert_eq!(lldp0, lldp_params);
+    assert_eq!(lldp0, lldp0_params);
+
+    let link1 = &roundtrip.links[1];
+    assert_eq!(&link1.link_name.to_string(), "phy1");
+    assert_eq!(link1.mtu, 4700);
+
+    let lldp1 = link1.lldp_link_config.clone().unwrap();
+    assert_eq!(lldp1, lldp1_params);
 
     let ifx0 = &roundtrip.interfaces[0];
     assert_eq!(&ifx0.interface_name.to_string(), "phy0");
@@ -259,7 +305,7 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
 
     // Update port settings. Should not see conflict.
     settings.bgp_peers.push(BgpPeerConfig {
-        link_name: link_name.clone(),
+        link_name: link0_name.clone(),
         peers: vec![BgpPeer {
             bgp_config: NameOrId::Name("as47".parse().unwrap()),
             interface_name: "phy0".parse().unwrap(),

schema/crdb/change-lldp-management-ip-to-inet/up01.sql

@@ -0,0 +1,2 @@
+ALTER TABLE IF EXISTS omicron.public.lldp_link_config
+DROP COLUMN IF EXISTS management_ip;

schema/crdb/change-lldp-management-ip-to-inet/up02.sql

@@ -0,0 +1,2 @@
+ALTER TABLE IF EXISTS omicron.public.lldp_link_config
+ADD COLUMN IF NOT EXISTS management_ip INET;

schema/crdb/dbinit.sql

@@ -3108,10 +3108,10 @@ CREATE TABLE IF NOT EXISTS omicron.public.lldp_link_config (
     chassis_id STRING(63),
     system_name STRING(63),
     system_description STRING(612),
-    management_ip TEXT,
     time_created TIMESTAMPTZ NOT NULL,
     time_modified TIMESTAMPTZ NOT NULL,
-    time_deleted TIMESTAMPTZ
+    time_deleted TIMESTAMPTZ,
+    management_ip INET
 );
 
 CREATE TABLE IF NOT EXISTS omicron.public.tx_eq_config (
@@ -6497,7 +6497,7 @@ INSERT INTO omicron.public.db_metadata (
     version,
     target_version
 ) VALUES
-    (TRUE, NOW(), NOW(), '177.0.0', NULL)
+    (TRUE, NOW(), NOW(), '178.0.0', NULL)
 ON CONFLICT DO NOTHING;
 
 COMMIT;