Open jgallagher opened 1 day ago
Is it a problem to have all zone services listen on the same IP? I think that's simplest and seems to be what we do everywhere. I'm having trouble coming up with a reason why we'd need to use more than one IP. The only one I could think of is if we run multiple instances of the same service and want to use the same hardcoded port.
Is it a problem to have all zone services listen on the same IP?
Not that I know of! But I wanted to point out that the two cleanup options are different here, and ask whether we care.
OmicronZoneType::InternalDns
contains two socket addresses because it's effectively running two services (a dropshot server for administration, and a DNS server): https://github.com/oxidecomputer/omicron/blob/f3ab3387b640de27875502209a55211087e9b23d/nexus-sled-agent-shared/src/inventory.rs#L232-L233We have several other variants that are running multiple services but which only contain a single socket address:
OmicronZoneType::Cockroach
has the address for cockroach itself, but does not describe the address for thecockroach-admin
server running alongsideOmicronZoneType::{Clickhouse,ClickhouseServer,ClickhouseKeeper}
have the address for clickhouse itself, but do not describe the address for theclickhouse-admin
serversled-agent
(and DNS, probably?) is forced to combine the IP from one address (e.g.,Cockroach.address
) with a hard-coded port number (COCKROACH_ADMIN_PORT
) in order to configure these supplementary services.This doesn't seem particularly urgent or terrible, as constant ports are all we use today, but is somewhat inconsistent. #6794 removed
OmicronZoneConfig::underlay_address
(the parent struct ofOmicronZoneType
) to remove some duplication; I think there are at least two possible ways to address this issue (one of which would undo #6794, and both of which would require some care to support compatibility across upgrades):SocketAddrV6
fields to the relevant variants for their supplementary servicesOmicronZoneConfig::underlay_address
, and replace the existingOmicronZoneType
SocketAddrV6
fields with just port numbers (and add port numbers for the missing services while we're there)These aren't equivalent: the first (which matches how our DNS zone types are described) allows different services in the zone to run on different IPs; the second would require all services to use the same IP. We've sprinkled assumptions that a given Omicron service zone only has one underlay IP throughout the codebase, but I'm not sure whether that was an intentional design choice or something we may want to change in the future.