fix: durable Ochre/RDS appliance lifecycle (static ENI, teardown GC, stale LSP) (#852)

* fix: join the daemon host-port ENI to the appliance security group

The customer ENI's self-referencing SG drops all ingress except its own
members, so the daemon's host-port ENI could open TCP but never complete a
postgres handshake. Carry the appliance's security groups onto the daemon ENI
at create, and re-associate an already-created one in place.

* fix: complete teardown GC when a volume's metadata doc is already gone

DeleteVolumeOnTerminate and DetachVolumeOnTerminate propagated the
NoSuchKey error from a missing volume metadata doc as a hard failure, so
the teardown reaper backed off forever and never purged the terminated
record. A missing metadata doc means the volume is already gone, so treat
it as success via objectstore.IsNoSuchKeyError at both call sites, with a
matching tolerance in the instance-cleaner adapter.

* fix: statically address the RDS customer ENI so it survives the DHCP lease

The customer ENI was configured via a one-shot udhcpc with no renewal
against OVN's 3600s lease, so the DB endpoint went address-less an hour
after every boot. Resolve the customer subnet's prefix and gateway at
launch and emit the ENI as a static NIC (CIDR + gateway, never the
default route), mirroring the mgmt NIC, so it no longer depends on the
lease being renewed. The guest static branch now installs the same
source-based return-path policy a non-default DHCP NIC gets.

* fix: delete the OVN logical switch port when an ENI is deleted

An ENI delete published vpc.delete-port fire-and-forget, so a lost or slow
message let the ENI record and its IP free through with the logical switch
port still in OVN. A recreate onto the same IP then had two ports carrying
it and the router latched the stale MAC, blackholing the datapath. Publish
the delete synchronously so the port is confirmed gone before reuse, and
make DeletePort idempotent so an already-absent port is a no-op.
This commit is contained in:
Josh Siviour
2026-08-21 15:11:02 +10:00
committed by GitHub
parent e22db02d24
commit bed994859c
19 changed files with 534 additions and 87 deletions
+27 -10
View File
@@ -8,11 +8,14 @@
# - NIC<n>_DHCP=1: the primary data ENI. OVN serves DHCP; we lease it (retrying
# one-shots on a budget until the cross-host datapath is up) so the Ec2 IMDS
# datasource can reach 169.254.169.254, and pin a /32 to it so a link-local
# 169.254.0.0/16 route on another NIC cannot hijack IMDS. A DHCP NIC with
# NIC<n>_DEFAULT=0 (an RDS DB VM's customer ENI) instead loses its leased
# default route and gets a source-based policy table for its replies.
# - NIC<n>_CIDR: a static NIC (mgmt0 on br-mgmt, which has no DHCP). Applied
# address-only; NIC<n>_DEFAULT is 0, so mgmt0 is never the default route.
# 169.254.0.0/16 route on another NIC cannot hijack IMDS.
# - NIC<n>_CIDR (+ optional NIC<n>_GW): a static NIC — mgmt0 on br-mgmt, and
# an RDS DB VM's customer ENI. The customer ENI is static rather than DHCP
# so it never depends on OVN's 3600s lease being renewed, which nothing
# does. NIC<n>_DEFAULT is 0 for both, so neither is ever the default
# route; when NIC<n>_GW is set, the NIC also gets the source-based
# return-path policy table a non-default DHCP NIC gets, so cross-subnet
# replies still egress correctly.
#
# The blob is shell KEY=value, matching daemon.buildNetcfgBlob and
# build/microvm/init.sh; interfaces are matched by MAC. No-op when the blob is
@@ -128,6 +131,7 @@ cidr_network() {
install_return_policy() {
iface="$1"
tid=$(( 100 + $2 ))
gw="${3:-}"
addr=$(ip -4 addr show dev "$iface" 2>/dev/null | awk '$1 == "inet" { print $2; exit }')
if [ -z "$addr" ]; then
@@ -136,9 +140,13 @@ install_return_policy() {
fi
ip4="${addr%%/*}"
net=$(cidr_network "$addr")
# The lease's gateway is OVN's per-subnet router IP, which reaches every
# subnet of the VPC. Read it before the default route is dropped from main.
gw=$(ip -4 route show default dev "$iface" 2>/dev/null | awk '{print $3; exit}')
# OVN's per-subnet router IP reaches every subnet of the VPC. A static NIC
# has no lease to read it from, so the caller passes it explicitly
# (NIC<n>_GW); a DHCP NIC reads it from its own default route instead,
# before that route is dropped below.
if [ -z "$gw" ]; then
gw=$(ip -4 route show default dev "$iface" 2>/dev/null | awk '{print $3; exit}')
fi
# Both passes rewrite the table, not just the rule: a rule survives an
# interface bounce but the routes go down with the device, and a rule whose
@@ -174,6 +182,7 @@ for n in 0 1 2 3 4 5; do
eval "dhcp=\${NIC${n}_DHCP:-}"
eval "cidr=\${NIC${n}_CIDR:-}"
eval "gw=\${NIC${n}_GW:-}"
eval "isdefault=\${NIC${n}_DEFAULT:-}"
ip link set "$iface" up
@@ -243,9 +252,17 @@ for n in 0 1 2 3 4 5; do
# NIC<n>_DEFAULT=0 means this NIC must never carry the default route: the
# mgmt NIC reaches the gateway on-link and a default via it would blackhole
# egress and (with a link-local /16) hijack IMDS. Enforce it — a DHCP client
# racing this NIC may have added one before we set it static.
# egress and (with a link-local /16) hijack IMDS. Enforce it on every pass,
# not just setup: nothing else re-adds a route to a static NIC, but the
# policy table itself goes down with the device on a stop/start re-attach.
if [ "$isdefault" != "1" ]; then
# A static NIC with a resolved gateway (an RDS DB VM's customer ENI) is
# cross-subnet reachable, so its replies need the same source-based
# return path a non-default DHCP NIC gets. mgmt0 has no NIC<n>_GW and
# is skipped — it only ever reaches the gateway on-link.
if [ -n "$gw" ]; then
install_return_policy "$iface" "$n" "$gw"
fi
drop_default_route "$iface"
fi
done
+12 -8
View File
@@ -187,10 +187,12 @@ func (d *Daemon) LaunchSystemInstance(input *handlers_elbv2.SystemInstanceInput)
d.recordENIInstanceOwner(extraAccount, extra.ENIID, accountID)
}
instance.ExtraENIs = append(instance.ExtraENIs, vm.ExtraENI{
ENIID: extra.ENIID,
ENIMac: extra.ENIMac,
ENIIP: extra.ENIIP,
SubnetID: extra.SubnetID,
ENIID: extra.ENIID,
ENIMac: extra.ENIMac,
ENIIP: extra.ENIIP,
SubnetID: extra.SubnetID,
ENICIDRPrefix: extra.ENICIDRPrefix,
Gateway: extra.Gateway,
})
}
} else if input.SubnetID != "" && d.vpcService != nil {
@@ -514,10 +516,12 @@ func (d *Daemon) launchAMISystemInstance(input *sysinstance.SystemInstanceInput)
}
}
inst.ExtraENIs = append(inst.ExtraENIs, vm.ExtraENI{
ENIID: extra.ENIID,
ENIMac: extra.ENIMac,
ENIIP: extra.ENIIP,
SubnetID: extra.SubnetID,
ENIID: extra.ENIID,
ENIMac: extra.ENIMac,
ENIIP: extra.ENIIP,
SubnetID: extra.SubnetID,
ENICIDRPrefix: extra.ENICIDRPrefix,
Gateway: extra.Gateway,
})
}
+3 -2
View File
@@ -14,6 +14,7 @@ import (
handlers_ec2_placementgroup "github.com/mulgadc/spinifex/spinifex/handlers/ec2/placementgroup"
"github.com/mulgadc/spinifex/spinifex/network/external/dhcp"
"github.com/mulgadc/spinifex/spinifex/network/topology"
"github.com/mulgadc/spinifex/spinifex/objectstore"
"github.com/mulgadc/spinifex/spinifex/otelsetup"
"github.com/mulgadc/spinifex/spinifex/tags"
"github.com/mulgadc/spinifex/spinifex/types"
@@ -707,7 +708,7 @@ func (a *instanceCleanerAdapter) DeleteVolumes(instance *vm.VM) error {
"name", ebsRequest.Name, "id", instance.ID)
continue
}
if err := a.d.volumeService.DetachVolumeOnTerminate(context.Background(), ebsRequest.Name, instance.AccountID); err != nil && !awserrors.IsNotFound(err) {
if err := a.d.volumeService.DetachVolumeOnTerminate(context.Background(), ebsRequest.Name, instance.AccountID); err != nil && !awserrors.IsNotFound(err) && !objectstore.IsNoSuchKeyError(err) {
slog.Error("Failed to detach volume on termination",
"name", ebsRequest.Name, "id", instance.ID, "err", err)
firstErr = cmp.Or(firstErr, err)
@@ -730,7 +731,7 @@ func (a *instanceCleanerAdapter) DeleteVolumes(instance *vm.VM) error {
// (above, via terminateCleanup) deliberately never clears a Boot
// volume's attachment, so without this the root volume still hits
// DeleteVolume's in-use guard here.
if err := a.d.volumeService.DeleteVolumeOnTerminate(context.Background(), ebsRequest.Name, instance.AccountID); err != nil && !awserrors.IsNotFound(err) {
if err := a.d.volumeService.DeleteVolumeOnTerminate(context.Background(), ebsRequest.Name, instance.AccountID); err != nil && !awserrors.IsNotFound(err) && !objectstore.IsNoSuchKeyError(err) {
slog.Error("Failed to delete volume on termination",
"name", ebsRequest.Name, "id", instance.ID, "err", err)
firstErr = cmp.Or(firstErr, err)
+14 -1
View File
@@ -1007,6 +1007,11 @@ func (s *VolumeServiceImpl) ModifyVolume(ctx context.Context, input *ec2.ModifyV
// are returned, not swallowed.
func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeID, accountID string) error {
if err := s.UpdateVolumeState(volumeID, "available", "", ""); err != nil {
// No metadata doc means the volume is already gone, per GetVolume's own
// contract, so absence here is success, not a teardown failure.
if objectstore.IsNoSuchKeyError(err) {
return nil
}
return fmt.Errorf("clear attachment before terminate delete: %w", err)
}
_, err := s.DeleteVolume(ctx, &ec2.DeleteVolumeInput{VolumeId: &volumeID}, accountID)
@@ -1020,7 +1025,15 @@ func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeI
// as DeleteVolumeOnTerminate: there is no live QEMU to hot-unplug on either
// terminate path.
func (s *VolumeServiceImpl) DetachVolumeOnTerminate(_ context.Context, volumeID, _ string) error {
return s.UpdateVolumeState(volumeID, "available", "", "")
if err := s.UpdateVolumeState(volumeID, "available", "", ""); err != nil {
// No metadata doc means the volume is already gone, per GetVolume's own
// contract, so absence here is success, not a teardown failure.
if objectstore.IsNoSuchKeyError(err) {
return nil
}
return err
}
return nil
}
// ForceDetachVolume clears a volume's attachment in the control plane without
@@ -1773,6 +1773,34 @@ func TestDeleteVolumeOnTerminate_ClearsAttachmentThenDeletes(t *testing.T) {
assert.Contains(t, err.Error(), awserrors.ErrorInvalidVolumeNotFound)
}
// TestDeleteVolumeOnTerminate_AlreadyGoneMetadataIsSuccess verifies GC
// re-driving the volumes dependent on an instance whose volume metadata
// document is already deleted is treated as success, not a hard failure that
// pins the reaper backoff at its cap forever.
func TestDeleteVolumeOnTerminate_AlreadyGoneMetadataIsSuccess(t *testing.T) {
kv := setupTestVolumeKV(t)
store := objectstore.NewMemoryObjectStore()
svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
svc.snapshotKV = kv
// No metadata doc is ever written for this volume ID.
err := svc.DeleteVolumeOnTerminate(context.Background(), "vol-already-gone", "")
require.NoError(t, err, "a volume with no metadata document is already gone, not a teardown failure")
}
// TestDetachVolumeOnTerminate_AlreadyGoneMetadataIsSuccess mirrors
// TestDeleteVolumeOnTerminate_AlreadyGoneMetadataIsSuccess for the
// DeleteOnTermination=false path.
func TestDetachVolumeOnTerminate_AlreadyGoneMetadataIsSuccess(t *testing.T) {
kv := setupTestVolumeKV(t)
store := objectstore.NewMemoryObjectStore()
svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
svc.snapshotKV = kv
err := svc.DetachVolumeOnTerminate(context.Background(), "vol-already-gone", "")
require.NoError(t, err, "a volume with no metadata document is already gone, not a teardown failure")
}
// TestDeleteVolumeOnTerminate_SurfacesDeleteFailure verifies a DeleteVolume
// failure downstream of the attachment clear is returned, not swallowed — the
// caller (deleteInstanceVolumes / instanceCleanerAdapter.DeleteVolumes) relies
+7 -11
View File
@@ -968,18 +968,14 @@ type portEventPayload struct {
SecurityGroupIds []string `json:"security_group_ids,omitempty"`
}
// publishPortEvent publishes a port lifecycle event fire-and-forget. Used for
// vpc.delete-port — failure on delete is harmless (the port is going away
// anyway and the reconciler converges any leftover OVN state).
// publishPortEvent sends vpc.delete-port via request-reply so vpcd confirms
// the LSP is gone before a same-IP recreate can collide with a stale one.
// Non-fatal: the ENI KV row is already gone, so a failure is only logged.
func (s *VPCServiceImpl) publishPortEvent(topic, eniId, subnetId, vpcId, privateIP, macAddr string, sgIds []string) {
utils.PublishEvent(s.natsConn, topic, portEventPayload{
NetworkInterfaceId: eniId,
SubnetId: subnetId,
VpcId: vpcId,
PrivateIpAddress: privateIP,
MacAddress: macAddr,
SecurityGroupIds: sgIds,
})
if err := s.requestPortEvent(topic, eniId, subnetId, vpcId, privateIP, macAddr, sgIds); err != nil {
slog.Warn("vpc: delete-port request failed, relying on reconciler orphan prune",
"topic", topic, "eniId", eniId, "err", err)
}
}
// requestPortEvent sends a port lifecycle event via request-reply so vpcd
+25
View File
@@ -582,6 +582,31 @@ func TestDeleteNetworkInterface_PublishesEvent(t *testing.T) {
}
}
// TestDeleteNetworkInterface_VpcdDeletePortError_NonFatal proves a vpcd-side
// OVN LSP delete failure surfaces as a log, not an API error: the ENI KV row
// is already gone by the time this runs, so a failed teardown must still let
// the caller (instance terminate, appliance teardown) converge.
func TestDeleteNetworkInterface_VpcdDeletePortError_NonFatal(t *testing.T) {
svc, nc := setupTestVPCServiceWithNC(t)
vpcId := createTestVPC(t, svc, "10.0.0.0/16")
subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
eniId := createTestENI(t, svc, subnetId)
testutil.OverrideVpcdStubResponse(nc, "vpc.delete-port",
[]byte(`{"success":false,"error":"forced-delete-port-error"}`))
_, err := svc.DeleteNetworkInterface(context.Background(), &ec2.DeleteNetworkInterfaceInput{
NetworkInterfaceId: aws.String(eniId),
}, testAccountID)
require.NoError(t, err, "a vpcd delete-port failure must not fail DeleteNetworkInterface")
// The ENI record itself is gone regardless of the OVN-side failure.
_, err = svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
NetworkInterfaceIds: []*string{aws.String(eniId)},
}, testAccountID)
assert.Error(t, err)
}
func TestModifyNetworkInterfaceAttribute_SecurityGroups(t *testing.T) {
svc := setupTestVPCService(t)
vpcId := createTestVPC(t, svc, "10.0.0.0/16")
+64 -10
View File
@@ -39,6 +39,7 @@ type vpcProvisioner interface {
CreateNetworkInterface(ctx context.Context, input *ec2.CreateNetworkInterfaceInput, accountID string) (*ec2.CreateNetworkInterfaceOutput, error)
DescribeNetworkInterfaces(ctx context.Context, input *ec2.DescribeNetworkInterfacesInput, accountID string) (*ec2.DescribeNetworkInterfacesOutput, error)
DescribeSubnets(ctx context.Context, input *ec2.DescribeSubnetsInput, accountID string) (*ec2.DescribeSubnetsOutput, error)
ModifyNetworkInterfaceAttribute(ctx context.Context, input *ec2.ModifyNetworkInterfaceAttributeInput, accountID string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error)
}
// HostPortDeps bundles the collaborators the daemon's appliance-VPC host
@@ -73,7 +74,7 @@ func daemonPortDescription(nodeID string) string {
// handlers/bedrock's helper of the same shape: description-filtered
// describe first, create only on a miss, so a restart reuses the same
// address rather than leaking one per launch.
func ensureDaemonENI(ctx context.Context, vpcSvc vpcProvisioner, subnetID, nodeID string) (*daemonENI, error) {
func ensureDaemonENI(ctx context.Context, vpcSvc vpcProvisioner, subnetID, nodeID string, groupIDs []string) (*daemonENI, error) {
desc := daemonPortDescription(nodeID)
out, err := vpcSvc.DescribeNetworkInterfaces(ctx, &ec2.DescribeNetworkInterfacesInput{
Filters: []*ec2.Filter{
@@ -94,6 +95,12 @@ func ensureDaemonENI(ctx context.Context, vpcSvc vpcProvisioner, subnetID, nodeI
// A record missing either field cannot back a host port, so fall
// through and mint a usable one rather than fail the connect.
if eni.id != "" && eni.ip != "" && eni.mac != "" {
// A reused ENI predates this node's SG membership, so re-assert
// it: without the appliance's groups the self-referencing SG
// drops the host port's traffic.
if err := ensureENIGroups(ctx, vpcSvc, ni, groupIDs); err != nil {
return nil, err
}
return eni, nil
}
slog.WarnContext(ctx, "ochrevector: ignoring incomplete daemon ENI record",
@@ -101,9 +108,14 @@ func ensureDaemonENI(ctx context.Context, vpcSvc vpcProvisioner, subnetID, nodeI
}
}
var groups []*string
if len(groupIDs) > 0 {
groups = aws.StringSlice(groupIDs)
}
created, err := vpcSvc.CreateNetworkInterface(ctx, &ec2.CreateNetworkInterfaceInput{
SubnetId: aws.String(subnetID),
Description: aws.String(desc),
Groups: groups,
TagSpecifications: []*ec2.TagSpecification{{
ResourceType: aws.String("network-interface"),
Tags: []*ec2.Tag{
@@ -130,6 +142,41 @@ func ensureDaemonENI(ctx context.Context, vpcSvc vpcProvisioner, subnetID, nodeI
return eni, nil
}
// ensureENIGroups makes the daemon ENI a member of the appliance's security
// groups, so the customer ENI's self-referencing SG authorizes the host port
// rather than dropping it. A no-op when the ENI already carries exactly those
// groups, or when the appliance exposed none.
func ensureENIGroups(ctx context.Context, vpcSvc vpcProvisioner, ni *ec2.NetworkInterface, groupIDs []string) error {
if len(groupIDs) == 0 || sameGroupSet(ni.Groups, groupIDs) {
return nil
}
_, err := vpcSvc.ModifyNetworkInterfaceAttribute(ctx, &ec2.ModifyNetworkInterfaceAttributeInput{
NetworkInterfaceId: ni.NetworkInterfaceId,
Groups: aws.StringSlice(groupIDs),
}, utils.GlobalAccountID)
if err != nil {
return fmt.Errorf("ochrevector: set daemon ENI %s security groups: %w", aws.StringValue(ni.NetworkInterfaceId), err)
}
return nil
}
// sameGroupSet reports whether current holds exactly the desired group IDs.
func sameGroupSet(current []*ec2.GroupIdentifier, desired []string) bool {
if len(current) != len(desired) {
return false
}
have := make(map[string]struct{}, len(current))
for _, g := range current {
have[aws.StringValue(g.GroupId)] = struct{}{}
}
for _, id := range desired {
if _, ok := have[id]; !ok {
return false
}
}
return true
}
// applianceInstanceTagKey mirrors the unexported rdsInstanceTagKey in
// handlers/rds/launch.go, which stamps it on the appliance's customer ENI at
// launch. Keep this literal in sync with that const; the e2e harness
@@ -145,9 +192,9 @@ const endpointENIDescriptionPrefix = "RDS endpoint ENI for "
// handlers/rds stamps on it at launch, returning the real private IP to dial
// and the subnet it lives in -- never the vanity endpoint hostname, which is
// unresolvable and unroutable from the daemon host.
func resolveApplianceTarget(ctx context.Context, deps HostPortDeps, identifier string) (dialIP, subnetID, subnetCIDR string, err error) {
func resolveApplianceTarget(ctx context.Context, deps HostPortDeps, identifier string) (dialIP, subnetID, subnetCIDR string, groupIDs []string, err error) {
if deps.VPC == nil {
return "", "", "", errors.New("ochrevector: no VPC provider configured; cannot resolve the appliance's ENI")
return "", "", "", nil, errors.New("ochrevector: no VPC provider configured; cannot resolve the appliance's ENI")
}
out, err := deps.VPC.DescribeNetworkInterfaces(ctx, &ec2.DescribeNetworkInterfacesInput{
@@ -156,7 +203,7 @@ func resolveApplianceTarget(ctx context.Context, deps HostPortDeps, identifier s
},
}, utils.GlobalAccountID)
if err != nil {
return "", "", "", fmt.Errorf("ochrevector: describe appliance ENI for %s: %w", identifier, err)
return "", "", "", nil, fmt.Errorf("ochrevector: describe appliance ENI for %s: %w", identifier, err)
}
// Both the endpoint ENI and the management NIC carry the rds-db-instance
// tag, but only the endpoint ENI (in the customer VPC) is reachable from the
@@ -177,22 +224,29 @@ func resolveApplianceTarget(ctx context.Context, deps HostPortDeps, identifier s
}
}
if ni == nil {
return "", "", "", fmt.Errorf("ochrevector: no customer ENI found for appliance %s", identifier)
return "", "", "", nil, fmt.Errorf("ochrevector: no customer ENI found for appliance %s", identifier)
}
dialIP = aws.StringValue(ni.PrivateIpAddress)
subnetID = aws.StringValue(ni.SubnetId)
// The daemon's host-port ENI must join these groups, or the customer ENI's
// self-referencing SG drops its traffic to the appliance.
for _, g := range ni.Groups {
if id := aws.StringValue(g.GroupId); id != "" {
groupIDs = append(groupIDs, id)
}
}
subOut, err := deps.VPC.DescribeSubnets(ctx, &ec2.DescribeSubnetsInput{
Filters: []*ec2.Filter{{Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{subnetID})}},
}, utils.GlobalAccountID)
if err != nil {
return "", "", "", fmt.Errorf("ochrevector: describe subnet %s: %w", subnetID, err)
return "", "", "", nil, fmt.Errorf("ochrevector: describe subnet %s: %w", subnetID, err)
}
if subOut == nil || len(subOut.Subnets) == 0 || aws.StringValue(subOut.Subnets[0].CidrBlock) == "" {
return "", "", "", fmt.Errorf("ochrevector: subnet %s has no cidr block", subnetID)
return "", "", "", nil, fmt.Errorf("ochrevector: subnet %s has no cidr block", subnetID)
}
subnetCIDR = aws.StringValue(subOut.Subnets[0].CidrBlock)
return dialIP, subnetID, subnetCIDR, nil
return dialIP, subnetID, subnetCIDR, groupIDs, nil
}
// ensureApplianceHostPort gives this daemon a routed presence in the subnet
@@ -213,7 +267,7 @@ func ensureApplianceHostPort(ctx context.Context, deps HostPortDeps, identifier
return "", "", errors.New("ochrevector: no VPC provider configured; cannot mint the daemon's appliance-VPC port")
}
dialIP, subnetID, subnetCIDR, err := resolveApplianceTarget(ctx, deps, identifier)
dialIP, subnetID, subnetCIDR, groupIDs, err := resolveApplianceTarget(ctx, deps, identifier)
if err != nil {
return "", "", err
}
@@ -222,7 +276,7 @@ func ensureApplianceHostPort(ctx context.Context, deps HostPortDeps, identifier
return "", "", fmt.Errorf("ochrevector: parse appliance subnet CIDR %q: %w", subnetCIDR, err)
}
eni, err := ensureDaemonENI(ctx, deps.VPC, subnetID, deps.NodeID)
eni, err := ensureDaemonENI(ctx, deps.VPC, subnetID, deps.NodeID, groupIDs)
if err != nil {
return "", "", err
}
+86 -11
View File
@@ -24,6 +24,7 @@ const (
testApplianceSubnet = "subnet-appliance-0001"
testApplianceEndpoint = "10.244.1.9"
testApplianceENIID = "eni-appliance-0001"
testApplianceSG = "sg-appliance-0001"
)
// fakeVPC is an in-memory vpcProvisioner: every CreateNetworkInterface call
@@ -32,11 +33,12 @@ const (
// lookup and the description-filtered daemon-ENI describe-or-create.
// subnets backs DescribeSubnets.
type fakeVPC struct {
mu sync.Mutex
nextID int
created []string
records map[string]*ec2.NetworkInterface
subnets []*ec2.Subnet
mu sync.Mutex
nextID int
created []string
sgModifies []string
records map[string]*ec2.NetworkInterface
subnets []*ec2.Subnet
}
var _ vpcProvisioner = (*fakeVPC)(nil)
@@ -54,6 +56,7 @@ func (f *fakeVPC) CreateNetworkInterface(_ context.Context, in *ec2.CreateNetwor
MacAddress: aws.String("02:00:00:00:00:01"),
SubnetId: in.SubnetId,
Description: in.Description,
Groups: groupIdentifiers(in.Groups),
}
if f.records == nil {
f.records = map[string]*ec2.NetworkInterface{}
@@ -62,6 +65,32 @@ func (f *fakeVPC) CreateNetworkInterface(_ context.Context, in *ec2.CreateNetwor
return &ec2.CreateNetworkInterfaceOutput{NetworkInterface: ni}, nil
}
// ModifyNetworkInterfaceAttribute records the call and re-associates the
// stored ENI's security groups, mirroring the in-place SG replace the real
// service does.
func (f *fakeVPC) ModifyNetworkInterfaceAttribute(_ context.Context, in *ec2.ModifyNetworkInterfaceAttributeInput, _ string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) {
f.mu.Lock()
defer f.mu.Unlock()
id := aws.StringValue(in.NetworkInterfaceId)
f.sgModifies = append(f.sgModifies, id)
if ni, ok := f.records[id]; ok && in.Groups != nil {
ni.Groups = groupIdentifiers(in.Groups)
}
return &ec2.ModifyNetworkInterfaceAttributeOutput{}, nil
}
// groupIdentifiers maps SG IDs to the describe-shaped GroupIdentifier slice.
func groupIdentifiers(ids []*string) []*ec2.GroupIdentifier {
if len(ids) == 0 {
return nil
}
out := make([]*ec2.GroupIdentifier, 0, len(ids))
for _, id := range ids {
out = append(out, &ec2.GroupIdentifier{GroupId: id})
}
return out
}
// putRecord seeds an ENI the fake did not mint, so a test can present the
// lookup with a record it would not otherwise produce.
func (f *fakeVPC) putRecord(eniID, subnetID, description, ip, mac string) {
@@ -92,6 +121,7 @@ func (f *fakeVPC) putApplianceENI(eniID, identifier, subnetID, ip string) {
NetworkInterfaceId: aws.String(eniID),
SubnetId: aws.String(subnetID),
PrivateIpAddress: aws.String(ip),
Groups: []*ec2.GroupIdentifier{{GroupId: aws.String(testApplianceSG)}},
TagSet: []*ec2.Tag{
{Key: aws.String(applianceInstanceTagKey), Value: aws.String(identifier)},
},
@@ -112,6 +142,7 @@ func (f *fakeVPC) putTaggedENIWithDescription(eniID, identifier, subnetID, ip, d
SubnetId: aws.String(subnetID),
PrivateIpAddress: aws.String(ip),
Description: aws.String(description),
Groups: []*ec2.GroupIdentifier{{GroupId: aws.String(testApplianceSG)}},
TagSet: []*ec2.Tag{
{Key: aws.String(applianceInstanceTagKey), Value: aws.String(identifier)},
},
@@ -291,6 +322,49 @@ func TestEnsureApplianceHostPort_CreatesENIAndHostPort(t *testing.T) {
assert.Equal(t, "02:00:00:00:00:01", calls[0].mac)
}
// The daemon's host-port ENI must join the appliance's security group, or the
// customer ENI's self-referencing SG silently drops its traffic to pg.
func TestEnsureApplianceHostPort_DaemonENIJoinsApplianceSG(t *testing.T) {
h := newHostPortHarness()
h.putSubnet(testApplianceSubnet, "10.244.1.0/24")
h.putApplianceENI(testApplianceIdentifier, testApplianceSubnet, testApplianceEndpoint)
_, eniID, err := ensureApplianceHostPort(t.Context(), h.deps(), testApplianceIdentifier)
require.NoError(t, err)
h.vpc.mu.Lock()
defer h.vpc.mu.Unlock()
require.Contains(t, h.vpc.records, eniID)
got := []string{}
for _, g := range h.vpc.records[eniID].Groups {
got = append(got, aws.StringValue(g.GroupId))
}
assert.Equal(t, []string{testApplianceSG}, got, "the created daemon ENI must carry the appliance SG")
}
// A daemon ENI that predates SG inheritance (reused by description, no groups)
// must be re-associated in place rather than left unauthorized.
func TestEnsureApplianceHostPort_ReusedENIGetsApplianceSG(t *testing.T) {
h := newHostPortHarness()
h.putSubnet(testApplianceSubnet, "10.244.1.0/24")
h.putApplianceENI(testApplianceIdentifier, testApplianceSubnet, testApplianceEndpoint)
h.vpc.putRecord("eni-legacy-daemon", testApplianceSubnet, daemonPortDescription(testNodeID), "10.244.1.50", "02:00:00:00:00:09")
_, eniID, err := ensureApplianceHostPort(t.Context(), h.deps(), testApplianceIdentifier)
require.NoError(t, err)
assert.Equal(t, "eni-legacy-daemon", eniID, "the existing daemon ENI should be reused")
assert.Empty(t, h.vpc.created, "reuse must not mint a new ENI")
h.vpc.mu.Lock()
defer h.vpc.mu.Unlock()
assert.Contains(t, h.vpc.sgModifies, "eni-legacy-daemon", "the reused ENI must be re-associated to the appliance SG")
got := []string{}
for _, g := range h.vpc.records["eni-legacy-daemon"].Groups {
got = append(got, aws.StringValue(g.GroupId))
}
assert.Equal(t, []string{testApplianceSG}, got)
}
// The port must carry the ENI address at the SUBNET's prefix length: a /32
// addresses the port and still leaves the appliance unreachable, which is
// the entire reason the port exists.
@@ -417,11 +491,12 @@ func TestResolveApplianceTarget_FindsTheTaggedApplianceENI(t *testing.T) {
h.putSubnet(testApplianceSubnet, "10.244.1.0/24")
h.putApplianceENI(testApplianceIdentifier, testApplianceSubnet, testApplianceEndpoint)
dialIP, subnetID, cidr, err := resolveApplianceTarget(t.Context(), h.deps(), testApplianceIdentifier)
dialIP, subnetID, cidr, groupIDs, err := resolveApplianceTarget(t.Context(), h.deps(), testApplianceIdentifier)
require.NoError(t, err)
assert.Equal(t, testApplianceEndpoint, dialIP)
assert.Equal(t, testApplianceSubnet, subnetID)
assert.Equal(t, "10.244.1.0/24", cidr)
assert.Equal(t, []string{testApplianceSG}, groupIDs, "the appliance SG must be surfaced so the daemon ENI can join it")
}
// The endpoint ENI and the management NIC share the rds-db-instance tag, but
@@ -441,7 +516,7 @@ func TestResolveApplianceTarget_PrefersEndpointENIOverManagementNIC(t *testing.T
h.vpc.putTaggedENIWithDescription("eni-endpoint", testApplianceIdentifier, testApplianceSubnet, testApplianceEndpoint,
endpointENIDescriptionPrefix+testApplianceIdentifier)
dialIP, subnetID, cidr, err := resolveApplianceTarget(t.Context(), h.deps(), testApplianceIdentifier)
dialIP, subnetID, cidr, _, err := resolveApplianceTarget(t.Context(), h.deps(), testApplianceIdentifier)
require.NoError(t, err)
require.Equal(t, testApplianceEndpoint, dialIP, "must dial the endpoint ENI, never the management NIC")
require.Equal(t, testApplianceSubnet, subnetID)
@@ -455,7 +530,7 @@ func TestResolveApplianceTarget_IgnoresOtherIdentifiersTags(t *testing.T) {
h.putSubnet(testApplianceSubnet, "10.244.1.0/24")
h.putApplianceENI("some-other-db", testApplianceSubnet, testApplianceEndpoint)
_, _, _, err := resolveApplianceTarget(t.Context(), h.deps(), testApplianceIdentifier)
_, _, _, _, err := resolveApplianceTarget(t.Context(), h.deps(), testApplianceIdentifier)
require.Error(t, err)
assert.Contains(t, err.Error(), "no customer ENI found")
}
@@ -464,7 +539,7 @@ func TestResolveApplianceTarget_ErrorsWhenSubnetHasNoCIDR(t *testing.T) {
h := newHostPortHarness()
h.putApplianceENI(testApplianceIdentifier, testApplianceSubnet, testApplianceEndpoint)
_, _, _, err := resolveApplianceTarget(t.Context(), h.deps(), testApplianceIdentifier)
_, _, _, _, err := resolveApplianceTarget(t.Context(), h.deps(), testApplianceIdentifier)
require.Error(t, err)
}
@@ -474,7 +549,7 @@ func TestResolveApplianceTarget_PropagatesDescribeFailure(t *testing.T) {
deps := h.deps()
deps.VPC = failing
_, _, _, err := resolveApplianceTarget(t.Context(), deps, testApplianceIdentifier)
_, _, _, _, err := resolveApplianceTarget(t.Context(), deps, testApplianceIdentifier)
require.Error(t, err)
assert.Contains(t, err.Error(), "nats timeout")
}
@@ -483,7 +558,7 @@ func TestResolveApplianceTarget_RequiresVPC(t *testing.T) {
h := newHostPortHarness()
noVPC := h.deps()
noVPC.VPC = nil
_, _, _, err := resolveApplianceTarget(t.Context(), noVPC, testApplianceIdentifier)
_, _, _, _, err := resolveApplianceTarget(t.Context(), noVPC, testApplianceIdentifier)
require.Error(t, err)
assert.Contains(t, err.Error(), "VPC provider")
}
+57 -5
View File
@@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"log/slog"
"net"
"slices"
"time"
@@ -84,6 +85,10 @@ type launchVPCProvisioner interface {
// start, so a deployment whose system VPC was rebuilt still gets one.
CreateSecurityGroup(ctx context.Context, input *ec2.CreateSecurityGroupInput, accountID string) (*ec2.CreateSecurityGroupOutput, error)
DescribeSecurityGroups(ctx context.Context, input *ec2.DescribeSecurityGroupsInput, accountID string) (*ec2.DescribeSecurityGroupsOutput, error)
// Read to configure the customer ENI statically: it must not depend on the
// OVN DHCP lease being renewed, so its prefix and gateway are resolved once
// at launch time instead.
DescribeSubnets(ctx context.Context, input *ec2.DescribeSubnetsInput, accountID string) (*ec2.DescribeSubnetsOutput, error)
}
// System-managed VMs get a mgmt-bridge NIC alongside their VPC NICs, which is
@@ -239,6 +244,15 @@ func LaunchDBInstanceVM(ctx context.Context, deps LaunchDeps, in LaunchInput) (o
})
}
// The customer ENI is configured statically, not via DHCP: OVN's 3600s
// lease is never renewed by the guest's one-shot udhcpc, so a DHCP customer
// ENI goes address-less an hour after boot. Resolve its prefix and gateway
// once here instead of depending on the lease surviving the VM's lifetime.
customerPrefix, customerGateway, err := customerENISubnetGateway(ctx, deps.VPC, in.AccountID, in.SubnetID)
if err != nil {
return nil, fmt.Errorf("rds: resolve customer subnet network for %s: %w", in.DBInstanceIdentifier, err)
}
sysOut, err := deps.Instance.LaunchSystemInstance(&sysinstance.SystemInstanceInput{
BootMode: sysinstance.BootAMI,
ManagedBy: tags.ManagedByRDS,
@@ -252,11 +266,13 @@ func LaunchDBInstanceVM(ctx context.Context, deps LaunchDeps, in LaunchInput) (o
ENIMac: systemENI.mac,
ENIIP: systemENI.ip,
ExtraENIs: []sysinstance.ExtraENIInput{{
ENIID: customerENI.id,
ENIMac: customerENI.mac,
ENIIP: customerENI.ip,
SubnetID: in.SubnetID,
AccountID: in.AccountID,
ENIID: customerENI.id,
ENIMac: customerENI.mac,
ENIIP: customerENI.ip,
SubnetID: in.SubnetID,
AccountID: in.AccountID,
ENICIDRPrefix: customerPrefix,
Gateway: customerGateway,
// The endpoint is the ENI's address, so it has to survive the
// terminate half of a replace the way the datadir volume does.
// A DeleteDBInstance deletes it explicitly once the VM is gone.
@@ -445,6 +461,42 @@ func resolveCustomerENI(ctx context.Context, vpcSvc launchVPCProvisioner, in Lau
return nil, fmt.Errorf("rds: endpoint ENI %s no longer exists", in.ExistingCustomerENI)
}
// customerENISubnetGateway resolves the customer subnet's prefix length and
// gateway IP (network address +1), matching OVN's per-subnet router
// placement (see network/topology.SubnetGatewayCIDR). Both are needed to
// configure the customer ENI statically instead of via DHCP.
func customerENISubnetGateway(ctx context.Context, vpcSvc launchVPCProvisioner, accountID, subnetID string) (prefixBits int, gateway string, err error) {
out, err := vpcSvc.DescribeSubnets(ctx, &ec2.DescribeSubnetsInput{
SubnetIds: aws.StringSlice([]string{subnetID}),
}, accountID)
if err != nil {
return 0, "", fmt.Errorf("rds: describe subnet %s: %w", subnetID, err)
}
if out == nil {
return 0, "", fmt.Errorf("rds: subnet %s not found", subnetID)
}
for _, subnet := range out.Subnets {
if aws.StringValue(subnet.SubnetId) != subnetID {
continue
}
_, ipNet, perr := net.ParseCIDR(aws.StringValue(subnet.CidrBlock))
if perr != nil {
return 0, "", fmt.Errorf("rds: subnet %s has an unparseable CIDR %q: %w",
subnetID, aws.StringValue(subnet.CidrBlock), perr)
}
ones, _ := ipNet.Mask.Size()
gw := ipNet.IP.To4()
if gw == nil {
return 0, "", fmt.Errorf("rds: subnet %s CIDR %q is not IPv4", subnetID, ipNet.String())
}
gwCopy := make(net.IP, len(gw))
copy(gwCopy, gw)
gwCopy[3]++
return ones, gwCopy.String(), nil
}
return 0, "", fmt.Errorf("rds: subnet %s not found", subnetID)
}
// Detach comes first because a terminated VM can leave the attachment record
// behind, which a plain delete rejects as InUse.
func deleteLaunchENI(ctx context.Context, vpcSvc launchVPCProvisioner, accountID, eniID string) {
+19 -1
View File
@@ -26,7 +26,11 @@ import (
const (
testCustomerAccount = "123456789012"
testDBSubnet = "subnet-customer-db"
testEngineAMI = "ami-rds-postgres-18"
// The customer subnet's CIDR, resolved so the launcher can configure the
// customer ENI statically. Gives a gateway of 10.0.0.1, matching the fake
// customer ENI IPs CreateNetworkInterface hands out.
testDBSubnetCIDR = "10.0.0.0/24"
testEngineAMI = "ami-rds-postgres-18"
// The persisted endpoint a replace re-attaches: its address is the DNS
// target and its MAC is what the replacement VM's NIC has to come up with.
@@ -266,6 +270,16 @@ func (f *fakeENIs) ModifyNetworkInterfaceAttribute(_ context.Context, in *ec2.Mo
return &ec2.ModifyNetworkInterfaceAttributeOutput{}, nil
}
// DescribeSubnets answers the customer subnet's CIDR, which the launcher
// resolves into the prefix and gateway it configures the customer ENI with.
func (f *fakeENIs) DescribeSubnets(_ context.Context, in *ec2.DescribeSubnetsInput, _ string) (*ec2.DescribeSubnetsOutput, error) {
var subnets []*ec2.Subnet
for _, id := range aws.StringValueSlice(in.SubnetIds) {
subnets = append(subnets, &ec2.Subnet{SubnetId: aws.String(id), CidrBlock: aws.String(testDBSubnetCIDR)})
}
return &ec2.DescribeSubnetsOutput{Subnets: subnets}, nil
}
func (f *fakeENIs) DescribeSecurityGroups(_ context.Context, in *ec2.DescribeSecurityGroupsInput, _ string) (*ec2.DescribeSecurityGroupsOutput, error) {
f.sgDescribed = append(f.sgDescribed, in)
if f.sgExistingGroups != nil {
@@ -601,6 +615,10 @@ func TestLaunchDBInstanceVMWiresBothNICs(t *testing.T) {
// The endpoint address lives on this NIC, so it has to survive the
// terminate half of a replace the way the datadir volume does.
DeleteOnTermination: aws.Bool(false),
// Resolved from the customer subnet's CIDR so the guest configures this
// NIC statically instead of depending on the OVN DHCP lease.
ENICIDRPrefix: 24,
Gateway: "10.0.0.1",
}, in.ExtraENIs[0], "the extra NIC must carry the customer account, or the daemon updates the wrong ENI record")
assert.Equal(t, "#cloud-config\n", in.UserData)
assert.Equal(t, "arn:aws:iam::000000000000:instance-profile/rdsInstanceRole", in.IamInstanceProfileArn)
@@ -105,6 +105,12 @@ type ExtraENIInput struct {
// ENI across a class-change replace). Nil keeps the attach-time default,
// which deletes the ENI with the VM.
DeleteOnTermination *bool `json:"delete_on_termination,omitempty"`
// ENICIDRPrefix and Gateway configure this ENI statically instead of via
// DHCP (an RDS DB VM's customer ENI, so it never depends on the OVN lease
// being renewed). Zero/empty keeps the DHCP path.
ENICIDRPrefix int `json:"eni_cidr_prefix,omitempty"`
Gateway string `json:"gateway,omitempty"`
}
// NICConfig describes a single network interface for a BootDirect microVM.
+21 -10
View File
@@ -347,6 +347,9 @@ func (m *liveManager) EnsurePort(ctx context.Context, spec PortSpec) error {
}
// DeletePort clears the ENI's port-group memberships and removes the LSP.
// Idempotent: an already-absent LSP (a retry, or a race with the reconciler's
// orphan prune) is a no-op success, not an error — mirrors ForceDeleteInstanceENI
// tolerating an already-gone ENI.
func (m *liveManager) DeletePort(ctx context.Context, spec PortSpec) error {
if m.ovn == nil {
return fmt.Errorf("OVN client not connected")
@@ -354,17 +357,25 @@ func (m *liveManager) DeletePort(ctx context.Context, spec PortSpec) error {
portName := Port(spec.PortID)
switchName := SubnetSwitch(spec.SubnetID)
if _, err := m.reconcilePortSGs(ctx, portName, nil); err != nil {
return fmt.Errorf("clear port group memberships for %q: %w", portName, err)
// Mirror EnsurePort's existence check: a Get that returns no error means
// the port is present, so delete it; any error means it is already gone
// (a retry, or a race with the reconciler's orphan prune) and we no-op.
if _, err := m.ovn.GetLogicalSwitchPort(ctx, portName); err == nil {
if _, err := m.reconcilePortSGs(ctx, portName, nil); err != nil {
return fmt.Errorf("clear port group memberships for %q: %w", portName, err)
}
if err := m.ovn.DeleteLogicalSwitchPort(ctx, switchName, portName); err != nil {
return fmt.Errorf("delete logical switch port %q on %q: %w", portName, switchName, err)
}
slog.Info("topology: DeletePort removed LSP",
"port", portName,
"switch", switchName,
"eni_id", spec.PortID,
)
return nil
}
if err := m.ovn.DeleteLogicalSwitchPort(ctx, switchName, portName); err != nil {
return fmt.Errorf("delete logical switch port %q on %q: %w", portName, switchName, err)
}
slog.Info("topology: DeletePort removed LSP",
"port", portName,
"switch", switchName,
"eni_id", spec.PortID,
)
slog.Info("topology: DeletePort found no LSP, already gone", "port", portName, "eni_id", spec.PortID)
return nil
}
@@ -137,6 +137,96 @@ func TestLiveManager_DeletePort(t *testing.T) {
}
}
// TestLiveManager_DeletePort_AlreadyAbsentIsNoop proves DeletePort is
// idempotent: a retry (or a race with the reconciler's orphan prune) against
// an already-gone LSP must not error, matching how the codebase treats
// already-gone resources elsewhere (e.g. ForceDeleteInstanceENI).
func TestLiveManager_DeletePort_AlreadyAbsentIsNoop(t *testing.T) {
mgr, _ := newLiveManagerForTest(t)
ctx := context.Background()
vpc := VPCSpec{VPCID: "vpc-dp2", CIDR: netip.MustParsePrefix("10.5.0.0/16")}
sub := SubnetSpec{SubnetID: "subnet-DP2", VPCID: vpc.VPCID, CIDR: netip.MustParsePrefix("10.5.1.0/24")}
_ = mgr.EnsureVPC(ctx, vpc)
_ = mgr.EnsureSubnet(ctx, sub)
port := PortSpec{PortID: "eni-never-created", SubnetID: sub.SubnetID, VPCID: vpc.VPCID}
if err := mgr.DeletePort(ctx, port); err != nil {
t.Fatalf("DeletePort on an absent LSP must be a no-op, got: %v", err)
}
// A second delete of a port that WAS created, once it's gone, is also a no-op.
mac, _ := net.ParseMAC("02:00:00:00:00:03")
port2 := PortSpec{
PortID: "eni-DP2", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
PrivateIP: netip.MustParseAddr("10.5.1.5"), MAC: mac,
}
if err := mgr.EnsurePort(ctx, port2); err != nil {
t.Fatalf("EnsurePort: %v", err)
}
if err := mgr.DeletePort(ctx, port2); err != nil {
t.Fatalf("first DeletePort: %v", err)
}
if err := mgr.DeletePort(ctx, port2); err != nil {
t.Fatalf("repeat DeletePort on an already-deleted LSP must be a no-op, got: %v", err)
}
}
// TestLiveManager_DeletePort_ThenRecreateSameIP_NoDuplicateLSP covers the
// teardown+recreate-on-the-same-IP scenario: after DeletePort removes the
// old ENI's LSP, a new ENI can EnsurePort the same private IP and OVN NB
// ends up with exactly one LSP carrying that address, not two.
func TestLiveManager_DeletePort_ThenRecreateSameIP_NoDuplicateLSP(t *testing.T) {
mgr, mockClient := newLiveManagerForTest(t)
ctx := context.Background()
vpc := VPCSpec{VPCID: "vpc-reuse", CIDR: netip.MustParsePrefix("172.31.0.0/16")}
sub := SubnetSpec{SubnetID: "subnet-reuse", VPCID: vpc.VPCID, CIDR: netip.MustParsePrefix("172.31.0.0/24")}
_ = mgr.EnsureVPC(ctx, vpc)
_ = mgr.EnsureSubnet(ctx, sub)
oldMAC, _ := net.ParseMAC("02:00:00:00:01:01")
oldPort := PortSpec{
PortID: "eni-old", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
PrivateIP: netip.MustParseAddr("172.31.0.4"), MAC: oldMAC,
}
if err := mgr.EnsurePort(ctx, oldPort); err != nil {
t.Fatalf("EnsurePort(old): %v", err)
}
if err := mgr.DeletePort(ctx, oldPort); err != nil {
t.Fatalf("DeletePort(old): %v", err)
}
newMAC, _ := net.ParseMAC("02:00:00:00:01:02")
newPort := PortSpec{
PortID: "eni-new", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
PrivateIP: netip.MustParseAddr("172.31.0.4"), MAC: newMAC,
}
if err := mgr.EnsurePort(ctx, newPort); err != nil {
t.Fatalf("EnsurePort(new): %v", err)
}
lsps, err := mockClient.ListLogicalSwitchPorts(ctx)
if err != nil {
t.Fatalf("ListLogicalSwitchPorts: %v", err)
}
matching := 0
for _, lsp := range lsps {
for _, addr := range lsp.Addresses {
if addr == newMAC.String()+" 172.31.0.4" || addr == oldMAC.String()+" 172.31.0.4" {
matching++
}
}
}
if matching != 1 {
t.Fatalf("expected exactly one LSP carrying 172.31.0.4, found %d", matching)
}
if _, err := mockClient.GetLogicalSwitchPort(ctx, Port(oldPort.PortID)); err == nil {
t.Fatal("old ENI's LSP must be gone after DeletePort")
}
if _, err := mockClient.GetLogicalSwitchPort(ctx, Port(newPort.PortID)); err != nil {
t.Fatalf("new ENI's LSP must exist: %v", err)
}
}
func TestLiveManager_DeleteSubnetAndVPC(t *testing.T) {
mgr, mockClient := newLiveManagerForTest(t)
ctx := context.Background()
+1
View File
@@ -94,6 +94,7 @@ var vpcdStubTopics = []string{
"vpc.update-sg",
"vpc.update-port-sgs",
"vpc.create-port",
"vpc.delete-port",
}
// vpcdStubRegistry holds the per-conn response map. Per-conn so concurrent tests with separate conns don't interfere.
+23 -13
View File
@@ -1129,14 +1129,17 @@ const EBSHotPlugSlotCount = 11
// which. The primary data ENI is marked DHCP (OVN serves it; it carries the
// default route, and bringing it up before cloud-init's network stage is what
// lets the Ec2 datasource reach IMDS — without it cloud-init brings up the mgmt
// NIC instead and falls to DataSourceNone). Every extra ENI is DHCP too, but
// never the default route. mgmt0 lives on br-mgmt with no DHCP, so its static
// address is delivered here; it is never the default route either. The blob
// must name every NIC QEMU is given: the guest configures by MAC, so a NIC the
// blob omits is left down and address-less, unreachable at its own VPC IP. The
// blob key format matches daemon.buildNetcfgBlob and build/microvm/init.sh.
// No-op without a mgmt NIC, so single-NIC guests are untouched — cloud-init
// brings their one NIC up.
// NIC instead and falls to DataSourceNone). Every extra ENI is static when a
// prefix and gateway were resolved at launch (an RDS DB VM's customer ENI is
// the case): DHCP would depend on the OVN lease being renewed, which nothing
// does, and the NIC goes address-less an hour after boot. mgmt0 lives on
// br-mgmt with no DHCP, so its static address is delivered here too; neither
// it nor an extra ENI is ever the default route. The blob must name every NIC
// QEMU is given: the guest configures by MAC, so a NIC the blob omits is left
// down and address-less, unreachable at its own VPC IP. The blob key format
// matches daemon.buildNetcfgBlob and build/microvm/init.sh. No-op without a
// mgmt NIC, so single-NIC guests are untouched — cloud-init brings their one
// NIC up.
func (m *Manager) appendSystemNetcfgFwCfg(instance *VM) error {
if instance.MgmtMAC == "" || instance.MgmtIP == "" {
return nil
@@ -1148,15 +1151,22 @@ func (m *Manager) appendSystemNetcfgFwCfg(instance *VM) error {
fmt.Fprintf(&b, "NIC%d_MAC=%s\nNIC%d_DHCP=1\nNIC%d_DEFAULT=1\n", n, instance.ENIMac, n, n)
n++
}
// Additional VPC ENIs: an RDS DB VM's customer-VPC NIC is the case. OVN
// serves DHCP on each, which is also what supplies their subnet routes.
// None may take the default route — that stays with the primary ENI,
// which carries the IMDS path.
// Additional VPC ENIs: an RDS DB VM's customer-VPC NIC is the case. Static
// when the launcher resolved a prefix and gateway, so it never depends on
// the OVN lease being renewed. Falls back to DHCP for an extra ENI that
// predates this field or whose subnet could not be resolved. None may take
// the default route — that stays with the primary ENI, which carries the
// IMDS path.
for _, extra := range instance.ExtraENIs {
if extra.ENIMac == "" {
continue
}
fmt.Fprintf(&b, "NIC%d_MAC=%s\nNIC%d_DHCP=1\nNIC%d_DEFAULT=0\n", n, extra.ENIMac, n, n)
if extra.ENICIDRPrefix > 0 && extra.Gateway != "" {
fmt.Fprintf(&b, "NIC%d_MAC=%s\nNIC%d_CIDR=%s/%d\nNIC%d_GW=%s\nNIC%d_DEFAULT=0\n",
n, extra.ENIMac, n, extra.ENIIP, extra.ENICIDRPrefix, n, extra.Gateway, n)
} else {
fmt.Fprintf(&b, "NIC%d_MAC=%s\nNIC%d_DHCP=1\nNIC%d_DEFAULT=0\n", n, extra.ENIMac, n, n)
}
n++
}
// Management NIC: static, off br-mgmt, never the default route.
+40 -2
View File
@@ -33,14 +33,52 @@ func TestAppendSystemNetcfgFwCfg(t *testing.T) {
// The guest configures by MAC, so a NIC the blob omits is left down and
// address-less. An RDS DB VM spans three: the system-VPC ENI, the customer
// ENI it is actually reached on, and mgmt0.
t.Run("names every NIC the VM is given, extras DHCP but never the default route", func(t *testing.T) {
// ENI it is actually reached on, and mgmt0. The customer ENI is static
// (NIC1_CIDR + NIC1_GW), not DHCP: OVN's lease is never renewed by the
// guest's one-shot udhcpc, so a DHCP customer ENI goes address-less an
// hour after boot.
t.Run("names every NIC the VM is given, customer ENI static and never the default route", func(t *testing.T) {
t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
inst := &VM{
ID: "i-rds01",
ENIMac: "02:3c:8f:54:bd:c9",
MgmtMAC: "02:aa:bb:cc:dd:ee",
MgmtIP: "10.20.0.5",
ExtraENIs: []ExtraENI{
{
ENIID: "eni-customer",
ENIMac: "02:31:ed:a2:ed:27",
ENIIP: "172.31.0.4",
ENICIDRPrefix: 20,
Gateway: "172.31.0.1",
},
},
}
require.NoError(t, m.appendSystemNetcfgFwCfg(inst))
require.Len(t, inst.Config.FwCfg, 1)
data, err := os.ReadFile(inst.Config.FwCfg[0].File)
require.NoError(t, err)
blob := string(data)
require.Equal(t,
"NIC0_MAC=02:3c:8f:54:bd:c9\nNIC0_DHCP=1\nNIC0_DEFAULT=1\n"+
"NIC1_MAC=02:31:ed:a2:ed:27\nNIC1_CIDR=172.31.0.4/20\nNIC1_GW=172.31.0.1\nNIC1_DEFAULT=0\n"+
"NIC2_MAC=02:aa:bb:cc:dd:ee\nNIC2_CIDR=10.20.0.5/24\nNIC2_DEFAULT=0\n",
blob)
require.NotContains(t, blob, "NIC1_DHCP", "the customer ENI must not depend on the OVN DHCP lease being renewed")
})
// An extra ENI without a resolved prefix/gateway (e.g. persisted state from
// before this field existed) falls back to DHCP rather than emitting a
// bogus static config.
t.Run("extra ENI without a resolved gateway falls back to DHCP", func(t *testing.T) {
t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
inst := &VM{
ID: "i-rds02",
ENIMac: "02:3c:8f:54:bd:c9",
MgmtMAC: "02:aa:bb:cc:dd:ee",
MgmtIP: "10.20.0.5",
ExtraENIs: []ExtraENI{
{ENIID: "eni-customer", ENIMac: "02:31:ed:a2:ed:27"},
},
+3 -2
View File
@@ -430,8 +430,9 @@ func (m *Manager) terminateCleanup(instance *VM) {
}
}
// ENI KV delete is sync; vpc.delete-port (OVN LSP) is fire-and-forget, so
// the OVN port removal is recorded pending (reconcile LSP prune reaps it).
// ENI KV delete is sync; vpc.delete-port (OVN LSP) is request-reply but
// non-fatal on failure, so the OVN port removal is recorded pending
// (reconcile LSP prune reaps anything the request-reply missed).
eniErr := m.deps.InstanceCleaner.DetachAndDeleteENI(instance)
if instance.ENIId != "" {
m.markTeardownResult(instance, TeardownENI, eniErr)
+8 -1
View File
@@ -33,12 +33,19 @@ type InstanceHealthState struct {
}
// ExtraENI describes an additional VPC network interface attached to a VM
// beyond the primary ENI. Only system VMs (ALBs) use multiple ENIs today.
// beyond the primary ENI. Only system VMs (ALBs, RDS DB VMs) use multiple
// ENIs today.
type ExtraENI struct {
ENIID string `json:"eni_id"`
ENIMac string `json:"eni_mac"`
ENIIP string `json:"eni_ip"`
SubnetID string `json:"subnet_id,omitempty"`
// ENICIDRPrefix and Gateway let this ENI be configured statically instead
// of via DHCP, so it never depends on the OVN lease being renewed (an RDS
// DB VM's customer ENI is the case). Zero/empty falls back to DHCP.
ENICIDRPrefix int `json:"eni_cidr_prefix,omitempty"`
Gateway string `json:"gateway,omitempty"`
}
type VM struct {