Skip to content

ExternalIpConfig is too complicated #9839

@bnaecker

Description

@bnaecker

The ExternalIpConfig enum is used to send the details of an instance's external IP addresses to sled agent. It's defined like this:

pub enum ExternalIpConfig {
/// Single-stack IPv4 external IP configuration.
V4(ExternalIpv4Config),
/// Single-stack IPv6 external IP configuration.
V6(ExternalIpv6Config),
/// Both IPv4 and IPv6 external IP configuration.
DualStack { v4: ExternalIpv4Config, v6: ExternalIpv6Config },
}

Each of the contained types looks like this:

#[derive(Clone, Debug, Serialize, PartialEq)]
pub struct ExternalIps<T>
where
T: ConcreteIp,
{
/// Source NAT configuration, for outbound-only connectivity.
source_nat: Option<SourceNatConfig<T>>,
/// An Ephemeral address for in- and outbound connectivity.
ephemeral_ip: Option<T>,
/// Additional Floating IPs for in- and outbound connectivity.
floating_ips: Vec<T>,
}

That's supposed to enforce the invariants we have to today at runtime. For example, you can't have a completely empty configuration, and (today) you must have an SNAT address. Instead, we indicate that there's no IP addresses at all to the sled-agent by wrapping the whole ExternalIpConfig enum in an Option. That's sent here:

pub external_ips: Option<ExternalIpConfig>,

In the InstanceSledLocalConfig part of the InstanceEnsureBody.

This is all too complicated. We've already had one bug around this (#9830), which we hit because there is no way to "update" the sled-agent's view of the external IP config when the user does something like attach an Ephemeral IP address. If they weren't dual-stack at the instance-creation time, they won't be now, which is really unhelpful.

The complexity of this type makes fixing the bug kind of tricky. We need to change the variant of this enum that the sled-agent has, when we're sent a new external IP to create, or fill in this optional set of external IPs entirely. It will also be harder to make future changes like #9003.

Because of time pressure, and the fact that this type appears in the sled-agent API, I'm working on a fix for #9830 that doesn't change it. But we should do that at the earliest opportunity after R18.

Metadata

Metadata

Assignees

No one assigned

    Labels

    networkingRelated to the networking.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions