Skip to content

Commit

Permalink
activate vpc route manager on instance create/destroy/migrate
Browse files Browse the repository at this point in the history
  • Loading branch information
rcgoodfellow committed Sep 28, 2024
1 parent 564ac71 commit 3efed37
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 23 deletions.
21 changes: 0 additions & 21 deletions illumos-utils/src/opte/port_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
14 changes: 12 additions & 2 deletions nexus/db-queries/src/db/datastore/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ impl DataStore {
})?;

self.link_default_gateway(
opctx,
ip_pool_resource.resource_id,
ip_pool_resource.ip_pool_id,
&conn,
Expand All @@ -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<crate::db::DbConnection>,
Expand Down Expand Up @@ -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<crate::db::DbConnection>,
Expand Down Expand Up @@ -666,6 +670,7 @@ impl DataStore {
public_error_from_diesel(e, ErrorHandler::Server)
})?;
}
self.vpc_increment_rpw_version(opctx, vpc.id()).await?;
}
}
Ok(())
Expand Down Expand Up @@ -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(())
}
Expand Down
21 changes: 21 additions & 0 deletions nexus/src/app/background/tasks/vpc_routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
_ => {}
}
Expand Down
6 changes: 6 additions & 0 deletions nexus/src/app/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -544,6 +546,8 @@ impl super::Nexus {
saga_params,
)
.await?;

self.background_tasks.task_vpc_route_manager.activate();
Ok(())
}

Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions nexus/tests/integration_tests/internet_gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ async fn test_setup(c: &ClientTestContext) {
floating_ip: NameOrId::Name(FLOATING_IP_NAME.parse().unwrap()),
}],
true,
None,
)
.await;

Expand Down

0 comments on commit 3efed37

Please sign in to comment.