Skip to content

Commit

Permalink
Live attach/detach of external IPs (#4694)
Browse files Browse the repository at this point in the history
This PR adds new endpoints to attach and detach external IPs to/from an
individual instance at runtime, when instances are either stopped or
started. These new endpoints are:

* POST `/v1/floating-ips/{floating_ip}/attach`
* POST `/v1/floating-ips/{floating_ip}/detach`
* POST `/v1/instances/{instance}/external-ips/ephemeral`
* DELETE `/v1/instances/{instance}/external-ips/ephemeral`

These follow and enforce the same rules as external IPs registered
during instance creation: at most one ephemeral IP, and at most 32
external IPs total.

`/v1/floating-ips/{floating_ip}/attach` includes a `kind` field to
account for future API resources which a FIP may be bound to -- such as
internet gateways, load balancers, and services.

## Interaction with other instance lifecycle changes and sagas

Both external IP modify sagas begin with an atomic update to external IP
attach state conditioned on $\mathit{state}\in[
\mathit{started},\mathit{stopped}]$. As a result, we know that an
external IP saga can only ever start before any other instance state
change occurs. We then only need to think about how these other
sagas/events must behave when called *during* an attach/detach, keeping
in mind that these are worst-case orderings: attach/detach are likely to
complete quickly.

### Instance start & migrate

Both of these sagas alter an instance's functional sled ID, which
controls whether NAT entry insertion and OPTE port state updates are
performed. If an IP attach/detach is incomplete when either saga reaches
`instance_ensure_dpd_config` or `instance_ensure_registered` (e.g., any
IP associated with the target instance is in attaching/detaching state),
the start/migrate will unwind with an HTTP 503.

Generally, neither should undo in practice since IP attach/detach are
fast operations -- particularly when an instance is formerly stopped.
This is used solely to guarantee that only one saga is accessing a given
external IP at a time, and that the update target remains unchanged.

### Instance stop & delete

These operations are either not sagaized (stop), or cannot unwind
(delete), and so we cannot block them using IP attach state. IP
attach/detach will unwind if a given sled-agent is no longer responsible
for an instance. Instance delete will force-detach IP addresses bound to
an instance, and if this is seen then IP attach will deliberately unwind
to potentially clean up NAT state. OPTE/DPD undo operations are
best-effort in such a case to prevent stuck sagas.

Instance stop and IP attach may interleave such that the latter adds
additional NAT entries after other network state is cleared. Because we
cannot unwind in this case, `instance_ensure_dpd_config` will now
attempt to remove leftover conflicting RPW entries if they are detected,
since we know they are a deviation from intended state.

## Additional/supporting changes

* Pool/floating IP specifiers in instance create now take `NameOrId`,
parameter names changed to match.
* External IP create/bind in instance create no longer double-resolves
name on saga unwind.
* `views::ExternalIp` can now contain `FloatingIp` body.
* DPD NAT insert/remove functions now perform single rule update via ID
instead of index into the EIP list -- index-based was unstable under
live addition/removal.
* NAT RPW ensure is now more authoritative, and will remove conflicting
entries if an initial insert fails.
* Pool `NameOrId` resolution for floating IP allocation pulled up from
`Datastore` into `Nexus`.

---

Closes #4630 and #4628.
  • Loading branch information
FelixMcFelix authored Jan 24, 2024
1 parent 9ac047e commit cc64304
Show file tree
Hide file tree
Showing 59 changed files with 4,776 additions and 525 deletions.
3 changes: 3 additions & 0 deletions dev-tools/omdb/src/bin/omdb/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ use nexus_db_model::ExternalIp;
use nexus_db_model::HwBaseboardId;
use nexus_db_model::Instance;
use nexus_db_model::InvCollection;
use nexus_db_model::IpAttachState;
use nexus_db_model::Project;
use nexus_db_model::Region;
use nexus_db_model::RegionSnapshot;
Expand Down Expand Up @@ -1705,6 +1706,7 @@ async fn cmd_db_eips(
ip: ipnetwork::IpNetwork,
ports: PortRange,
kind: String,
state: IpAttachState,
owner: Owner,
}

Expand Down Expand Up @@ -1789,6 +1791,7 @@ async fn cmd_db_eips(
first: ip.first_port.into(),
last: ip.last_port.into(),
},
state: ip.state,
kind: format!("{:?}", ip.kind),
owner,
};
Expand Down
13 changes: 8 additions & 5 deletions end-to-end-tests/src/instance_launch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use anyhow::{ensure, Context as _, Result};
use async_trait::async_trait;
use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError};
use oxide_client::types::{
ByteCount, DiskCreate, DiskSource, ExternalIpCreate, InstanceCpuCount,
InstanceCreate, InstanceDiskAttachment, InstanceNetworkInterfaceAttachment,
SshKeyCreate,
ByteCount, DiskCreate, DiskSource, ExternalIp, ExternalIpCreate,
InstanceCpuCount, InstanceCreate, InstanceDiskAttachment,
InstanceNetworkInterfaceAttachment, SshKeyCreate,
};
use oxide_client::{ClientDisksExt, ClientInstancesExt, ClientSessionExt};
use russh::{ChannelMsg, Disconnect};
Expand Down Expand Up @@ -70,7 +70,7 @@ async fn instance_launch() -> Result<()> {
name: disk_name.clone(),
}],
network_interfaces: InstanceNetworkInterfaceAttachment::Default,
external_ips: vec![ExternalIpCreate::Ephemeral { pool_name: None }],
external_ips: vec![ExternalIpCreate::Ephemeral { pool: None }],
user_data: String::new(),
start: true,
})
Expand All @@ -87,7 +87,10 @@ async fn instance_launch() -> Result<()> {
.items
.first()
.context("no external IPs")?
.ip;
.clone();
let ExternalIp::Ephemeral { ip: ip_addr } = ip_addr else {
anyhow::bail!("IP bound to instance was not ephemeral as required.")
};
eprintln!("instance external IP: {}", ip_addr);

// poll serial for login prompt, waiting 5 min max
Expand Down
10 changes: 10 additions & 0 deletions illumos-utils/src/opte/illumos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use omicron_common::api::internal::shared::NetworkInterfaceKind;
use opte_ioctl::OpteHdl;
use slog::info;
use slog::Logger;
use std::net::IpAddr;

#[derive(thiserror::Error, Debug)]
pub enum Error {
Expand Down Expand Up @@ -46,6 +47,15 @@ pub enum Error {

#[error("Tried to release non-existent port ({0}, {1:?})")]
ReleaseMissingPort(uuid::Uuid, NetworkInterfaceKind),

#[error("Tried to update external IPs on non-existent port ({0}, {1:?})")]
ExternalIpUpdateMissingPort(uuid::Uuid, NetworkInterfaceKind),

#[error("Could not find Primary NIC")]
NoPrimaryNic,

#[error("Can't attach new ephemeral IP {0}, currently have {1}")]
ImplicitEphemeralIpDetach(IpAddr, IpAddr),
}

/// Delete all xde devices on the system.
Expand Down
10 changes: 10 additions & 0 deletions illumos-utils/src/opte/non_illumos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use slog::Logger;

use crate::addrobj::AddrObject;
use omicron_common::api::internal::shared::NetworkInterfaceKind;
use std::net::IpAddr;

#[derive(thiserror::Error, Debug)]
pub enum Error {
Expand All @@ -16,6 +17,15 @@ pub enum Error {

#[error("Tried to release non-existent port ({0}, {1:?})")]
ReleaseMissingPort(uuid::Uuid, NetworkInterfaceKind),

#[error("Tried to update external IPs on non-existent port ({0}, {1:?})")]
ExternalIpUpdateMissingPort(uuid::Uuid, NetworkInterfaceKind),

#[error("Could not find Primary NIC")]
NoPrimaryNic,

#[error("Can't attach new ephemeral IP {0}, currently have {1}")]
ImplicitEphemeralIpDetach(IpAddr, IpAddr),
}

pub fn initialize_xde_driver(
Expand Down
116 changes: 116 additions & 0 deletions illumos-utils/src/opte/port_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use oxide_vpc::api::MacAddr;
use oxide_vpc::api::RouterTarget;
use oxide_vpc::api::SNat4Cfg;
use oxide_vpc::api::SNat6Cfg;
use oxide_vpc::api::SetExternalIpsReq;
use oxide_vpc::api::VpcCfg;
use slog::debug;
use slog::error;
Expand Down Expand Up @@ -398,6 +399,121 @@ impl PortManager {
Ok((port, ticket))
}

/// Ensure external IPs for an OPTE port are up to date.
#[cfg_attr(not(target_os = "illumos"), allow(unused_variables))]
pub fn external_ips_ensure(
&self,
nic_id: Uuid,
nic_kind: NetworkInterfaceKind,
source_nat: Option<SourceNatConfig>,
ephemeral_ip: Option<IpAddr>,
floating_ips: &[IpAddr],
) -> Result<(), Error> {
let ports = self.inner.ports.lock().unwrap();
let port = ports.get(&(nic_id, nic_kind)).ok_or_else(|| {
Error::ExternalIpUpdateMissingPort(nic_id, nic_kind)
})?;

// XXX: duplicates parts of macro logic in `create_port`.
macro_rules! ext_ip_cfg {
($ip:expr, $log_prefix:literal, $ip_t:path, $cidr_t:path,
$ipcfg_e:path, $ipcfg_t:ident, $snat_t:ident) => {{
let snat = match source_nat {
Some(snat) => {
let $ip_t(snat_ip) = snat.ip else {
error!(
self.inner.log,
concat!($log_prefix, " SNAT config");
"snat_ip" => ?snat.ip,
);
return Err(Error::InvalidPortIpConfig);
};
let ports = snat.first_port..=snat.last_port;
Some($snat_t { external_ip: snat_ip.into(), ports })
}
None => None,
};
let ephemeral_ip = match ephemeral_ip {
Some($ip_t(ip)) => Some(ip.into()),
Some(_) => {
error!(
self.inner.log,
concat!($log_prefix, " ephemeral IP");
"ephemeral_ip" => ?ephemeral_ip,
);
return Err(Error::InvalidPortIpConfig);
}
None => None,
};
let floating_ips: Vec<_> = floating_ips
.iter()
.copied()
.map(|ip| match ip {
$ip_t(ip) => Ok(ip.into()),
_ => {
error!(
self.inner.log,
concat!($log_prefix, " ephemeral IP");
"ephemeral_ip" => ?ephemeral_ip,
);
Err(Error::InvalidPortIpConfig)
}
})
.collect::<Result<Vec<_>, _>>()?;

ExternalIpCfg {
ephemeral_ip,
snat,
floating_ips,
}
}}
}

// TODO-completeness: support dual-stack. We'll need to explicitly store
// a v4 and a v6 ephemeral IP + SNat + gateway + ... in `InstanceInner`
// to have enough info to build both.
let mut v4_cfg = None;
let mut v6_cfg = None;
match port.gateway().ip {
IpAddr::V4(_) => {
v4_cfg = Some(ext_ip_cfg!(
ip,
"Expected IPv4",
IpAddr::V4,
IpCidr::Ip4,
IpCfg::Ipv4,
Ipv4Cfg,
SNat4Cfg
))
}
IpAddr::V6(_) => {
v6_cfg = Some(ext_ip_cfg!(
ip,
"Expected IPv6",
IpAddr::V6,
IpCidr::Ip6,
IpCfg::Ipv6,
Ipv6Cfg,
SNat6Cfg
))
}
}

let req = SetExternalIpsReq {
port_name: port.name().into(),
external_ips_v4: v4_cfg,
external_ips_v6: v6_cfg,
};

#[cfg(target_os = "illumos")]
let hdl = opte_ioctl::OpteHdl::open(opte_ioctl::OpteHdl::XDE_CTL)?;

#[cfg(target_os = "illumos")]
hdl.set_external_ips(&req)?;

Ok(())
}

#[cfg(target_os = "illumos")]
pub fn firewall_rules_ensure(
&self,
Expand Down
Loading

0 comments on commit cc64304

Please sign in to comment.