From f34452883ce7f6f8a7aae5081320fffcbba87c0d Mon Sep 17 00:00:00 2001 From: Ben Duncan <5178243+benduncan@users.noreply.github.com> Date: Mon, 23 Mar 2026 06:57:25 +0000 Subject: [PATCH] refactor: move bootstrap config to admin init, add IGW ID, eliminate race - admin init now pre-generates VPC/subnet/IGW IDs and writes [bootstrap] to spinifex.toml before services ever start. No race condition. - EnsureDefaultVPC accepts optional BootstrapIDs to use pre-determined IDs instead of generating random ones. - CreateInternetGatewayWithID allows daemon to use the bootstrap IGW ID. - Removed writeBootstrapConfig from daemon (no longer needed). - Fixed bootstrap account_id: uses admin account (000000000001) not system account (000000000000). - reconcileIGW now sets spinifex:igw_id in OVN external IDs. --- cmd/spinifex/cmd/admin.go | 15 ++++++++ cmd/spinifex/cmd/service.go | 1 + cmd/spinifex/cmd/templates/spinifex.toml | 10 +++++ spinifex/admin/admin.go | 9 +++++ spinifex/config/config.go | 1 + spinifex/daemon/daemon.go | 16 +++++--- spinifex/daemon/daemon_handlers_vpc.go | 47 ++++------------------- spinifex/handlers/ec2/igw/service_impl.go | 10 +++++ spinifex/handlers/ec2/vpc/service_impl.go | 18 ++++++++- spinifex/services/vpcd/reconcile.go | 2 +- spinifex/services/vpcd/reconcile_test.go | 1 + spinifex/services/vpcd/topology.go | 34 +++++++++------- spinifex/services/vpcd/vpcd.go | 1 + 13 files changed, 104 insertions(+), 61 deletions(-) diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index db9b746c5..7071b3f29 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -796,6 +796,14 @@ func runAdminInit(cmd *cobra.Command, args []string) { fmt.Printf("✅ Created: multi-node predastore.toml (node ID: %d)\n", predastoreNodeID) } + // Pre-generate default VPC/subnet/IGW IDs for bootstrap config. + // These are written to [bootstrap] in spinifex.toml so vpcd can + // create OVN topology on first boot. The daemon uses the same IDs + // when it creates the records in NATS KV via EnsureDefaultVPC. + bootstrapVpcId := utils.GenerateResourceID("vpc") + bootstrapSubnetId := utils.GenerateResourceID("subnet") + bootstrapIgwId := utils.GenerateResourceID("igw") + configSettings := admin.ConfigSettings{ AccessKey: accessKey, SecretKey: secretKey, @@ -827,6 +835,13 @@ func runAdminInit(cmd *cobra.Command, args []string) { PoolEnd: poolEnd, PoolGateway: externalGateway, PoolPrefixLen: externalPrefixLen, + + BootstrapAccountId: admin.DefaultAccountID(), + BootstrapVpcId: bootstrapVpcId, + BootstrapSubnetId: bootstrapSubnetId, + BootstrapIgwId: bootstrapIgwId, + BootstrapCidr: handlers_ec2_vpc.DefaultVPCCidr, + BootstrapSubnetCidr: handlers_ec2_vpc.DefaultSubnetCidr, } // Print external networking summary diff --git a/cmd/spinifex/cmd/service.go b/cmd/spinifex/cmd/service.go index e44703832..8f96abe45 100644 --- a/cmd/spinifex/cmd/service.go +++ b/cmd/spinifex/cmd/service.go @@ -721,6 +721,7 @@ var vpcdStartCmd = &cobra.Command{ AccountID: clusterConfig.Bootstrap.AccountID, VpcId: clusterConfig.Bootstrap.VpcId, SubnetId: clusterConfig.Bootstrap.SubnetId, + IgwId: clusterConfig.Bootstrap.IgwId, Cidr: clusterConfig.Bootstrap.Cidr, SubnetCidr: clusterConfig.Bootstrap.SubnetCidr, } diff --git a/cmd/spinifex/cmd/templates/spinifex.toml b/cmd/spinifex/cmd/templates/spinifex.toml index 81e1f8061..c09187853 100644 --- a/cmd/spinifex/cmd/templates/spinifex.toml +++ b/cmd/spinifex/cmd/templates/spinifex.toml @@ -22,6 +22,16 @@ gateway_ip = "{{.PoolGatewayIP}}" {{- end}} prefix_len = {{if .PoolPrefixLen}}{{.PoolPrefixLen}}{{else}}24{{end}} {{- end}} +{{- if .BootstrapVpcId}} + +[bootstrap] +account_id = "{{.BootstrapAccountId}}" +vpc_id = "{{.BootstrapVpcId}}" +subnet_id = "{{.BootstrapSubnetId}}" +igw_id = "{{.BootstrapIgwId}}" +cidr = "{{.BootstrapCidr}}" +subnet_cidr = "{{.BootstrapSubnetCidr}}" +{{- end}} [nodes.{{.Node}}] node = "{{.Node}}" diff --git a/spinifex/admin/admin.go b/spinifex/admin/admin.go index 921ab0b14..684ef7545 100644 --- a/spinifex/admin/admin.go +++ b/spinifex/admin/admin.go @@ -77,6 +77,15 @@ type ConfigSettings struct { // Other nodes in the cluster (for config source of truth) RemoteNodes []RemoteNode + + // Bootstrap: pre-generated default VPC IDs for vpcd reconciliation. + // Written by admin init so [bootstrap] exists before services start. + BootstrapAccountId string + BootstrapVpcId string + BootstrapSubnetId string + BootstrapIgwId string + BootstrapCidr string + BootstrapSubnetCidr string } // PredastoreNodeConfig describes a single Predastore node for multi-node config generation. diff --git a/spinifex/config/config.go b/spinifex/config/config.go index 1b7f55ea7..1e498734c 100644 --- a/spinifex/config/config.go +++ b/spinifex/config/config.go @@ -44,6 +44,7 @@ type BootstrapConfig struct { AccountID string `mapstructure:"account_id"` VpcId string `mapstructure:"vpc_id"` SubnetId string `mapstructure:"subnet_id"` + IgwId string `mapstructure:"igw_id"` Cidr string `mapstructure:"cidr"` SubnetCidr string `mapstructure:"subnet_cidr"` } diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 4d8b314f3..23d695671 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -578,13 +578,17 @@ func (d *Daemon) Start() error { // (matches AWS: every account has a default VPC with IGW + default SG) if d.vpcService != nil { for _, accountID := range []string{utils.GlobalAccountID, admin.DefaultAccountID()} { - vpcInfo, err := d.vpcService.EnsureDefaultVPC(accountID) - if err != nil { - slog.Error("Failed to ensure default VPC", "accountID", accountID, "error", err) + // Pass bootstrap IDs for the admin account so EnsureDefaultVPC uses + // the same IDs that admin init wrote to [bootstrap] in spinifex.toml. + var opts []handlers_ec2_vpc.BootstrapIDs + if accountID == admin.DefaultAccountID() && d.clusterConfig != nil && d.clusterConfig.Bootstrap.VpcId != "" { + opts = append(opts, handlers_ec2_vpc.BootstrapIDs{ + VpcId: d.clusterConfig.Bootstrap.VpcId, + SubnetId: d.clusterConfig.Bootstrap.SubnetId, + }) } - // Write [bootstrap] to spinifex.toml for vpcd reconciliation (admin account only) - if vpcInfo != nil && accountID == admin.DefaultAccountID() && d.configPath != "" { - writeBootstrapConfig(d.configPath, accountID, vpcInfo) + if _, err := d.vpcService.EnsureDefaultVPC(accountID, opts...); err != nil { + slog.Error("Failed to ensure default VPC", "accountID", accountID, "error", err) } } // Ensure default VPC has an IGW and default security group diff --git a/spinifex/daemon/daemon_handlers_vpc.go b/spinifex/daemon/daemon_handlers_vpc.go index 2c928c4ba..fdf552984 100644 --- a/spinifex/daemon/daemon_handlers_vpc.go +++ b/spinifex/daemon/daemon_handlers_vpc.go @@ -2,13 +2,9 @@ package daemon import ( "encoding/json" - "fmt" "log/slog" - "os" - "strings" "github.com/aws/aws-sdk-go/service/ec2" - handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" "github.com/mulgadc/spinifex/spinifex/admin" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" @@ -114,8 +110,14 @@ func (d *Daemon) ensureDefaultVPCInfrastructure() { } } if !hasIGW { - // Create and attach an IGW - createOut, err := d.igwService.CreateInternetGateway(&ec2.CreateInternetGatewayInput{}, accountID) + // Create and attach an IGW — use bootstrap ID if available + var createOut *ec2.CreateInternetGatewayOutput + var err error + if accountID == admin.DefaultAccountID() && d.clusterConfig != nil && d.clusterConfig.Bootstrap.IgwId != "" { + createOut, err = d.igwService.CreateInternetGatewayWithID(&ec2.CreateInternetGatewayInput{}, accountID, d.clusterConfig.Bootstrap.IgwId) + } else { + createOut, err = d.igwService.CreateInternetGateway(&ec2.CreateInternetGatewayInput{}, accountID) + } if err != nil { slog.Error("Failed to create default IGW", "accountID", accountID, "err", err) continue @@ -187,36 +189,3 @@ func (d *Daemon) ensureDefaultVPCInfrastructure() { } } -// writeBootstrapConfig appends the [bootstrap] section to spinifex.toml if not already present. -// This enables vpcd to reconcile OVN topology on startup for the default VPC. -// configPath is the path to spinifex.toml (e.g., /home/user/spinifex/config/spinifex.toml). -func writeBootstrapConfig(configPath, accountID string, info *handlers_ec2_vpc.DefaultVPCInfo) { - tomlPath := configPath - - // Check if [bootstrap] already exists - data, err := os.ReadFile(tomlPath) - if err != nil { - slog.Debug("writeBootstrapConfig: could not read spinifex.toml", "err", err) - return - } - if strings.Contains(string(data), "[bootstrap]") { - return // already present - } - - section := fmt.Sprintf("\n[bootstrap]\naccount_id = %q\nvpc_id = %q\nsubnet_id = %q\ncidr = %q\nsubnet_cidr = %q\n", - accountID, info.VpcId, info.SubnetId, info.Cidr, info.SubnetCidr) - - f, err := os.OpenFile(tomlPath, os.O_APPEND|os.O_WRONLY, 0644) - if err != nil { - slog.Warn("writeBootstrapConfig: could not open spinifex.toml for append", "err", err) - return - } - defer f.Close() - - if _, err := f.WriteString(section); err != nil { - slog.Warn("writeBootstrapConfig: failed to write [bootstrap] section", "err", err) - } else { - slog.Info("Wrote [bootstrap] section to spinifex.toml", - "vpcId", info.VpcId, "subnetId", info.SubnetId) - } -} diff --git a/spinifex/handlers/ec2/igw/service_impl.go b/spinifex/handlers/ec2/igw/service_impl.go index 7e58534f5..29d6e1417 100644 --- a/spinifex/handlers/ec2/igw/service_impl.go +++ b/spinifex/handlers/ec2/igw/service_impl.go @@ -75,8 +75,18 @@ func NewIGWServiceImplWithNATS(cfg *config.Config, natsConn *nats.Conn) (*IGWSer } // CreateInternetGateway creates a new Internet Gateway (initially detached) +// CreateInternetGatewayWithID creates an IGW with a pre-determined ID. +// Used by bootstrap to ensure the IGW ID matches [bootstrap] in spinifex.toml. +func (s *IGWServiceImpl) CreateInternetGatewayWithID(input *ec2.CreateInternetGatewayInput, accountID, igwID string) (*ec2.CreateInternetGatewayOutput, error) { + return s.createIGW(input, accountID, igwID) +} + func (s *IGWServiceImpl) CreateInternetGateway(input *ec2.CreateInternetGatewayInput, accountID string) (*ec2.CreateInternetGatewayOutput, error) { igwID := utils.GenerateResourceID("igw") + return s.createIGW(input, accountID, igwID) +} + +func (s *IGWServiceImpl) createIGW(input *ec2.CreateInternetGatewayInput, accountID, igwID string) (*ec2.CreateInternetGatewayOutput, error) { record := IGWRecord{ InternetGatewayId: igwID, diff --git a/spinifex/handlers/ec2/vpc/service_impl.go b/spinifex/handlers/ec2/vpc/service_impl.go index c5c37c2c0..f9739cef3 100644 --- a/spinifex/handlers/ec2/vpc/service_impl.go +++ b/spinifex/handlers/ec2/vpc/service_impl.go @@ -708,11 +708,19 @@ type DefaultVPCInfo struct { SubnetCidr string } +// BootstrapIDs holds pre-generated resource IDs from the [bootstrap] config. +// When provided, EnsureDefaultVPC uses these IDs instead of generating random ones, +// ensuring consistency between admin init, daemon, and vpcd. +type BootstrapIDs struct { + VpcId string + SubnetId string +} + // EnsureDefaultVPC creates a default VPC and subnet if none exists for the given account. // This matches AWS behavior where a default VPC is present on account creation. // Safe to call multiple times — no-ops if a default VPC already exists. // Returns the default VPC info (whether newly created or pre-existing). -func (s *VPCServiceImpl) EnsureDefaultVPC(accountID string) (*DefaultVPCInfo, error) { +func (s *VPCServiceImpl) EnsureDefaultVPC(accountID string, bootstrap ...BootstrapIDs) (*DefaultVPCInfo, error) { if s.vpcKV == nil { return nil, nil // No persistence, skip } @@ -752,13 +760,16 @@ func (s *VPCServiceImpl) EnsureDefaultVPC(accountID string) (*DefaultVPCInfo, er } } - // Create default VPC + // Create default VPC — use bootstrap IDs if provided for consistency vni, err := s.nextVNI() if err != nil { return nil, fmt.Errorf("allocate VNI for default VPC: %w", err) } vpcID := utils.GenerateResourceID("vpc") + if len(bootstrap) > 0 && bootstrap[0].VpcId != "" { + vpcID = bootstrap[0].VpcId + } vpcRecord := VPCRecord{ VpcId: vpcID, CidrBlock: DefaultVPCCidr, @@ -787,6 +798,9 @@ func (s *VPCServiceImpl) EnsureDefaultVPC(accountID string) (*DefaultVPCInfo, er // Create default subnet (public — matches AWS default VPC behavior) subnetID := utils.GenerateResourceID("subnet") + if len(bootstrap) > 0 && bootstrap[0].SubnetId != "" { + subnetID = bootstrap[0].SubnetId + } subnetRecord := SubnetRecord{ SubnetId: subnetID, VpcId: vpcID, diff --git a/spinifex/services/vpcd/reconcile.go b/spinifex/services/vpcd/reconcile.go index dc39bbab2..3cc20252f 100644 --- a/spinifex/services/vpcd/reconcile.go +++ b/spinifex/services/vpcd/reconcile.go @@ -64,7 +64,7 @@ func Reconcile(ctx context.Context, topo *TopologyHandler, bootstrap *BootstrapV extSwitchName := "ext-" + bootstrap.VpcId if _, err := topo.ovn.GetLogicalSwitch(ctx, extSwitchName); err != nil { slog.Info("vpcd reconcile: creating IGW topology", "switch", extSwitchName) - if err := topo.reconcileIGW(ctx, bootstrap.VpcId); err != nil { + if err := topo.reconcileIGW(ctx, bootstrap.VpcId, bootstrap.IgwId); err != nil { slog.Error("vpcd reconcile: failed to create IGW topology", "err", err) } else { result.IGWsCreated++ diff --git a/spinifex/services/vpcd/reconcile_test.go b/spinifex/services/vpcd/reconcile_test.go index 9f4a0bf15..1b0d6d6e0 100644 --- a/spinifex/services/vpcd/reconcile_test.go +++ b/spinifex/services/vpcd/reconcile_test.go @@ -139,6 +139,7 @@ func TestReconcile_PartialState(t *testing.T) { // Pre-create just the router (simulating partial OVN state) ctx := context.Background() _ = topo.reconcileVPC(ctx, "vpc-partial", "172.31.0.0/16") + // IGW ID not needed for pre-creating just the router // Reconcile should skip router but create subnet + IGW result := Reconcile(ctx, topo, bootstrap) diff --git a/spinifex/services/vpcd/topology.go b/spinifex/services/vpcd/topology.go index 8d8dfdcb3..9d14be3cc 100644 --- a/spinifex/services/vpcd/topology.go +++ b/spinifex/services/vpcd/topology.go @@ -990,7 +990,7 @@ func (h *TopologyHandler) reconcileSubnet(ctx context.Context, subnetId, vpcId, // reconcileIGW creates the OVN external switch, gateway router port, SNAT rule, // default route, and gateway chassis for a VPC's internet gateway. -func (h *TopologyHandler) reconcileIGW(ctx context.Context, vpcId string) error { +func (h *TopologyHandler) reconcileIGW(ctx context.Context, vpcId, igwId string) error { routerName := "vpc-" + vpcId extSwitchName := "ext-" + vpcId extPortName := "ext-port-" + vpcId @@ -1018,27 +1018,35 @@ func (h *TopologyHandler) reconcileIGW(ctx context.Context, vpcId string) error wanGateway = pool.Gateway } + // Build external IDs with optional IGW ID + extIDs := map[string]string{ + "spinifex:vpc_id": vpcId, + "spinifex:role": "external", + } + if igwId != "" { + extIDs["spinifex:igw_id"] = igwId + } + // 1. Create external logical switch extSwitch := &nbdb.LogicalSwitch{ - Name: extSwitchName, - ExternalIDs: map[string]string{ - "spinifex:vpc_id": vpcId, - "spinifex:role": "external", - }, + Name: extSwitchName, + ExternalIDs: extIDs, } if err := h.ovn.CreateLogicalSwitch(ctx, extSwitch); err != nil { return fmt.Errorf("create external switch %s: %w", extSwitchName, err) } // 2. Create localnet port + portExtIDs := map[string]string{"spinifex:vpc_id": vpcId} + if igwId != "" { + portExtIDs["spinifex:igw_id"] = igwId + } localnetPort := &nbdb.LogicalSwitchPort{ - Name: extPortName, - Type: "localnet", - Addresses: []string{"unknown"}, - Options: map[string]string{"network_name": "external"}, - ExternalIDs: map[string]string{ - "spinifex:vpc_id": vpcId, - }, + Name: extPortName, + Type: "localnet", + Addresses: []string{"unknown"}, + Options: map[string]string{"network_name": "external"}, + ExternalIDs: portExtIDs, } if err := h.ovn.CreateLogicalSwitchPort(ctx, extSwitchName, localnetPort); err != nil { _ = h.ovn.DeleteLogicalSwitch(ctx, extSwitchName) diff --git a/spinifex/services/vpcd/vpcd.go b/spinifex/services/vpcd/vpcd.go index 8ae11e625..02159d604 100644 --- a/spinifex/services/vpcd/vpcd.go +++ b/spinifex/services/vpcd/vpcd.go @@ -32,6 +32,7 @@ type BootstrapVPC struct { AccountID string VpcId string SubnetId string + IgwId string Cidr string SubnetCidr string }