diff --git a/illumos-utils/src/opte/port_manager.rs b/illumos-utils/src/opte/port_manager.rs index d1ba60b714c..f22987f2372 100644 --- a/illumos-utils/src/opte/port_manager.rs +++ b/illumos-utils/src/opte/port_manager.rs @@ -343,28 +343,7 @@ impl PortManager { system_routes_v6(ipv6, is_service, &mut routes, &port) } }; - /* - let system_routes = - routes.entry(port.system_router_key()).or_insert_with(|| { - let mut routes = HashSet::new(); - - // Services do not talk to one another via OPTE, but do need - // to reach out over the Internet *before* nexus is up to give - // us real rules. The easiest bet is to instantiate these here. - if is_service { - routes.insert(ResolvedVpcRoute { - dest: "0.0.0.0/0".parse().unwrap(), - target: ApiRouterTarget::InternetGateway, - }); - routes.insert(ResolvedVpcRoute { - dest: "::/0".parse().unwrap(), - target: ApiRouterTarget::InternetGateway, - }); - } - RouteSet { version: None, routes, active_ports: 0 } - }); - */ system_routes.active_ports += 1; // Clone is needed to get borrowck on our side, sadly. let system_routes = system_routes.clone(); diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index 28dcba3a7d7..25b1b3bcb98 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -511,6 +511,7 @@ impl DataStore { })?; self.link_default_gateway( + opctx, ip_pool_resource.resource_id, ip_pool_resource.ip_pool_id, &conn, @@ -522,6 +523,7 @@ impl DataStore { async fn link_default_gateway( &self, + opctx: &OpContext, silo_id: Uuid, ip_pool_id: Uuid, conn: &async_bb8_diesel::Connection, @@ -606,12 +608,14 @@ impl DataStore { } }; } + self.vpc_increment_rpw_version(opctx, vpc.id()).await?; } } Ok(()) } async fn unlink_default_gateway( &self, + opctx: &OpContext, silo_id: Uuid, ip_pool_id: Uuid, conn: &async_bb8_diesel::Connection, @@ -666,6 +670,7 @@ impl DataStore { public_error_from_diesel(e, ErrorHandler::Server) })?; } + self.vpc_increment_rpw_version(opctx, vpc.id()).await?; } } Ok(()) @@ -907,8 +912,13 @@ impl DataStore { )) })?; - self.unlink_default_gateway(authz_silo.id(), authz_pool.id(), &conn) - .await?; + self.unlink_default_gateway( + opctx, + authz_silo.id(), + authz_pool.id(), + &conn, + ) + .await?; Ok(()) } diff --git a/nexus/src/app/background/tasks/vpc_routes.rs b/nexus/src/app/background/tasks/vpc_routes.rs index a5e4ec69b66..6d2d319ead4 100644 --- a/nexus/src/app/background/tasks/vpc_routes.rs +++ b/nexus/src/app/background/tasks/vpc_routes.rs @@ -235,6 +235,27 @@ impl BackgroundTask for VpcRouteManager { sled.id() ); //TODO(ry) continue; + // Currently, this version is bumped in response to + // events that change the routes. This has to be + // done explicitly by the programmer in response to + // any event that may result in a difference in + // route resolution calculation. With the + // introduction of internet gateway targets that + // are parameterized on source IP, route resolution + // computations can change based on events that are + // not directly modifying VPC routes - like the + // linkiage of an IP pool to a silo, or the + // allocation of an external IP address and + // attachment of that IP address to a service or + // instance. This broadened context for changes + // influencing route resolution makes manual + // tracking of a router version easy to get wrong + // and I feel like it will be a bug magnet. + // + // I think we should move decisions around change + // propagation to be based on actual delta + // calculation, rather than trying to manually + // maintain a signal. } _ => {} } diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 83ccf6bbb8d..7fc62ae58e2 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -482,6 +482,8 @@ impl super::Nexus { } } + self.background_tasks.task_vpc_route_manager.activate(); + // TODO: This operation should return the instance as it was created. // Refetching the instance state here won't return that version of the // instance if its state changed between the time the saga finished and @@ -544,6 +546,8 @@ impl super::Nexus { saga_params, ) .await?; + + self.background_tasks.task_vpc_route_manager.activate(); Ok(()) } @@ -596,6 +600,8 @@ impl super::Nexus { ) .await?; + self.background_tasks.task_vpc_route_manager.activate(); + // TODO correctness TODO robustness TODO design // Should we lookup the instance again here? // See comment in project_create_instance. diff --git a/nexus/tests/integration_tests/internet_gateway.rs b/nexus/tests/integration_tests/internet_gateway.rs index 620e3c36596..4ca405c1c2c 100644 --- a/nexus/tests/integration_tests/internet_gateway.rs +++ b/nexus/tests/integration_tests/internet_gateway.rs @@ -299,6 +299,7 @@ async fn test_setup(c: &ClientTestContext) { floating_ip: NameOrId::Name(FLOATING_IP_NAME.parse().unwrap()), }], true, + None, ) .await;