Fix filters for probe v2p mappings (#5960)

During the changes made in #5917, we accidentally filtered for _instances_ when querying for probe v2p mappings. This PR fixes that filter, and also updates the v2p mapping and vpc router RPWs to use the `SledFilter::VpcRouting` enum.
oxidecomputer · Jul 3, 2024 · 30b6713 · 30b6713
1 parent 4d9b147
commit 30b6713
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 25 deletions.
diff --git a/dev-tools/omdb/tests/usage_errors.out b/dev-tools/omdb/tests/usage_errors.out
@@ -290,7 +290,7 @@ Options:
             for discretionary services)
           - query-during-inventory: Sleds whose sled agents should be queried for inventory
           - reservation-create:     Sleds on which reservations can be created
-          - v2p-mapping:            Sleds which should be sent OPTE V2P mappings
+          - vpc-routing:            Sleds which should be sent OPTE V2P mappings and Routing rules
           - vpc-firewall:           Sleds which should be sent VPC firewall rules
 
       --log-level <LOG_LEVEL>

diff --git a/nexus/db-queries/src/db/datastore/v2p_mapping.rs b/nexus/db-queries/src/db/datastore/v2p_mapping.rs
@@ -35,6 +35,7 @@ impl DataStore {
 
         opctx.check_complex_operations_allowed()?;
 
+        // Query for instance v2p mappings
         let mut mappings = Vec::new();
         let mut paginator = Paginator::new(SQL_BATCH_SIZE);
         while let Some(p) = paginator.next() {
@@ -66,7 +67,7 @@ impl DataStore {
                 .filter(
                     network_interface::kind.eq(NetworkInterfaceKind::Instance),
                 )
-                .sled_filter(SledFilter::V2PMapping)
+                .sled_filter(SledFilter::VpcRouting)
                 .select((
                     NetworkInterface::as_select(),
                     Sled::as_select(),
@@ -92,6 +93,7 @@ impl DataStore {
             mappings.extend(batch);
         }
 
+        // Query for probe v2p mappings
         let mut paginator = Paginator::new(SQL_BATCH_SIZE);
         while let Some(p) = paginator.next() {
             let batch: Vec<_> =
@@ -113,10 +115,8 @@ impl DataStore {
                 )
                 .inner_join(sled_dsl::sled.on(sled_dsl::id.eq(probe_dsl::sled)))
                 .filter(network_interface::time_deleted.is_null())
-                .filter(
-                    network_interface::kind.eq(NetworkInterfaceKind::Instance),
-                )
-                .sled_filter(SledFilter::V2PMapping)
+                .filter(network_interface::kind.eq(NetworkInterfaceKind::Probe))
+                .sled_filter(SledFilter::VpcRouting)
                 .select((
                     NetworkInterface::as_select(),
                     Sled::as_select(),

diff --git a/nexus/src/app/background/tasks/v2p_mappings.rs b/nexus/src/app/background/tasks/v2p_mappings.rs
@@ -2,12 +2,10 @@ use std::{collections::HashSet, sync::Arc};
 
 use futures::future::BoxFuture;
 use futures::FutureExt;
-use nexus_db_model::{Sled, SledState};
+use nexus_db_model::Sled;
 use nexus_db_queries::{context::OpContext, db::DataStore};
 use nexus_networking::sled_client_from_address;
-use nexus_types::{
-    deployment::SledFilter, external_api::views::SledPolicy, identity::Asset,
-};
+use nexus_types::{deployment::SledFilter, identity::Asset};
 use omicron_common::api::external::Vni;
 use serde_json::json;
 use sled_agent_client::types::VirtualNetworkInterfaceHost;
@@ -44,23 +42,19 @@ impl BackgroundTask for V2PManager {
 
             // Get sleds
             // we only care about sleds that are active && inservice
-            let sleds = match self.datastore.sled_list_all_batched(opctx, SledFilter::InService).await
+            let sleds = match self.datastore.sled_list_all_batched(opctx, SledFilter::VpcRouting).await
             {
                 Ok(v) => v,
                 Err(e) => {
                     let msg = format!("failed to enumerate sleds: {:#}", e);
                     error!(&log, "{msg}");
                     return json!({"error": msg});
                 }
-            }
-            .into_iter()
-            .filter(|sled| {
-                matches!(sled.state(), SledState::Active)
-                    && matches!(sled.policy(), SledPolicy::InService { .. })
-            });
+            };
 
             // Map sled db records to sled-agent clients
             let sled_clients: Vec<(Sled, sled_agent_client::Client)> = sleds
+                .into_iter()
                 .map(|sled| {
                     let client = sled_client_from_address(
                         sled.id(),

diff --git a/nexus/src/app/background/tasks/vpc_routes.rs b/nexus/src/app/background/tasks/vpc_routes.rs
@@ -59,7 +59,7 @@ impl BackgroundTask for VpcRouteManager {
 
             let sleds = match self
                 .datastore
-                .sled_list_all_batched(opctx, SledFilter::InService)
+                .sled_list_all_batched(opctx, SledFilter::VpcRouting)
                 .await
             {
                 Ok(v) => v,

diff --git a/nexus/types/src/deployment/planning_input.rs b/nexus/types/src/deployment/planning_input.rs
@@ -484,8 +484,8 @@ pub enum SledFilter {
     /// Sleds on which reservations can be created.
     ReservationCreate,
 
-    /// Sleds which should be sent OPTE V2P mappings.
-    V2PMapping,
+    /// Sleds which should be sent OPTE V2P mappings and Routing rules.
+    VpcRouting,
 
     /// Sleds which should be sent VPC firewall rules.
     VpcFirewall,
@@ -541,7 +541,7 @@ impl SledPolicy {
                 SledFilter::InService => true,
                 SledFilter::QueryDuringInventory => true,
                 SledFilter::ReservationCreate => true,
-                SledFilter::V2PMapping => true,
+                SledFilter::VpcRouting => true,
                 SledFilter::VpcFirewall => true,
             },
             SledPolicy::InService {
@@ -553,7 +553,7 @@ impl SledPolicy {
                 SledFilter::InService => true,
                 SledFilter::QueryDuringInventory => true,
                 SledFilter::ReservationCreate => false,
-                SledFilter::V2PMapping => true,
+                SledFilter::VpcRouting => true,
                 SledFilter::VpcFirewall => true,
             },
             SledPolicy::Expunged => match filter {
@@ -563,7 +563,7 @@ impl SledPolicy {
                 SledFilter::InService => false,
                 SledFilter::QueryDuringInventory => false,
                 SledFilter::ReservationCreate => false,
-                SledFilter::V2PMapping => false,
+                SledFilter::VpcRouting => false,
                 SledFilter::VpcFirewall => false,
             },
         }
@@ -595,7 +595,7 @@ impl SledState {
                 SledFilter::InService => true,
                 SledFilter::QueryDuringInventory => true,
                 SledFilter::ReservationCreate => true,
-                SledFilter::V2PMapping => true,
+                SledFilter::VpcRouting => true,
                 SledFilter::VpcFirewall => true,
             },
             SledState::Decommissioned => match filter {
@@ -605,7 +605,7 @@ impl SledState {
                 SledFilter::InService => false,
                 SledFilter::QueryDuringInventory => false,
                 SledFilter::ReservationCreate => false,
-                SledFilter::V2PMapping => false,
+                SledFilter::VpcRouting => false,
                 SledFilter::VpcFirewall => false,
             },
         }