diff --git a/spinifex/handlers/ec2/vpc/eni.go b/spinifex/handlers/ec2/vpc/eni.go index a33ca7f99..f433a9f9e 100644 --- a/spinifex/handlers/ec2/vpc/eni.go +++ b/spinifex/handlers/ec2/vpc/eni.go @@ -14,6 +14,7 @@ import ( "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/filterutil" "github.com/mulgadc/spinifex/spinifex/network/topology" + "github.com/mulgadc/spinifex/spinifex/tags" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go/jetstream" ) @@ -83,6 +84,11 @@ type ENIRecord struct { // interface and the value DescribeNetworkInterfaces already advertised // before this field backed it. DeleteOnTermination *bool `json:"delete_on_termination,omitempty"` + // SuppressDHCP marks a statically-addressed ENI whose port must never get + // an OVN DHCP lease, so the guest's dhcpcd never flushes its static + // address at lease expiry. Set from the internal dhcp-disabled tag at + // create; threaded straight through to the port layer. + SuppressDHCP bool `json:"suppress_dhcp,omitempty"` } // eniIsLiveAttachment reports whether the ENI record is a live attachment to @@ -155,6 +161,13 @@ func (s *VPCServiceImpl) CreateNetworkInterface(ctx context.Context, input *ec2. description = *input.Description } + // The dhcp-disabled tag is an internal signal from the launch path (a + // statically-addressed customer ENI), not a customer-visible tag: strip it + // from the persisted tag set once consumed. + eniTags := utils.ExtractTags(input.TagSpecifications, "network-interface") + suppressDHCP := eniTags[tags.DHCPDisabledKey] == tags.DHCPDisabledValue + delete(eniTags, tags.DHCPDisabledKey) + record := ENIRecord{ NetworkInterfaceId: eniId, SubnetId: subnetId, @@ -165,8 +178,9 @@ func (s *VPCServiceImpl) CreateNetworkInterface(ctx context.Context, input *ec2. Description: description, Status: "available", SecurityGroupIds: sgIdsIn, - Tags: utils.ExtractTags(input.TagSpecifications, "network-interface"), + Tags: eniTags, CreatedAt: time.Now(), + SuppressDHCP: suppressDHCP, } data, err := json.Marshal(record) @@ -183,7 +197,7 @@ func (s *VPCServiceImpl) CreateNetworkInterface(ctx context.Context, input *ec2. // caller. Fire-and-forget would let CreateNetworkInterface return success // while the LSP joins zero port groups (NATS hiccup or vpcd OVSDB error), // leaving the port unrestricted until the 30s reconciler heals it. - if err := s.requestPortEvent("vpc.create-port", eniId, subnetId, subnet.VpcId, privateIP, macAddr, sgIdsIn); err != nil { + if err := s.requestPortEvent("vpc.create-port", eniId, subnetId, subnet.VpcId, privateIP, macAddr, sgIdsIn, record.SuppressDHCP); err != nil { slog.ErrorContext(ctx, "CreateNetworkInterface: vpcd create-port failed", "eniId", eniId, "err", err) return nil, err } @@ -255,7 +269,7 @@ func (s *VPCServiceImpl) deleteNetworkInterface(ctx context.Context, eniId, acco // Publish vpc.delete-port event for vpcd topology cleanup. SG IDs are // included for consistency with create-port; vpcd's delete handler reads // current memberships from the libovsdb cache rather than the event. - s.publishPortEvent("vpc.delete-port", eniId, record.SubnetId, record.VpcId, record.PrivateIpAddress, record.MacAddress, record.SecurityGroupIds) + s.publishPortEvent("vpc.delete-port", eniId, record.SubnetId, record.VpcId, record.PrivateIpAddress, record.MacAddress, record.SecurityGroupIds, false) return &ec2.DeleteNetworkInterfaceOutput{}, nil } @@ -364,7 +378,7 @@ func (s *VPCServiceImpl) DetachAndDeleteENI(ctx context.Context, accountID, eniI s.releaseENISideEffects(ctx, eniID, accountID, &record) slog.InfoContext(ctx, "DetachAndDeleteENI completed", "eniId", eniID, "accountID", accountID, "force", force) - s.publishPortEvent("vpc.delete-port", eniID, record.SubnetId, record.VpcId, record.PrivateIpAddress, record.MacAddress, record.SecurityGroupIds) + s.publishPortEvent("vpc.delete-port", eniID, record.SubnetId, record.VpcId, record.PrivateIpAddress, record.MacAddress, record.SecurityGroupIds, false) return true, nil } @@ -966,13 +980,16 @@ type portEventPayload struct { PrivateIpAddress string `json:"private_ip_address"` MacAddress string `json:"mac_address"` SecurityGroupIds []string `json:"security_group_ids,omitempty"` + // SuppressDHCP, set on create, skips attaching the subnet's dhcpv4_options + // to this port's LSP. Irrelevant on delete. + SuppressDHCP bool `json:"suppress_dhcp,omitempty"` } // 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) { - if err := s.requestPortEvent(topic, eniId, subnetId, vpcId, privateIP, macAddr, sgIds); err != nil { +func (s *VPCServiceImpl) publishPortEvent(topic, eniId, subnetId, vpcId, privateIP, macAddr string, sgIds []string, suppressDHCP bool) { + if err := s.requestPortEvent(topic, eniId, subnetId, vpcId, privateIP, macAddr, sgIds, suppressDHCP); err != nil { slog.Warn("vpc: delete-port request failed, relying on reconciler orphan prune", "topic", topic, "eniId", eniId, "err", err) } @@ -980,7 +997,7 @@ func (s *VPCServiceImpl) publishPortEvent(topic, eniId, subnetId, vpcId, private // requestPortEvent sends a port lifecycle event via request-reply so vpcd // OVSDB failures surface to the caller rather than being swallowed. -func (s *VPCServiceImpl) requestPortEvent(topic, eniId, subnetId, vpcId, privateIP, macAddr string, sgIds []string) error { +func (s *VPCServiceImpl) requestPortEvent(topic, eniId, subnetId, vpcId, privateIP, macAddr string, sgIds []string, suppressDHCP bool) error { return utils.RequestEvent(s.natsConn, topic, portEventPayload{ NetworkInterfaceId: eniId, SubnetId: subnetId, @@ -988,6 +1005,7 @@ func (s *VPCServiceImpl) requestPortEvent(topic, eniId, subnetId, vpcId, private PrivateIpAddress: privateIP, MacAddress: macAddr, SecurityGroupIds: sgIds, + SuppressDHCP: suppressDHCP, }, vpcdSGEventTimeout) } diff --git a/spinifex/handlers/ec2/vpc/eni_test.go b/spinifex/handlers/ec2/vpc/eni_test.go index 39452efa5..a2057ad50 100644 --- a/spinifex/handlers/ec2/vpc/eni_test.go +++ b/spinifex/handlers/ec2/vpc/eni_test.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/mulgadc/spinifex/spinifex/tags" "github.com/mulgadc/spinifex/spinifex/testutil" "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" @@ -103,6 +104,86 @@ func TestCreateNetworkInterface_WithTags(t *testing.T) { assert.Equal(t, "my-eni", *out.NetworkInterface.TagSet[0].Value) } +// TestCreateNetworkInterface_DHCPDisabledTag_SuppressesDHCP proves the internal +// dhcp-disabled tag (set by the RDS launch path on the customer endpoint ENI) +// marks the ENIRecord SuppressDHCP, threads it into the vpc.create-port event +// for vpcd, and never persists as a customer-visible tag. +func TestCreateNetworkInterface_DHCPDisabledTag_SuppressesDHCP(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") + + eventCh := make(chan *nats.Msg, 1) + sub, err := nc.Subscribe("vpc.create-port", func(msg *nats.Msg) { + eventCh <- msg + }) + require.NoError(t, err) + defer func() { _ = sub.Unsubscribe() }() + + out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{ + SubnetId: aws.String(subnetId), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("network-interface"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("rds-endpoint")}, + {Key: aws.String(tags.DHCPDisabledKey), Value: aws.String(tags.DHCPDisabledValue)}, + }, + }}, + }, testAccountID) + require.NoError(t, err) + eniId := *out.NetworkInterface.NetworkInterfaceId + + // The internal tag never reaches the customer-visible tag set. + require.Len(t, out.NetworkInterface.TagSet, 1) + assert.Equal(t, "Name", *out.NetworkInterface.TagSet[0].Key) + + rec, err := svc.GetENIRecord(testAccountID, eniId) + require.NoError(t, err) + assert.True(t, rec.SuppressDHCP, "ENIRecord.SuppressDHCP must be set from the internal tag") + _, hasInternalTag := rec.Tags[tags.DHCPDisabledKey] + assert.False(t, hasInternalTag, "internal dhcp-disabled tag must not be persisted") + + select { + case msg := <-eventCh: + assert.Contains(t, string(msg.Data), `"suppress_dhcp":true`) + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for vpc.create-port event") + } +} + +// TestCreateNetworkInterface_NoDHCPDisabledTag_LeavesDHCPEnabled proves an +// ordinary ENI (no internal tag) is unaffected: SuppressDHCP stays false and +// the wire event carries no suppress_dhcp marker. +func TestCreateNetworkInterface_NoDHCPDisabledTag_LeavesDHCPEnabled(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") + + eventCh := make(chan *nats.Msg, 1) + sub, err := nc.Subscribe("vpc.create-port", func(msg *nats.Msg) { + eventCh <- msg + }) + require.NoError(t, err) + defer func() { _ = sub.Unsubscribe() }() + + out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{ + SubnetId: aws.String(subnetId), + }, testAccountID) + require.NoError(t, err) + eniId := *out.NetworkInterface.NetworkInterfaceId + + rec, err := svc.GetENIRecord(testAccountID, eniId) + require.NoError(t, err) + assert.False(t, rec.SuppressDHCP) + + select { + case msg := <-eventCh: + assert.NotContains(t, string(msg.Data), `"suppress_dhcp":true`) + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for vpc.create-port event") + } +} + func TestDeleteNetworkInterface(t *testing.T) { svc := setupTestVPCService(t) vpcId := createTestVPC(t, svc, "10.0.0.0/16") diff --git a/spinifex/handlers/rds/launch.go b/spinifex/handlers/rds/launch.go index 6daaf4f20..724496de2 100644 --- a/spinifex/handlers/rds/launch.go +++ b/spinifex/handlers/rds/launch.go @@ -223,7 +223,7 @@ func LaunchDBInstanceVM(ctx context.Context, deps LaunchDeps, in LaunchInput) (o }() systemENI, err := createLaunchENI(ctx, deps.VPC, utils.GlobalAccountID, systemSubnetID, []string{systemSGID}, - "RDS management NIC for "+in.DBInstanceIdentifier, in.DBInstanceIdentifier) + "RDS management NIC for "+in.DBInstanceIdentifier, in.DBInstanceIdentifier, false) if err != nil { return nil, err } @@ -387,21 +387,27 @@ type launchENI struct { // groups must be non-empty for either NIC: an ENI created with none is placed in // its VPC's default group, whose sole ingress rule admits every other member of // itself — which for the shared system VPC is every DB VM in the deployment. -func createLaunchENI(ctx context.Context, vpcSvc launchVPCProvisioner, accountID, subnetID string, groups []string, description, dbInstanceID string) (*launchENI, error) { +// suppressDHCP marks the customer endpoint ENI, which is always statically +// addressed; the system/primary NIC keeps DHCP for IMDS bootstrap. +func createLaunchENI(ctx context.Context, vpcSvc launchVPCProvisioner, accountID, subnetID string, groups []string, description, dbInstanceID string, suppressDHCP bool) (*launchENI, error) { var groupIDs []*string if len(groups) > 0 { groupIDs = aws.StringSlice(groups) } + eniTags := []*ec2.Tag{ + {Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByRDS)}, + {Key: aws.String(rdsInstanceTagKey), Value: aws.String(dbInstanceID)}, + } + if suppressDHCP { + eniTags = append(eniTags, &ec2.Tag{Key: aws.String(tags.DHCPDisabledKey), Value: aws.String(tags.DHCPDisabledValue)}) + } out, err := vpcSvc.CreateNetworkInterface(ctx, &ec2.CreateNetworkInterfaceInput{ SubnetId: aws.String(subnetID), Description: aws.String(description), Groups: groupIDs, TagSpecifications: []*ec2.TagSpecification{{ ResourceType: aws.String("network-interface"), - Tags: []*ec2.Tag{ - {Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByRDS)}, - {Key: aws.String(rdsInstanceTagKey), Value: aws.String(dbInstanceID)}, - }, + Tags: eniTags, }}, }, accountID) if err != nil { @@ -427,7 +433,7 @@ func createLaunchENI(ctx context.Context, vpcSvc launchVPCProvisioner, accountID func resolveCustomerENI(ctx context.Context, vpcSvc launchVPCProvisioner, in LaunchInput) (*launchENI, error) { if in.ExistingCustomerENI == "" { return createLaunchENI(ctx, vpcSvc, in.AccountID, in.SubnetID, in.SecurityGroupIDs, - "RDS endpoint ENI for "+in.DBInstanceIdentifier, in.DBInstanceIdentifier) + "RDS endpoint ENI for "+in.DBInstanceIdentifier, in.DBInstanceIdentifier, true) } // The old VM is gone by now but its attachment record can outlive it, and a diff --git a/spinifex/handlers/rds/launch_test.go b/spinifex/handlers/rds/launch_test.go index f86bd96a8..40515a7bc 100644 --- a/spinifex/handlers/rds/launch_test.go +++ b/spinifex/handlers/rds/launch_test.go @@ -594,6 +594,13 @@ func TestLaunchDBInstanceVMWiresBothNICs(t *testing.T) { assert.Equal(t, "mydb", tagOf(eni.TagSpecifications, rdsInstanceTagKey)) } + // Only the customer endpoint ENI is statically addressed; the system NIC + // still needs its OVN DHCP lease for IMDS bootstrap. + assert.Empty(t, tagOf(sysENI.TagSpecifications, tags.DHCPDisabledKey), + "the system NIC must keep DHCP") + assert.Equal(t, tags.DHCPDisabledValue, tagOf(custENI.TagSpecifications, tags.DHCPDisabledKey), + "the customer endpoint ENI must be marked to suppress DHCP") + // The VM itself: a system-account instance off the engine AMI, carrying the // managed-by tag that hides it from the customer's EC2 API, with the // customer ENI injected cross-account as an extra NIC. diff --git a/spinifex/network/reconcile/apply.go b/spinifex/network/reconcile/apply.go index 76aa9cd2f..d72097ce9 100644 --- a/spinifex/network/reconcile/apply.go +++ b/spinifex/network/reconcile/apply.go @@ -302,8 +302,10 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual "spinifex:vpc_id": spec.VPCID, }, } - if dhcpOpts, derr := r.ovn.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", spec.SubnetID); derr == nil && dhcpOpts != nil { - lsp.DHCPv4Options = &dhcpOpts.UUID + if !spec.SuppressDHCP { + if dhcpOpts, derr := r.ovn.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", spec.SubnetID); derr == nil && dhcpOpts != nil { + lsp.DHCPv4Options = &dhcpOpts.UUID + } } if err := r.ovn.CreateLogicalSwitchPortInGroups(ctx, switchName, lsp, desiredPGs); err != nil { slog.Error("reconcile/apply: create ENI port failed", "port", portName, "err", err) diff --git a/spinifex/network/reconcile/apply_test.go b/spinifex/network/reconcile/apply_test.go index 9536b8d3f..ff18a0767 100644 --- a/spinifex/network/reconcile/apply_test.go +++ b/spinifex/network/reconcile/apply_test.go @@ -76,6 +76,44 @@ func TestReconcile_TopoOrder_VPCThenSubnetThenPort(t *testing.T) { } } +// TestReconcile_PortSuppressDHCP proves applyPorts' create branch never sets +// DHCPv4Options for a SuppressDHCP port (a statically-addressed customer +// ENI), while an ordinary port on the same subnet still gets one — this is +// the drift/recreate path, distinct from EnsurePort's, that must not re-add +// a lease the guest's dhcpcd would flush the static address on. +func TestReconcile_PortSuppressDHCP(t *testing.T) { + rec, m := newTestReconciler(t) + ctx := context.Background() + + intent := freshIntent(t) + mac, _ := net.ParseMAC("02:00:00:00:00:09") + intent.Ports["eni-static"] = topology.PortSpec{ + PortID: "eni-static", SubnetID: "subnet-a", VPCID: "vpc-a", + PrivateIP: netip.MustParseAddr("10.0.1.11"), MAC: mac, + SuppressDHCP: true, + } + + if err := rec.Reconcile(ctx, intent); err != nil { + t.Fatalf("Reconcile: %v", err) + } + + dhcpLSP := m.Ports["port-"+intent.Ports["eni-a"].PortID] + if dhcpLSP == nil { + t.Fatal("ordinary ENI port not created") + } + if dhcpLSP.DHCPv4Options == nil { + t.Errorf("ordinary ENI's LSP must carry dhcpv4_options") + } + + staticLSP := m.Ports["port-eni-static"] + if staticLSP == nil { + t.Fatal("static ENI port not created") + } + if staticLSP.DHCPv4Options != nil { + t.Errorf("SuppressDHCP ENI's LSP must not carry dhcpv4_options, got %q", *staticLSP.DHCPv4Options) + } +} + func TestReconcile_PortJoinsPortGroupAtomically(t *testing.T) { rec, m := newTestReconciler(t) ctx := context.Background() diff --git a/spinifex/network/reconcile/intent.go b/spinifex/network/reconcile/intent.go index f4cbb5c23..8c0ca9112 100644 --- a/spinifex/network/reconcile/intent.go +++ b/spinifex/network/reconcile/intent.go @@ -392,13 +392,14 @@ func loadPorts(ctx context.Context, js jetstream.JetStream, localVPCs map[string // zero/invalid when absent. User EIPs come from the EIP bucket instead. publicIP, _ := netip.ParseAddr(rec.PublicIpAddress) out[rec.NetworkInterfaceId] = topology.PortSpec{ - PortID: rec.NetworkInterfaceId, - SubnetID: rec.SubnetId, - VPCID: rec.VpcId, - PrivateIP: addr, - MAC: mac, - SGIDs: append([]string(nil), rec.SecurityGroupIds...), - PublicIP: publicIP, + PortID: rec.NetworkInterfaceId, + SubnetID: rec.SubnetId, + VPCID: rec.VpcId, + PrivateIP: addr, + MAC: mac, + SGIDs: append([]string(nil), rec.SecurityGroupIds...), + PublicIP: publicIP, + SuppressDHCP: rec.SuppressDHCP, } } return nil diff --git a/spinifex/network/reconcile/intent_test.go b/spinifex/network/reconcile/intent_test.go index 1f84757a0..cd5bf895c 100644 --- a/spinifex/network/reconcile/intent_test.go +++ b/spinifex/network/reconcile/intent_test.go @@ -109,6 +109,53 @@ func TestLoadIntentFromKV_TransitiveSubnetFilter(t *testing.T) { } } +// TestLoadIntentFromKV_PortSuppressDHCP proves loadPorts carries the ENI +// record's SuppressDHCP flag into the intent PortSpec, so a delete+recreate +// (drift) of a statically-addressed customer ENI's LSP never re-adds DHCP. +func TestLoadIntentFromKV_PortSuppressDHCP(t *testing.T) { + js := startKV(t) + + testutil.SeedKV(t, js, handlers_ec2_vpc.KVBucketVPCs, map[string][]byte{ + "acct/vpc-a": mustJSON(t, handlers_ec2_vpc.VPCRecord{ + VpcId: "vpc-a", CidrBlock: "10.0.0.0/16", AZ: "us-east-1a", CreatedAt: time.Now(), + }), + }) + + testutil.SeedKV(t, js, handlers_ec2_vpc.KVBucketENIs, map[string][]byte{ + "acct/eni-static": mustJSON(t, handlers_ec2_vpc.ENIRecord{ + NetworkInterfaceId: "eni-static", SubnetId: "subnet-a", VpcId: "vpc-a", + PrivateIpAddress: "10.0.1.10", MacAddress: "02:00:00:00:00:01", + SuppressDHCP: true, CreatedAt: time.Now(), + }), + "acct/eni-dhcp": mustJSON(t, handlers_ec2_vpc.ENIRecord{ + NetworkInterfaceId: "eni-dhcp", SubnetId: "subnet-a", VpcId: "vpc-a", + PrivateIpAddress: "10.0.1.11", MacAddress: "02:00:00:00:00:02", + CreatedAt: time.Now(), + }), + }) + + intent, err := LoadIntentFromKV(context.Background(), js, "us-east-1a") + if err != nil { + t.Fatalf("LoadIntentFromKV: %v", err) + } + + staticPort, ok := intent.Ports["eni-static"] + if !ok { + t.Fatalf("static ENI missing from intent") + } + if !staticPort.SuppressDHCP { + t.Errorf("static ENI's PortSpec.SuppressDHCP = false, want true") + } + + dhcpPort, ok := intent.Ports["eni-dhcp"] + if !ok { + t.Fatalf("dhcp ENI missing from intent") + } + if dhcpPort.SuppressDHCP { + t.Errorf("ordinary ENI's PortSpec.SuppressDHCP = true, want false") + } +} + func TestLoadIntentFromKV_EIPStateFilter(t *testing.T) { js := startKV(t) diff --git a/spinifex/network/subscribers/events.go b/spinifex/network/subscribers/events.go index 993e0ce18..b11b3d704 100644 --- a/spinifex/network/subscribers/events.go +++ b/spinifex/network/subscribers/events.go @@ -49,6 +49,9 @@ type PortEvent struct { PrivateIpAddress string `json:"private_ip_address"` MacAddress string `json:"mac_address"` SecurityGroupIds []string `json:"security_group_ids,omitempty"` + // SuppressDHCP, set on create, skips attaching the subnet's dhcpv4_options + // to this port's LSP. Irrelevant on delete. + SuppressDHCP bool `json:"suppress_dhcp,omitempty"` } // UpdatePortSGsEvent: vpc.update-port-sgs. Declarative — vpcd diffs diff --git a/spinifex/network/subscribers/topology.go b/spinifex/network/subscribers/topology.go index 6899c971d..3d7f9c656 100644 --- a/spinifex/network/subscribers/topology.go +++ b/spinifex/network/subscribers/topology.go @@ -131,12 +131,13 @@ func (s *Subscriber) handleCreatePort(msg *nats.Msg) { return } if err := s.topology.EnsurePort(context.Background(), topology.PortSpec{ - PortID: evt.NetworkInterfaceId, - SubnetID: evt.SubnetId, - VPCID: evt.VpcId, - PrivateIP: ip, - MAC: mac, - SGIDs: evt.SecurityGroupIds, + PortID: evt.NetworkInterfaceId, + SubnetID: evt.SubnetId, + VPCID: evt.VpcId, + PrivateIP: ip, + MAC: mac, + SGIDs: evt.SecurityGroupIds, + SuppressDHCP: evt.SuppressDHCP, }); err != nil { slog.Error("subscribers: EnsurePort failed", "eni_id", evt.NetworkInterfaceId, "err", err) respond(msg, err) diff --git a/spinifex/network/topology/manager.go b/spinifex/network/topology/manager.go index d2216ef19..5bba9b37a 100644 --- a/spinifex/network/topology/manager.go +++ b/spinifex/network/topology/manager.go @@ -62,4 +62,8 @@ type PortSpec struct { // It does not affect the L2 LSP; the reconciler reads it to exempt the // instance from its subnet egress drop gate (the dnat_and_snat datapath). PublicIP netip.Addr + // SuppressDHCP, when true, skips attaching the subnet's dhcpv4_options to + // this port's LSP. Set for statically-addressed ENIs whose guest must + // never take an OVN DHCP lease. + SuppressDHCP bool } diff --git a/spinifex/network/topology/manager_live.go b/spinifex/network/topology/manager_live.go index 4e92fbd0d..4ed1de147 100644 --- a/spinifex/network/topology/manager_live.go +++ b/spinifex/network/topology/manager_live.go @@ -322,8 +322,10 @@ func (m *liveManager) EnsurePort(ctx context.Context, spec PortSpec) error { "spinifex:vpc_id": spec.VPCID, }, } - if dhcpOpts, err := m.ovn.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", spec.SubnetID); err == nil { - lsp.DHCPv4Options = &dhcpOpts.UUID + if !spec.SuppressDHCP { + if dhcpOpts, err := m.ovn.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", spec.SubnetID); err == nil { + lsp.DHCPv4Options = &dhcpOpts.UUID + } } pgNames := make([]string, 0, len(spec.SGIDs)) diff --git a/spinifex/network/topology/manager_live_test.go b/spinifex/network/topology/manager_live_test.go index 10e0f4210..3d029bb96 100644 --- a/spinifex/network/topology/manager_live_test.go +++ b/spinifex/network/topology/manager_live_test.go @@ -114,6 +114,56 @@ func TestLiveManager_EnsurePort(t *testing.T) { } } +// TestLiveManager_EnsurePort_SuppressDHCP proves a statically-addressed ENI's +// LSP never gets dhcpv4_options, while an ordinary ENI on the same subnet +// still does — the guest's dhcpcd must never take a lease on the static NIC. +func TestLiveManager_EnsurePort_SuppressDHCP(t *testing.T) { + mgr, mockClient := newLiveManagerForTest(t) + ctx := context.Background() + + vpc := VPCSpec{VPCID: "vpc-nodhcp", CIDR: netip.MustParsePrefix("10.7.0.0/16")} + sub := SubnetSpec{SubnetID: "subnet-nodhcp", VPCID: vpc.VPCID, CIDR: netip.MustParsePrefix("10.7.1.0/24")} + if err := mgr.EnsureVPC(ctx, vpc); err != nil { + t.Fatalf("EnsureVPC: %v", err) + } + if err := mgr.EnsureSubnet(ctx, sub); err != nil { + t.Fatalf("EnsureSubnet: %v", err) + } + + dhcpMAC, _ := net.ParseMAC("02:00:00:00:02:01") + dhcpPort := PortSpec{ + PortID: "eni-dhcp", SubnetID: sub.SubnetID, VPCID: vpc.VPCID, + PrivateIP: netip.MustParseAddr("10.7.1.10"), MAC: dhcpMAC, + } + if err := mgr.EnsurePort(ctx, dhcpPort); err != nil { + t.Fatalf("EnsurePort(dhcp): %v", err) + } + dhcpLSP, err := mockClient.GetLogicalSwitchPort(ctx, Port(dhcpPort.PortID)) + if err != nil { + t.Fatalf("dhcp port LSP missing: %v", err) + } + if dhcpLSP.DHCPv4Options == nil { + t.Errorf("ordinary ENI's LSP must carry dhcpv4_options") + } + + staticMAC, _ := net.ParseMAC("02:00:00:00:02:02") + staticPort := PortSpec{ + PortID: "eni-static", SubnetID: sub.SubnetID, VPCID: vpc.VPCID, + PrivateIP: netip.MustParseAddr("10.7.1.11"), MAC: staticMAC, + SuppressDHCP: true, + } + if err := mgr.EnsurePort(ctx, staticPort); err != nil { + t.Fatalf("EnsurePort(static): %v", err) + } + staticLSP, err := mockClient.GetLogicalSwitchPort(ctx, Port(staticPort.PortID)) + if err != nil { + t.Fatalf("static port LSP missing: %v", err) + } + if staticLSP.DHCPv4Options != nil { + t.Errorf("SuppressDHCP ENI's LSP must not carry dhcpv4_options, got %q", *staticLSP.DHCPv4Options) + } +} + func TestLiveManager_DeletePort(t *testing.T) { mgr, mockClient := newLiveManagerForTest(t) ctx := context.Background() diff --git a/spinifex/tags/tags.go b/spinifex/tags/tags.go index fb75f630a..b6748aff9 100644 --- a/spinifex/tags/tags.go +++ b/spinifex/tags/tags.go @@ -42,6 +42,15 @@ const ( // GPUVendorAMD identifies an AMD-driver GPU node AMI. GPUVendorAMD = "amd" + + // DHCPDisabledKey marks an ENI whose logical switch port must not get OVN + // DHCP options — used for statically-addressed customer ENIs whose lease + // would otherwise be flushed by the guest's dhcpcd at expiry. Internal: + // stripped from the persisted user-visible tag set at CreateNetworkInterface. + DHCPDisabledKey = "spinifex:dhcp" + + // DHCPDisabledValue is the DHCPDisabledKey value that suppresses DHCP. + DHCPDisabledValue = "disabled" ) // IsSystemManaged reports whether a ManagedBy value denotes a Spinifex