Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omicron services need more unique ids #2922

Merged
merged 5 commits into from
Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions common/src/sql/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ CREATE TABLE omicron.public.service (

/* FK into the Sled table */
sled_id UUID NOT NULL,
/* For services in illumos zones, the zone's unique id (for debugging) */
zone_id UUID,
/* The IP address of the service. */
ip INET NOT NULL,
/* The UDP or TCP port on which the service listens. */
Expand Down
1 change: 1 addition & 0 deletions nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ table! {
time_modified -> Timestamptz,

sled_id -> Uuid,
zone_id -> Nullable<Uuid>,
ip -> Inet,
port -> Int4,
kind -> crate::ServiceKindEnum,
Expand Down
3 changes: 3 additions & 0 deletions nexus/db-model/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub struct Service {
identity: ServiceIdentity,

pub sled_id: Uuid,
pub zone_id: Option<Uuid>,
pub ip: ipv6::Ipv6Addr,
pub port: SqlU16,
pub kind: ServiceKind,
Expand All @@ -27,12 +28,14 @@ impl Service {
pub fn new(
id: Uuid,
sled_id: Uuid,
zone_id: Option<Uuid>,
addr: SocketAddrV6,
kind: ServiceKind,
) -> Self {
Self {
identity: ServiceIdentity::new(id),
sled_id,
zone_id,
ip: addr.ip().into(),
port: addr.port().into(),
kind,
Expand Down
16 changes: 13 additions & 3 deletions nexus/db-queries/src/db/datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1099,7 +1099,8 @@ mod test {
let addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0);
let kind = ServiceKind::Nexus;

let service1 = Service::new(service1_id, sled_id, addr, kind);
let service1 =
Service::new(service1_id, sled_id, Some(service1_id), addr, kind);
let result =
datastore.service_upsert(&opctx, service1.clone()).await.unwrap();
assert_eq!(service1.id(), result.id());
Expand All @@ -1108,7 +1109,7 @@ mod test {

let service2_id =
"fe5b6e3d-dfee-47b4-8719-c54f78912c0b".parse().unwrap();
let service2 = Service::new(service2_id, sled_id, addr, kind);
let service2 = Service::new(service2_id, sled_id, None, addr, kind);
let result =
datastore.service_upsert(&opctx, service2.clone()).await.unwrap();
assert_eq!(service2.id(), result.id());
Expand All @@ -1117,7 +1118,13 @@ mod test {

let service3_id = Uuid::new_v4();
let kind = ServiceKind::Oximeter;
let service3 = Service::new(service3_id, sled_id, addr, kind);
let service3 = Service::new(
service3_id,
sled_id,
Some(Uuid::new_v4()),
addr,
kind,
);
let result =
datastore.service_upsert(&opctx, service3.clone()).await.unwrap();
assert_eq!(service3.id(), result.id());
Expand All @@ -1139,9 +1146,11 @@ mod test {
.unwrap();
assert_eq!(services[0].id(), service1.id());
assert_eq!(services[0].sled_id, service1.sled_id);
assert_eq!(services[0].zone_id, service1.zone_id);
assert_eq!(services[0].kind, service1.kind);
assert_eq!(services[1].id(), service2.id());
assert_eq!(services[1].sled_id, service2.sled_id);
assert_eq!(services[1].zone_id, service2.zone_id);
assert_eq!(services[1].kind, service2.kind);
assert_eq!(services.len(), 2);

Expand All @@ -1160,6 +1169,7 @@ mod test {
.unwrap();
assert_eq!(services[0].id(), service3.id());
assert_eq!(services[0].sled_id, service3.sled_id);
assert_eq!(services[0].zone_id, service3.zone_id);
assert_eq!(services[0].kind, service3.kind);
assert_eq!(services.len(), 1);

Expand Down
25 changes: 19 additions & 6 deletions nexus/db-queries/src/db/datastore/rack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ impl DataStore {
let service_db = db::model::Service::new(
service.service_id,
service.sled_id,
service.zone_id,
service.address,
service.kind.into(),
);
Expand Down Expand Up @@ -624,9 +625,11 @@ mod test {
let sled = create_test_sled(&datastore).await;

let nexus_ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4));
let nexus_id = Uuid::new_v4();
let services = vec![internal_params::ServicePutRequest {
service_id: Uuid::new_v4(),
service_id: nexus_id,
sled_id: sled.id(),
zone_id: Some(nexus_id),
address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0),
kind: internal_params::ServiceKind::Nexus {
external_address: nexus_ip,
Expand Down Expand Up @@ -718,18 +721,22 @@ mod test {
// Ask for two Nexus services, with different external IPs.
let nexus_ip_start = Ipv4Addr::new(1, 2, 3, 4);
let nexus_ip_end = Ipv4Addr::new(1, 2, 3, 5);
let nexus_id1 = Uuid::new_v4();
let nexus_id2 = Uuid::new_v4();
let mut services = vec![
internal_params::ServicePutRequest {
service_id: Uuid::new_v4(),
service_id: nexus_id1,
sled_id: sled.id(),
zone_id: Some(nexus_id1),
address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0),
kind: internal_params::ServiceKind::Nexus {
external_address: IpAddr::V4(nexus_ip_start),
},
},
internal_params::ServicePutRequest {
service_id: Uuid::new_v4(),
service_id: nexus_id2,
sled_id: sled.id(),
zone_id: Some(nexus_id2),
address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 456, 0, 0),
kind: internal_params::ServiceKind::Nexus {
external_address: IpAddr::V4(nexus_ip_end),
Expand Down Expand Up @@ -904,9 +911,11 @@ mod test {
let sled = create_test_sled(&datastore).await;

let nexus_ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4));
let nexus_id = Uuid::new_v4();
let services = vec![internal_params::ServicePutRequest {
service_id: Uuid::new_v4(),
service_id: nexus_id,
sled_id: sled.id(),
zone_id: Some(nexus_id),
address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0),
kind: internal_params::ServiceKind::Nexus {
external_address: nexus_ip,
Expand Down Expand Up @@ -957,19 +966,23 @@ mod test {

// Request two services which happen to be using the same IP address.
let nexus_ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4));
let nexus_id1 = Uuid::new_v4();
let nexus_id2 = Uuid::new_v4();

let services = vec![
internal_params::ServicePutRequest {
service_id: Uuid::new_v4(),
service_id: nexus_id1,
sled_id: sled.id(),
zone_id: Some(nexus_id1),
address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0),
kind: internal_params::ServiceKind::Nexus {
external_address: nexus_ip,
},
},
internal_params::ServicePutRequest {
service_id: Uuid::new_v4(),
service_id: nexus_id2,
sled_id: sled.id(),
zone_id: Some(nexus_id2),
address: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 123, 0, 0),
kind: internal_params::ServiceKind::Nexus {
external_address: nexus_ip,
Expand Down
2 changes: 2 additions & 0 deletions nexus/src/app/background/dns_servers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ mod test {
.values(Service::new(
Uuid::new_v4(),
Uuid::new_v4(),
Some(Uuid::new_v4()),
SocketAddrV6::new(Ipv6Addr::LOCALHOST, 1, 0, 0),
ServiceKind::InternalDnsConfig,
))
Expand All @@ -255,6 +256,7 @@ mod test {
Service::new(
Uuid::new_v4(),
Uuid::new_v4(),
Some(Uuid::new_v4()),
SocketAddrV6::new(Ipv6Addr::LOCALHOST, i + 2, 0, 0),
ServiceKind::InternalDnsConfig,
)
Expand Down
1 change: 1 addition & 0 deletions nexus/src/app/background/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ pub mod test {
&opctx,
Uuid::new_v4(),
cptestctx.sled_agent.sled_agent.id,
Some(Uuid::new_v4()),
new_dns_addr,
ServiceKind::InternalDnsConfig.into(),
)
Expand Down
4 changes: 3 additions & 1 deletion nexus/src/app/sled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ impl super::Nexus {
opctx: &OpContext,
id: Uuid,
sled_id: Uuid,
zone_id: Option<Uuid>,
address: SocketAddrV6,
kind: ServiceKind,
) -> Result<(), Error> {
Expand All @@ -270,7 +271,8 @@ impl super::Nexus {
"service_id" => id.to_string(),
"address" => address.to_string(),
);
let service = db::model::Service::new(id, sled_id, address, kind);
let service =
db::model::Service::new(id, sled_id, zone_id, address, kind);
self.db_datastore.service_upsert(opctx, service).await?;

if kind == ServiceKind::ExternalDnsConfig {
Expand Down
28 changes: 26 additions & 2 deletions nexus/test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,28 @@ pub async fn test_setup_with_config<N: NexusServer>(
SocketAddr::V4(_) => panic!("expected DNS server to have IPv6 address"),
SocketAddr::V6(addr) => addr,
};
let dns_service_internal = ServicePutRequest {
let dns_server_dns_address_internal = match sled_agent
.dns_server
.local_address()
{
SocketAddr::V4(_) => panic!("expected DNS server to have IPv6 address"),
SocketAddr::V6(addr) => *addr,
};
let dns_server_zone = Uuid::new_v4();
let dns_service_config = ServicePutRequest {
service_id: Uuid::new_v4(),
sled_id: sa_id,
zone_id: Some(dns_server_zone),
address: dns_server_address_internal,
kind: ServiceKind::InternalDnsConfig,
};
let dns_service_dns = ServicePutRequest {
service_id: Uuid::new_v4(),
sled_id: sa_id,
zone_id: Some(dns_server_zone),
address: dns_server_dns_address_internal,
kind: ServiceKind::InternalDns,
};
let dns_server_address_external = match external_dns_config_server
.local_addr()
{
Expand All @@ -235,12 +251,14 @@ pub async fn test_setup_with_config<N: NexusServer>(
let dns_service_external = ServicePutRequest {
service_id: Uuid::new_v4(),
sled_id: sa_id,
zone_id: Some(Uuid::new_v4()),
address: dns_server_address_external,
kind: ServiceKind::ExternalDnsConfig,
};
let nexus_service = ServicePutRequest {
service_id: Uuid::new_v4(),
sled_id: sa_id,
zone_id: Some(Uuid::new_v4()),
address: SocketAddrV6::new(
match nexus_internal_addr.ip() {
IpAddr::V4(addr) => addr.to_ipv6_mapped(),
Expand All @@ -260,10 +278,16 @@ pub async fn test_setup_with_config<N: NexusServer>(
};
let external_dns_zone_name =
internal_dns::names::DNS_ZONE_EXTERNAL_TESTING.to_string();

let server = N::start(
nexus_internal,
&config,
vec![dns_service_internal, dns_service_external, nexus_service],
vec![
dns_service_config,
dns_service_dns,
dns_service_external,
nexus_service,
],
&external_dns_zone_name,
)
.await;
Expand Down
1 change: 1 addition & 0 deletions nexus/types/src/internal_api/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ impl fmt::Display for ServiceKind {
pub struct ServicePutRequest {
pub service_id: Uuid,
pub sled_id: Uuid,
pub zone_id: Option<Uuid>,

/// Address on which a service is responding to requests.
pub address: SocketAddrV6,
Expand Down
5 changes: 5 additions & 0 deletions openapi/nexus-internal.json
Original file line number Diff line number Diff line change
Expand Up @@ -2460,6 +2460,11 @@
"sled_id": {
"type": "string",
"format": "uuid"
},
"zone_id": {
"nullable": true,
"type": "string",
"format": "uuid"
}
},
"required": [
Expand Down
18 changes: 17 additions & 1 deletion openapi/sled-agent.json
Original file line number Diff line number Diff line change
Expand Up @@ -2001,7 +2001,7 @@
"services": {
"type": "array",
"items": {
"$ref": "#/components/schemas/ServiceType"
"$ref": "#/components/schemas/ServiceZoneService"
}
},
"zone_type": {
Expand All @@ -2015,6 +2015,22 @@
"zone_type"
]
},
"ServiceZoneService": {
"type": "object",
"properties": {
"details": {
"$ref": "#/components/schemas/ServiceType"
},
"id": {
"type": "string",
"format": "uuid"
}
},
"required": [
"details",
"id"
]
},
"SetVirtualNetworkInterfaceHost": {
"description": "A mapping from a virtual NIC to a physical host",
"type": "object",
Expand Down
16 changes: 15 additions & 1 deletion sled-agent/src/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ pub struct ServiceZoneRequest {
#[serde(default)]
pub gz_addresses: Vec<Ipv6Addr>,
// Services that should be run in the zone
pub services: Vec<ServiceType>,
pub services: Vec<ServiceZoneService>,
}

impl From<ServiceZoneRequest> for sled_agent_client::types::ServiceZoneRequest {
Expand All @@ -485,6 +485,20 @@ impl From<ServiceZoneRequest> for sled_agent_client::types::ServiceZoneRequest {
}
}

#[derive(
Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq, Hash,
)]
pub struct ServiceZoneService {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can probably do another pass on the naming here to collapse "Service" out of this in a separate PR, but the addition of this structure makes sense to me!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look!

pub id: Uuid,
pub details: ServiceType,
}

impl From<ServiceZoneService> for sled_agent_client::types::ServiceZoneService {
fn from(s: ServiceZoneService) -> Self {
Self { id: s.id, details: s.details.into() }
}
}

/// Used to request that the Sled initialize certain services on initialization.
///
/// This may be used to record that certain sleds are responsible for
Expand Down
Loading