diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index ac77f25e1..66dbe2299 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -239,7 +239,8 @@ func init() { adminInitCmd.Flags().Int("nodes", 3, "Number of nodes to expect for cluster") adminInitCmd.Flags().String("host", "", "Leader node to join (if not specified, tries multicast discovery)") adminInitCmd.Flags().Int("port", 4432, "Port to bind cluster services on") - adminInitCmd.Flags().String("bind", "127.0.0.1", "IP address to bind services to (e.g., 10.11.12.1 for multi-node)") + adminInitCmd.Flags().String("bind", "0.0.0.0", "IP address to bind services to (e.g., 10.11.12.1 for multi-node). Default 0.0.0.0 listens on all interfaces.") + adminInitCmd.Flags().String("advertise", "", "External IP that off-host clients (ALB VMs, remote operators) should dial. Auto-detected from default route when empty.") adminInitCmd.Flags().String("cluster-bind", "", "IP address to bind NATS cluster services to (e.g., 10.11.12.1 for multi-node)") adminInitCmd.Flags().String("cluster-routes", "", "NATS cluster hosts for routing specify multiple with comma (e.g., 10.11.12.1:4248,10.11.12.2:4248 for multi-node)") adminInitCmd.Flags().String("predastore-nodes", "", "Comma-separated IPs for multi-node Predastore cluster (e.g., 10.11.12.1,10.11.12.2,10.11.12.3). Requires >= 3 nodes.") @@ -267,6 +268,7 @@ func init() { adminJoinCmd.Flags().String("data-dir", "", "Data directory for this node (default: ~/spinifex)") adminJoinCmd.Flags().Int("port", 4432, "Port to bind cluster services on") adminJoinCmd.Flags().String("bind", "0.0.0.0", "IP address to bind services to (e.g., 10.11.12.2 for multi-node on single host)") + adminJoinCmd.Flags().String("advertise", "", "External IP this node advertises to other cluster members. Defaults to --bind, or auto-detected WAN IP when --bind is 0.0.0.0.") adminJoinCmd.Flags().String("cluster-bind", "", "IP address to bind NATS cluster services to (e.g., 10.11.12.1 for multi-node)") adminJoinCmd.Flags().String("cluster-routes", "", "NATS cluster hosts for routing specify multiple with comma (e.g., 10.11.12.1:4248,10.11.12.2:4248 for multi-node)") adminJoinCmd.Flags().String("token", "", "Join token from the init node (required)") @@ -607,6 +609,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { nodes, _ := cmd.Flags().GetInt("nodes") port, _ := cmd.Flags().GetInt("port") bindIP, _ := cmd.Flags().GetString("bind") + advertiseFlag, _ := cmd.Flags().GetString("advertise") clusterBind, _ := cmd.Flags().GetString("cluster-bind") clusterRoutesStr, _ := cmd.Flags().GetString("cluster-routes") var clusterRoutes []string @@ -776,6 +779,19 @@ func runAdminInit(cmd *cobra.Command, args []string) { os.Exit(1) } + // Resolve the off-host advertise IP. DetectNetwork may have been skipped + // when --no-external is set; detect lazily if we need the WAN IP. + if advertiseFlag == "" && (bindIP == "0.0.0.0" || bindIP == "127.0.0.1") && detectedNet == nil { + if d, derr := admin.DetectNetwork(); derr == nil { + detectedNet = d + } + } + advertiseIP, err := resolveAdvertiseIP(bindIP, advertiseFlag, detectedNet) + if err != nil { + fmt.Fprintf(os.Stderr, "āŒ Error: %v\n", err) + os.Exit(1) + } + // Validate port range if port < 1 || port > 65535 { fmt.Fprintf(os.Stderr, "āŒ Error: Port must be between 1 and 65535, got: %d\n", port) @@ -787,7 +803,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { clusterBind = bindIP } - fmt.Printf("Initializing Spinifex with bind IP: %s, port: %d\n", bindIP, port) + fmt.Printf("Initializing Spinifex with bind IP: %s, advertise IP: %s, port: %d\n", bindIP, advertiseIP, port) // Default config directory if configDir == "" { @@ -873,8 +889,9 @@ func runAdminInit(cmd *cobra.Command, args []string) { } spxRoot = filepath.Clean(spxRoot) - // Determine if this is a multi-node formation - isMultiNode := nodes >= 2 && bindIP != "0.0.0.0" && bindIP != "127.0.0.1" + // Determine if this is a multi-node formation. Operator intent comes from + // --nodes, not from whether --bind was left at the 0.0.0.0 default. + isMultiNode := nodes >= 2 if isMultiNode { // Build cluster-wide network config for propagation to joining nodes @@ -904,7 +921,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { } runAdminInitMultiNode(cmd, accessKey, secretKey, accountID, natsToken, clusterName, - configDir, spxRoot, certPath, region, az, node, bindIP, clusterBind, + configDir, spxRoot, certPath, region, az, node, bindIP, advertiseIP, clusterBind, port, nodes, formationTimeoutStr, tokenTTLStr, services, networkConfig) return } @@ -987,6 +1004,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { Az: az, Port: portStr, BindIP: bindIP, + AdvertiseIP: advertiseIP, ClusterBindIP: clusterBind, ClusterRoutes: clusterRoutes, ClusterName: clusterName, @@ -1040,7 +1058,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { os.Exit(1) } - finalizeNodeSetup(spxRoot, certPath, bootstrapResult.AdminAccessKey, bootstrapResult.AdminSecretKey, region, bindIP) + finalizeNodeSetup(spxRoot, certPath, bootstrapResult.AdminAccessKey, bootstrapResult.AdminSecretKey, region, bindIP, advertiseIP) // Print success message fmt.Println("\nšŸŽ‰ Spinifex initialization complete!") @@ -1048,6 +1066,9 @@ func runAdminInit(cmd *cobra.Command, args []string) { fmt.Println("šŸ”— Configuration:") fmt.Printf(" Config file: %s\n", spinifexTomlPath) fmt.Printf(" Data directory: %s\n", spxRoot) + fmt.Printf(" Bind IP: %s (listen)\n", bindIP) + fmt.Printf(" Advertise IP: %s (off-host dial target)\n", advertiseIP) + fmt.Printf(" Loopback: 127.0.0.1 (in-process dial target)\n") fmt.Println() } @@ -1055,7 +1076,7 @@ func runAdminInit(cmd *cobra.Command, args []string) { // It starts a formation server, registers this node, waits for all nodes to join, // then generates configs with complete cluster topology. func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, natsToken, clusterName, - configDir, spxRoot, certPath, region, az, node, bindIP, clusterBind string, + configDir, spxRoot, certPath, region, az, node, bindIP, advertiseIP, clusterBind string, port, expectedNodes int, formationTimeoutStr, tokenTTLStr string, services []string, networkConfig *formation.NetworkConfig) { formationTimeout, err := time.ParseDuration(formationTimeoutStr) if err != nil { @@ -1124,12 +1145,13 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, // Register self (init node) as the first node selfNode := formation.NodeInfo{ - Name: node, - BindIP: bindIP, - ClusterIP: clusterBind, - Region: region, - AZ: az, - Port: port, + Name: node, + BindIP: bindIP, + AdvertiseIP: advertiseIP, + ClusterIP: clusterBind, + Region: region, + AZ: az, + Port: port, } if err := fs.RegisterNode(selfNode); err != nil { fmt.Fprintf(os.Stderr, "āŒ Error registering self: %v\n", err) @@ -1217,6 +1239,7 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, Az: az, Port: portStr, BindIP: bindIP, + AdvertiseIP: advertiseIP, ClusterBindIP: clusterBind, ClusterRoutes: clusterRoutes, ClusterName: clusterName, @@ -1239,7 +1262,7 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, os.Exit(1) } - finalizeNodeSetup(spxRoot, certPath, bootstrapResult.AdminAccessKey, bootstrapResult.AdminSecretKey, region, bindIP) + finalizeNodeSetup(spxRoot, certPath, bootstrapResult.AdminAccessKey, bootstrapResult.AdminSecretKey, region, bindIP, advertiseIP) // Keep formation server running briefly so joining nodes can fetch complete status fmt.Println("\nā³ Waiting for joining nodes to fetch cluster data...") @@ -1255,9 +1278,14 @@ func runAdminInitMultiNode(cmd *cobra.Command, accessKey, secretKey, accountID, fmt.Println("\nšŸŽ‰ Cluster formation complete!") fmt.Printf(" Cluster: %s (%d nodes)\n", clusterName, expectedNodes) fmt.Printf(" Region: %s\n", region) + fmt.Printf(" Bind: %s Advertise: %s Loopback: 127.0.0.1\n", bindIP, advertiseIP) fmt.Println(" Nodes:") for name, n := range allNodes { - fmt.Printf(" - %s (%s)\n", name, n.BindIP) + adv := n.AdvertiseIP + if adv == "" { + adv = n.BindIP + } + fmt.Printf(" - %s (bind=%s advertise=%s)\n", name, n.BindIP, adv) } fmt.Println("\nšŸ“‹ Next steps:") fmt.Println(" 1. Start services on ALL nodes:") @@ -1280,6 +1308,7 @@ func runAdminJoin(cmd *cobra.Command, args []string) { dataDir, _ := cmd.Flags().GetString("data-dir") port, _ := cmd.Flags().GetInt("port") bindIP, _ := cmd.Flags().GetString("bind") + advertiseFlag, _ := cmd.Flags().GetString("advertise") configDir, _ := cmd.Flags().GetString("config-dir") clusterBind, _ := cmd.Flags().GetString("cluster-bind") services, _ := cmd.Flags().GetStringSlice("services") @@ -1307,6 +1336,20 @@ func runAdminJoin(cmd *cobra.Command, args []string) { os.Exit(1) } + // Resolve the off-host advertise IP before the JoinRequest is built so + // peers record a reachable dial target instead of 0.0.0.0. + var detectedNet *admin.DetectedNetwork + if advertiseFlag == "" && (bindIP == "0.0.0.0" || bindIP == "127.0.0.1") { + if d, derr := admin.DetectNetwork(); derr == nil { + detectedNet = d + } + } + advertiseIP, err := resolveAdvertiseIP(bindIP, advertiseFlag, detectedNet) + if err != nil { + fmt.Fprintf(os.Stderr, "āŒ Error: %v\n", err) + os.Exit(1) + } + // Validate port range if port < 1 || port > 65535 { fmt.Fprintf(os.Stderr, "āŒ Error: Port must be between 1 and 65535, got: %d\n", port) @@ -1329,17 +1372,19 @@ func runAdminJoin(cmd *cobra.Command, args []string) { fmt.Printf("Region: %s\n", region) fmt.Printf("AZ: %s\n", az) fmt.Printf("Bind IP: %s\n", bindIP) + fmt.Printf("Advertise IP: %s\n", advertiseIP) fmt.Printf("Port: %d\n\n", port) // POST join request to formation server joinReq := formation.JoinRequest{ NodeInfo: formation.NodeInfo{ - Name: node, - BindIP: bindIP, - ClusterIP: clusterBind, - Region: region, - AZ: az, - Port: port, + Name: node, + BindIP: bindIP, + AdvertiseIP: advertiseIP, + ClusterIP: clusterBind, + Region: region, + AZ: az, + Port: port, }, } @@ -1588,6 +1633,7 @@ func runAdminJoin(cmd *cobra.Command, args []string) { Az: az, Port: portStr, BindIP: bindIP, + AdvertiseIP: advertiseIP, ClusterBindIP: clusterBind, ClusterRoutes: clusterRoutes, ClusterName: creds.ClusterName, @@ -1610,17 +1656,44 @@ func runAdminJoin(cmd *cobra.Command, args []string) { os.Exit(1) } - finalizeNodeSetup(dataDir, caCertPath, creds.AdminAccessKey, creds.AdminSecretKey, creds.Region, bindIP) + finalizeNodeSetup(dataDir, caCertPath, creds.AdminAccessKey, creds.AdminSecretKey, creds.Region, bindIP, advertiseIP) // Print cluster summary fmt.Println("\nšŸŽ‰ Node successfully joined cluster!") fmt.Printf(" Cluster: %s (%d nodes)\n", creds.ClusterName, len(statusResp.Nodes)) + fmt.Printf(" Bind: %s Advertise: %s Loopback: 127.0.0.1\n", bindIP, advertiseIP) fmt.Println(" Nodes:") for name, n := range statusResp.Nodes { - fmt.Printf(" - %s (%s)\n", name, n.BindIP) + adv := n.AdvertiseIP + if adv == "" { + adv = n.BindIP + } + fmt.Printf(" - %s (bind=%s advertise=%s)\n", name, n.BindIP, adv) } } +// resolveAdvertiseIP picks the off-host dial target for this node. +// Precedence: explicit --advertise flag > specific --bind IP > auto-detected +// WAN IP > loopback (with warning that off-host clients cannot reach this node). +func resolveAdvertiseIP(bindIP, advertiseFlag string, detected *admin.DetectedNetwork) (string, error) { + if advertiseFlag != "" { + if net.ParseIP(advertiseFlag) == nil { + return "", fmt.Errorf("--advertise: invalid IP %q", advertiseFlag) + } + return advertiseFlag, nil + } + if bindIP != "" && bindIP != "0.0.0.0" && bindIP != "127.0.0.1" { + return bindIP, nil + } + if detected != nil && detected.WAN != nil && detected.WAN.IP != "" { + return detected.WAN.IP, nil + } + fmt.Fprintln(os.Stderr, + "āš ļø Could not auto-detect a WAN IP. Off-host clients (ALB VMs, remote operators) "+ + "will not be able to reach this node. Re-run with --advertise to fix.") + return "127.0.0.1", nil +} + // buildRemoteNodes converts formation NodeInfo into RemoteNode entries, // excluding the local node. This puts all cluster members into spinifex.toml // so config is the source of truth for expected cluster membership. @@ -1630,9 +1703,15 @@ func buildRemoteNodes(allNodes map[string]formation.NodeInfo, localNode string) if name == localNode { continue } + // Prefer the peer's advertise IP (off-host dial target); fall back + // to BindIP for pre-siv-8 joiners that didn't send AdvertiseIP. + host := n.AdvertiseIP + if host == "" { + host = n.BindIP + } remote = append(remote, admin.RemoteNode{ Name: name, - Host: n.BindIP, + Host: host, Region: n.Region, AZ: n.AZ, Services: n.Services, @@ -1885,10 +1964,12 @@ func generateAndWriteConfigs(dirs configDirs, spinifexTomlPath string, settings } // finalizeNodeSetup configures AWS credentials, creates service directories, -// and sets ownership when running as root. -func finalizeNodeSetup(dataDir, certPath, adminAccessKey, adminSecretKey, region, bindIP string) { +// and sets ownership when running as root. advertiseIP is the off-host dial +// target for this node; it is threaded through SetupAWSCredentials's wanIP +// param for a future --operator-endpoint flag (see SetupAWSCredentials doc). +func finalizeNodeSetup(dataDir, certPath, adminAccessKey, adminSecretKey, region, bindIP, advertiseIP string) { fmt.Println("\nšŸ”§ Configuring AWS credentials...") - if err := admin.SetupAWSCredentials(adminAccessKey, adminSecretKey, region, certPath, bindIP); err != nil { + if err := admin.SetupAWSCredentials(adminAccessKey, adminSecretKey, region, certPath, bindIP, advertiseIP); err != nil { fmt.Fprintf(os.Stderr, "Warning: Could not update AWS credentials: %v\n", err) } else { fmt.Println("āœ… AWS credentials configured") diff --git a/cmd/spinifex/cmd/admin_test.go b/cmd/spinifex/cmd/admin_test.go index 4fdb92cac..5ce6871a2 100644 --- a/cmd/spinifex/cmd/admin_test.go +++ b/cmd/spinifex/cmd/admin_test.go @@ -4,10 +4,15 @@ import ( "bytes" "errors" "fmt" + "os" + "path/filepath" "testing" + "github.com/mulgadc/spinifex/spinifex/admin" + "github.com/mulgadc/spinifex/spinifex/formation" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // Regression guard: source URL must print for every error kind, not just @@ -32,3 +37,129 @@ func TestPrintChecksumError(t *testing.T) { assert.Contains(t, out, "spx admin images import --name "+imageName+" --force") } } + +// buildRemoteNodes must prefer AdvertiseIP (off-host dial target) and fall +// back to BindIP when the peer pre-dates siv-8 and didn't send AdvertiseIP. +func TestBuildRemoteNodes_AdvertiseFallback(t *testing.T) { + nodes := map[string]formation.NodeInfo{ + "node1": {Name: "node1", BindIP: "10.0.0.1", AdvertiseIP: "203.0.113.1"}, + "node2": {Name: "node2", BindIP: "10.0.0.2"}, // legacy joiner + "node3": {Name: "node3", BindIP: "10.0.0.3", AdvertiseIP: "203.0.113.3"}, + } + got := buildRemoteNodes(nodes, "node3") + if assert.Len(t, got, 2) { + assert.Equal(t, "node1", got[0].Name) + assert.Equal(t, "203.0.113.1", got[0].Host, "advertise wins when set") + assert.Equal(t, "node2", got[1].Name) + assert.Equal(t, "10.0.0.2", got[1].Host, "bind fallback when advertise empty") + } +} + +func TestResolveAdvertiseIP(t *testing.T) { + wan := &admin.DetectedNetwork{WAN: &admin.DetectedInterface{IP: "192.168.1.21"}} + noWAN := &admin.DetectedNetwork{} + + tests := []struct { + name string + bindIP string + advertise string + detected *admin.DetectedNetwork + want string + wantErr bool + }{ + {"single-node default, WAN detected", "0.0.0.0", "", wan, "192.168.1.21", false}, + {"single-node default, no WAN", "0.0.0.0", "", noWAN, "127.0.0.1", false}, + {"single-node explicit advertise", "0.0.0.0", "203.0.113.5", wan, "203.0.113.5", false}, + {"multi-node init, specific bind", "10.11.12.1", "", nil, "10.11.12.1", false}, + {"multi-node dual-homed override", "10.11.12.1", "203.0.113.5", nil, "203.0.113.5", false}, + {"loopback-only test", "127.0.0.1", "", noWAN, "127.0.0.1", false}, + {"invalid advertise flag", "0.0.0.0", "not-an-ip", wan, "", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := resolveAdvertiseIP(tt.bindIP, tt.advertise, tt.detected) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +// Generated spinifex.toml must include both host (listen, BindIP) and +// advertise (off-host dial target, AdvertiseIP) for the local node. +func TestSpinifexTomlTemplate_AdvertiseField(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "spinifex.toml") + settings := admin.ConfigSettings{ + Node: "node1", + Az: "ap-southeast-2a", + Port: "4432", + Region: "ap-southeast-2", + BindIP: "0.0.0.0", + AdvertiseIP: "192.168.1.21", + AccessKey: "AKIATEST", + SecretKey: "SECRET", + AccountID: "123456789012", + NatsToken: "token", + ConfigDir: dir, + OVNNBAddr: "tcp:127.0.0.1:6641", + OVNSBAddr: "tcp:127.0.0.1:6642", + } + require.NoError(t, admin.GenerateConfigFile(path, spinifexTomlTemplate, settings)) + + data, err := os.ReadFile(path) + require.NoError(t, err) + content := string(data) + assert.Contains(t, content, `host = "0.0.0.0"`, "listen address") + assert.Contains(t, content, `advertise = "192.168.1.21"`, "off-host dial target") +} + +// Empty AdvertiseIP (e.g. loading an existing cluster pre-siv-8) must NOT +// render an empty advertise = "" line — downstream fallback to Host kicks in. +func TestSpinifexTomlTemplate_AdvertiseOmittedWhenEmpty(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "spinifex.toml") + settings := admin.ConfigSettings{ + Node: "node1", + Az: "ap-southeast-2a", + Port: "4432", + Region: "ap-southeast-2", + BindIP: "10.11.12.1", + AccessKey: "AKIATEST", + SecretKey: "SECRET", + AccountID: "123456789012", + NatsToken: "token", + ConfigDir: dir, + OVNNBAddr: "tcp:127.0.0.1:6641", + OVNSBAddr: "tcp:127.0.0.1:6642", + } + require.NoError(t, admin.GenerateConfigFile(path, spinifexTomlTemplate, settings)) + data, _ := os.ReadFile(path) + assert.NotContains(t, string(data), "advertise =") +} + +// iam.nats_url must not embed a bare 0.0.0.0 — the predastore process dials +// its own NATS via loopback when listening on wildcard. +func TestPredastoreMultinodeTemplate_NatsURLLoopbackShim(t *testing.T) { + nodes := []admin.PredastoreNodeConfig{ + {ID: 1, Host: "10.0.0.1"}, + {ID: 2, Host: "10.0.0.2"}, + {ID: 3, Host: "10.0.0.3"}, + } + content, err := admin.GenerateMultiNodePredastoreConfig( + predastoreMultiNodeTemplate, nodes, "AK", "SK", "ap-southeast-2", "token", "/tmp", "0.0.0.0", + ) + require.NoError(t, err) + assert.Contains(t, content, `nats_url = "nats://localhost:4222"`) + assert.NotContains(t, content, `nats_url = "nats://0.0.0.0:4222"`) + + // Specific bind IP → stays as-is. + content2, err := admin.GenerateMultiNodePredastoreConfig( + predastoreMultiNodeTemplate, nodes, "AK", "SK", "ap-southeast-2", "token", "/tmp", "10.11.12.1", + ) + require.NoError(t, err) + assert.Contains(t, content2, `nats_url = "nats://10.11.12.1:4222"`) +} diff --git a/cmd/spinifex/cmd/templates/predastore-multinode.toml b/cmd/spinifex/cmd/templates/predastore-multinode.toml index a0b180ba7..7b99141e4 100644 --- a/cmd/spinifex/cmd/templates/predastore-multinode.toml +++ b/cmd/spinifex/cmd/templates/predastore-multinode.toml @@ -63,7 +63,7 @@ burst = 200 # IAM authentication via NATS KV (enables multi-account S3 access) [iam] -nats_url = "nats://{{.BindIP}}:4222" +nats_url = "nats://{{if eq .BindIP "0.0.0.0"}}localhost{{else}}{{.BindIP}}{{end}}:4222" nats_token = "{{.NatsToken}}" nats_ca_cert = "{{.ConfigDir}}/ca.pem" master_key_path = "{{.ConfigDir}}/master.key" diff --git a/cmd/spinifex/cmd/templates/spinifex.toml b/cmd/spinifex/cmd/templates/spinifex.toml index 432dd70f0..7ecc41e63 100644 --- a/cmd/spinifex/cmd/templates/spinifex.toml +++ b/cmd/spinifex/cmd/templates/spinifex.toml @@ -42,6 +42,9 @@ subnet_cidr = "{{.BootstrapSubnetCidr}}" [nodes.{{.Node}}] node = "{{.Node}}" host = "{{.BindIP}}" +{{- if .AdvertiseIP}} +advertise = "{{.AdvertiseIP}}" +{{- end}} region = "{{.Region}}" az = "{{.Az}}" {{- if .Services}} diff --git a/scripts/setup-ovn.sh b/scripts/setup-ovn.sh index cb1fbbb4d..b12fe39ed 100755 --- a/scripts/setup-ovn.sh +++ b/scripts/setup-ovn.sh @@ -20,6 +20,10 @@ # --macvlan Create macvlan off --wan-iface instead of moving NIC directly. # SSH-safe for single-NIC hosts where WAN NIC carries SSH. # --dhcp Obtain gateway IP via DHCP on the WAN bridge interface +# --mgmt-bridge=NAME OVS bridge for system-instance control plane (default: br-mgmt) +# --mgmt-cidr=CIDR IPv4 CIDR to assign on the mgmt bridge (default: 10.15.8.1/24) +# --mgmt-iface=NAME Physical/virtual NIC to enslave to the mgmt bridge (multi-node only) +# --no-mgmt-bridge Skip mgmt bridge provisioning (for dev-networking hosts) # --ovn-remote=ADDR OVN SB DB address (default: tcp:127.0.0.1:6642) # --encap-ip=IP Geneve tunnel endpoint IP (default: auto-detect) # --chassis-id=ID OVN chassis identifier (default: chassis-$(hostname -s)) @@ -55,6 +59,10 @@ WAN_BRIDGE="" WAN_IFACE="" MACVLAN_MODE=false EXTERNAL_DHCP=false +MGMT_BRIDGE_ENABLED=true +MGMT_BRIDGE="br-mgmt" +MGMT_CIDR="10.15.8.1/24" +MGMT_IFACE="" OVN_REMOTE="tcp:127.0.0.1:6642" ENCAP_IP="" CHASSIS_ID="" @@ -67,11 +75,15 @@ for arg in "$@"; do --dhcp) EXTERNAL_DHCP=true ;; --wan-bridge=*) WAN_BRIDGE="${arg#*=}" ;; --wan-iface=*) WAN_IFACE="${arg#*=}" ;; + --mgmt-bridge=*) MGMT_BRIDGE="${arg#*=}" ;; + --mgmt-cidr=*) MGMT_CIDR="${arg#*=}" ;; + --mgmt-iface=*) MGMT_IFACE="${arg#*=}" ;; + --no-mgmt-bridge) MGMT_BRIDGE_ENABLED=false ;; --ovn-remote=*) OVN_REMOTE="${arg#*=}" ;; --encap-ip=*) ENCAP_IP="${arg#*=}" ;; --chassis-id=*) CHASSIS_ID="${arg#*=}" ;; --help|-h) - head -44 "$0" | tail -42 + head -50 "$0" | tail -48 exit 0 ;; *) @@ -451,6 +463,59 @@ if [ -n "$WAN_BRIDGE" ]; then fi fi +# --- Step 3d: Management bridge for system-instance control plane --- +# br-mgmt is an OVS bridge (not L2-learning Linux bridge, not part of OVN +# overlay). fail-mode=standalone makes it behave like a plain L2 switch +# without OVN flows; it is excluded from ovn-bridge-mappings so +# ovn-controller ignores it. System instance TAPs attach via ovs-vsctl +# add-port (see SetupMgmtTapDevice in daemon/network.go). +if [ "$MGMT_BRIDGE_ENABLED" = true ]; then + echo "" + echo "Step 3d: Configuring management bridge ($MGMT_BRIDGE)..." + + sudo ovs-vsctl --may-exist add-br "$MGMT_BRIDGE" + sudo ovs-vsctl set Bridge "$MGMT_BRIDGE" \ + fail-mode=standalone \ + other-config:disable-in-band=true + sudo ip link set "$MGMT_BRIDGE" up + echo " $MGMT_BRIDGE: OVS bridge, fail-mode=standalone, up" + + # Enslave physical/virtual mgmt NIC when provided (multi-node deployments + # where the node's mgmt subnet spans multiple hosts). + if [ -n "$MGMT_IFACE" ]; then + if ! ip link show "$MGMT_IFACE" >/dev/null 2>&1; then + echo " ERROR: mgmt interface $MGMT_IFACE does not exist" + ip -o link show | awk -F': ' '{print " " $2}' + exit 1 + fi + + # Drop any existing IP on the NIC — IP belongs on the bridge. + sudo ip addr flush dev "$MGMT_IFACE" || true + sudo ovs-vsctl --may-exist add-port "$MGMT_BRIDGE" "$MGMT_IFACE" + sudo ip link set "$MGMT_IFACE" up + echo " $MGMT_IFACE: port on $MGMT_BRIDGE" + fi + + if [ -n "$MGMT_CIDR" ]; then + sudo ip addr replace "$MGMT_CIDR" dev "$MGMT_BRIDGE" + echo " $MGMT_BRIDGE: address $MGMT_CIDR" + + # Persist the IP across reboots via a systemd-networkd drop-in. The + # OVS bridge definition itself survives in ovsdb; only the L3 IP + # needs re-applying on boot. + MGMT_NETD_UNIT="/etc/systemd/network/10-spinifex-mgmt.network" + sudo tee "$MGMT_NETD_UNIT" >/dev/null </dev/null | awk '{print $4}' | head -1) + echo " $MGMT_BRIDGE:$(printf '%*s' $((15 - ${#MGMT_BRIDGE})) '') OK (${MGMT_IP_ACTUAL:-no IP})" + else + echo " $MGMT_BRIDGE:$(printf '%*s' $((15 - ${#MGMT_BRIDGE})) '') FAILED" + OK=false + fi +fi + # Check ovn-controller if sudo ovs-appctl -t ovn-controller version >/dev/null 2>&1 || systemctl is-active --quiet ovn-controller 2>/dev/null; then echo " ovn-controller: OK" diff --git a/spinifex/admin/admin.go b/spinifex/admin/admin.go index 679c831a8..b2448a046 100644 --- a/spinifex/admin/admin.go +++ b/spinifex/admin/admin.go @@ -48,6 +48,9 @@ type ConfigSettings struct { Az string Port string BindIP string + // AdvertiseIP is the off-host dial target rendered into the local node's + // [nodes.X].advertise field. Empty → callers fall back to BindIP. + AdvertiseIP string // Cluster settings ClusterBindIP string @@ -783,9 +786,14 @@ func GenerateSelfSignedCert(certPath, keyPath string) error { } // SetupAWSCredentials updates ~/.aws/credentials and ~/.aws/config. -// bindIP is the IP the AWS gateway listens on. If empty or "0.0.0.0", defaults to "localhost". +// bindIP is the IP the AWS gateway listens on. If empty or "0.0.0.0", the +// operator's local ~/.aws/config endpoint_url falls back to "localhost" (this +// runs on the same box as the gateway). wanIP is reserved for a future +// --operator-endpoint flag that will let remote operators point their CLI at +// the host's WAN IP; today it is accepted but unused. // When running under sudo, writes to SUDO_USER's home instead of root's. -func SetupAWSCredentials(accessKey, secretKey, region, certPath, bindIP string) error { +func SetupAWSCredentials(accessKey, secretKey, region, certPath, bindIP, wanIP string) error { + _ = wanIP homeDir, err := os.UserHomeDir() if err != nil { return err diff --git a/spinifex/admin/admin_test.go b/spinifex/admin/admin_test.go index b4aa57253..a8a2b0434 100644 --- a/spinifex/admin/admin_test.go +++ b/spinifex/admin/admin_test.go @@ -222,7 +222,7 @@ func TestSetupAWSCredentials_CreatesFiles(t *testing.T) { dir := t.TempDir() t.Setenv("HOME", dir) - err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/path/to/ca.pem", "") + err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/path/to/ca.pem", "", "") require.NoError(t, err) credData, _ := os.ReadFile(filepath.Join(dir, ".aws", "credentials")) @@ -245,7 +245,7 @@ func TestSetupAWSCredentials_PreservesExistingProfiles(t *testing.T) { "aws_access_key_id": "EXISTING_KEY", })) - err := SetupAWSCredentials("NEWAKEY", "NEWSECRET", "us-west-2", "/ca.pem", "") + err := SetupAWSCredentials("NEWAKEY", "NEWSECRET", "us-west-2", "/ca.pem", "", "") require.NoError(t, err) data, _ := os.ReadFile(filepath.Join(awsDir, "credentials")) @@ -258,7 +258,7 @@ func TestSetupAWSCredentials_UsesBindIP(t *testing.T) { dir := t.TempDir() t.Setenv("HOME", dir) - err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/ca.pem", "10.11.12.1") + err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/ca.pem", "10.11.12.1", "") require.NoError(t, err) configData, _ := os.ReadFile(filepath.Join(dir, ".aws", "config")) @@ -270,7 +270,7 @@ func TestSetupAWSCredentials_FallsBackToLocalhostForWildcard(t *testing.T) { dir := t.TempDir() t.Setenv("HOME", dir) - err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/ca.pem", "0.0.0.0") + err := SetupAWSCredentials("AKIATEST123", "secret123", "us-east-1", "/ca.pem", "0.0.0.0", "") require.NoError(t, err) configData, _ := os.ReadFile(filepath.Join(dir, ".aws", "config")) diff --git a/spinifex/admin/dial.go b/spinifex/admin/dial.go new file mode 100644 index 000000000..ccaa96d9a --- /dev/null +++ b/spinifex/admin/dial.go @@ -0,0 +1,42 @@ +package admin + +import "net" + +// DialTarget returns a host:port suitable for an in-process client to dial, +// given a configured listen address. Inputs of the form "0.0.0.0:N" or "[::]:N" +// are rewritten to "127.0.0.1:N" or "[::1]:N" — a service listening on the +// wildcard is reachable on loopback, and loopback is the TLS SAN that the +// daemon's own cert always covers. Loopback, specific IPs, and bare hostnames +// pass through unchanged. Malformed input (no host:port) passes through so +// callers still see the original value in error messages. +func DialTarget(listenAddr string) string { + host, port, err := net.SplitHostPort(listenAddr) + if err != nil { + return listenAddr + } + switch host { + case "0.0.0.0", "": + return net.JoinHostPort("127.0.0.1", port) + case "::": + return net.JoinHostPort("::1", port) + } + return listenAddr +} + +// AdvertiseHost picks the host that off-host clients should dial for a service +// listening on listenAddr. If the listen host is a wildcard (0.0.0.0, ::, or +// empty), the caller-supplied advertiseIP is returned; otherwise the listen +// host itself is returned. Port is intentionally dropped — callers append the +// port appropriate for the protocol (some services listen and advertise on +// different ports). +func AdvertiseHost(listenAddr, advertiseIP string) string { + host, _, err := net.SplitHostPort(listenAddr) + if err != nil { + host = listenAddr + } + switch host { + case "0.0.0.0", "::", "": + return advertiseIP + } + return host +} diff --git a/spinifex/admin/dial_test.go b/spinifex/admin/dial_test.go new file mode 100644 index 000000000..910d17bfb --- /dev/null +++ b/spinifex/admin/dial_test.go @@ -0,0 +1,59 @@ +package admin + +import "testing" + +func TestDialTarget(t *testing.T) { + t.Parallel() + cases := []struct { + name string + in string + want string + }{ + {"ipv4 wildcard", "0.0.0.0:4222", "127.0.0.1:4222"}, + {"ipv6 wildcard", "[::]:4222", "[::1]:4222"}, + {"ipv4 loopback passthrough", "127.0.0.1:4222", "127.0.0.1:4222"}, + {"ipv6 loopback passthrough", "[::1]:4222", "[::1]:4222"}, + {"specific ipv4 passthrough", "192.168.1.5:4222", "192.168.1.5:4222"}, + {"hostname passthrough", "nats.cluster.local:4222", "nats.cluster.local:4222"}, + {"empty host rewrites to loopback", ":4222", "127.0.0.1:4222"}, + {"malformed passthrough", "malformed", "malformed"}, + {"ipv4 wildcard tls port", "0.0.0.0:8443", "127.0.0.1:8443"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := DialTarget(tc.in) + if got != tc.want { + t.Fatalf("DialTarget(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestAdvertiseHost(t *testing.T) { + t.Parallel() + cases := []struct { + name string + listen string + advertiseIP string + want string + }{ + {"ipv4 wildcard resolves", "0.0.0.0:9999", "192.168.1.5", "192.168.1.5"}, + {"ipv6 wildcard resolves", "[::]:9999", "192.168.1.5", "192.168.1.5"}, + {"specific ip passthrough", "10.11.12.1:9999", "192.168.1.5", "10.11.12.1"}, + {"loopback passthrough", "127.0.0.1:9999", "192.168.1.5", "127.0.0.1"}, + {"hostname passthrough", "nats.cluster.local:9999", "192.168.1.5", "nats.cluster.local"}, + {"no-port input treated as host", "0.0.0.0", "192.168.1.5", "192.168.1.5"}, + {"empty listen falls back to advertise", "", "192.168.1.5", "192.168.1.5"}, + {"empty advertise with wildcard returns empty", "0.0.0.0:9999", "", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := AdvertiseHost(tc.listen, tc.advertiseIP) + if got != tc.want { + t.Fatalf("AdvertiseHost(%q, %q) = %q, want %q", tc.listen, tc.advertiseIP, got, tc.want) + } + }) + } +} diff --git a/spinifex/config/config.go b/spinifex/config/config.go index 92ef94ee6..d480038e5 100644 --- a/spinifex/config/config.go +++ b/spinifex/config/config.go @@ -55,12 +55,15 @@ type BootstrapConfig struct { // Config holds all configuration for the application type Config struct { // Node config - Node string `json:"Node" mapstructure:"node"` - Host string `json:"Host" mapstructure:"host"` // Unique hostname or IP of this node - Region string `json:"Region" mapstructure:"region"` - AZ string `json:"AZ" mapstructure:"az"` - DataDir string `json:"DataDir" mapstructure:"data_dir"` - Services []string `json:"Services" mapstructure:"services"` // Which services this node runs locally + Node string `json:"Node" mapstructure:"node"` + Host string `json:"Host" mapstructure:"host"` // Unique hostname or IP of this node + // AdvertiseIP is the off-host dial target for this node. Empty → callers + // fall back to Host for backward compat with pre-siv-8 cluster configs. + AdvertiseIP string `json:"AdvertiseIP" mapstructure:"advertise"` + Region string `json:"Region" mapstructure:"region"` + AZ string `json:"AZ" mapstructure:"az"` + DataDir string `json:"DataDir" mapstructure:"data_dir"` + Services []string `json:"Services" mapstructure:"services"` // Which services this node runs locally Daemon DaemonConfig `json:"Daemon" mapstructure:"daemon"` NATS NATSConfig `json:"NATS" mapstructure:"nats"` diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index ea7a79759..08b01bdc2 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -569,15 +569,15 @@ func (d *Daemon) Start() error { if err := d.jsManager.WriteServiceManifest( d.node, d.config.GetServices(), - d.config.NATS.Host, - d.config.Predastore.Host, + admin.DialTarget(d.config.NATS.Host), + admin.DialTarget(d.config.Predastore.Host), ); err != nil { slog.Warn("Failed to write service manifest", "error", err) } } // Create services before loading/launching instances, since LaunchInstance depends on them - store := objectstore.NewS3ObjectStoreFromConfig(d.config.Predastore.Host, d.config.Predastore.Region, d.config.Predastore.AccessKey, d.config.Predastore.SecretKey) + store := objectstore.NewS3ObjectStoreFromConfig(admin.DialTarget(d.config.Predastore.Host), d.config.Predastore.Region, d.config.Predastore.AccessKey, d.config.Predastore.SecretKey) d.instanceService = handlers_ec2_instance.NewInstanceServiceImpl(d.config, d.resourceMgr.instanceTypes, d.natsConn, &d.Instances, store) d.keyService = handlers_ec2_key.NewKeyServiceImpl(d.config) d.imageService = handlers_ec2_image.NewImageServiceImpl(d.config, d.natsConn) @@ -844,7 +844,7 @@ func (d *Daemon) Start() error { // be ready immediately after daemon start (e.g. if start-dev.sh is still // launching services). This retries for up to 5 minutes before giving up. func (d *Daemon) connectNATS() error { - nc, err := utils.ConnectNATSWithRetry(d.config.NATS.Host, d.config.NATS.ACL.Token, d.config.NATS.CACert, d.natsRetryOpts...) + nc, err := utils.ConnectNATSWithRetry(admin.DialTarget(d.config.NATS.Host), d.config.NATS.ACL.Token, d.config.NATS.CACert, d.natsRetryOpts...) if err != nil { return err } @@ -993,7 +993,7 @@ func (d *Daemon) checkViperblockReady() bool { // checkPredastoreReady checks if predastore is reachable via TCP. func (d *Daemon) checkPredastoreReady() bool { - host := d.config.Predastore.Host + host := admin.DialTarget(d.config.Predastore.Host) if host == "" { return true // no predastore configured, skip check } @@ -2782,9 +2782,15 @@ func (d *Daemon) wireLBAgentConfig() { } // Resolve gateway URL — the address LB VMs use to reach the AWS gateway. - // Internet-facing LB VMs reach it via VPC networking; internal LB VMs - // reach it via the management NIC (br-mgmt) with a host route added - // to their cloud-init when AWSGW binds to a specific WAN IP. + // Precedence: + // 1. AWSGW listens on a specific WAN IP + br-mgmt present → use WAN IP + // and add host route via br-mgmt for internal LBs. + // 2. br-mgmt present + AWSGW on 0.0.0.0 → br-mgmt IP (both LB flavours + // reach the daemon via mgmt). + // 3. This node's AdvertiseIP (single-node default install) → AdvertiseIP. + // 4. DevNetworking shim → 10.0.2.2. + // 5. AWSGW bound to specific IP (no br-mgmt, no advertise) → that IP. + // 6. Else: error and skip assignment — no silent empty URL. var gatewayHost string awsgwBindIP := "" if d.config.AWSGW.Host != "" { @@ -2793,20 +2799,23 @@ func (d *Daemon) wireLBAgentConfig() { } } - if d.mgmtBridgeIP != "" { - if awsgwBindIP != "" && awsgwBindIP != "0.0.0.0" && !net.ParseIP(awsgwBindIP).IsLoopback() { - // Multi-node: AWSGW listens on a specific WAN IP. Use that IP - // as the gateway URL. Internal LBs will get a host route via - // br-mgmt to reach it (see LaunchSystemInstance). - gatewayHost = awsgwBindIP - d.mgmtRouteVia = awsgwBindIP - } else { - // Single-node: AWSGW listens on all interfaces — br-mgmt IP works. - gatewayHost = d.mgmtBridgeIP - } - } else if d.config.Daemon.DevNetworking { + advertiseIP := d.config.AdvertiseIP + + switch { + case d.mgmtBridgeIP != "" && awsgwBindIP != "" && awsgwBindIP != "0.0.0.0" && !net.ParseIP(awsgwBindIP).IsLoopback(): + // Multi-node: AWSGW listens on a specific WAN IP. Use that IP as the + // gateway URL; internal LBs get a host route via br-mgmt. + gatewayHost = awsgwBindIP + d.mgmtRouteVia = awsgwBindIP + case d.mgmtBridgeIP != "": + // Multi-node with AWSGW on 0.0.0.0 — br-mgmt reaches all LBs. + gatewayHost = d.mgmtBridgeIP + case advertiseIP != "" && advertiseIP != "0.0.0.0": + // Single-node default install — off-host clients dial the advertised IP. + gatewayHost = advertiseIP + case d.config.Daemon.DevNetworking: gatewayHost = "10.0.2.2" - } else if awsgwBindIP != "" && awsgwBindIP != "0.0.0.0" { + case awsgwBindIP != "" && awsgwBindIP != "0.0.0.0": gatewayHost = awsgwBindIP } @@ -2823,7 +2832,8 @@ func (d *Daemon) wireLBAgentConfig() { d.elbv2Service.GatewayURL = gatewayURL slog.Info("LB agent gateway URL configured", "url", gatewayURL) } else { - slog.Error("LB agent gateway URL not configured: no reachable host found — all CreateLoadBalancer calls will fail") + slog.Error("LB agent gateway URL not configured: no reachable host found — CreateLoadBalancer will fail until --advertise or br-mgmt is configured", + "awsgwBindIP", awsgwBindIP, "mgmtBridgeIP", d.mgmtBridgeIP, "advertiseIP", advertiseIP) } // Pass mgmt route info so lbVMUserData can add a bootcmd route for diff --git a/spinifex/daemon/daemon_wire_lb_test.go b/spinifex/daemon/daemon_wire_lb_test.go index 52e8620de..21fdab452 100644 --- a/spinifex/daemon/daemon_wire_lb_test.go +++ b/spinifex/daemon/daemon_wire_lb_test.go @@ -172,3 +172,33 @@ func TestWireLBAgentConfig_NoMgmtRoute(t *testing.T) { assert.Empty(t, d.mgmtRouteVia) } + +// AdvertiseIP covers the single-node default install (no br-mgmt, AWSGW on +// wildcard): the gateway URL falls back to the node's advertised off-host IP. +// Replaces the siv-6 "empty gateway URL" silent failure. +func TestWireLBAgentConfig_GatewayURL_AdvertiseFallback(t *testing.T) { + cfg := &config.Config{ + AdvertiseIP: "192.168.1.21", + AWSGW: config.AWSGWConfig{Host: "0.0.0.0:9999"}, + } + d := newWireLBTestDaemon(t, cfg) + + d.wireLBAgentConfig() + + assert.Equal(t, "https://192.168.1.21:9999", d.elbv2Service.GatewayURL) + assert.Empty(t, d.mgmtRouteVia) +} + +// When there's no br-mgmt, no AdvertiseIP, no DevNetworking, and AWSGW is on +// the wildcard, the gateway URL must stay empty and the daemon must log an +// error (rather than silently assigning a broken URL like pre-siv-8). +func TestWireLBAgentConfig_GatewayURL_NoAdvertiseNoBridge(t *testing.T) { + cfg := &config.Config{ + AWSGW: config.AWSGWConfig{Host: "0.0.0.0:9999"}, + } + d := newWireLBTestDaemon(t, cfg) + + d.wireLBAgentConfig() + + assert.Empty(t, d.elbv2Service.GatewayURL) +} diff --git a/spinifex/daemon/network.go b/spinifex/daemon/network.go index 2aca7b2e1..57e76bc76 100644 --- a/spinifex/daemon/network.go +++ b/spinifex/daemon/network.go @@ -203,15 +203,17 @@ func GetBridgeIPv4(bridgeName string) (string, error) { return "", fmt.Errorf("no IPv4 address on %s", bridgeName) } -// SetupMgmtTapDevice creates a TAP device and adds it to the management Linux bridge. -// Unlike VPC TAPs (OVS/OVN), this is a plain Linux bridge TAP. +// SetupMgmtTapDevice creates a TAP device and attaches it to the management +// OVS bridge (provisioned by scripts/setup-ovn.sh). br-mgmt uses +// fail-mode=standalone and is excluded from ovn-bridge-mappings, so it +// behaves as a plain L2 switch outside the OVN overlay. func SetupMgmtTapDevice(instanceID, mac, bridge string) (string, error) { tapName := MgmtTapName(instanceID) // Clean up stale TAP if present if _, err := os.Stat("/sys/class/net/" + tapName); err == nil { slog.Warn("Stale mgmt tap found, cleaning up", "tap", tapName) - if err := sudoCommand("ip", "link", "set", tapName, "nomaster").Run(); err != nil { + if err := sudoCommand("ovs-vsctl", "--if-exists", "del-port", bridge, tapName).Run(); err != nil { slog.Warn("Failed to remove stale mgmt tap from bridge", "tap", tapName, "err", err) } if err := sudoCommand("ip", "tuntap", "del", "dev", tapName, "mode", "tap").Run(); err != nil { @@ -230,8 +232,8 @@ func SetupMgmtTapDevice(instanceID, mac, bridge string) (string, error) { return "", fmt.Errorf("bring up mgmt tap %s: %s: %w", tapName, strings.TrimSpace(string(out)), err) } - // Add to Linux bridge - if out, err := sudoCommand("ip", "link", "set", tapName, "master", bridge).CombinedOutput(); err != nil { + // Attach to OVS mgmt bridge + if out, err := sudoCommand("ovs-vsctl", "--may-exist", "add-port", bridge, tapName).CombinedOutput(); err != nil { _ = sudoCommand("ip", "tuntap", "del", "dev", tapName, "mode", "tap").Run() return "", fmt.Errorf("add mgmt tap to %s: %s: %w", bridge, strings.TrimSpace(string(out)), err) } @@ -240,10 +242,11 @@ func SetupMgmtTapDevice(instanceID, mac, bridge string) (string, error) { return tapName, nil } -// CleanupMgmtTapDevice removes a management TAP device from its bridge and deletes it. +// CleanupMgmtTapDevice removes a management TAP device from the mgmt OVS +// bridge and deletes it. func CleanupMgmtTapDevice(tapName string) error { - // Remove from bridge - if out, err := sudoCommand("ip", "link", "set", tapName, "nomaster").CombinedOutput(); err != nil { + // Detach from whichever OVS bridge holds this tap (usually br-mgmt) + if out, err := sudoCommand("ovs-vsctl", "--if-exists", "del-port", tapName).CombinedOutput(); err != nil { slog.Warn("Failed to remove mgmt tap from bridge", "tap", tapName, "err", err, "out", strings.TrimSpace(string(out))) } diff --git a/spinifex/formation/formation.go b/spinifex/formation/formation.go index e0a29acc8..811cdc48e 100644 --- a/spinifex/formation/formation.go +++ b/spinifex/formation/formation.go @@ -17,13 +17,17 @@ import ( // NodeInfo describes a node participating in cluster formation. type NodeInfo struct { - Name string `json:"name"` - BindIP string `json:"bind_ip"` - ClusterIP string `json:"cluster_ip"` - Region string `json:"region"` - AZ string `json:"az"` - Port int `json:"port"` - Services []string `json:"services,omitempty"` + Name string `json:"name"` + // BindIP is the listen address on the joining node. + BindIP string `json:"bind_ip"` + // AdvertiseIP is the off-host dial target for this node. Empty → + // peers fall back to BindIP for backward compat with older joiners. + AdvertiseIP string `json:"advertise_ip,omitempty"` + ClusterIP string `json:"cluster_ip"` + Region string `json:"region"` + AZ string `json:"az"` + Port int `json:"port"` + Services []string `json:"services,omitempty"` } // JoinRequest is the payload POSTed by joining nodes. diff --git a/spinifex/services/awsgw/awsgw.go b/spinifex/services/awsgw/awsgw.go index add0370b3..3850a375c 100644 --- a/spinifex/services/awsgw/awsgw.go +++ b/spinifex/services/awsgw/awsgw.go @@ -11,6 +11,7 @@ import ( "time" "github.com/mulgadc/predastore/ratelimit" + "github.com/mulgadc/spinifex/spinifex/admin" "github.com/mulgadc/spinifex/spinifex/config" "github.com/mulgadc/spinifex/spinifex/gateway" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" @@ -102,7 +103,7 @@ func launchService(config *config.ClusterConfig) error { // Connect to NATS for service communication. On concurrent startup the // local NATS server may not be listening yet, so retry with backoff. - natsConn, err := utils.ConnectNATSWithRetry(nodeConfig.NATS.Host, nodeConfig.NATS.ACL.Token, nodeConfig.NATS.CACert) + natsConn, err := utils.ConnectNATSWithRetry(admin.DialTarget(nodeConfig.NATS.Host), nodeConfig.NATS.ACL.Token, nodeConfig.NATS.CACert) if err != nil { return err }