diff --git a/.github/workflows/e2e-nightly.yml b/.github/workflows/e2e-nightly.yml index f64434dd6..b349567c8 100644 --- a/.github/workflows/e2e-nightly.yml +++ b/.github/workflows/e2e-nightly.yml @@ -32,6 +32,7 @@ jobs: - { cell: 9, install: binary, topo: real-multi, net: static, host: ubuntu-24.04, arch: x86_64, runner: ci-multi, extra: "" } - { cell: 10, install: binary, topo: real-multi, net: dhcp, host: ubuntu-24.04, arch: x86_64, runner: ci-multi, extra: "" } - { cell: 17, install: binary, topo: single, net: static, host: debian-13, arch: x86_64, runner: ci-pseudo, extra: "tofu-examples" } + - { cell: 18, install: binary, topo: single, net: static, host: debian-13, arch: x86_64, runner: ci-single, extra: "reboot-single" } runs-on: [ self-hosted, "${{ matrix.runner }}" ] timeout-minutes: 35 @@ -138,7 +139,7 @@ jobs: --test-tar "${RUNNER_TEMP}/spinifex-tests.tar" - name: Run Single Node E2E - if: steps.gate.outputs.selected == 'true' && matrix.topo == 'single' && matrix.extra != 'tofu-examples' + if: steps.gate.outputs.selected == 'true' && matrix.topo == 'single' && matrix.extra == '' working-directory: mulga/scripts/tofu-cluster run: | INVENTORY=$(tofu output -json inventory) @@ -177,6 +178,24 @@ jobs: ssh -o StrictHostKeyChecking=no -i "$SSH_KEY" tf-user@${WAN_IP} \ "cd ~/e2e-tests && ./run-tofu-examples-e2e.sh" + - name: Run Reboot Resilience E2E + if: steps.gate.outputs.selected == 'true' && matrix.extra == 'reboot-single' + working-directory: mulga/scripts/tofu-cluster + # Driver runs from the runner (NOT via ssh-and-run) so it survives the + # systemctl-reboot-induced ssh disconnect mid-run. + run: | + INVENTORY=$(tofu output -json inventory) + WAN_IP=$(echo "$INVENTORY" | jq -r '.nodes[0].wan_ip') + SSH_KEY=$(echo "$INVENTORY" | jq -r '.ssh_key_path') + SSH_KEY="${SSH_KEY/#\~/$HOME}" + + REBOOT_DIR="${RUNNER_TEMP}/reboot-e2e" + mkdir -p "$REBOOT_DIR" + tar xf "${RUNNER_TEMP}/spinifex-tests.tar" -C "$REBOOT_DIR" + chmod +x "$REBOOT_DIR/run-reboot-e2e.sh" + + WAN_IP="$WAN_IP" SSH_KEY="$SSH_KEY" "$REBOOT_DIR/run-reboot-e2e.sh" + - name: Teardown if: always() working-directory: mulga/scripts/tofu-cluster diff --git a/cmd/spinifex/cmd/admin_test.go b/cmd/spinifex/cmd/admin_test.go index c03d61469..6686b7468 100644 --- a/cmd/spinifex/cmd/admin_test.go +++ b/cmd/spinifex/cmd/admin_test.go @@ -146,27 +146,43 @@ func TestSpinifexTomlTemplate_AdvertiseOmittedWhenEmpty(t *testing.T) { // when it's a bridge (Linux or OVS, br-* prefix). The old detectedWanBridge() // returned hardcoded "br-ext" for Linux bridges — that value broke DHCP on // consumer-router LANs because br-ext never sees LAN DHCP traffic (mulga-998). -func TestDetectedDhcpBindBridge_LinuxBridgeDefaultRoute(t *testing.T) { - detected := &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "br-wan"}} - assert.Equal(t, "br-wan", detectedDhcpBindBridge(detected), - "Linux bridge (WAN NIC enslaved) must return the bridge name, not 'br-ext'") -} - -func TestDetectedDhcpBindBridge_OVSBridgeDefaultRoute(t *testing.T) { - detected := &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "br-ext"}} - assert.Equal(t, "br-ext", detectedDhcpBindBridge(detected), - "OVS bridge (WAN NIC on br-*) returned verbatim") -} - -func TestDetectedDhcpBindBridge_PhysicalNIC(t *testing.T) { - detected := &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "enp0s3"}} - assert.Equal(t, "br-wan", detectedDhcpBindBridge(detected), - "bare NIC defaults to 'br-wan' (the bridge the installer will create)") -} - -func TestDetectedDhcpBindBridge_NilWAN(t *testing.T) { - assert.Empty(t, detectedDhcpBindBridge(nil)) - assert.Empty(t, detectedDhcpBindBridge(&admin.DetectedNetwork{})) +func TestDetectedDhcpBindBridge(t *testing.T) { + tests := []struct { + name string + detected *admin.DetectedNetwork + want string + }{ + { + name: "Linux bridge default route returns bridge name (not 'br-ext')", + detected: &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "br-wan"}}, + want: "br-wan", + }, + { + name: "OVS bridge default route returned verbatim", + detected: &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "br-ext"}}, + want: "br-ext", + }, + { + name: "physical NIC defaults to 'br-wan' (bridge the installer creates)", + detected: &admin.DetectedNetwork{WAN: &admin.DetectedInterface{Name: "enp0s3"}}, + want: "br-wan", + }, + { + name: "nil DetectedNetwork", + detected: nil, + want: "", + }, + { + name: "DetectedNetwork with nil WAN", + detected: &admin.DetectedNetwork{}, + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, detectedDhcpBindBridge(tt.detected)) + }) + } } // Legacy `wan_bridge` TOML key must fail-start vpcd with guidance, not silently diff --git a/cmd/spinifex/cmd/paths.go b/cmd/spinifex/cmd/paths.go index 51c201613..bebdf720d 100644 --- a/cmd/spinifex/cmd/paths.go +++ b/cmd/spinifex/cmd/paths.go @@ -54,12 +54,15 @@ func DefaultConfigFile() string { return filepath.Join(DefaultConfigDir(), "spinifex.toml") } +// productionMarkerPath identifies a production install by directory presence. +// Created by setup.sh during binary install. Overridable in tests. +var productionMarkerPath = "/etc/spinifex" + // isProductionLayout returns true when running in a production install. -// Detected by: /etc/spinifex directory exists (created by setup.sh during -// binary install). No root check — allows non-root users (e.g. tf-user) to -// run CLI commands like `spx get nodes` without sudo or --config flags. +// No root check — allows non-root users (e.g. tf-user) to run CLI commands +// like `spx get nodes` without sudo or --config flags. func isProductionLayout() bool { - if info, err := os.Stat("/etc/spinifex"); err == nil && info.IsDir() { + if info, err := os.Stat(productionMarkerPath); err == nil && info.IsDir() { return true } return false diff --git a/cmd/spinifex/cmd/paths_test.go b/cmd/spinifex/cmd/paths_test.go index bc36156d0..31e8a49b8 100644 --- a/cmd/spinifex/cmd/paths_test.go +++ b/cmd/spinifex/cmd/paths_test.go @@ -1,57 +1,77 @@ package cmd import ( - "os" "path/filepath" "testing" + + "github.com/stretchr/testify/assert" ) -func TestDefaultConfigDir_Dev(t *testing.T) { - // Without /etc/spinifex should get dev paths - if _, err := os.Stat("/etc/spinifex"); err == nil { - t.Skip("test requires /etc/spinifex to not exist") - } - - homeDir, _ := os.UserHomeDir() - expected := filepath.Join(homeDir, "spinifex", "config") - got := DefaultConfigDir() - if got != expected { - t.Errorf("DefaultConfigDir() = %q, want %q", got, expected) - } +// withProductionMarker temporarily overrides the marker path used to detect a +// production install. Tests that pass an empty string force "dev" mode by +// pointing at a non-existent path; tests that pass t.TempDir() force +// "production" mode by pointing at a real directory. +func withProductionMarker(t *testing.T, path string) { + t.Helper() + orig := productionMarkerPath + productionMarkerPath = path + t.Cleanup(func() { productionMarkerPath = orig }) } -func TestDefaultDataDir_Dev(t *testing.T) { - if _, err := os.Stat("/etc/spinifex"); err == nil { - t.Skip("test requires /etc/spinifex to not exist") - } - - homeDir, _ := os.UserHomeDir() - expected := filepath.Join(homeDir, "spinifex") - got := DefaultDataDir() - if got != expected { - t.Errorf("DefaultDataDir() = %q, want %q", got, expected) - } +func TestIsProductionLayout(t *testing.T) { + t.Run("absent marker returns false", func(t *testing.T) { + withProductionMarker(t, filepath.Join(t.TempDir(), "does-not-exist")) + assert.False(t, isProductionLayout()) + }) + t.Run("present marker returns true", func(t *testing.T) { + withProductionMarker(t, t.TempDir()) + assert.True(t, isProductionLayout()) + }) } -func TestDefaultConfigFile(t *testing.T) { - if _, err := os.Stat("/etc/spinifex"); err == nil { - t.Skip("test requires /etc/spinifex to not exist") +func TestDefaultPaths(t *testing.T) { + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + t.Setenv("SUDO_USER", "") + + tests := []struct { + name string + productionMode bool + wantConfigDir string + wantDataDir string + wantConfigFile string + wantLogDir string // for LogDirFor("/data/dir") + }{ + { + name: "dev layout", + productionMode: false, + wantConfigDir: filepath.Join(homeDir, "spinifex", "config"), + wantDataDir: filepath.Join(homeDir, "spinifex"), + wantConfigFile: filepath.Join(homeDir, "spinifex", "config", "spinifex.toml"), + wantLogDir: "/data/dir/logs", + }, + { + name: "production layout", + productionMode: true, + wantConfigDir: "/etc/spinifex", + wantDataDir: "/var/lib/spinifex", + wantConfigFile: "/etc/spinifex/spinifex.toml", + wantLogDir: "/var/log/spinifex", + }, } - got := DefaultConfigFile() - homeDir, _ := os.UserHomeDir() - expected := filepath.Join(homeDir, "spinifex", "config", "spinifex.toml") - if got != expected { - t.Errorf("DefaultConfigFile() = %q, want %q", got, expected) - } -} - -func TestIsProductionLayout_NoEtcSpinifex(t *testing.T) { - if _, err := os.Stat("/etc/spinifex"); err == nil { - t.Skip("test requires /etc/spinifex to not exist") - } - - if isProductionLayout() { - t.Error("isProductionLayout() = true without /etc/spinifex, want false") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.productionMode { + withProductionMarker(t, t.TempDir()) + } else { + withProductionMarker(t, filepath.Join(t.TempDir(), "absent")) + } + + assert.Equal(t, tt.wantConfigDir, DefaultConfigDir()) + assert.Equal(t, tt.wantDataDir, DefaultDataDir()) + assert.Equal(t, tt.wantConfigFile, DefaultConfigFile()) + assert.Equal(t, tt.wantLogDir, LogDirFor("/data/dir")) + }) } } diff --git a/cmd/spinifex/cmd/version_test.go b/cmd/spinifex/cmd/version_test.go deleted file mode 100644 index a66fb6fc1..000000000 --- a/cmd/spinifex/cmd/version_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package cmd - -import ( - "testing" -) - -func TestVersionVarsExist(t *testing.T) { - // Verify the version variables are set (defaults when not injected via ldflags) - if Version == "" { - t.Error("Version should not be empty") - } - if Commit == "" { - t.Error("Commit should not be empty") - } -} - -func TestVersionCommand(t *testing.T) { - // Verify the version command is registered - found := false - for _, cmd := range rootCmd.Commands() { - if cmd.Use == "version" { - found = true - break - } - } - if !found { - t.Error("version command not registered on rootCmd") - } -} diff --git a/spinifex/config/config_test.go b/spinifex/config/config_test.go index 1051da5fe..6c2a9844d 100644 --- a/spinifex/config/config_test.go +++ b/spinifex/config/config_test.go @@ -217,40 +217,53 @@ expected_nodes = 3 // Tests for HasService / GetServices -func TestHasService_ExplicitList(t *testing.T) { - c := Config{Services: []string{"nats", "daemon"}} +func TestHasService(t *testing.T) { + tests := []struct { + name string + services []string + query string + want bool + }{ + {"explicit list, member", []string{"nats", "daemon"}, "nats", true}, + {"explicit list, second member", []string{"nats", "daemon"}, "daemon", true}, + {"explicit list, non-member", []string{"nats", "daemon"}, "predastore", false}, + {"explicit list, queried name unknown", []string{"nats"}, "unknown", false}, + // A typo in the configured list ("natz") must not satisfy a query for + // the real service name — Contains is name-exact, no fuzzy match. + {"explicit list with typo entry", []string{"natz"}, "nats", false}, + {"nil list defaults to AllServices", nil, "viperblock", true}, + {"empty list defaults to AllServices", []string{}, "ui", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := Config{Services: tt.services} + assert.Equal(t, tt.want, c.HasService(tt.query)) + }) + } - assert.True(t, c.HasService("nats")) - assert.True(t, c.HasService("daemon")) - assert.False(t, c.HasService("predastore")) - assert.False(t, c.HasService("viperblock")) - assert.False(t, c.HasService("ui")) -} - -func TestHasService_EmptyListBackwardCompat(t *testing.T) { - c := Config{} // no Services set - - // Empty list means all services + // Sanity: empty list grants every documented service. + empty := Config{} for _, svc := range AllServices { - assert.True(t, c.HasService(svc), "expected %s to be available with empty list", svc) + assert.True(t, empty.HasService(svc), "empty list must include %q", svc) } } -func TestHasService_UnknownService(t *testing.T) { - c := Config{Services: []string{"nats"}} - assert.False(t, c.HasService("unknown")) -} - -func TestGetServices_DefaultsToAll(t *testing.T) { - c := Config{} - services := c.GetServices() - assert.Equal(t, AllServices, services) -} - -func TestGetServices_ExplicitList(t *testing.T) { - c := Config{Services: []string{"nats", "predastore"}} - services := c.GetServices() - assert.Equal(t, []string{"nats", "predastore"}, services) +func TestGetServices(t *testing.T) { + tests := []struct { + name string + services []string + want []string + }{ + {"nil defaults to AllServices", nil, AllServices}, + {"empty defaults to AllServices", []string{}, AllServices}, + {"explicit list returned as-is", []string{"nats", "predastore"}, []string{"nats", "predastore"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := Config{Services: tt.services} + assert.Equal(t, tt.want, c.GetServices()) + }) + } } // Tests for NodeBaseDir diff --git a/spinifex/daemon/daemon_mgmt_ip_test.go b/spinifex/daemon/daemon_mgmt_ip_test.go index de476ade9..cec24e439 100644 --- a/spinifex/daemon/daemon_mgmt_ip_test.go +++ b/spinifex/daemon/daemon_mgmt_ip_test.go @@ -41,40 +41,51 @@ func TestNewMgmtIPAllocator(t *testing.T) { } func TestMgmtIPAllocator_Allocate(t *testing.T) { - a, err := NewMgmtIPAllocator("10.15.8.1") - if err != nil { - t.Fatal(err) + tests := []struct { + name string + bridgeIP string + firstIP string + secondIP string + }{ + {"primary subnet", "10.15.8.1", "10.15.8.10", "10.15.8.11"}, + {"different subnet", "192.168.1.33", "192.168.1.10", "192.168.1.11"}, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a, err := NewMgmtIPAllocator(tt.bridgeIP) + if err != nil { + t.Fatal(err) + } - // First allocation gets .10 - ip, err := a.Allocate("i-first") - if err != nil { - t.Fatal(err) - } - if ip != "10.15.8.10" { - t.Errorf("first IP = %q, want 10.15.8.10", ip) - } + ip, err := a.Allocate("i-first") + if err != nil { + t.Fatal(err) + } + if ip != tt.firstIP { + t.Errorf("first IP = %q, want %q", ip, tt.firstIP) + } - // Second allocation gets .11 - ip, err = a.Allocate("i-second") - if err != nil { - t.Fatal(err) - } - if ip != "10.15.8.11" { - t.Errorf("second IP = %q, want 10.15.8.11", ip) - } + ip, err = a.Allocate("i-second") + if err != nil { + t.Fatal(err) + } + if ip != tt.secondIP { + t.Errorf("second IP = %q, want %q", ip, tt.secondIP) + } - // Re-allocating same instance returns same IP - ip, err = a.Allocate("i-first") - if err != nil { - t.Fatal(err) - } - if ip != "10.15.8.10" { - t.Errorf("re-allocate = %q, want 10.15.8.10", ip) - } + // Re-allocating same instance returns same IP + ip, err = a.Allocate("i-first") + if err != nil { + t.Fatal(err) + } + if ip != tt.firstIP { + t.Errorf("re-allocate = %q, want %q", ip, tt.firstIP) + } - if a.AllocatedCount() != 2 { - t.Errorf("count = %d, want 2", a.AllocatedCount()) + if a.AllocatedCount() != 2 { + t.Errorf("count = %d, want 2", a.AllocatedCount()) + } + }) } } @@ -171,21 +182,6 @@ func TestMgmtIPAllocator_Rebuild(t *testing.T) { } } -func TestMgmtIPAllocator_DifferentSubnet(t *testing.T) { - a, err := NewMgmtIPAllocator("192.168.1.33") - if err != nil { - t.Fatal(err) - } - - ip, err := a.Allocate("i-test") - if err != nil { - t.Fatal(err) - } - if ip != "192.168.1.10" { - t.Errorf("IP = %q, want 192.168.1.10", ip) - } -} - func TestMgmtIPAllocator_Concurrent(t *testing.T) { a, err := NewMgmtIPAllocator("10.15.8.1") if err != nil { diff --git a/spinifex/daemon/device_map_test.go b/spinifex/daemon/device_map_test.go index a64a4b60d..daea929f7 100644 --- a/spinifex/daemon/device_map_test.go +++ b/spinifex/daemon/device_map_test.go @@ -53,142 +53,105 @@ func TestExtractPCIIndex(t *testing.T) { } } -// TestBuildDeviceMap tests the core device mapping logic using the example -// QMP query-block response from qmp.go:72 +// TestBuildDeviceMap covers the core device-mapping logic across boot devices, +// hot-plugged virtio peripherals, non-virtio devices that must be filtered out, +// PCI-index ordering across mixed boot/hot-plug input, and the empty case. func TestBuildDeviceMap(t *testing.T) { - // Simulate the exact QMP response from the example in qmp.go - devices := []qmp.BlockDevice{ + tests := []struct { + name string + devices []qmp.BlockDevice + want map[string]string // expected entries (subset checked with Equal) + absent []string // keys that must NOT appear + wantLen int + }{ { - IOStatus: "ok", - Device: "os", - Locked: false, - Inserted: &qmp.BlockInserted{ - Image: qmp.BlockImage{ - VirtualSize: 4294967296, - Filename: "nbd://127.0.0.1:44801", - Format: "raw", + name: "empty input", + devices: nil, + want: map[string]string{}, + wantLen: 0, + }, + { + // Mirrors the example QMP query-block response from qmp.go:72 + name: "boot devices with non-virtio devices filtered out", + devices: []qmp.BlockDevice{ + { + IOStatus: "ok", + Device: "os", + Inserted: &qmp.BlockInserted{ + Image: qmp.BlockImage{VirtualSize: 4294967296, Filename: "nbd://127.0.0.1:44801", Format: "raw"}, + }, + QDev: "/machine/peripheral-anon/device[0]/virtio-backend", + Type: "unknown", }, - }, - QDev: "/machine/peripheral-anon/device[0]/virtio-backend", - Type: "unknown", - }, - { - IOStatus: "ok", - Device: "cloudinit", - Locked: false, - Inserted: &qmp.BlockInserted{ - Image: qmp.BlockImage{ - VirtualSize: 1048576, - Filename: "nbd://127.0.0.1:42911", - Format: "raw", + { + IOStatus: "ok", + Device: "cloudinit", + Inserted: &qmp.BlockInserted{ + Image: qmp.BlockImage{VirtualSize: 1048576, Filename: "nbd://127.0.0.1:42911", Format: "raw"}, + RO: true, + }, + QDev: "/machine/peripheral-anon/device[3]/virtio-backend", + Type: "unknown", }, - RO: true, + {Device: "ide1-cd0", Removable: true, QDev: "/machine/unattached/device[24]"}, + {Device: "floppy0", Removable: true, QDev: "/machine/unattached/device[18]"}, + {Device: "sd0", Removable: true}, }, - QDev: "/machine/peripheral-anon/device[3]/virtio-backend", - Type: "unknown", + want: map[string]string{"os": "/dev/vda", "cloudinit": "/dev/vdb"}, + absent: []string{"ide1-cd0", "floppy0", "sd0"}, + wantLen: 2, }, { - IOStatus: "ok", - Device: "ide1-cd0", - Locked: false, - Removable: true, - QDev: "/machine/unattached/device[24]", - Type: "unknown", + // Hot-plugged devices have empty Device field and use the + // /machine/peripheral//virtio-backend QDev path format. + name: "boot plus hot-plugged devices", + devices: []qmp.BlockDevice{ + {Device: "os", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"}, + {Device: "cloudinit", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"}, + {Device: "", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral/vdisk-vol-abc123/virtio-backend"}, + {Device: "", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral/vdisk-vol-def456/virtio-backend"}, + {Device: "floppy0", QDev: "/machine/unattached/device[18]"}, + {Device: "sd0"}, + {Device: "ide1-cd0", QDev: "/machine/unattached/device[24]"}, + }, + want: map[string]string{ + "os": "/dev/vda", + "cloudinit": "/dev/vdb", + "vdisk-vol-abc123": "/dev/vdc", + "vdisk-vol-def456": "/dev/vdd", + }, + wantLen: 4, }, { - Device: "floppy0", - Locked: false, - Removable: true, - QDev: "/machine/unattached/device[18]", - Type: "unknown", - }, - { - Device: "sd0", - Locked: false, - Removable: true, - Type: "unknown", + // Boot devices returned out of order: lowest PCI index wins /dev/vda; + // hot-plugged devices sort after boot devices. + name: "PCI-index ordering across mixed boot and hot-plug", + devices: []qmp.BlockDevice{ + {Device: "cloudinit", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral-anon/device[5]/virtio-backend"}, + {Device: "os", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral-anon/device[1]/virtio-backend"}, + {Device: "", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral/vdisk-vol-123/virtio-backend"}, + }, + want: map[string]string{ + "os": "/dev/vda", + "cloudinit": "/dev/vdb", + "vdisk-vol-123": "/dev/vdc", + }, + wantLen: 3, }, } - result := buildDeviceMap(devices) - - assert.Equal(t, "/dev/vda", result["os"], "root disk should be /dev/vda") - assert.Equal(t, "/dev/vdb", result["cloudinit"], "cloudinit should be /dev/vdb") - assert.NotContains(t, result, "ide1-cd0", "ide device should be excluded") - assert.NotContains(t, result, "floppy0", "floppy should be excluded") - assert.NotContains(t, result, "sd0", "sd device should be excluded") - assert.Len(t, result, 2, "should only have 2 virtio devices") -} - -func TestBuildDeviceMapWithHotplug(t *testing.T) { - // Hot-plugged devices have empty Device field and use - // /machine/peripheral//virtio-backend QDev path format - devices := []qmp.BlockDevice{ - { - Device: "os", - Inserted: &qmp.BlockInserted{}, - QDev: "/machine/peripheral-anon/device[0]/virtio-backend", - }, - { - Device: "cloudinit", - Inserted: &qmp.BlockInserted{}, - QDev: "/machine/peripheral-anon/device[3]/virtio-backend", - }, - { - Device: "", - Inserted: &qmp.BlockInserted{}, - QDev: "/machine/peripheral/vdisk-vol-abc123/virtio-backend", - }, - { - Device: "", - Inserted: &qmp.BlockInserted{}, - QDev: "/machine/peripheral/vdisk-vol-def456/virtio-backend", - }, - // Legacy devices to filter out - {Device: "floppy0", QDev: "/machine/unattached/device[18]"}, - {Device: "sd0"}, - {Device: "ide1-cd0", QDev: "/machine/unattached/device[24]"}, + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := buildDeviceMap(tt.devices) + for k, v := range tt.want { + assert.Equal(t, v, got[k], "device %q", k) + } + for _, k := range tt.absent { + assert.NotContains(t, got, k) + } + assert.Len(t, got, tt.wantLen) + }) } - - result := buildDeviceMap(devices) - - assert.Equal(t, "/dev/vda", result["os"]) - assert.Equal(t, "/dev/vdb", result["cloudinit"]) - assert.Equal(t, "/dev/vdc", result["vdisk-vol-abc123"]) - assert.Equal(t, "/dev/vdd", result["vdisk-vol-def456"]) - assert.Len(t, result, 4) -} - -func TestBuildDeviceMapEmpty(t *testing.T) { - result := buildDeviceMap(nil) - assert.Empty(t, result) -} - -func TestBuildDeviceMapPCIOrdering(t *testing.T) { - // Boot devices returned out of order + hot-plugged device - devices := []qmp.BlockDevice{ - { - Device: "cloudinit", - Inserted: &qmp.BlockInserted{}, - QDev: "/machine/peripheral-anon/device[5]/virtio-backend", - }, - { - Device: "os", - Inserted: &qmp.BlockInserted{}, - QDev: "/machine/peripheral-anon/device[1]/virtio-backend", - }, - { - Device: "", - Inserted: &qmp.BlockInserted{}, - QDev: "/machine/peripheral/vdisk-vol-123/virtio-backend", - }, - } - - result := buildDeviceMap(devices) - - assert.Equal(t, "/dev/vda", result["os"], "lowest PCI index gets /dev/vda") - assert.Equal(t, "/dev/vdb", result["cloudinit"], "second boot device") - assert.Equal(t, "/dev/vdc", result["vdisk-vol-123"], "hot-plugged device sorts after boot devices") } func TestExtractPeripheralName(t *testing.T) { diff --git a/spinifex/daemon/heartbeat_test.go b/spinifex/daemon/heartbeat_test.go index e20ac3a63..e0d7ccd13 100644 --- a/spinifex/daemon/heartbeat_test.go +++ b/spinifex/daemon/heartbeat_test.go @@ -1,6 +1,7 @@ package daemon import ( + "encoding/json" "testing" "github.com/mulgadc/spinifex/spinifex/config" @@ -90,19 +91,21 @@ func TestHeartbeatReflectsAllocation(t *testing.T) { assert.Less(t, after.AvailableVCPU, before.AvailableVCPU, "AvailableVCPU should decrease") } -// TestHeartbeatKVRoundTrip verifies that heartbeat can be written to and read from KV. -func TestHeartbeatKVRoundTrip(t *testing.T) { +// TestHeartbeatKVContract pins the on-wire JSON shape and the KV key layout +// for WriteHeartbeat/ReadHeartbeat. A renamed struct tag or changed key +// prefix would break cross-version cluster reads — a pure round-trip would +// not catch either, so we assert against the raw KV bytes. +func TestHeartbeatKVContract(t *testing.T) { nc, err := nats.Connect(sharedJSNATSURL) require.NoError(t, err) defer nc.Close() jsm, err := NewJetStreamManager(nc, 1) require.NoError(t, err) - err = jsm.InitClusterStateBucket() - require.NoError(t, err) + require.NoError(t, jsm.InitClusterStateBucket()) h := &Heartbeat{ - Node: "kv-test-node", + Node: "kv-contract-node", Epoch: 3, Timestamp: "2025-01-01T00:00:00Z", Services: []string{"daemon", "nats"}, @@ -113,20 +116,28 @@ func TestHeartbeatKVRoundTrip(t *testing.T) { AvailableMem: 24.0, } - err = jsm.WriteHeartbeat(h) + require.NoError(t, jsm.WriteHeartbeat(h)) + + // Verify the KV key prefix and the JSON tag names directly. Reading back + // via ReadHeartbeat alone would also pass against a typo'd tag. + entry, err := jsm.clusterKV.Get("heartbeat." + h.Node) require.NoError(t, err) - loaded, err := jsm.ReadHeartbeat("kv-test-node") + var raw map[string]any + require.NoError(t, json.Unmarshal(entry.Value(), &raw)) + assert.Equal(t, "kv-contract-node", raw["node"]) + assert.Equal(t, float64(3), raw["epoch"]) + assert.Equal(t, "2025-01-01T00:00:00Z", raw["timestamp"]) + assert.Equal(t, []any{"daemon", "nats"}, raw["services"]) + assert.Equal(t, float64(2), raw["vm_count"]) + assert.Equal(t, float64(4), raw["allocated_vcpu"]) + assert.Equal(t, float64(12), raw["available_vcpu"]) + assert.Equal(t, 8.0, raw["allocated_mem_gb"]) + assert.Equal(t, 24.0, raw["available_mem_gb"]) + + // And ReadHeartbeat decodes the same value. + loaded, err := jsm.ReadHeartbeat(h.Node) require.NoError(t, err) require.NotNil(t, loaded) - - assert.Equal(t, h.Node, loaded.Node) - assert.Equal(t, h.Epoch, loaded.Epoch) - assert.Equal(t, h.Timestamp, loaded.Timestamp) - assert.Equal(t, h.Services, loaded.Services) - assert.Equal(t, h.VMCount, loaded.VMCount) - assert.Equal(t, h.AllocatedVCPU, loaded.AllocatedVCPU) - assert.Equal(t, h.AvailableVCPU, loaded.AvailableVCPU) - assert.Equal(t, h.AllocatedMem, loaded.AllocatedMem) - assert.Equal(t, h.AvailableMem, loaded.AvailableMem) + assert.Equal(t, h, loaded) } diff --git a/spinifex/daemon/network.go b/spinifex/daemon/network.go index dbeff6d5e..580152a2e 100644 --- a/spinifex/daemon/network.go +++ b/spinifex/daemon/network.go @@ -36,6 +36,8 @@ type NetworkPlumber interface { // OVSNetworkPlumber implements NetworkPlumber using system commands. type OVSNetworkPlumber struct{} +var _ NetworkPlumber = (*OVSNetworkPlumber)(nil) + func (p *OVSNetworkPlumber) SetupTapDevice(eniId, mac string) error { tapName := TapDeviceName(eniId) ifaceID := OVSIfaceID(eniId) diff --git a/spinifex/daemon/network_test.go b/spinifex/daemon/network_test.go index 57a587062..7d6b2222f 100644 --- a/spinifex/daemon/network_test.go +++ b/spinifex/daemon/network_test.go @@ -34,25 +34,6 @@ func TestTapDeviceName(t *testing.T) { } } -func TestOVSIfaceID(t *testing.T) { - tests := []struct { - eniId string - expected string - }{ - {"eni-abc123", "port-eni-abc123"}, - {"eni-abc123def456789", "port-eni-abc123def456789"}, - } - - for _, tt := range tests { - t.Run(tt.eniId, func(t *testing.T) { - got := OVSIfaceID(tt.eniId) - if got != tt.expected { - t.Errorf("OVSIfaceID(%q) = %q, want %q", tt.eniId, got, tt.expected) - } - }) - } -} - // MockNetworkPlumber records calls for testing. type MockNetworkPlumber struct { SetupCalls []mockSetupCall @@ -61,6 +42,8 @@ type MockNetworkPlumber struct { CleanupErr error } +var _ NetworkPlumber = (*MockNetworkPlumber)(nil) + type mockSetupCall struct { ENIId string MAC string @@ -232,12 +215,6 @@ func TestGenerateDevMAC(t *testing.T) { } } -func TestNetworkPlumber_InterfaceCompliance(t *testing.T) { - // Verify both types satisfy the interface - var _ NetworkPlumber = &OVSNetworkPlumber{} - var _ NetworkPlumber = &MockNetworkPlumber{} -} - func TestSetupExtraENINICs_AppendsOnePerExtra(t *testing.T) { mock := &MockNetworkPlumber{} d := &Daemon{networkPlumber: mock} @@ -371,62 +348,6 @@ func TestCleanupExtraENITaps_ErrorsAreLogged(t *testing.T) { } } -func TestOVNHealthStatus_Fields(t *testing.T) { - // Verify OVNHealthStatus struct can be used for health reporting - status := OVNHealthStatus{ - BrIntExists: true, - OVNControllerUp: true, - ChassisID: "chassis-node1", - EncapIP: "10.0.0.1", - OVNRemote: "tcp:10.0.0.1:6642", - } - - if !status.BrIntExists { - t.Error("expected BrIntExists to be true") - } - if !status.OVNControllerUp { - t.Error("expected OVNControllerUp to be true") - } - if status.ChassisID != "chassis-node1" { - t.Errorf("ChassisID = %q, want 'chassis-node1'", status.ChassisID) - } - if status.EncapIP != "10.0.0.1" { - t.Errorf("EncapIP = %q, want '10.0.0.1'", status.EncapIP) - } - if status.OVNRemote != "tcp:10.0.0.1:6642" { - t.Errorf("OVNRemote = %q, want 'tcp:10.0.0.1:6642'", status.OVNRemote) - } -} - -func TestOVNHealthStatus_Defaults(t *testing.T) { - // Zero-value OVNHealthStatus should indicate nothing is ready - var status OVNHealthStatus - - if status.BrIntExists { - t.Error("zero-value BrIntExists should be false") - } - if status.OVNControllerUp { - t.Error("zero-value OVNControllerUp should be false") - } - if status.ChassisID != "" { - t.Errorf("zero-value ChassisID should be empty, got %q", status.ChassisID) - } -} - -func TestCheckOVNHealth_ReturnsStatus(t *testing.T) { - // CheckOVNHealth should return a status struct without panicking, - // even when OVS/OVN tools are not installed (CI environment). - // On a dev machine without OVN, all fields will be zero values. - status := CheckOVNHealth() - - // On CI without OVS, both should be false — just verify no panic - _ = status.BrIntExists - _ = status.OVNControllerUp - _ = status.ChassisID - _ = status.EncapIP - _ = status.OVNRemote -} - func TestFindInterfaceByIP_InvalidIP(t *testing.T) { _, err := findInterfaceByIP("not-an-ip") if err == nil { @@ -571,6 +492,8 @@ func TestOVSIfaceID_Format(t *testing.T) { eniId string expected string }{ + {"eni-abc123", "port-eni-abc123"}, + {"eni-abc123def456789", "port-eni-abc123def456789"}, {"eni-short", "port-eni-short"}, {"eni-", "port-eni-"}, {"", "port-"}, diff --git a/spinifex/daemon/shutdown_test.go b/spinifex/daemon/shutdown_test.go index 34fabdd32..d946ce838 100644 --- a/spinifex/daemon/shutdown_test.go +++ b/spinifex/daemon/shutdown_test.go @@ -11,88 +11,6 @@ import ( "github.com/stretchr/testify/require" ) -// TestShutdownACKMarshal verifies JSON round-trip for ShutdownACK. -func TestShutdownACKMarshal(t *testing.T) { - ack := ShutdownACK{ - Node: "node1", - Phase: "gate", - Stopped: []string{"awsgw", "spinifex-ui"}, - } - - data, err := json.Marshal(ack) - require.NoError(t, err) - - var decoded ShutdownACK - err = json.Unmarshal(data, &decoded) - require.NoError(t, err) - - assert.Equal(t, ack.Node, decoded.Node) - assert.Equal(t, ack.Phase, decoded.Phase) - assert.Equal(t, ack.Stopped, decoded.Stopped) - assert.Empty(t, decoded.Error) -} - -// TestShutdownACKWithError verifies JSON round-trip for ShutdownACK with an error. -func TestShutdownACKWithError(t *testing.T) { - ack := ShutdownACK{ - Node: "node2", - Phase: "drain", - Error: "failed to stop VMs", - } - - data, err := json.Marshal(ack) - require.NoError(t, err) - - var decoded ShutdownACK - err = json.Unmarshal(data, &decoded) - require.NoError(t, err) - - assert.Equal(t, "failed to stop VMs", decoded.Error) - assert.Nil(t, decoded.Stopped) -} - -// TestShutdownRequestMarshal verifies JSON round-trip for ShutdownRequest. -func TestShutdownRequestMarshal(t *testing.T) { - req := ShutdownRequest{ - Phase: "drain", - Force: true, - Timeout: 120, - } - - data, err := json.Marshal(req) - require.NoError(t, err) - - var decoded ShutdownRequest - err = json.Unmarshal(data, &decoded) - require.NoError(t, err) - - assert.Equal(t, "drain", decoded.Phase) - assert.True(t, decoded.Force) - assert.Equal(t, 120, decoded.Timeout) -} - -// TestShutdownProgressMarshal verifies JSON round-trip for ShutdownProgress. -func TestShutdownProgressMarshal(t *testing.T) { - progress := ShutdownProgress{ - Node: "node1", - Phase: "drain", - Total: 5, - Remaining: 3, - } - - data, err := json.Marshal(progress) - require.NoError(t, err) - - var decoded ShutdownProgress - err = json.Unmarshal(data, &decoded) - require.NoError(t, err) - - assert.Equal(t, "node1", decoded.Node) - assert.Equal(t, "drain", decoded.Phase) - assert.Equal(t, 5, decoded.Total) - assert.Equal(t, 3, decoded.Remaining) -} - // TestClusterShutdownStateKVRoundTrip verifies cluster shutdown state can be stored and retrieved from KV. func TestClusterShutdownStateKVRoundTrip(t *testing.T) { nc, err := nats.Connect(sharedJSNATSURL) diff --git a/spinifex/handlers/ec2/image/service_impl_test.go b/spinifex/handlers/ec2/image/service_impl_test.go index 74b2b0114..6a714b068 100644 --- a/spinifex/handlers/ec2/image/service_impl_test.go +++ b/spinifex/handlers/ec2/image/service_impl_test.go @@ -315,25 +315,6 @@ func TestDescribeImages_MixedExistingAndMissing(t *testing.T) { assert.Equal(t, awserrors.ErrorInvalidAMIIDNotFound, err.Error()) } -func TestDescribeImages_FilterByOwnerSelf(t *testing.T) { - svc, store := setupTestImageService(t) - - // Create an AMI owned by the caller's account - createTestAMIConfigWithOwner(t, store, "ami-selfowned", "self-owned-ami", testAccountID) - - // Create a system AMI (should not appear with --owners self) - createTestAMIConfigWithOwner(t, store, "ami-system", "system-ami", "spinifex") - - // Filter by "self" should return only the caller's AMI - result, err := svc.DescribeImages(&ec2.DescribeImagesInput{ - Owners: []*string{aws.String("self")}, - }, testAccountID) - require.NoError(t, err) - require.NotNil(t, result) - assert.Len(t, result.Images, 1) - assert.Equal(t, "ami-selfowned", *result.Images[0].ImageId) -} - func TestCreateImageFromInstance_DuplicateName(t *testing.T) { svc, store := setupTestImageService(t) @@ -733,27 +714,6 @@ func TestDescribeImages_WithTags(t *testing.T) { assert.Len(t, img.BlockDeviceMappings, 1) } -func TestDescribeImages_FilterByExplicitOwnerID(t *testing.T) { - svc, store := setupTestImageService(t) - - createTestAMIConfigWithOwner(t, store, "ami-owned1", "owned-ami", testAccountID) - - // Filter by explicit account ID - result, err := svc.DescribeImages(&ec2.DescribeImagesInput{ - Owners: []*string{aws.String(testAccountID)}, - }, testAccountID) - require.NoError(t, err) - require.Len(t, result.Images, 1) - assert.Equal(t, "ami-owned1", *result.Images[0].ImageId) - - // Filter by wrong account ID — should return empty - result, err = svc.DescribeImages(&ec2.DescribeImagesInput{ - Owners: []*string{aws.String("999999999999")}, - }, testAccountID) - require.NoError(t, err) - assert.Empty(t, result.Images) -} - func TestDescribeImages_OwnerFilterNilEntry(t *testing.T) { svc, store := setupTestImageService(t) @@ -865,40 +825,163 @@ func createTestAMIConfigFull(t *testing.T, store *objectstore.MemoryObjectStore, require.NoError(t, err) } -func TestDescribeImages_FilterByName(t *testing.T) { - svc, store := setupTestImageService(t) - createTestAMIConfigWithName(t, store, "ami-aaa", "debian-12") - createTestAMIConfigWithName(t, store, "ami-bbb", "ubuntu-22") - - out, err := svc.DescribeImages(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - {Name: aws.String("name"), Values: []*string{aws.String("debian-12")}}, +// TestDescribeImages_FilterBy verifies single-attribute filters. Each subtest +// builds its own AMI fixtures and asserts which IDs the filter selects. +// Multi-filter, wildcard, error-path, and no-filter cases live in their own +// dedicated tests below. +func TestDescribeImages_FilterBy(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, store *objectstore.MemoryObjectStore) + input *ec2.DescribeImagesInput + wantIDs []string // matched image IDs (order not asserted) + wantNone bool // expect zero results + }{ + { + name: "owner self excludes other owners", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigWithOwner(t, store, "ami-selfowned", "self-owned-ami", testAccountID) + createTestAMIConfigWithOwner(t, store, "ami-system", "system-ami", "spinifex") + }, + input: &ec2.DescribeImagesInput{Owners: []*string{aws.String("self")}}, + wantIDs: []string{"ami-selfowned"}, }, - }, testAccountID) - require.NoError(t, err) - assert.Len(t, out.Images, 1) - assert.Equal(t, "ami-aaa", *out.Images[0].ImageId) -} - -func TestDescribeImages_FilterByArchitecture(t *testing.T) { - svc, store := setupTestImageService(t) - createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ - ImageID: "ami-x86", Name: "x86-img", Architecture: "x86_64", - RootDeviceType: "ebs", VolumeSizeGiB: 8, - }) - createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ - ImageID: "ami-arm", Name: "arm-img", Architecture: "arm64", - RootDeviceType: "ebs", VolumeSizeGiB: 8, - }) - - out, err := svc.DescribeImages(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - {Name: aws.String("architecture"), Values: []*string{aws.String("arm64")}}, + { + name: "explicit owner ID matches caller", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigWithOwner(t, store, "ami-owned1", "owned-ami", testAccountID) + }, + input: &ec2.DescribeImagesInput{Owners: []*string{aws.String(testAccountID)}}, + wantIDs: []string{"ami-owned1"}, }, - }, testAccountID) - require.NoError(t, err) - assert.Len(t, out.Images, 1) - assert.Equal(t, "ami-arm", *out.Images[0].ImageId) + { + name: "explicit owner ID with wrong account returns empty", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigWithOwner(t, store, "ami-owned1", "owned-ami", testAccountID) + }, + input: &ec2.DescribeImagesInput{Owners: []*string{aws.String("999999999999")}}, + wantNone: true, + }, + { + name: "name", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigWithName(t, store, "ami-aaa", "debian-12") + createTestAMIConfigWithName(t, store, "ami-bbb", "ubuntu-22") + }, + input: &ec2.DescribeImagesInput{Filters: []*ec2.Filter{ + {Name: aws.String("name"), Values: []*string{aws.String("debian-12")}}, + }}, + wantIDs: []string{"ami-aaa"}, + }, + { + name: "architecture", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ + ImageID: "ami-x86", Name: "x86-img", Architecture: "x86_64", + RootDeviceType: "ebs", VolumeSizeGiB: 8, + }) + createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ + ImageID: "ami-arm", Name: "arm-img", Architecture: "arm64", + RootDeviceType: "ebs", VolumeSizeGiB: 8, + }) + }, + input: &ec2.DescribeImagesInput{Filters: []*ec2.Filter{ + {Name: aws.String("architecture"), Values: []*string{aws.String("arm64")}}, + }}, + wantIDs: []string{"ami-arm"}, + }, + { + name: "tag", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ + ImageID: "ami-tagged", Name: "tagged-img", Architecture: "x86_64", + RootDeviceType: "ebs", VolumeSizeGiB: 8, + Tags: map[string]string{"Environment": "prod"}, + }) + createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ + ImageID: "ami-untagged", Name: "untagged-img", Architecture: "x86_64", + RootDeviceType: "ebs", VolumeSizeGiB: 8, + }) + }, + input: &ec2.DescribeImagesInput{Filters: []*ec2.Filter{ + {Name: aws.String("tag:Environment"), Values: []*string{aws.String("prod")}}, + }}, + wantIDs: []string{"ami-tagged"}, + }, + { + name: "state available", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigWithName(t, store, "ami-aaa", "test") + }, + input: &ec2.DescribeImagesInput{Filters: []*ec2.Filter{ + {Name: aws.String("state"), Values: []*string{aws.String("available")}}, + }}, + wantIDs: []string{"ami-aaa"}, + }, + { + name: "state deregistered returns empty", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigWithName(t, store, "ami-aaa", "test") + }, + input: &ec2.DescribeImagesInput{Filters: []*ec2.Filter{ + {Name: aws.String("state"), Values: []*string{aws.String("deregistered")}}, + }}, + wantNone: true, + }, + { + name: "virtualization type", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ + ImageID: "ami-hvm", Name: "hvm-img", Architecture: "x86_64", + Virtualization: "hvm", RootDeviceType: "ebs", VolumeSizeGiB: 8, + }) + createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ + ImageID: "ami-pv", Name: "pv-img", Architecture: "x86_64", + Virtualization: "paravirtual", RootDeviceType: "ebs", VolumeSizeGiB: 8, + }) + }, + input: &ec2.DescribeImagesInput{Filters: []*ec2.Filter{ + {Name: aws.String("virtualization-type"), Values: []*string{aws.String("hvm")}}, + }}, + wantIDs: []string{"ami-hvm"}, + }, + { + name: "root device type", + setup: func(t *testing.T, store *objectstore.MemoryObjectStore) { + createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ + ImageID: "ami-ebs", Name: "ebs-img", Architecture: "x86_64", + Virtualization: "hvm", RootDeviceType: "ebs", VolumeSizeGiB: 8, + }) + createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ + ImageID: "ami-is", Name: "is-img", Architecture: "x86_64", + Virtualization: "hvm", RootDeviceType: "instance-store", VolumeSizeGiB: 8, + }) + }, + input: &ec2.DescribeImagesInput{Filters: []*ec2.Filter{ + {Name: aws.String("root-device-type"), Values: []*string{aws.String("ebs")}}, + }}, + wantIDs: []string{"ami-ebs"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, store := setupTestImageService(t) + tt.setup(t, store) + + out, err := svc.DescribeImages(tt.input, testAccountID) + require.NoError(t, err) + if tt.wantNone { + assert.Empty(t, out.Images) + return + } + gotIDs := make([]string, len(out.Images)) + for i, img := range out.Images { + gotIDs[i] = aws.StringValue(img.ImageId) + } + assert.ElementsMatch(t, tt.wantIDs, gotIDs) + }) + } } func TestDescribeImages_FilterMultipleValues_OR(t *testing.T) { @@ -987,92 +1070,6 @@ func TestDescribeImages_FilterNoFilters(t *testing.T) { assert.Len(t, out.Images, 2) } -func TestDescribeImages_FilterByTag(t *testing.T) { - svc, store := setupTestImageService(t) - createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ - ImageID: "ami-tagged", Name: "tagged-img", Architecture: "x86_64", - RootDeviceType: "ebs", VolumeSizeGiB: 8, - Tags: map[string]string{"Environment": "prod"}, - }) - createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ - ImageID: "ami-untagged", Name: "untagged-img", Architecture: "x86_64", - RootDeviceType: "ebs", VolumeSizeGiB: 8, - }) - - out, err := svc.DescribeImages(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - {Name: aws.String("tag:Environment"), Values: []*string{aws.String("prod")}}, - }, - }, testAccountID) - require.NoError(t, err) - assert.Len(t, out.Images, 1) - assert.Equal(t, "ami-tagged", *out.Images[0].ImageId) -} - -func TestDescribeImages_FilterByState(t *testing.T) { - svc, store := setupTestImageService(t) - createTestAMIConfigWithName(t, store, "ami-aaa", "test") - - // All images have state "available" - out, err := svc.DescribeImages(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - {Name: aws.String("state"), Values: []*string{aws.String("available")}}, - }, - }, testAccountID) - require.NoError(t, err) - assert.Len(t, out.Images, 1) - - out, err = svc.DescribeImages(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - {Name: aws.String("state"), Values: []*string{aws.String("deregistered")}}, - }, - }, testAccountID) - require.NoError(t, err) - assert.Empty(t, out.Images) -} - -func TestDescribeImages_FilterByVirtualizationType(t *testing.T) { - svc, store := setupTestImageService(t) - createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ - ImageID: "ami-hvm", Name: "hvm-img", Architecture: "x86_64", - Virtualization: "hvm", RootDeviceType: "ebs", VolumeSizeGiB: 8, - }) - createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ - ImageID: "ami-pv", Name: "pv-img", Architecture: "x86_64", - Virtualization: "paravirtual", RootDeviceType: "ebs", VolumeSizeGiB: 8, - }) - - out, err := svc.DescribeImages(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - {Name: aws.String("virtualization-type"), Values: []*string{aws.String("hvm")}}, - }, - }, testAccountID) - require.NoError(t, err) - assert.Len(t, out.Images, 1) - assert.Equal(t, "ami-hvm", *out.Images[0].ImageId) -} - -func TestDescribeImages_FilterByRootDeviceType(t *testing.T) { - svc, store := setupTestImageService(t) - createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ - ImageID: "ami-ebs", Name: "ebs-img", Architecture: "x86_64", - Virtualization: "hvm", RootDeviceType: "ebs", VolumeSizeGiB: 8, - }) - createTestAMIConfigFull(t, store, viperblock.AMIMetadata{ - ImageID: "ami-is", Name: "is-img", Architecture: "x86_64", - Virtualization: "hvm", RootDeviceType: "instance-store", VolumeSizeGiB: 8, - }) - - out, err := svc.DescribeImages(&ec2.DescribeImagesInput{ - Filters: []*ec2.Filter{ - {Name: aws.String("root-device-type"), Values: []*string{aws.String("ebs")}}, - }, - }, testAccountID) - require.NoError(t, err) - assert.Len(t, out.Images, 1) - assert.Equal(t, "ami-ebs", *out.Images[0].ImageId) -} - // --- RegisterImage tests --- // putTestSnapshotConfig writes a SnapshotConfig at {snapshotID}/metadata.json, diff --git a/spinifex/handlers/ec2/instance/service_impl_test.go b/spinifex/handlers/ec2/instance/service_impl_test.go index b92d86789..76c09d8be 100644 --- a/spinifex/handlers/ec2/instance/service_impl_test.go +++ b/spinifex/handlers/ec2/instance/service_impl_test.go @@ -10,32 +10,12 @@ 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/config" - "github.com/mulgadc/spinifex/spinifex/objectstore" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/mulgadc/spinifex/spinifex/vm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestNewInstanceServiceImpl(t *testing.T) { - cfg := &config.Config{} - instanceTypes := map[string]*ec2.InstanceTypeInfo{ - "t3.micro": {InstanceType: aws.String("t3.micro")}, - } - store := objectstore.NewMemoryObjectStore() - instances := &vm.Instances{VMS: make(map[string]*vm.VM)} - - svc := NewInstanceServiceImpl(cfg, instanceTypes, nil, instances, store) - - require.NotNil(t, svc) - assert.Equal(t, cfg, svc.config) - assert.Equal(t, instanceTypes, svc.instanceTypes) - assert.Nil(t, svc.natsConn) - assert.Equal(t, instances, svc.instances) - assert.Equal(t, store, svc.objectStore) -} - func TestGenerateHostname(t *testing.T) { tests := []struct { name string diff --git a/spinifex/handlers/ec2/key/service_impl_test.go b/spinifex/handlers/ec2/key/service_impl_test.go index e7a2923d3..5b0c709c6 100644 --- a/spinifex/handlers/ec2/key/service_impl_test.go +++ b/spinifex/handlers/ec2/key/service_impl_test.go @@ -52,27 +52,6 @@ func importTestKey(t *testing.T, svc *KeyServiceImpl, keyName string) *ec2.Impor return out } -// --- TestKeyPairMetadataMarshaling (kept — validates JSON roundtrip independently) --- - -func TestKeyPairMetadataMarshaling(t *testing.T) { - metadata := ec2.CreateKeyPairOutput{ - KeyPairId: aws.String("key-12345"), - KeyFingerprint: aws.String("SHA256:abcdef1234567890"), - KeyName: aws.String("test-key"), - } - - data, err := json.Marshal(metadata) - require.NoError(t, err) - - var unmarshaled ec2.CreateKeyPairOutput - err = json.Unmarshal(data, &unmarshaled) - require.NoError(t, err) - - assert.Equal(t, *metadata.KeyPairId, *unmarshaled.KeyPairId) - assert.Equal(t, *metadata.KeyFingerprint, *unmarshaled.KeyFingerprint) - assert.Equal(t, *metadata.KeyName, *unmarshaled.KeyName) -} - // ============================================================ // CreateKeyPair Tests // ============================================================ diff --git a/spinifex/handlers/ec2/volume/service_impl_test.go b/spinifex/handlers/ec2/volume/service_impl_test.go index 28d5eb50d..abc37b6ac 100644 --- a/spinifex/handlers/ec2/volume/service_impl_test.go +++ b/spinifex/handlers/ec2/volume/service_impl_test.go @@ -1444,16 +1444,6 @@ func TestDescribeVolumeStatus_SlowPath_SkipsBrokenConfig(t *testing.T) { assert.Len(t, output.VolumeStatuses, 1) } -func TestNewVolumeServiceImplWithStore_WithSnapshotKV(t *testing.T) { - kv := setupTestVolumeKV(t) - store := objectstore.NewMemoryObjectStore() - cfg := &config.Config{ - Predastore: config.PredastoreConfig{Bucket: "test-bucket"}, - } - svc := NewVolumeServiceImplWithStore(cfg, store, nil, kv) - assert.NotNil(t, svc.snapshotKV) -} - func TestDeleteVolume_NATSErrorResponse(t *testing.T) { kv := setupTestVolumeKV(t) store := objectstore.NewMemoryObjectStore() diff --git a/spinifex/handlers/elbv2/agent_types_test.go b/spinifex/handlers/elbv2/agent_types_test.go index 81508bd8e..e42d37780 100644 --- a/spinifex/handlers/elbv2/agent_types_test.go +++ b/spinifex/handlers/elbv2/agent_types_test.go @@ -8,72 +8,83 @@ import ( "github.com/stretchr/testify/assert" ) -func TestToHealthReport_AllFieldsPopulated(t *testing.T) { - in := &LBAgentHeartbeatInput{ - LBID: aws.String("lb-abc123"), - Servers: []*LBAgentServerStatus{ - {Backend: aws.String("tg-1"), Server: aws.String("10.0.0.1:80"), Status: aws.String("UP")}, - {Backend: aws.String("tg-2"), Server: aws.String("10.0.0.2:80"), Status: aws.String("DOWN")}, +// TestToHealthReport verifies the LBAgentHeartbeatInput.toHealthReport conversion, +// in particular that aws.StringValue handles nil pointers without panicking. +func TestToHealthReport(t *testing.T) { + tests := []struct { + name string + in *LBAgentHeartbeatInput + wantLB string + wantSrv []lbagent.ServerStatus + }{ + { + name: "all fields populated", + in: &LBAgentHeartbeatInput{ + LBID: aws.String("lb-abc123"), + Servers: []*LBAgentServerStatus{ + {Backend: aws.String("tg-1"), Server: aws.String("10.0.0.1:80"), Status: aws.String("UP")}, + {Backend: aws.String("tg-2"), Server: aws.String("10.0.0.2:80"), Status: aws.String("DOWN")}, + }, + }, + wantLB: "lb-abc123", + wantSrv: []lbagent.ServerStatus{ + {Backend: "tg-1", Server: "10.0.0.1:80", Status: "UP"}, + {Backend: "tg-2", Server: "10.0.0.2:80", Status: "DOWN"}, + }, + }, + { + name: "nil LBID", + in: &LBAgentHeartbeatInput{ + LBID: nil, + Servers: []*LBAgentServerStatus{ + {Backend: aws.String("tg-1"), Server: aws.String("10.0.0.1:80"), Status: aws.String("UP")}, + }, + }, + wantLB: "", + wantSrv: []lbagent.ServerStatus{ + {Backend: "tg-1", Server: "10.0.0.1:80", Status: "UP"}, + }, + }, + { + name: "nil servers", + in: &LBAgentHeartbeatInput{LBID: aws.String("lb-abc123"), Servers: nil}, + wantLB: "lb-abc123", + wantSrv: nil, + }, + { + name: "empty input", + in: &LBAgentHeartbeatInput{}, + wantLB: "", + wantSrv: nil, + }, + { + name: "nil server fields", + in: &LBAgentHeartbeatInput{ + LBID: aws.String("lb-abc123"), + Servers: []*LBAgentServerStatus{ + {Backend: nil, Server: nil, Status: nil}, + {Backend: aws.String("tg-1"), Server: nil, Status: aws.String("MAINT")}, + {Backend: nil, Server: aws.String("10.0.0.3:80"), Status: nil}, + }, + }, + wantLB: "lb-abc123", + wantSrv: []lbagent.ServerStatus{ + {Backend: "", Server: "", Status: ""}, + {Backend: "tg-1", Server: "", Status: "MAINT"}, + {Backend: "", Server: "10.0.0.3:80", Status: ""}, + }, }, } - report := in.toHealthReport() - - assert.Equal(t, "lb-abc123", report.LBID) - assert.Len(t, report.Servers, 2) - assert.Equal(t, lbagent.ServerStatus{Backend: "tg-1", Server: "10.0.0.1:80", Status: "UP"}, report.Servers[0]) - assert.Equal(t, lbagent.ServerStatus{Backend: "tg-2", Server: "10.0.0.2:80", Status: "DOWN"}, report.Servers[1]) -} - -func TestToHealthReport_NilLBID(t *testing.T) { - in := &LBAgentHeartbeatInput{ - LBID: nil, - Servers: []*LBAgentServerStatus{ - {Backend: aws.String("tg-1"), Server: aws.String("10.0.0.1:80"), Status: aws.String("UP")}, - }, + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + report := tt.in.toHealthReport() + assert.Equal(t, tt.wantLB, report.LBID) + if tt.wantSrv == nil { + assert.Empty(t, report.Servers) + } else { + assert.Equal(t, tt.wantSrv, report.Servers) + } + }) } - - report := in.toHealthReport() - - assert.Equal(t, "", report.LBID) - assert.Len(t, report.Servers, 1) -} - -func TestToHealthReport_NilServers(t *testing.T) { - in := &LBAgentHeartbeatInput{ - LBID: aws.String("lb-abc123"), - Servers: nil, - } - - report := in.toHealthReport() - - assert.Equal(t, "lb-abc123", report.LBID) - assert.Empty(t, report.Servers) -} - -func TestToHealthReport_EmptyInput(t *testing.T) { - in := &LBAgentHeartbeatInput{} - - report := in.toHealthReport() - - assert.Equal(t, "", report.LBID) - assert.Empty(t, report.Servers) -} - -func TestToHealthReport_NilServerFields(t *testing.T) { - in := &LBAgentHeartbeatInput{ - LBID: aws.String("lb-abc123"), - Servers: []*LBAgentServerStatus{ - {Backend: nil, Server: nil, Status: nil}, - {Backend: aws.String("tg-1"), Server: nil, Status: aws.String("MAINT")}, - {Backend: nil, Server: aws.String("10.0.0.3:80"), Status: nil}, - }, - } - - report := in.toHealthReport() - - assert.Len(t, report.Servers, 3) - assert.Equal(t, lbagent.ServerStatus{Backend: "", Server: "", Status: ""}, report.Servers[0]) - assert.Equal(t, lbagent.ServerStatus{Backend: "tg-1", Server: "", Status: "MAINT"}, report.Servers[1]) - assert.Equal(t, lbagent.ServerStatus{Backend: "", Server: "10.0.0.3:80", Status: ""}, report.Servers[2]) } diff --git a/spinifex/handlers/elbv2/arn_test.go b/spinifex/handlers/elbv2/arn_test.go index 7faea1a06..9f1c475ba 100644 --- a/spinifex/handlers/elbv2/arn_test.go +++ b/spinifex/handlers/elbv2/arn_test.go @@ -8,6 +8,59 @@ import ( "github.com/stretchr/testify/require" ) +// extractAgentEnv parses the cloud-config user-data into the env-var map that +// cloud-init will write to /etc/conf.d/lb-agent. We don't pull in a YAML +// dependency just for tests — the format is stable enough that line-based +// parsing catches the things substring matching misses (misplaced keys, +// broken indentation, content that escaped the agent-conf block). +// +// Returns the parsed env map plus the `runcmd:` section's body (raw lines) +// so callers can assert on the launch command shape. +func extractAgentEnv(t *testing.T, ud string) (envs map[string]string, runcmd []string) { + t.Helper() + require.True(t, strings.HasPrefix(ud, "#cloud-config\n"), + "user-data must start with #cloud-config directive") + + envs = map[string]string{} + const agentPathMarker = " - path: /etc/conf.d/lb-agent" + const contentMarker = " content: |" + const envIndent = " " // 6 spaces under `content: |` + + lines := strings.Split(ud, "\n") + inAgentEnv := false + inRuncmd := false + for i := 0; i < len(lines); i++ { + line := lines[i] + switch { + case line == agentPathMarker: + require.Less(t, i+1, len(lines), "agent path entry truncated") + require.Equal(t, contentMarker, lines[i+1], + "agent path must be followed by `content: |` block") + inAgentEnv = true + i++ // skip the content marker + case inAgentEnv: + if !strings.HasPrefix(line, envIndent) { + inAgentEnv = false + i-- // re-process this line in case it starts another section + continue + } + kv := strings.TrimPrefix(line, envIndent) + k, v, ok := strings.Cut(kv, "=") + require.True(t, ok, "agent env line missing '=': %q", line) + envs[k] = v + case line == "runcmd:": + inRuncmd = true + case inRuncmd: + if !strings.HasPrefix(line, " ") { + inRuncmd = false + continue + } + runcmd = append(runcmd, line) + } + } + return envs, runcmd +} + func TestBuildLBArn(t *testing.T) { arn := buildLBArn("us-east-1", "123456789012", "my-alb", "50dc6c495c0c9188", LoadBalancerTypeApplication) assert.Equal(t, "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-alb/50dc6c495c0c9188", arn) @@ -33,7 +86,12 @@ func TestBuildListenerArn_NLB(t *testing.T) { assert.Equal(t, "arn:aws:elasticloadbalancing:eu-west-1:999888777666:listener/net/my-nlb/lbid123/listener456", arn) } -func TestLbVMUserData_ContainsLBID(t *testing.T) { +// TestLbVMUserData_Structural parses the generated cloud-config and asserts +// the structure: agent env vars land in /etc/conf.d/lb-agent, runcmd starts +// lb-agent via OpenRC (not the bare binary), the CA cert section — owned by +// the instance service's cloud-init template — is absent, and no bootcmd is +// emitted in the single-node default. +func TestLbVMUserData_Structural(t *testing.T) { svc := &ELBv2ServiceImpl{ GatewayURL: "https://192.168.1.33:9999", SystemAccessKey: "AKIAIOSFODNN7EXAMPLE", @@ -42,79 +100,56 @@ func TestLbVMUserData_ContainsLBID(t *testing.T) { } ud, err := svc.lbVMUserData("lb-abc123") require.NoError(t, err) - assert.Contains(t, ud, "#cloud-config") - assert.Contains(t, ud, "write_files:") - assert.Contains(t, ud, "/etc/conf.d/lb-agent") - assert.Contains(t, ud, "LB_LB_ID=lb-abc123") - assert.Contains(t, ud, "LB_GATEWAY_URL=https://192.168.1.33:9999") - assert.Contains(t, ud, "LB_ACCESS_KEY=AKIAIOSFODNN7EXAMPLE") - assert.Contains(t, ud, "LB_SECRET_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY") - assert.Contains(t, ud, "LB_REGION=ap-southeast-2") - // NATS URL should no longer be present - assert.NotContains(t, ud, "NATS") -} -func TestLbVMUserData_WriteFilesThenRuncmd(t *testing.T) { - // Cloud-init guarantees write_files runs before runcmd. The agent is NOT - // enabled at boot via OpenRC — cloud-init is the sole trigger. - svc := &ELBv2ServiceImpl{ - GatewayURL: "https://10.0.2.2:9999", - SystemAccessKey: "AKID", - SystemSecretKey: "SECRET", + envs, runcmd := extractAgentEnv(t, ud) + assert.Equal(t, map[string]string{ + "LB_LB_ID": "lb-abc123", + "LB_GATEWAY_URL": "https://192.168.1.33:9999", + "LB_ACCESS_KEY": "AKIAIOSFODNN7EXAMPLE", + "LB_SECRET_KEY": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + "LB_REGION": "ap-southeast-2", + }, envs) + for k := range envs { + assert.NotContains(t, k, "NATS", "NATS URL must not leak into agent config") } - ud, err := svc.lbVMUserData("lb-test") - require.NoError(t, err) - assert.Contains(t, ud, "write_files:") - assert.Contains(t, ud, "/etc/conf.d/lb-agent") - assert.Contains(t, ud, "rc-service") - // Must not invoke the binary directly — OpenRC manages the process. + + // Agent is launched via OpenRC, never as a bare binary path. + require.Len(t, runcmd, 1) + assert.Contains(t, runcmd[0], `[ "rc-service", "lb-agent", "start" ]`) assert.NotContains(t, ud, "/usr/local/bin/lb-agent") - // write_files must appear before runcmd in the output - wfIdx := strings.Index(ud, "write_files:") - rcIdx := strings.Index(ud, "runcmd:") - assert.Greater(t, rcIdx, wfIdx, "write_files must precede runcmd") -} - -func TestLbVMUserData_NoCACert(t *testing.T) { - // CA cert is injected by the instance service's cloud-init template, - // not by lbVMUserData — verify it's not duplicated here. - svc := &ELBv2ServiceImpl{ - GatewayURL: "https://192.168.1.33:9999", - SystemAccessKey: "AKID", - SystemSecretKey: "SECRET", - } - ud, err := svc.lbVMUserData("lb-noca") - require.NoError(t, err) + // CA cert is injected upstream by the instance service's template. assert.NotContains(t, ud, "ca_certs:") + + // Single-node default: no MgmtRoute set means no bootcmd. + assert.NotContains(t, ud, "bootcmd:") } -func TestLbVMUserData_EmptyGatewayURL(t *testing.T) { - svc := &ELBv2ServiceImpl{ - SystemAccessKey: "AKID", - SystemSecretKey: "SECRET", +// TestLbVMUserData_MissingCredentials covers the three required-field error +// paths (gateway URL, access key, secret key) in one table-driven test. +func TestLbVMUserData_MissingCredentials(t *testing.T) { + tests := []struct { + name string + svc *ELBv2ServiceImpl + }{ + { + name: "missing GatewayURL", + svc: &ELBv2ServiceImpl{SystemAccessKey: "AKID", SystemSecretKey: "SECRET"}, + }, + { + name: "missing SystemAccessKey", + svc: &ELBv2ServiceImpl{GatewayURL: "https://10.0.0.1:9999", SystemSecretKey: "SECRET"}, + }, + { + name: "missing SystemSecretKey", + svc: &ELBv2ServiceImpl{GatewayURL: "https://10.0.0.1:9999", SystemAccessKey: "AKID"}, + }, } - _, err := svc.lbVMUserData("lb-test") - require.Error(t, err) - assert.Contains(t, err.Error(), "missing system credentials") -} - -func TestLbVMUserData_EmptyAccessKey(t *testing.T) { - svc := &ELBv2ServiceImpl{ - GatewayURL: "https://10.0.0.1:9999", - SystemSecretKey: "SECRET", + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := tt.svc.lbVMUserData("lb-test") + require.Error(t, err) + assert.Contains(t, err.Error(), "missing system credentials") + }) } - _, err := svc.lbVMUserData("lb-test") - require.Error(t, err) - assert.Contains(t, err.Error(), "missing system credentials") -} - -func TestLbVMUserData_EmptySecretKey(t *testing.T) { - svc := &ELBv2ServiceImpl{ - GatewayURL: "https://10.0.0.1:9999", - SystemAccessKey: "AKID", - } - _, err := svc.lbVMUserData("lb-test") - require.Error(t, err) - assert.Contains(t, err.Error(), "missing system credentials") } diff --git a/spinifex/handlers/elbv2/coverage_test.go b/spinifex/handlers/elbv2/coverage_test.go index 8d9bb8773..95aaf0578 100644 --- a/spinifex/handlers/elbv2/coverage_test.go +++ b/spinifex/handlers/elbv2/coverage_test.go @@ -9,47 +9,10 @@ import ( "github.com/stretchr/testify/require" ) -// --- CreateTargetGroup: all health check fields + target type + tags --- +// Tests in this file cover paths NOT exercised by service_impl_test.go: +// filter-by-ARN family, missing-ARN error paths, RegisterTargets nil/not-found edges. -func TestCreateTargetGroup_AllHealthCheckFields(t *testing.T) { - svc := setupTestService(t) - - out, err := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{ - Name: aws.String("full-hc"), - Port: aws.Int64(8080), - Protocol: aws.String("HTTPS"), - VpcId: aws.String("vpc-123"), - TargetType: aws.String("ip"), - HealthCheckProtocol: aws.String("HTTPS"), - HealthCheckPort: aws.String("8443"), - HealthCheckPath: aws.String("/health"), - HealthCheckIntervalSeconds: aws.Int64(15), - HealthCheckTimeoutSeconds: aws.Int64(5), - HealthyThresholdCount: aws.Int64(3), - UnhealthyThresholdCount: aws.Int64(2), - Matcher: &elbv2.Matcher{HttpCode: aws.String("200")}, - Tags: []*elbv2.Tag{ - {Key: aws.String("env"), Value: aws.String("prod")}, - {Key: aws.String("team"), Value: aws.String("platform")}, - }, - }, testAccountID) - - require.NoError(t, err) - tg := out.TargetGroups[0] - assert.Equal(t, "ip", *tg.TargetType) - assert.Equal(t, "HTTPS", *tg.HealthCheckProtocol) - assert.Equal(t, "8443", *tg.HealthCheckPort) - assert.Equal(t, "/health", *tg.HealthCheckPath) - assert.Equal(t, int64(15), *tg.HealthCheckIntervalSeconds) - assert.Equal(t, int64(5), *tg.HealthCheckTimeoutSeconds) - assert.Equal(t, int64(3), *tg.HealthyThresholdCount) - assert.Equal(t, int64(2), *tg.UnhealthyThresholdCount) - assert.Equal(t, "200", *tg.Matcher.HttpCode) - assert.Equal(t, int64(8080), *tg.Port) - assert.Equal(t, "vpc-123", *tg.VpcId) -} - -// --- DescribeLoadBalancers: ARN filter --- +// --- DescribeLoadBalancers: ARN filter (service_impl_test.go covers FilterByName) --- func TestDescribeLoadBalancers_FilterByArn(t *testing.T) { svc := setupTestService(t) @@ -65,7 +28,7 @@ func TestDescribeLoadBalancers_FilterByArn(t *testing.T) { assert.Equal(t, "lb-arn-a", *desc.LoadBalancers[0].LoadBalancerName) } -// --- DescribeTargetGroups: by name --- +// --- DescribeTargetGroups: by name and by ARN (service_impl_test.go only covers FilterByLBArn) --- func TestDescribeTargetGroups_FilterByName(t *testing.T) { svc := setupTestService(t) @@ -81,8 +44,6 @@ func TestDescribeTargetGroups_FilterByName(t *testing.T) { assert.Equal(t, "tg-alpha", *desc.TargetGroups[0].TargetGroupName) } -// --- DescribeTargetGroups: by ARN --- - func TestDescribeTargetGroups_FilterByArn(t *testing.T) { svc := setupTestService(t) @@ -97,7 +58,7 @@ func TestDescribeTargetGroups_FilterByArn(t *testing.T) { assert.Equal(t, "tg-arn-a", *desc.TargetGroups[0].TargetGroupName) } -// --- DescribeListeners: filter by listener ARN --- +// --- DescribeListeners: filter by listener ARN (service_impl_test.go covers FilterByLBArn) --- func TestDescribeListeners_FilterByListenerArn(t *testing.T) { svc := setupTestService(t) @@ -124,7 +85,7 @@ func TestDescribeListeners_FilterByListenerArn(t *testing.T) { assert.Equal(t, int64(80), *desc.Listeners[0].Port) } -// --- RegisterTargets: nil target id skipped --- +// --- RegisterTargets edge cases --- func TestRegisterTargets_NilTargetSkipped(t *testing.T) { svc := setupTestService(t) @@ -147,17 +108,12 @@ func TestRegisterTargets_NilTargetSkipped(t *testing.T) { assert.Equal(t, "i-valid", *health.TargetHealthDescriptions[0].Target.Id) } -// --- RegisterTargets / DeregisterTargets: missing ARN --- - -func TestRegisterTargets_MissingArn(t *testing.T) { +func TestRegisterTargets_TGNotFound(t *testing.T) { svc := setupTestService(t) - _, err := svc.RegisterTargets(&elbv2.RegisterTargetsInput{}, testAccountID) - assert.Error(t, err) -} - -func TestDeregisterTargets_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DeregisterTargets(&elbv2.DeregisterTargetsInput{}, testAccountID) + _, err := svc.RegisterTargets(&elbv2.RegisterTargetsInput{ + TargetGroupArn: aws.String("arn:nonexistent"), + Targets: []*elbv2.TargetDescription{{Id: aws.String("i-abc")}}, + }, testAccountID) assert.Error(t, err) } @@ -169,26 +125,76 @@ func TestDeregisterTargets_TGNotFound(t *testing.T) { assert.Error(t, err) } -// --- CreateListener: custom protocol --- +// --- Missing-ARN error paths across the service surface --- -func TestCreateListener_CustomProtocol(t *testing.T) { +func TestMissingArnReturnsError(t *testing.T) { svc := setupTestService(t) - lb, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-proto")}, testAccountID) - tg, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-proto")}, testAccountID) + tests := []struct { + name string + call func() error + }{ + { + name: "DeleteLoadBalancer", + call: func() error { + _, err := svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{}, testAccountID) + return err + }, + }, + { + name: "DeleteTargetGroup", + call: func() error { + _, err := svc.DeleteTargetGroup(&elbv2.DeleteTargetGroupInput{}, testAccountID) + return err + }, + }, + { + name: "DeleteListener", + call: func() error { + _, err := svc.DeleteListener(&elbv2.DeleteListenerInput{}, testAccountID) + return err + }, + }, + { + name: "RegisterTargets", + call: func() error { + _, err := svc.RegisterTargets(&elbv2.RegisterTargetsInput{}, testAccountID) + return err + }, + }, + { + name: "DeregisterTargets", + call: func() error { + _, err := svc.DeregisterTargets(&elbv2.DeregisterTargetsInput{}, testAccountID) + return err + }, + }, + { + name: "DescribeTargetHealth", + call: func() error { + _, err := svc.DescribeTargetHealth(&elbv2.DescribeTargetHealthInput{}, testAccountID) + return err + }, + }, + { + name: "CreateListener missing LB ARN", + call: func() error { + _, err := svc.CreateListener(&elbv2.CreateListenerInput{ + DefaultActions: []*elbv2.Action{{Type: aws.String("forward")}}, + }, testAccountID) + return err + }, + }, + } - out, err := svc.CreateListener(&elbv2.CreateListenerInput{ - LoadBalancerArn: lb.LoadBalancers[0].LoadBalancerArn, - Protocol: aws.String("HTTPS"), - Port: aws.Int64(443), - DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tg.TargetGroups[0].TargetGroupArn}}, - }, testAccountID) - require.NoError(t, err) - assert.Equal(t, "HTTPS", *out.Listeners[0].Protocol) - assert.Equal(t, int64(443), *out.Listeners[0].Port) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Error(t, tt.call()) + }) + } } -// --- CreateListener: missing actions --- +// --- CreateListener: missing actions (separate from missing-ARN path) --- func TestCreateListener_MissingActions(t *testing.T) { svc := setupTestService(t) @@ -199,99 +205,3 @@ func TestCreateListener_MissingActions(t *testing.T) { }, testAccountID) assert.Error(t, err) } - -// --- DeleteTargetGroup: missing ARN --- - -func TestDeleteTargetGroup_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DeleteTargetGroup(&elbv2.DeleteTargetGroupInput{}, testAccountID) - assert.Error(t, err) -} - -// --- DeleteListener: missing ARN --- - -func TestDeleteListener_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DeleteListener(&elbv2.DeleteListenerInput{}, testAccountID) - assert.Error(t, err) -} - -// --- DescribeTargetHealth: missing ARN --- - -func TestDescribeTargetHealth_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DescribeTargetHealth(&elbv2.DescribeTargetHealthInput{}, testAccountID) - assert.Error(t, err) -} - -// --- DescribeLoadBalancers: empty filters returns all --- - -func TestDescribeLoadBalancers_All(t *testing.T) { - svc := setupTestService(t) - - _, _ = svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-all-a")}, testAccountID) - _, _ = svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-all-b")}, testAccountID) - - desc, err := svc.DescribeLoadBalancers(&elbv2.DescribeLoadBalancersInput{}, testAccountID) - require.NoError(t, err) - assert.GreaterOrEqual(t, len(desc.LoadBalancers), 2) -} - -// --- DescribeTargetGroups: all --- - -func TestDescribeTargetGroups_All(t *testing.T) { - svc := setupTestService(t) - - _, _ = svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-all-a")}, testAccountID) - _, _ = svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-all-b")}, testAccountID) - - desc, err := svc.DescribeTargetGroups(&elbv2.DescribeTargetGroupsInput{}, testAccountID) - require.NoError(t, err) - assert.GreaterOrEqual(t, len(desc.TargetGroups), 2) -} - -// --- DescribeListeners: all (no LB ARN filter) --- - -func TestDescribeListeners_All(t *testing.T) { - svc := setupTestService(t) - - lb, _ := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{Name: aws.String("lb-lall")}, testAccountID) - tg, _ := svc.CreateTargetGroup(&elbv2.CreateTargetGroupInput{Name: aws.String("tg-lall")}, testAccountID) - _, _ = svc.CreateListener(&elbv2.CreateListenerInput{ - LoadBalancerArn: lb.LoadBalancers[0].LoadBalancerArn, - DefaultActions: []*elbv2.Action{{Type: aws.String("forward"), TargetGroupArn: tg.TargetGroups[0].TargetGroupArn}}, - }, testAccountID) - - desc, err := svc.DescribeListeners(&elbv2.DescribeListenersInput{}, testAccountID) - require.NoError(t, err) - assert.GreaterOrEqual(t, len(desc.Listeners), 1) -} - -// --- DeleteLoadBalancer: missing ARN --- - -func TestDeleteLoadBalancer_MissingArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.DeleteLoadBalancer(&elbv2.DeleteLoadBalancerInput{}, testAccountID) - assert.Error(t, err) -} - -// --- RegisterTargets: TG not found --- - -func TestRegisterTargets_TGNotFound(t *testing.T) { - svc := setupTestService(t) - _, err := svc.RegisterTargets(&elbv2.RegisterTargetsInput{ - TargetGroupArn: aws.String("arn:nonexistent"), - Targets: []*elbv2.TargetDescription{{Id: aws.String("i-abc")}}, - }, testAccountID) - assert.Error(t, err) -} - -// --- CreateListener: missing LB ARN --- - -func TestCreateListener_MissingLBArn(t *testing.T) { - svc := setupTestService(t) - _, err := svc.CreateListener(&elbv2.CreateListenerInput{ - DefaultActions: []*elbv2.Action{{Type: aws.String("forward")}}, - }, testAccountID) - assert.Error(t, err) -} diff --git a/spinifex/handlers/elbv2/launcher_test.go b/spinifex/handlers/elbv2/launcher_test.go index 8cba9679c..df9152580 100644 --- a/spinifex/handlers/elbv2/launcher_test.go +++ b/spinifex/handlers/elbv2/launcher_test.go @@ -1,12 +1,11 @@ package handlers_elbv2 import ( - "fmt" "sync" - "testing" ) -// mockSystemInstanceLauncher implements SystemInstanceLauncher for tests. +// mockSystemInstanceLauncher implements SystemInstanceLauncher for tests in +// service_impl_test.go and service_impl_vpc_test.go. type mockSystemInstanceLauncher struct { mu sync.Mutex launchCalls []*SystemInstanceInput @@ -39,142 +38,5 @@ func (m *mockSystemInstanceLauncher) waitTerminate() { } } -func TestSystemInstanceInput_Fields(t *testing.T) { - input := &SystemInstanceInput{ - InstanceType: "t3.nano", - ImageID: "ami-abc123", - SubnetID: "subnet-xyz", - UserData: "#!/bin/bash\necho hello", - ENIID: "eni-test123", - ENIMac: "02:00:00:11:22:33", - ENIIP: "10.0.1.5", - } - - if input.InstanceType != "t3.nano" { - t.Errorf("InstanceType = %q, want %q", input.InstanceType, "t3.nano") - } - if input.ImageID != "ami-abc123" { - t.Errorf("ImageID = %q, want %q", input.ImageID, "ami-abc123") - } - if input.SubnetID != "subnet-xyz" { - t.Errorf("SubnetID = %q, want %q", input.SubnetID, "subnet-xyz") - } - if input.ENIID != "eni-test123" { - t.Errorf("ENIID = %q, want %q", input.ENIID, "eni-test123") - } - if input.ENIMac != "02:00:00:11:22:33" { - t.Errorf("ENIMac = %q, want %q", input.ENIMac, "02:00:00:11:22:33") - } - if input.ENIIP != "10.0.1.5" { - t.Errorf("ENIIP = %q, want %q", input.ENIIP, "10.0.1.5") - } -} - -func TestMockSystemInstanceLauncher_Launch(t *testing.T) { - mock := &mockSystemInstanceLauncher{ - launchResult: &SystemInstanceOutput{ - InstanceID: "i-test123", - PrivateIP: "10.0.1.5", - }, - } - - input := &SystemInstanceInput{ - InstanceType: "t3.nano", - ImageID: "ami-abc123", - SubnetID: "subnet-xyz", - UserData: "#!/bin/bash\necho hello", - } - - out, err := mock.LaunchSystemInstance(input) - if err != nil { - t.Fatalf("LaunchSystemInstance: %v", err) - } - if out.InstanceID != "i-test123" { - t.Errorf("InstanceID = %q, want %q", out.InstanceID, "i-test123") - } - if out.PrivateIP != "10.0.1.5" { - t.Errorf("PrivateIP = %q, want %q", out.PrivateIP, "10.0.1.5") - } - if len(mock.launchCalls) != 1 { - t.Fatalf("expected 1 launch call, got %d", len(mock.launchCalls)) - } - if mock.launchCalls[0].InstanceType != "t3.nano" { - t.Errorf("launch call InstanceType = %q, want %q", mock.launchCalls[0].InstanceType, "t3.nano") - } -} - -func TestMockSystemInstanceLauncher_LaunchError(t *testing.T) { - mock := &mockSystemInstanceLauncher{ - launchErr: fmt.Errorf("insufficient capacity"), - } - - _, err := mock.LaunchSystemInstance(&SystemInstanceInput{ - InstanceType: "t3.nano", - ImageID: "ami-abc123", - }) - if err == nil { - t.Fatal("expected error from LaunchSystemInstance") - } - if err.Error() != "insufficient capacity" { - t.Errorf("error = %q, want %q", err.Error(), "insufficient capacity") - } -} - -func TestMockSystemInstanceLauncher_Terminate(t *testing.T) { - mock := &mockSystemInstanceLauncher{} - - err := mock.TerminateSystemInstance("i-test123") - if err != nil { - t.Fatalf("TerminateSystemInstance: %v", err) - } - if len(mock.terminateCalls) != 1 { - t.Fatalf("expected 1 terminate call, got %d", len(mock.terminateCalls)) - } - if mock.terminateCalls[0] != "i-test123" { - t.Errorf("terminate call = %q, want %q", mock.terminateCalls[0], "i-test123") - } -} - -func TestMockSystemInstanceLauncher_TerminateError(t *testing.T) { - mock := &mockSystemInstanceLauncher{ - terminateErr: fmt.Errorf("instance not found"), - } - - err := mock.TerminateSystemInstance("i-nonexistent") - if err == nil { - t.Fatal("expected error from TerminateSystemInstance") - } -} - -func TestMockSystemInstanceLauncher_WithPreCreatedENI(t *testing.T) { - mock := &mockSystemInstanceLauncher{ - launchResult: &SystemInstanceOutput{ - InstanceID: "i-alb123", - PrivateIP: "10.201.1.6", - }, - } - - input := &SystemInstanceInput{ - InstanceType: "t3.nano", - ImageID: "ami-abc123", - SubnetID: "subnet-vpc1", - UserData: "#!/bin/bash\napt-get install haproxy", - ENIID: "eni-alb456", - ENIMac: "02:00:00:aa:bb:cc", - ENIIP: "10.201.1.6", - } - - out, err := mock.LaunchSystemInstance(input) - if err != nil { - t.Fatalf("LaunchSystemInstance: %v", err) - } - if out.InstanceID != "i-alb123" { - t.Errorf("InstanceID = %q, want %q", out.InstanceID, "i-alb123") - } - if mock.launchCalls[0].ENIID != "eni-alb456" { - t.Errorf("ENIID = %q, want %q", mock.launchCalls[0].ENIID, "eni-alb456") - } -} - -// Verify the interface is satisfied by the mock at compile time. +// Compile-time check that the mock satisfies the production interface. var _ SystemInstanceLauncher = (*mockSystemInstanceLauncher)(nil) diff --git a/spinifex/handlers/elbv2/service_nats.go b/spinifex/handlers/elbv2/service_nats.go index edf9ad100..a01423675 100644 --- a/spinifex/handlers/elbv2/service_nats.go +++ b/spinifex/handlers/elbv2/service_nats.go @@ -24,6 +24,8 @@ type NATSELBv2Service struct { natsConn *nats.Conn } +var _ ELBv2Service = (*NATSELBv2Service)(nil) + // NewNATSELBv2Service creates a new NATS-based ELBv2 service. func NewNATSELBv2Service(conn *nats.Conn) ELBv2Service { return &NATSELBv2Service{natsConn: conn} diff --git a/spinifex/handlers/elbv2/service_nats_test.go b/spinifex/handlers/elbv2/service_nats_test.go deleted file mode 100644 index e304488a2..000000000 --- a/spinifex/handlers/elbv2/service_nats_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package handlers_elbv2 - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -// Verify NATSELBv2Service implements ELBv2Service at compile time. -var _ ELBv2Service = (*NATSELBv2Service)(nil) - -func TestNewNATSELBv2Service_NilConn(t *testing.T) { - // Should create the service even with nil conn (conn is used at call time, not construction) - svc := NewNATSELBv2Service(nil) - assert.NotNil(t, svc) -} diff --git a/spinifex/handlers/elbv2/store_test.go b/spinifex/handlers/elbv2/store_test.go index 97fb922d4..ab9d4bf86 100644 --- a/spinifex/handlers/elbv2/store_test.go +++ b/spinifex/handlers/elbv2/store_test.go @@ -68,66 +68,162 @@ func newTestListener(id, lbArn string) *ListenerRecord { } } -// --- Load Balancer tests --- +func TestLoadBalancerStoreLifecycle(t *testing.T) { + t.Run("put and get", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutLoadBalancer(newTestLB("getput1", "lb-getput1"))) -func TestPutAndGetLoadBalancer(t *testing.T) { - store := setupTestStore(t) - lb := newTestLB("abc123", "my-alb") + got, err := store.GetLoadBalancer("getput1") + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "getput1", got.LoadBalancerID) + }) - err := store.PutLoadBalancer(lb) - require.NoError(t, err) + t.Run("get not found", func(t *testing.T) { + store := setupTestStore(t) + got, err := store.GetLoadBalancer("nonexistent") + require.NoError(t, err) + assert.Nil(t, got) + }) - got, err := store.GetLoadBalancer("abc123") - require.NoError(t, err) - require.NotNil(t, got) - assert.Equal(t, lb.Name, got.Name) - assert.Equal(t, lb.LoadBalancerArn, got.LoadBalancerArn) - assert.Equal(t, lb.VpcId, got.VpcId) - assert.Equal(t, lb.Scheme, got.Scheme) + t.Run("delete removes record", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutLoadBalancer(newTestLB("del1", "lb-del1"))) + require.NoError(t, store.DeleteLoadBalancer("del1")) + + got, err := store.GetLoadBalancer("del1") + require.NoError(t, err) + assert.Nil(t, got) + }) + + t.Run("delete idempotent", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.DeleteLoadBalancer("doesnt-exist")) + }) + + t.Run("list returns all", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutLoadBalancer(newTestLB("a", "lb-a"))) + require.NoError(t, store.PutLoadBalancer(newTestLB("b", "lb-b"))) + + records, err := store.ListLoadBalancers() + require.NoError(t, err) + assert.Len(t, records, 2) + }) + + t.Run("list empty", func(t *testing.T) { + store := setupTestStore(t) + records, err := store.ListLoadBalancers() + require.NoError(t, err) + assert.Empty(t, records) + }) } -func TestGetLoadBalancer_NotFound(t *testing.T) { - store := setupTestStore(t) - got, err := store.GetLoadBalancer("nonexistent") - require.NoError(t, err) - assert.Nil(t, got) +func TestTargetGroupStoreLifecycle(t *testing.T) { + t.Run("put and get", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutTargetGroup(newTestTG("getput1", "tg-getput1"))) + + got, err := store.GetTargetGroup("getput1") + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "getput1", got.TargetGroupID) + }) + + t.Run("get not found", func(t *testing.T) { + store := setupTestStore(t) + got, err := store.GetTargetGroup("nonexistent") + require.NoError(t, err) + assert.Nil(t, got) + }) + + t.Run("delete removes record", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutTargetGroup(newTestTG("del1", "tg-del1"))) + require.NoError(t, store.DeleteTargetGroup("del1")) + + got, err := store.GetTargetGroup("del1") + require.NoError(t, err) + assert.Nil(t, got) + }) + + t.Run("delete idempotent", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.DeleteTargetGroup("doesnt-exist")) + }) + + t.Run("list returns all", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutTargetGroup(newTestTG("a", "tg-a"))) + require.NoError(t, store.PutTargetGroup(newTestTG("b", "tg-b"))) + + records, err := store.ListTargetGroups() + require.NoError(t, err) + assert.Len(t, records, 2) + }) + + t.Run("list empty", func(t *testing.T) { + store := setupTestStore(t) + records, err := store.ListTargetGroups() + require.NoError(t, err) + assert.Empty(t, records) + }) } -func TestStoreDeleteLoadBalancer(t *testing.T) { - store := setupTestStore(t) - lb := newTestLB("del123", "delete-me") +func TestListenerStoreLifecycle(t *testing.T) { + lbArn := "arn:aws:elasticloadbalancing:us-east-1:" + testAccountID + ":loadbalancer/app/test/lb1" - require.NoError(t, store.PutLoadBalancer(lb)) - require.NoError(t, store.DeleteLoadBalancer("del123")) + t.Run("put and get", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutListener(newTestListener("getput1", lbArn))) - got, err := store.GetLoadBalancer("del123") - require.NoError(t, err) - assert.Nil(t, got) + got, err := store.GetListener("getput1") + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "getput1", got.ListenerID) + }) + + t.Run("get not found", func(t *testing.T) { + store := setupTestStore(t) + got, err := store.GetListener("nonexistent") + require.NoError(t, err) + assert.Nil(t, got) + }) + + t.Run("delete removes record", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutListener(newTestListener("del1", lbArn))) + require.NoError(t, store.DeleteListener("del1")) + + got, err := store.GetListener("del1") + require.NoError(t, err) + assert.Nil(t, got) + }) + + t.Run("delete idempotent", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.DeleteListener("doesnt-exist")) + }) + + t.Run("list returns all", func(t *testing.T) { + store := setupTestStore(t) + require.NoError(t, store.PutListener(newTestListener("a", lbArn))) + require.NoError(t, store.PutListener(newTestListener("b", lbArn))) + + records, err := store.ListListeners() + require.NoError(t, err) + assert.Len(t, records, 2) + }) + + t.Run("list empty", func(t *testing.T) { + store := setupTestStore(t) + records, err := store.ListListeners() + require.NoError(t, err) + assert.Empty(t, records) + }) } -func TestDeleteLoadBalancer_Idempotent(t *testing.T) { - store := setupTestStore(t) - err := store.DeleteLoadBalancer("doesnt-exist") - require.NoError(t, err) -} - -func TestListLoadBalancers(t *testing.T) { - store := setupTestStore(t) - - require.NoError(t, store.PutLoadBalancer(newTestLB("lb1", "alb-one"))) - require.NoError(t, store.PutLoadBalancer(newTestLB("lb2", "alb-two"))) - - lbs, err := store.ListLoadBalancers() - require.NoError(t, err) - assert.Len(t, lbs, 2) -} - -func TestListLoadBalancers_Empty(t *testing.T) { - store := setupTestStore(t) - lbs, err := store.ListLoadBalancers() - require.NoError(t, err) - assert.Nil(t, lbs) -} +// --- LB-specific lookups (no equivalent on TG/Listener) --- func TestGetLoadBalancerByArn(t *testing.T) { store := setupTestStore(t) @@ -138,11 +234,8 @@ func TestGetLoadBalancerByArn(t *testing.T) { require.NoError(t, err) require.NotNil(t, got) assert.Equal(t, lb.Name, got.Name) -} -func TestGetLoadBalancerByArn_NotFound(t *testing.T) { - store := setupTestStore(t) - got, err := store.GetLoadBalancerByArn("arn:nonexistent") + got, err = store.GetLoadBalancerByArn("arn:nonexistent") require.NoError(t, err) assert.Nil(t, got) } @@ -158,53 +251,23 @@ func TestGetLoadBalancerByName(t *testing.T) { assert.Equal(t, lb.LoadBalancerID, got.LoadBalancerID) } -// --- Target Group tests --- - -func TestPutAndGetTargetGroup(t *testing.T) { +func TestPutLoadBalancer_Update(t *testing.T) { store := setupTestStore(t) - tg := newTestTG("tg123", "my-tg") + lb := newTestLB("upd123", "updatable") + require.NoError(t, store.PutLoadBalancer(lb)) - require.NoError(t, store.PutTargetGroup(tg)) + lb.State = StateFailed + lb.ENIs = []string{"eni-111", "eni-222"} + require.NoError(t, store.PutLoadBalancer(lb)) - got, err := store.GetTargetGroup("tg123") + got, err := store.GetLoadBalancer("upd123") require.NoError(t, err) require.NotNil(t, got) - assert.Equal(t, tg.Name, got.Name) - assert.Equal(t, tg.Protocol, got.Protocol) - assert.Equal(t, tg.Port, got.Port) - assert.Equal(t, tg.HealthCheck.Path, got.HealthCheck.Path) + assert.Equal(t, StateFailed, got.State) + assert.Equal(t, []string{"eni-111", "eni-222"}, got.ENIs) } -func TestGetTargetGroup_NotFound(t *testing.T) { - store := setupTestStore(t) - got, err := store.GetTargetGroup("nonexistent") - require.NoError(t, err) - assert.Nil(t, got) -} - -func TestStoreDeleteTargetGroup(t *testing.T) { - store := setupTestStore(t) - tg := newTestTG("tgdel", "delete-tg") - - require.NoError(t, store.PutTargetGroup(tg)) - require.NoError(t, store.DeleteTargetGroup("tgdel")) - - got, err := store.GetTargetGroup("tgdel") - require.NoError(t, err) - assert.Nil(t, got) -} - -func TestListTargetGroups(t *testing.T) { - store := setupTestStore(t) - - require.NoError(t, store.PutTargetGroup(newTestTG("tg1", "tg-one"))) - require.NoError(t, store.PutTargetGroup(newTestTG("tg2", "tg-two"))) - require.NoError(t, store.PutTargetGroup(newTestTG("tg3", "tg-three"))) - - tgs, err := store.ListTargetGroups() - require.NoError(t, err) - assert.Len(t, tgs, 3) -} +// --- TG-specific lookups + targets --- func TestGetTargetGroupByArn(t *testing.T) { store := setupTestStore(t) @@ -250,43 +313,7 @@ func TestTargetGroupWithTargets(t *testing.T) { assert.Equal(t, "10.0.1.11", got.Targets[1].PrivateIP) } -// --- Listener tests --- - -func TestPutAndGetListener(t *testing.T) { - store := setupTestStore(t) - lbArn := "arn:aws:elasticloadbalancing:us-east-1:" + testAccountID + ":loadbalancer/app/my-alb/lb123" - l := newTestListener("lst123", lbArn) - - require.NoError(t, store.PutListener(l)) - - got, err := store.GetListener("lst123") - require.NoError(t, err) - require.NotNil(t, got) - assert.Equal(t, ProtocolHTTP, got.Protocol) - assert.Equal(t, int64(80), got.Port) - assert.Len(t, got.DefaultActions, 1) - assert.Equal(t, ActionTypeForward, got.DefaultActions[0].Type) -} - -func TestGetListener_NotFound(t *testing.T) { - store := setupTestStore(t) - got, err := store.GetListener("nonexistent") - require.NoError(t, err) - assert.Nil(t, got) -} - -func TestStoreDeleteListener(t *testing.T) { - store := setupTestStore(t) - lbArn := "arn:aws:elasticloadbalancing:us-east-1:" + testAccountID + ":loadbalancer/app/test/lb1" - l := newTestListener("lstdel", lbArn) - - require.NoError(t, store.PutListener(l)) - require.NoError(t, store.DeleteListener("lstdel")) - - got, err := store.GetListener("lstdel") - require.NoError(t, err) - assert.Nil(t, got) -} +// --- Listener-specific lookups --- func TestListListenersByLB(t *testing.T) { store := setupTestStore(t) @@ -304,12 +331,10 @@ func TestListListenersByLB(t *testing.T) { require.NoError(t, store.PutListener(l2)) require.NoError(t, store.PutListener(l3)) - // Should return only listeners for lbArn1 listeners, err := store.ListListenersByLB(lbArn1) require.NoError(t, err) assert.Len(t, listeners, 2) - // Should return only listener for lbArn2 listeners, err = store.ListListenersByLB(lbArn2) require.NoError(t, err) assert.Len(t, listeners, 1) @@ -327,10 +352,9 @@ func TestGetListenerByArn(t *testing.T) { assert.Equal(t, l.ListenerID, got.ListenerID) } -// --- Cross-resource isolation tests --- +// --- Cross-resource isolation: shared IDs across record types must not collide --- func TestResourceIsolation(t *testing.T) { - // Verify that LB, TG, and Listener records don't interfere with each other store := setupTestStore(t) lb := newTestLB("shared1", "alb-shared") @@ -341,7 +365,6 @@ func TestResourceIsolation(t *testing.T) { require.NoError(t, store.PutTargetGroup(tg)) require.NoError(t, store.PutListener(l)) - // Each should be retrievable independently gotLB, err := store.GetLoadBalancer("shared1") require.NoError(t, err) require.NotNil(t, gotLB) @@ -357,14 +380,6 @@ func TestResourceIsolation(t *testing.T) { require.NotNil(t, gotL) assert.Equal(t, ProtocolHTTP, gotL.Protocol) - // List operations should return correct counts - lbs, _ := store.ListLoadBalancers() - assert.Len(t, lbs, 1) - tgs, _ := store.ListTargetGroups() - assert.Len(t, tgs, 1) - listeners, _ := store.ListListeners() - assert.Len(t, listeners, 1) - // Deleting one type should not affect others require.NoError(t, store.DeleteLoadBalancer("shared1")) gotTG, _ = store.GetTargetGroup("shared1") @@ -373,23 +388,6 @@ func TestResourceIsolation(t *testing.T) { assert.NotNil(t, gotL) } -func TestPutLoadBalancer_Update(t *testing.T) { - store := setupTestStore(t) - lb := newTestLB("upd123", "updatable") - require.NoError(t, store.PutLoadBalancer(lb)) - - // Update state - lb.State = StateFailed - lb.ENIs = []string{"eni-111", "eni-222"} - require.NoError(t, store.PutLoadBalancer(lb)) - - got, err := store.GetLoadBalancer("upd123") - require.NoError(t, err) - require.NotNil(t, got) - assert.Equal(t, StateFailed, got.State) - assert.Equal(t, []string{"eni-111", "eni-222"}, got.ENIs) -} - func TestTargetGroupsForLB(t *testing.T) { store := setupTestStore(t) @@ -398,7 +396,6 @@ func TestTargetGroupsForLB(t *testing.T) { require.NoError(t, err) assert.Nil(t, tgs) - // Create LB, TG, and listener linking them lb := newTestLB("tgflb1", "my-alb") tg := newTestTG("tg001", "my-tg") require.NoError(t, store.PutLoadBalancer(lb)) diff --git a/spinifex/handlers/elbv2/types_test.go b/spinifex/handlers/elbv2/types_test.go index 2c66b8372..f3d748885 100644 --- a/spinifex/handlers/elbv2/types_test.go +++ b/spinifex/handlers/elbv2/types_test.go @@ -18,63 +18,3 @@ func TestDefaultHealthCheck(t *testing.T) { assert.Equal(t, int64(DefaultUnhealthyThreshold), hc.UnhealthyThreshold) assert.Equal(t, DefaultHealthCheckMatcher, hc.Matcher) } - -func TestHealthCheckConfig_CustomValues(t *testing.T) { - hc := HealthCheckConfig{ - Protocol: ProtocolHTTPS, - Port: "8443", - Path: "/healthz", - IntervalSeconds: 10, - TimeoutSeconds: 3, - HealthyThreshold: 2, - UnhealthyThreshold: 3, - Matcher: "200-299", - } - - assert.Equal(t, ProtocolHTTPS, hc.Protocol) - assert.Equal(t, "8443", hc.Port) - assert.Equal(t, "/healthz", hc.Path) - assert.Equal(t, int64(10), hc.IntervalSeconds) -} - -func TestLoadBalancerRecord_Fields(t *testing.T) { - lb := LoadBalancerRecord{ - Name: "my-alb", - Scheme: SchemeInternetFacing, - Type: LoadBalancerTypeApplication, - State: StateProvisioning, - SecurityGroups: []string{"sg-111", "sg-222"}, - Subnets: []string{"subnet-aaa", "subnet-bbb"}, - IPAddressType: IPAddressTypeIPv4, - } - - assert.Equal(t, "internet-facing", lb.Scheme) - assert.Equal(t, "application", lb.Type) - assert.Equal(t, "provisioning", lb.State) - assert.Len(t, lb.SecurityGroups, 2) - assert.Len(t, lb.Subnets, 2) -} - -func TestTarget_Fields(t *testing.T) { - target := Target{ - Id: "i-abc123", - Port: 8080, - HealthState: TargetHealthHealthy, - HealthDesc: "Target is healthy", - PrivateIP: "10.0.1.50", - } - - assert.Equal(t, "i-abc123", target.Id) - assert.Equal(t, int64(8080), target.Port) - assert.Equal(t, "healthy", target.HealthState) -} - -func TestListenerAction_Fields(t *testing.T) { - action := ListenerAction{ - Type: ActionTypeForward, - TargetGroupArn: "arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/my-tg/abc123", - } - - assert.Equal(t, "forward", action.Type) - assert.Contains(t, action.TargetGroupArn, "targetgroup") -} diff --git a/spinifex/nbd/nbd_test.go b/spinifex/nbd/nbd_test.go index e34c2fb87..a5111b04e 100644 --- a/spinifex/nbd/nbd_test.go +++ b/spinifex/nbd/nbd_test.go @@ -153,28 +153,6 @@ func TestBuildArgs_NotVerbose(t *testing.T) { } } -func TestBuildArgs_PluginArgValues(t *testing.T) { - cfg := &NBDKitConfig{ - Socket: "/tmp/nbd.sock", - PidFile: "/tmp/nbd.pid", - PluginPath: "/plugin.so", - Size: 0, - Volume: "", - CacheSize: 0, - ShardWAL: false, - } - - args, err := cfg.buildArgs() - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - assertContains(t, args, "size=0") - assertContains(t, args, "volume=") - assertContains(t, args, "cache_size=0") - assertContains(t, args, "shardwal=false") -} - func TestBuildArgs_TCPPortValue(t *testing.T) { tests := []struct { port int @@ -267,13 +245,6 @@ func assertArgs(t *testing.T, expected, got []string) { } } -func assertContains(t *testing.T, args []string, val string) { - t.Helper() - if indexOf(args, val) < 0 { - t.Errorf("expected args to contain %q, got: %v", val, args) - } -} - func indexOf(args []string, val string) int { for i, a := range args { if a == val { diff --git a/spinifex/utils/account_test.go b/spinifex/utils/account_test.go index 7458c9976..d1bd004a4 100644 --- a/spinifex/utils/account_test.go +++ b/spinifex/utils/account_test.go @@ -67,69 +67,50 @@ func startJSNATSServer(t *testing.T) (*nats.Conn, nats.JetStreamContext) { return nc, js } -func TestWriteVersion_WritesKey(t *testing.T) { +// TestVersionStateMachine walks WriteVersion/ReadVersion through every state +// transition the migration helper guarantees: +// - unset → ReadVersion returns 0 with no error +// - first write persists the value +// - same version is a no-op (idempotent) +// - higher version replaces the stored value (upgrade) +// - lower version is a no-op (no downgrade) +// +// One bucket is reused across steps so each Write is exercised against the +// state the prior Write left behind — that's the only way to catch a +// regression that, say, made WriteVersion unconditionally overwrite. +func TestVersionStateMachine(t *testing.T) { _, js := startJSNATSServer(t) - - kv, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: "test-write-version"}) + kv, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: "test-version-fsm"}) require.NoError(t, err) - err = WriteVersion(kv, 1) + // Unset → 0. + v, err := ReadVersion(kv) require.NoError(t, err) + assert.Equal(t, 0, v, "ReadVersion on unset bucket") + steps := []struct { + name string + write int + want int // expected ReadVersion after the write + }{ + {"first write persists", 1, 1}, + {"same version is no-op", 1, 1}, + {"higher version upgrades", 2, 2}, + {"lower version is no-op (no downgrade)", 1, 2}, + {"larger jump upgrades", 5, 5}, + } + for _, step := range steps { + t.Run(step.name, func(t *testing.T) { + require.NoError(t, WriteVersion(kv, step.write)) + v, err := ReadVersion(kv) + require.NoError(t, err) + assert.Equal(t, step.want, v) + }) + } + + // Round-trip the raw KV value to confirm the encoding is what readers + // outside this package would expect. entry, err := kv.Get(VersionKey) require.NoError(t, err) - assert.Equal(t, "1", string(entry.Value())) -} - -func TestReadVersion_ReturnsZeroWhenUnset(t *testing.T) { - _, js := startJSNATSServer(t) - - kv, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: "test-read-unset"}) - require.NoError(t, err) - - v, err := ReadVersion(kv) - require.NoError(t, err) - assert.Equal(t, 0, v) -} - -func TestWriteVersion_IdempotentOnSameVersion(t *testing.T) { - _, js := startJSNATSServer(t) - - kv, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: "test-idempotent"}) - require.NoError(t, err) - - require.NoError(t, WriteVersion(kv, 1)) - require.NoError(t, WriteVersion(kv, 1)) - - v, err := ReadVersion(kv) - require.NoError(t, err) - assert.Equal(t, 1, v) -} - -func TestWriteVersion_UpdatesOnHigherVersion(t *testing.T) { - _, js := startJSNATSServer(t) - - kv, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: "test-upgrade"}) - require.NoError(t, err) - - require.NoError(t, WriteVersion(kv, 1)) - require.NoError(t, WriteVersion(kv, 2)) - - v, err := ReadVersion(kv) - require.NoError(t, err) - assert.Equal(t, 2, v) -} - -func TestWriteVersion_NoOpOnLowerVersion(t *testing.T) { - _, js := startJSNATSServer(t) - - kv, err := js.CreateKeyValue(&nats.KeyValueConfig{Bucket: "test-downgrade"}) - require.NoError(t, err) - - require.NoError(t, WriteVersion(kv, 3)) - require.NoError(t, WriteVersion(kv, 1)) - - v, err := ReadVersion(kv) - require.NoError(t, err) - assert.Equal(t, 3, v) + assert.Equal(t, "5", string(entry.Value())) } diff --git a/spinifex/utils/conversion_test.go b/spinifex/utils/conversion_test.go index a4c7cf4ae..550e28512 100644 --- a/spinifex/utils/conversion_test.go +++ b/spinifex/utils/conversion_test.go @@ -7,17 +7,18 @@ import ( "github.com/stretchr/testify/assert" ) +// Each wrapper has one or two clamp branches; tests cover the no-branch +// happy path and each branch. Boundary tests against MaxInt64/MaxUint8 etc. +// only exercise stdlib casts, so they are intentionally omitted. + func TestSafeInt64ToUint64(t *testing.T) { tests := []struct { name string in int64 want uint64 }{ - {"Zero", 0, 0}, - {"Positive", 42, 42}, - {"MaxInt64", math.MaxInt64, uint64(math.MaxInt64)}, - {"Negative", -1, 0}, - {"MinInt64", math.MinInt64, 0}, + {"positive passes through", 42, 42}, + {"negative clamps to 0", -1, 0}, } for _, tt := range tests { @@ -33,13 +34,9 @@ func TestSafeIntToUint8(t *testing.T) { in int want uint8 }{ - {"Zero", 0, 0}, - {"Normal", 128, 128}, - {"Max uint8", 255, 255}, - {"Above max", 256, 255}, - {"Large", 1000, 255}, - {"Negative", -1, 0}, - {"Large negative", -100, 0}, + {"in range passes through", 128, 128}, + {"above max clamps", math.MaxUint8 + 1, math.MaxUint8}, + {"negative clamps to 0", -1, 0}, } for _, tt := range tests { @@ -55,10 +52,8 @@ func TestSafeIntToUint64(t *testing.T) { in int want uint64 }{ - {"Zero", 0, 0}, - {"Positive", 42, 42}, - {"Negative", -1, 0}, - {"Large negative", -999, 0}, + {"positive passes through", 42, 42}, + {"negative clamps to 0", -1, 0}, } for _, tt := range tests { @@ -74,11 +69,8 @@ func TestSafeUint64ToInt64(t *testing.T) { in uint64 want int64 }{ - {"Zero", 0, 0}, - {"Normal", 42, 42}, - {"MaxInt64", uint64(math.MaxInt64), math.MaxInt64}, - {"Above MaxInt64", uint64(math.MaxInt64) + 1, math.MaxInt64}, - {"MaxUint64", math.MaxUint64, math.MaxInt64}, + {"in range passes through", 42, 42}, + {"above MaxInt64 clamps", uint64(math.MaxInt64) + 1, math.MaxInt64}, } for _, tt := range tests { diff --git a/spinifex/vm/vm_test.go b/spinifex/vm/vm_test.go index d41f74430..0a63b0556 100644 --- a/spinifex/vm/vm_test.go +++ b/spinifex/vm/vm_test.go @@ -285,60 +285,62 @@ func TestExecute_NoGraphic(t *testing.T) { } func TestExecute_SerialSocketAndConsoleLog(t *testing.T) { - cfg := Config{ - CPUCount: 1, - Memory: 512, - Architecture: "x86_64", - SerialSocket: "/run/serial.sock", - ConsoleLogPath: "/var/log/console.log", - Drives: []Drive{{File: "disk.img", Format: "raw"}}, - } + t.Run("both set emits chardev and serial", func(t *testing.T) { + cfg := Config{ + CPUCount: 1, + Memory: 512, + Architecture: "x86_64", + SerialSocket: "/run/serial.sock", + ConsoleLogPath: "/var/log/console.log", + Drives: []Drive{{File: "disk.img", Format: "raw"}}, + } - cmd, err := cfg.Execute() - assert.NoError(t, err) + cmd, err := cfg.Execute() + assert.NoError(t, err) - args := cmd.Args[1:] - chardev := argValue(args, "-chardev") - assert.Contains(t, chardev, "socket,id=console0") - assert.Contains(t, chardev, "path=/run/serial.sock") - assert.Contains(t, chardev, "logfile=/var/log/console.log") - assert.Equal(t, "chardev:console0", argValue(args, "-serial")) -} + args := cmd.Args[1:] + chardev := argValue(args, "-chardev") + assert.Contains(t, chardev, "socket,id=console0") + assert.Contains(t, chardev, "path=/run/serial.sock") + assert.Contains(t, chardev, "logfile=/var/log/console.log") + assert.Equal(t, "chardev:console0", argValue(args, "-serial")) + }) -func TestExecute_SerialSocketOnly(t *testing.T) { - cfg := Config{ - CPUCount: 1, - Memory: 512, - Architecture: "x86_64", - SerialSocket: "/run/serial.sock", - Drives: []Drive{{File: "disk.img", Format: "raw"}}, - } + // Boundary: the production guard requires BOTH fields. Flipping && to || + // would silently emit invalid -chardev args; these subtests catch that. + t.Run("serial socket alone emits nothing", func(t *testing.T) { + cfg := Config{ + CPUCount: 1, + Memory: 512, + Architecture: "x86_64", + SerialSocket: "/run/serial.sock", + Drives: []Drive{{File: "disk.img", Format: "raw"}}, + } - cmd, err := cfg.Execute() - assert.NoError(t, err) + cmd, err := cfg.Execute() + assert.NoError(t, err) - args := cmd.Args[1:] - // When ConsoleLogPath is empty, -chardev and -serial should not appear - assert.Empty(t, argValue(args, "-chardev")) - assert.Empty(t, argValue(args, "-serial")) -} + args := cmd.Args[1:] + assert.Empty(t, argValue(args, "-chardev")) + assert.Empty(t, argValue(args, "-serial")) + }) -func TestExecute_ConsoleLogPathOnly(t *testing.T) { - cfg := Config{ - CPUCount: 1, - Memory: 512, - Architecture: "x86_64", - ConsoleLogPath: "/var/log/console.log", - Drives: []Drive{{File: "disk.img", Format: "raw"}}, - } + t.Run("console log alone emits nothing", func(t *testing.T) { + cfg := Config{ + CPUCount: 1, + Memory: 512, + Architecture: "x86_64", + ConsoleLogPath: "/var/log/console.log", + Drives: []Drive{{File: "disk.img", Format: "raw"}}, + } - cmd, err := cfg.Execute() - assert.NoError(t, err) + cmd, err := cfg.Execute() + assert.NoError(t, err) - args := cmd.Args[1:] - // When SerialSocket is empty, -chardev and -serial should not appear - assert.Empty(t, argValue(args, "-chardev")) - assert.Empty(t, argValue(args, "-serial")) + args := cmd.Args[1:] + assert.Empty(t, argValue(args, "-chardev")) + assert.Empty(t, argValue(args, "-serial")) + }) } func TestExecute_NetDevs(t *testing.T) { diff --git a/tests/e2e/run-reboot-e2e.sh b/tests/e2e/run-reboot-e2e.sh new file mode 100755 index 000000000..7ea9c5fcb --- /dev/null +++ b/tests/e2e/run-reboot-e2e.sh @@ -0,0 +1,529 @@ +#!/bin/bash +# run-reboot-e2e.sh — single-node host reboot resilience driver (cell #18). +# +# Runs from the GitHub Actions runner (NOT via ssh-and-run) so the script +# survives the systemctl-reboot-induced ssh disconnect mid-run. Each step that +# touches the cluster opens its own ssh session into the spinifex node. +# +# Required env: +# WAN_IP — single-node cluster's WAN IP +# SSH_KEY — path to ssh key the runner can use to reach tf-user@WAN_IP +# +# Sequence: +# 1. Pre-reboot: VPC + 2 app EC2s + internet-facing ALB; verify round-robin. +# 2. Snapshot pre-reboot state (instance IPs, ALB ENI MAC, ovn-nbctl show). +# 3. systemctl reboot the host; poll TCP/22 from the runner until SSH returns. +# 4. Wait for spinifex services + AWS gateway to respond. +# 5. Post-reboot assertions (each a separate pass/fail line). +# 6. Cleanup via EXIT trap. +# +# This cell is expected red until the OVN persistence fix lands — the +# `ovn-nbctl show` diff assertion is the canonical reproducer for that bug. + +set -u + +WAN_IP="${WAN_IP:?WAN_IP is required}" +SSH_KEY="${SSH_KEY:?SSH_KEY is required}" +SSH_KEY="${SSH_KEY/#\~/$HOME}" + +REBOOT_WAIT_SECS="${REBOOT_WAIT_SECS:-300}" +DAEMON_READY_SECS="${DAEMON_READY_SECS:-180}" +INSTANCE_RUNNING_SECS="${INSTANCE_RUNNING_SECS:-120}" +LB_RECOVER_SECS="${LB_RECOVER_SECS:-180}" + +NODE_KEY_PATH="\$HOME/reboot-e2e-key" +NODE_USERDATA_PATH="\$HOME/reboot-e2e-userdata.txt" + +SSH_OPTS=( + -i "$SSH_KEY" + -o StrictHostKeyChecking=no + -o UserKnownHostsFile=/dev/null + -o ConnectTimeout=10 + -o LogLevel=ERROR +) + +PASSED=0 +FAILED=0 + +log() { echo "[$(date +%H:%M:%S)] $*"; } +pass() { echo " ✅ $1"; PASSED=$((PASSED + 1)); } +fail() { echo " ❌ $1"; FAILED=$((FAILED + 1)); } + +node_ssh() { ssh "${SSH_OPTS[@]}" "tf-user@${WAN_IP}" "$@"; } + +# Pipe a file's contents into a path on the node (tf-user-owned). +put_to_node() { + local dest="$1" + node_ssh "cat > $dest" +} + +# Run an aws cli call on the node with the spinifex profile already configured. +node_aws_ec2() { node_ssh "AWS_PROFILE=spinifex aws ec2 $*"; } +node_aws_elbv2() { node_ssh "AWS_PROFILE=spinifex aws elbv2 $*"; } + +# ========================================================================== +# Resource tracking for cleanup +# ========================================================================== +VPC_ID="" +SUBNET_ID="" +IGW_ID="" +APP_INSTANCE_IDS=() +ALB_LISTENER_ARN="" +ALB_LB_ARN="" +ALB_TG_ARN="" + +cleanup() { + local exit_code=$? + echo "" + echo "=== Cleanup ===" + + if ! node_ssh "true" 2>/dev/null; then + log "Cleanup skipped: node unreachable (exit=$exit_code)" + echo "Reboot E2E: $PASSED passed, $FAILED failed" + if [ $FAILED -gt 0 ] || [ $exit_code -ne 0 ]; then exit 1; fi + exit 0 + fi + + [ -n "$ALB_LISTENER_ARN" ] && node_aws_elbv2 "delete-listener --listener-arn $ALB_LISTENER_ARN" >/dev/null 2>&1 || true + [ -n "$ALB_LB_ARN" ] && node_aws_elbv2 "delete-load-balancer --load-balancer-arn $ALB_LB_ARN" >/dev/null 2>&1 || true + [ -n "$ALB_TG_ARN" ] && node_aws_elbv2 "delete-target-group --target-group-arn $ALB_TG_ARN" >/dev/null 2>&1 || true + + for inst_id in "${APP_INSTANCE_IDS[@]}"; do + [ -n "$inst_id" ] && node_aws_ec2 "terminate-instances --instance-ids $inst_id" >/dev/null 2>&1 || true + done + if [ ${#APP_INSTANCE_IDS[@]} -gt 0 ]; then + for attempt in $(seq 1 30); do + local all_terminated=true + for inst_id in "${APP_INSTANCE_IDS[@]}"; do + [ -z "$inst_id" ] && continue + local state + state=$(node_aws_ec2 "describe-instances --instance-ids $inst_id --query 'Reservations[0].Instances[0].State.Name' --output text" 2>/dev/null || echo "terminated") + if [ "$state" != "terminated" ]; then all_terminated=false; break; fi + done + if [ "$all_terminated" = true ]; then break; fi + sleep 2 + done + fi + + node_aws_ec2 "delete-key-pair --key-name reboot-e2e-key" >/dev/null 2>&1 || true + node_ssh "rm -f $NODE_KEY_PATH $NODE_USERDATA_PATH" >/dev/null 2>&1 || true + + if [ -n "$IGW_ID" ] && [ -n "$VPC_ID" ]; then + node_aws_ec2 "detach-internet-gateway --internet-gateway-id $IGW_ID --vpc-id $VPC_ID" >/dev/null 2>&1 || true + node_aws_ec2 "delete-internet-gateway --internet-gateway-id $IGW_ID" >/dev/null 2>&1 || true + fi + [ -n "$SUBNET_ID" ] && node_aws_ec2 "delete-subnet --subnet-id $SUBNET_ID" >/dev/null 2>&1 || true + [ -n "$VPC_ID" ] && node_aws_ec2 "delete-vpc --vpc-id $VPC_ID" >/dev/null 2>&1 || true + + echo "" + echo "=================================" + echo "Reboot E2E: $PASSED passed, $FAILED failed" + echo "=================================" + + if [ $FAILED -gt 0 ]; then exit 1; fi + exit $exit_code +} +trap cleanup EXIT + +dump_diagnostics() { + log "--- spinifex service journals (last 100 lines each) ---" + for svc in spinifex-nats spinifex-predastore spinifex-viperblock \ + spinifex-daemon spinifex-awsgw spinifex-vpcd \ + ovn-controller ovs-vswitchd; do + echo "=== ${svc} ===" + node_ssh "sudo journalctl -u ${svc} --no-pager -n 100 2>/dev/null || true" + done +} + +# ========================================================================== +# HTTP traffic burst from the spinifex node against an ALB URL. +# ========================================================================== +run_http_burst() { + local url="$1" label="$2" num="${3:-20}" + log "Sending $num HTTP requests to $url ($label)..." + local results + results=$(node_ssh "for i in \$(seq 1 $num); do curl -s --max-time 5 '$url/' 2>/dev/null; echo; done" 2>/dev/null || true) + + declare -A counts + local total_ok=0 + while IFS= read -r line; do + local inst + inst=$(echo "$line" | jq -r '.instance_id // empty' 2>/dev/null) + if [ -n "$inst" ]; then + counts[$inst]=$(( ${counts[$inst]:-0} + 1 )) + total_ok=$((total_ok + 1)) + fi + done <<< "$results" + + local unique=${#counts[@]} + echo " Distribution:" + for inst_id in "${!counts[@]}"; do echo " $inst_id: ${counts[$inst_id]} responses"; done + + if [ "$unique" -ge 2 ]; then + pass "$label round-robin: $unique unique instances" + else + fail "$label round-robin: expected >=2 unique responders, got $unique" + fi + if [ "$total_ok" -ge $((num / 2)) ]; then + pass "$label success rate: $total_ok/$num" + else + fail "$label success rate: only $total_ok/$num" + fi +} + +wait_for_lb_active() { + local lb_arn="$1" label="$2" timeout="${3:-270}" + log "Waiting for $label active (up to ${timeout}s)..." + local state="" + for attempt in $(seq 1 $((timeout / 3))); do + state=$(node_aws_elbv2 "describe-load-balancers --load-balancer-arns $lb_arn --query 'LoadBalancers[0].State.Code' --output text" 2>/dev/null || true) + if [ "$state" = "active" ]; then + pass "$label active" + return 0 + fi + sleep 3 + done + fail "$label did not reach active (last state: ${state:-unknown})" + return 1 +} + +wait_for_targets_healthy() { + local tg_arn="$1" expected="$2" label="$3" timeout="${4:-180}" + log "Waiting for $label targets healthy (up to ${timeout}s)..." + local healthy_count=0 total_count=0 + for attempt in $(seq 1 $((timeout / 5))); do + local out + out=$(node_aws_elbv2 "describe-target-health --target-group-arn $tg_arn --output json" 2>/dev/null || echo '{}') + healthy_count=$(echo "$out" | jq '[.TargetHealthDescriptions[] | select(.TargetHealth.State == "healthy")] | length' 2>/dev/null || echo 0) + total_count=$(echo "$out" | jq '.TargetHealthDescriptions | length' 2>/dev/null || echo 0) + if [ "$healthy_count" -eq "$expected" ]; then + pass "$label: $expected/$expected targets healthy" + return 0 + fi + sleep 5 + done + fail "$label: only $healthy_count/$total_count healthy after ${timeout}s" + return 1 +} + +# ========================================================================== +# Phase 0: Prerequisites +# ========================================================================== +echo "=================================" +echo "Reboot Resilience E2E (cell 18)" +echo "=================================" +log "WAN_IP: $WAN_IP" + +log "Verifying SSH to node..." +node_ssh "hostname" >/dev/null || { fail "ssh to $WAN_IP"; exit 1; } +pass "ssh to $WAN_IP" + +log "Discovering instance type + AMI..." +INSTANCE_TYPE=$(node_aws_ec2 "describe-instance-types --query 'InstanceTypes[*].InstanceType' --output text" \ + | tr ' \t' '\n\n' | grep -m1 'nano' || true) +if [ -z "$INSTANCE_TYPE" ]; then fail "no nano instance type available"; exit 1; fi +pass "instance type: $INSTANCE_TYPE" + +ALL_IMAGES=$(node_aws_ec2 "describe-images --output json" 2>/dev/null) +AMI_ID=$(echo "$ALL_IMAGES" | jq -r '[.Images[] | select(.Name | test("^ami-ubuntu"))][0].ImageId // empty') +if [ -z "$AMI_ID" ]; then + AMI_ID=$(echo "$ALL_IMAGES" | jq -r '[.Images[] | select(.Name | test("alpine") | not)][0].ImageId // empty') +fi +if [ -z "$AMI_ID" ]; then AMI_ID=$(echo "$ALL_IMAGES" | jq -r '.Images[0].ImageId // empty'); fi +if [ -z "$AMI_ID" ]; then fail "no AMI found"; exit 1; fi +pass "AMI: $AMI_ID" + +# Create key pair, capturing the private key material so we can ssh into guests +node_aws_ec2 "delete-key-pair --key-name reboot-e2e-key" >/dev/null 2>&1 || true +KEY_RESPONSE=$(node_aws_ec2 "create-key-pair --key-name reboot-e2e-key --output json" 2>/dev/null) \ + || { fail "create-key-pair"; exit 1; } +KEY_MATERIAL=$(echo "$KEY_RESPONSE" | jq -r '.KeyMaterial') +if [ -z "$KEY_MATERIAL" ] || [ "$KEY_MATERIAL" = "null" ]; then + fail "create-key-pair returned no KeyMaterial"; exit 1 +fi +printf '%s' "$KEY_MATERIAL" | put_to_node "$NODE_KEY_PATH" +node_ssh "chmod 600 $NODE_KEY_PATH" || { fail "chmod key"; exit 1; } +pass "key pair: reboot-e2e-key" + +# Stage user-data on the node (HTTP responder for round-robin verification) +APP_USER_DATA=$(cat <<'USERDATA' +#!/bin/bash +INSTANCE_ID=$(hostname) +mkdir -p /tmp/httpd && cd /tmp/httpd +echo "{\"instance_id\": \"${INSTANCE_ID}\"}" > index.html +nohup python3 -m http.server 80 --bind 0.0.0.0 > /dev/null 2>&1 & +USERDATA +) +printf '%s' "$APP_USER_DATA" | put_to_node "$NODE_USERDATA_PATH" + +# ========================================================================== +# Phase 1: VPC + Subnet +# ========================================================================== +log "Creating VPC..." +VPC_ID=$(node_aws_ec2 "create-vpc --cidr-block 10.210.0.0/16 --output json" | jq -r '.Vpc.VpcId') +if [ -z "$VPC_ID" ] || [ "$VPC_ID" = "null" ]; then fail "create-vpc"; exit 1; fi +pass "create-vpc: $VPC_ID" + +IGW_ID=$(node_aws_ec2 "create-internet-gateway --output json" | jq -r '.InternetGateway.InternetGatewayId') +node_aws_ec2 "attach-internet-gateway --internet-gateway-id $IGW_ID --vpc-id $VPC_ID" >/dev/null \ + || { fail "attach-internet-gateway"; exit 1; } +pass "internet gateway: $IGW_ID (attached)" + +SUBNET_ID=$(node_aws_ec2 "create-subnet --vpc-id $VPC_ID --cidr-block 10.210.1.0/24 --output json" | jq -r '.Subnet.SubnetId') +if [ -z "$SUBNET_ID" ] || [ "$SUBNET_ID" = "null" ]; then fail "create-subnet"; exit 1; fi +pass "create-subnet: $SUBNET_ID" +node_aws_ec2 "modify-subnet-attribute --subnet-id $SUBNET_ID --map-public-ip-on-launch" >/dev/null || true + +# ========================================================================== +# Phase 2: Launch app EC2 instances +# ========================================================================== +log "Launching 2 app instances..." +for i in 1 2; do + INST=$(node_aws_ec2 "run-instances \ + --image-id $AMI_ID \ + --instance-type $INSTANCE_TYPE \ + --key-name reboot-e2e-key \ + --subnet-id $SUBNET_ID \ + --user-data file://$NODE_USERDATA_PATH \ + --output json" | jq -r '.Instances[0].InstanceId') + if [ -z "$INST" ] || [ "$INST" = "null" ]; then fail "run-instances (app $i)"; exit 1; fi + APP_INSTANCE_IDS+=("$INST") + log " app$i: $INST" +done +pass "launched ${#APP_INSTANCE_IDS[@]} app instances" + +log "Waiting for instances to reach running..." +for inst_id in "${APP_INSTANCE_IDS[@]}"; do + for attempt in $(seq 1 60); do + STATE=$(node_aws_ec2 "describe-instances --instance-ids $inst_id --query 'Reservations[0].Instances[0].State.Name' --output text" 2>/dev/null) + if [ "$STATE" = "running" ]; then break; fi + if [ $attempt -eq 60 ]; then fail "instance $inst_id stuck in $STATE"; exit 1; fi + sleep 2 + done +done +pass "all instances running" + +declare -A PRE_INSTANCE_IPS +for inst_id in "${APP_INSTANCE_IDS[@]}"; do + IP=$(node_aws_ec2 "describe-instances --instance-ids $inst_id --query 'Reservations[0].Instances[0].PrivateIpAddress' --output text" 2>/dev/null) + if [ -z "$IP" ] || [ "$IP" = "None" ]; then fail "$inst_id has no PrivateIpAddress"; exit 1; fi + PRE_INSTANCE_IPS[$inst_id]="$IP" + log " $inst_id -> $IP" +done +pass "all instances have private IPs" + +# ========================================================================== +# Phase 3: ALB (internet-facing) +# ========================================================================== +log "Creating HTTP target group..." +ALB_TG_ARN=$(node_aws_elbv2 "create-target-group \ + --name reboot-e2e-tg \ + --protocol HTTP --port 80 \ + --vpc-id $VPC_ID \ + --health-check-path /index.html \ + --health-check-interval-seconds 5 \ + --healthy-threshold-count 2 \ + --unhealthy-threshold-count 2 \ + --output json" | jq -r '.TargetGroups[0].TargetGroupArn') +if [ -z "$ALB_TG_ARN" ] || [ "$ALB_TG_ARN" = "null" ]; then fail "create-target-group"; exit 1; fi +pass "target group: $ALB_TG_ARN" + +node_aws_elbv2 "register-targets --target-group-arn $ALB_TG_ARN \ + --targets Id=${APP_INSTANCE_IDS[0]} Id=${APP_INSTANCE_IDS[1]}" >/dev/null \ + || { fail "register-targets"; exit 1; } +pass "registered 2 targets" + +log "Creating internet-facing ALB..." +ALB_LB_ARN=$(node_aws_elbv2 "create-load-balancer \ + --name reboot-e2e-alb \ + --scheme internet-facing \ + --subnets $SUBNET_ID \ + --output json" | jq -r '.LoadBalancers[0].LoadBalancerArn') +if [ -z "$ALB_LB_ARN" ] || [ "$ALB_LB_ARN" = "null" ]; then fail "create-load-balancer"; exit 1; fi +pass "ALB: $ALB_LB_ARN" + +ALB_LB_ID=$(echo "$ALB_LB_ARN" | sed 's|.*/||') + +ALB_LISTENER_ARN=$(node_aws_elbv2 "create-listener \ + --load-balancer-arn $ALB_LB_ARN \ + --protocol HTTP --port 80 \ + --default-actions Type=forward,TargetGroupArn=$ALB_TG_ARN \ + --output json" | jq -r '.Listeners[0].ListenerArn') +if [ -z "$ALB_LISTENER_ARN" ] || [ "$ALB_LISTENER_ARN" = "null" ]; then fail "create-listener"; exit 1; fi +pass "listener: $ALB_LISTENER_ARN" + +wait_for_lb_active "$ALB_LB_ARN" "ALB" || { dump_diagnostics; exit 1; } + +ENI_OUT=$(node_aws_ec2 "describe-network-interfaces \ + --filters Name=description,Values='ELB app/reboot-e2e-alb/${ALB_LB_ID}' \ + --output json" 2>/dev/null) +ALB_PUBLIC_IP=$(echo "$ENI_OUT" | jq -r '.NetworkInterfaces[0].Association.PublicIp // empty') +ALB_ENI_ID=$(echo "$ENI_OUT" | jq -r '.NetworkInterfaces[0].NetworkInterfaceId // empty') +ALB_ENI_MAC=$(echo "$ENI_OUT" | jq -r '.NetworkInterfaces[0].MacAddress // empty') +if [ -z "$ALB_PUBLIC_IP" ] || [ "$ALB_PUBLIC_IP" = "null" ]; then fail "ALB has no public IP"; exit 1; fi +pass "ALB ENI $ALB_ENI_ID public IP: $ALB_PUBLIC_IP" + +wait_for_targets_healthy "$ALB_TG_ARN" 2 "ALB" || { dump_diagnostics; exit 1; } + +# ========================================================================== +# Phase 4: Pre-reboot traffic burst +# ========================================================================== +echo "" +echo "=== Phase 4: Pre-reboot traffic ===" +run_http_burst "http://${ALB_PUBLIC_IP}:80" "ALB pre-reboot" + +# ========================================================================== +# Phase 5: Snapshot pre-reboot state +# ========================================================================== +echo "" +echo "=== Phase 5: Snapshot pre-reboot state ===" +PRE_OVN=$(node_ssh "sudo ovn-nbctl show 2>/dev/null" || echo "") +PRE_OVN_LS_COUNT=$(echo "$PRE_OVN" | grep -c '^switch ' || true) +PRE_OVN_PORT_COUNT=$(echo "$PRE_OVN" | grep -c '^ port ' || true) +log "ovn-nbctl: ${PRE_OVN_LS_COUNT} logical switches, ${PRE_OVN_PORT_COUNT} ports" +log "ALB ENI MAC: ${ALB_ENI_MAC:-}" +pass "pre-reboot state snapshot captured" + +# ========================================================================== +# Phase 6: Reboot the host +# ========================================================================== +echo "" +echo "=== Phase 6: systemctl reboot ===" +REBOOT_START=$(date +%s) +log "Issuing reboot via ssh (connection will drop)..." +node_ssh "sudo systemctl reboot" >/dev/null 2>&1 || true + +# Give the host a moment to actually start shutting down before we begin polling +sleep 5 + +log "Polling TCP/22 (timeout ${REBOOT_WAIT_SECS}s)..." +HOST_UP=false +for attempt in $(seq 1 $((REBOOT_WAIT_SECS / 5))); do + if timeout 5 bash -c "/dev/null; then + if node_ssh "true" 2>/dev/null; then + HOST_UP=true + break + fi + fi + sleep 5 +done +REBOOT_ELAPSED=$(( $(date +%s) - REBOOT_START )) +if [ "$HOST_UP" = true ]; then + pass "host SSH back after ${REBOOT_ELAPSED}s" +else + fail "host SSH did not come back within ${REBOOT_WAIT_SECS}s" + exit 1 +fi + +# ========================================================================== +# Phase 7: Wait for spinifex services + AWS gateway readiness +# ========================================================================== +echo "" +echo "=== Phase 7: Wait for spinifex readiness ===" +log "Service status:" +for svc in spinifex-daemon spinifex-predastore spinifex-viperblock \ + spinifex-nats spinifex-awsgw spinifex-vpcd \ + ovn-controller ovs-vswitchd ovn-northd; do + state=$(node_ssh "systemctl is-active $svc" 2>/dev/null || echo "n/a") + log " $svc: $state" +done + +log "Polling describe-instance-types (timeout ${DAEMON_READY_SECS}s)..." +DAEMON_OK=false +for attempt in $(seq 1 $((DAEMON_READY_SECS / 5))); do + if node_aws_ec2 "describe-instance-types --query 'InstanceTypes[0].InstanceType' --output text" >/dev/null 2>&1; then + DAEMON_OK=true + break + fi + sleep 5 +done +if [ "$DAEMON_OK" = true ]; then + pass "AWS gateway responding post-reboot" +else + fail "AWS gateway did not respond within ${DAEMON_READY_SECS}s" + dump_diagnostics + exit 1 +fi + +# ========================================================================== +# Phase 8: Post-reboot assertions +# ========================================================================== +echo "" +echo "=== Phase 8: Post-reboot assertions ===" + +# 8.1: instances back to running with same IDs +log "Polling instance state (timeout ${INSTANCE_RUNNING_SECS}s)..." +ALL_RUNNING=false +for attempt in $(seq 1 $((INSTANCE_RUNNING_SECS / 5))); do + all=true + for inst_id in "${APP_INSTANCE_IDS[@]}"; do + STATE=$(node_aws_ec2 "describe-instances --instance-ids $inst_id --query 'Reservations[0].Instances[0].State.Name' --output text" 2>/dev/null || echo "missing") + if [ "$STATE" != "running" ]; then all=false; break; fi + done + if [ "$all" = true ]; then ALL_RUNNING=true; break; fi + sleep 5 +done +if [ "$ALL_RUNNING" = true ]; then + pass "all instances back to running" +else + fail "instances did not all reach running within ${INSTANCE_RUNNING_SECS}s" +fi + +# 8.2: private IPs preserved (IPAM persistence) +log "Verifying private IP persistence..." +IPS_PRESERVED=true +for inst_id in "${APP_INSTANCE_IDS[@]}"; do + POST_IP=$(node_aws_ec2 "describe-instances --instance-ids $inst_id --query 'Reservations[0].Instances[0].PrivateIpAddress' --output text" 2>/dev/null) + if [ "$POST_IP" != "${PRE_INSTANCE_IPS[$inst_id]}" ]; then + fail "$inst_id IP drift: pre=${PRE_INSTANCE_IPS[$inst_id]} post=$POST_IP" + IPS_PRESERVED=false + fi +done +if [ "$IPS_PRESERVED" = true ]; then + pass "private IPs preserved across reboot" +fi + +# 8.3: guest VMs actually rebooted (uptime check via two-hop ssh) +log "Verifying guest VMs rebooted (ssh into each guest from spinifex node)..." +GUESTS_REBOOTED=true +for inst_id in "${APP_INSTANCE_IDS[@]}"; do + GUEST_IP="${PRE_INSTANCE_IPS[$inst_id]}" + GUEST_UPTIME=$(node_ssh "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o ConnectTimeout=10 -o LogLevel=ERROR -i $NODE_KEY_PATH ubuntu@${GUEST_IP} 'cat /proc/uptime'" 2>/dev/null | awk '{print int($1)}') + if [ -z "$GUEST_UPTIME" ]; then + fail "$inst_id ($GUEST_IP): guest ssh failed" + GUESTS_REBOOTED=false + elif [ "$GUEST_UPTIME" -gt 600 ]; then + fail "$inst_id ($GUEST_IP): uptime=${GUEST_UPTIME}s — guest did not reboot" + GUESTS_REBOOTED=false + else + log " $inst_id ($GUEST_IP): uptime=${GUEST_UPTIME}s ✓" + fi +done +if [ "$GUESTS_REBOOTED" = true ]; then + pass "all guests rebooted (uptime < 600s)" +fi + +# 8.4: ALB back to active +wait_for_lb_active "$ALB_LB_ARN" "ALB post-reboot" "$LB_RECOVER_SECS" || true + +# 8.5: targets healthy again +wait_for_targets_healthy "$ALB_TG_ARN" 2 "ALB post-reboot" "$LB_RECOVER_SECS" || true + +# 8.6: re-run the traffic burst +echo "" +log "Re-running traffic burst against same ALB..." +run_http_burst "http://${ALB_PUBLIC_IP}:80" "ALB post-reboot" + +# 8.7: ovn-nbctl diff (this is where the known persistence bug lands) +log "Diffing ovn-nbctl show against pre-reboot snapshot..." +POST_OVN=$(node_ssh "sudo ovn-nbctl show 2>/dev/null" || echo "") +POST_OVN_LS_COUNT=$(echo "$POST_OVN" | grep -c '^switch ' || true) +POST_OVN_PORT_COUNT=$(echo "$POST_OVN" | grep -c '^ port ' || true) +log "ovn-nbctl post: ${POST_OVN_LS_COUNT} logical switches, ${POST_OVN_PORT_COUNT} ports (pre: ${PRE_OVN_LS_COUNT} / ${PRE_OVN_PORT_COUNT})" +if [ "$POST_OVN_LS_COUNT" -eq "$PRE_OVN_LS_COUNT" ] && [ "$POST_OVN_PORT_COUNT" -eq "$PRE_OVN_PORT_COUNT" ]; then + pass "ovn-nbctl logical-switch + port counts match pre-reboot" +else + fail "ovn-nbctl drift: switches ${PRE_OVN_LS_COUNT}->${POST_OVN_LS_COUNT}, ports ${PRE_OVN_PORT_COUNT}->${POST_OVN_PORT_COUNT}" +fi + +# Cleanup runs via the EXIT trap.