diff --git a/lib/autostandby/README.md b/lib/autostandby/README.md index f729d7c6..200dd712 100644 --- a/lib/autostandby/README.md +++ b/lib/autostandby/README.md @@ -15,6 +15,7 @@ A VM is considered active when there is at least one tracked TCP flow where: That means: - inbound client connections keep the VM awake +- a client mid-handshake (`SYN_SENT`) counts as activity, so a freshly restored guest that has not yet answered the SYN it was woken for is not put back into standby - replies to outbound guest requests do not keep the VM awake - same-host clients count by default diff --git a/lib/autostandby/classifier_test.go b/lib/autostandby/classifier_test.go index bc778069..dc175ef4 100644 --- a/lib/autostandby/classifier_test.go +++ b/lib/autostandby/classifier_test.go @@ -55,10 +55,16 @@ func TestActiveInboundCountCountsOnlyQualifyingInboundTCP(t *testing.T) { OriginalDestinationPort: 8080, TCPState: TCPStateTimeWait, }, + { + OriginalSourceIP: netip.MustParseAddr("7.7.7.7"), + OriginalDestinationIP: netip.MustParseAddr("192.168.100.10"), + OriginalDestinationPort: 8080, + TCPState: TCPStateSynSent, + }, }) require.NoError(t, err) - assert.Equal(t, 1, count) + assert.Equal(t, 2, count) assert.Equal(t, 5*time.Minute, idleTimeout) } diff --git a/lib/autostandby/controller_test.go b/lib/autostandby/controller_test.go index cd6cd5a1..b0cd602e 100644 --- a/lib/autostandby/controller_test.go +++ b/lib/autostandby/controller_test.go @@ -314,6 +314,70 @@ func TestConnectionEventsClearIdleAndStartCountdown(t *testing.T) { require.NotNil(t, status.IdleSince) } +func TestSynSentConnectionKeepsInstanceActive(t *testing.T) { + t.Parallel() + + store := newFakeInstanceStore([]Instance{{ + ID: "inst-synsent", + Name: "inst-synsent", + State: StateRunning, + NetworkEnabled: true, + IP: "192.168.100.33", + AutoStandby: &Policy{Enabled: true, IdleTimeout: "1m"}, + }}) + now := time.Date(2026, 4, 6, 11, 0, 0, 0, time.UTC) + controller := NewController(store, &fakeConnectionSource{}, ControllerOptions{ + Now: func() time.Time { return now }, + }) + require.NoError(t, controller.startupResync(context.Background())) + + // A half-open handshake against a slow guest arrives as NEW in SYN_SENT. + controller.handleConnectionEvent(context.Background(), ConnectionEvent{ + Type: ConnectionEventNew, + Connection: Connection{ + OriginalSourceIP: mustAddr("1.2.3.4"), + OriginalSourcePort: 50005, + OriginalDestinationIP: mustAddr("192.168.100.33"), + OriginalDestinationPort: 8080, + TCPState: TCPStateSynSent, + }, + ObservedAt: now.Add(5 * time.Second), + }) + + status := controller.Describe(store.instances[0]) + require.Equal(t, StatusActive, status.Status) + require.Equal(t, 1, status.ActiveInboundCount) + require.Nil(t, status.IdleSince) +} + +func TestSnapshotCountsSynSentConnection(t *testing.T) { + t.Parallel() + + store := newFakeInstanceStore([]Instance{{ + ID: "inst-synsent-snap", + Name: "inst-synsent-snap", + State: StateRunning, + NetworkEnabled: true, + IP: "192.168.100.34", + AutoStandby: &Policy{Enabled: true, IdleTimeout: "1m"}, + }}) + source := &fakeConnectionSource{connections: []Connection{{ + OriginalSourceIP: mustAddr("1.2.3.4"), + OriginalSourcePort: 50006, + OriginalDestinationIP: mustAddr("192.168.100.34"), + OriginalDestinationPort: 8080, + TCPState: TCPStateSynSent, + }}} + controller := NewController(store, source, ControllerOptions{ + Now: func() time.Time { return time.Date(2026, 4, 6, 11, 0, 0, 0, time.UTC) }, + }) + require.NoError(t, controller.startupResync(context.Background())) + + status := controller.Describe(store.instances[0]) + require.Equal(t, StatusActive, status.Status) + require.Equal(t, 1, status.ActiveInboundCount) +} + func TestConnectionUpdateWithInactiveTCPStateStartsCountdown(t *testing.T) { t.Parallel() diff --git a/lib/autostandby/types.go b/lib/autostandby/types.go index 38165056..0dfa319b 100644 --- a/lib/autostandby/types.go +++ b/lib/autostandby/types.go @@ -62,10 +62,14 @@ const ( TCPStateRetrans TCPState = 11 ) -// Active reports whether the TCP state should keep a VM awake. +// Active reports whether the TCP state should keep a VM awake. SYN_SENT +// counts: a client mid-handshake is inbound demand, and a freshly restored +// guest can take several seconds to answer the SYN it was woken for — going +// back to standby in that window orphans the connection, since wake-on-traffic +// does not exist. func (s TCPState) Active() bool { switch s { - case TCPStateSynRecv, TCPStateEstablished, TCPStateFinWait, TCPStateCloseWait, TCPStateLastAck: + case TCPStateSynSent, TCPStateSynRecv, TCPStateEstablished, TCPStateFinWait, TCPStateCloseWait, TCPStateLastAck: return true default: return false diff --git a/lib/instances/auto_standby_integration_linux_test.go b/lib/instances/auto_standby_integration_linux_test.go index 2b13a77b..2c542e40 100644 --- a/lib/instances/auto_standby_integration_linux_test.go +++ b/lib/instances/auto_standby_integration_linux_test.go @@ -9,6 +9,7 @@ import ( "log/slog" "net" "os" + "os/exec" "testing" "time" @@ -89,12 +90,8 @@ func (s integrationAutoStandbyStore) SubscribeInstanceEvents() (<-chan autostand return dst, unsub, nil } -func TestAutoStandbyCloudHypervisorActiveInboundTCP(t *testing.T) { - requireAutoStandbyE2EManualRun(t) - requireKVMAccess(t) - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) - defer cancel() +func setupAutoStandbyE2EInstance(t *testing.T, ctx context.Context, name string) (*manager, *autostandby.ConntrackSource, *Instance) { + t.Helper() mgr, _ := setupCompressionTestManagerForHypervisor(t, hypervisor.TypeCloudHypervisor) require.NoError(t, mgr.networkManager.Initialize(ctx, nil)) @@ -110,7 +107,7 @@ func TestAutoStandbyCloudHypervisorActiveInboundTCP(t *testing.T) { } inst, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ - Name: "auto-standby-e2e", + Name: name, Image: integrationTestImageRef(t, "docker.io/library/nginx:alpine"), Size: 1024 * 1024 * 1024, HotplugSize: 512 * 1024 * 1024, @@ -137,13 +134,40 @@ func TestAutoStandbyCloudHypervisorActiveInboundTCP(t *testing.T) { require.NoError(t, waitForExecAgent(ctx, mgr, inst.Id, 30*time.Second)) require.NoError(t, waitForLogMessage(ctx, mgr, inst.Id, "start worker processes", 45*time.Second)) - conn, err := dialGuestPortWithRetry(inst.IP, 80, 15*time.Second) - require.NoError(t, err) - defer func() { - if conn != nil { - _ = conn.Close() - } + return mgr, connSource, inst +} + +func startAutoStandbyE2EController(t *testing.T, ctx context.Context, mgr *manager, connSource *autostandby.ConntrackSource) { + t.Helper() + + controllerCtx, controllerCancel := context.WithCancel(ctx) + controllerDone := make(chan error, 1) + controller := autostandby.NewController( + integrationAutoStandbyStore{manager: mgr}, + connSource, + autostandby.ControllerOptions{ + Log: slog.Default(), + ReconnectDelay: 250 * time.Millisecond, + }, + ) + go func() { + controllerDone <- controller.Run(controllerCtx) }() + t.Cleanup(func() { + controllerCancel() + select { + case err := <-controllerDone: + if err != nil { + t.Logf("auto-standby controller exited with error during cleanup: %v", err) + } + case <-time.After(2 * time.Second): + t.Log("timed out waiting for auto-standby controller shutdown") + } + }) +} + +func requireActiveInboundEventually(t *testing.T, ctx context.Context, connSource *autostandby.ConntrackSource, inst *Instance, msg string) { + t.Helper() require.Eventually(t, func() bool { conns, err := connSource.ListConnections(ctx) @@ -168,47 +192,145 @@ func TestAutoStandbyCloudHypervisorActiveInboundTCP(t *testing.T) { return false } return count > 0 - }, 10*time.Second, 200*time.Millisecond, "host->guest TCP connection never appeared in conntrack") + }, 10*time.Second, 200*time.Millisecond, msg) +} - controllerCtx, controllerCancel := context.WithCancel(ctx) - controllerDone := make(chan error, 1) - controller := autostandby.NewController( - integrationAutoStandbyStore{manager: mgr}, - connSource, - autostandby.ControllerOptions{ - Log: slog.Default(), - ReconnectDelay: 250 * time.Millisecond, - }, - ) - go func() { - controllerDone <- controller.Run(controllerCtx) +func TestAutoStandbyCloudHypervisorActiveInboundTCP(t *testing.T) { + requireAutoStandbyE2EManualRun(t) + requireKVMAccess(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + mgr, connSource, inst := setupAutoStandbyE2EInstance(t, ctx, "auto-standby-e2e") + instanceID := inst.Id + + conn, err := dialGuestPortWithRetry(inst.IP, 80, 15*time.Second) + require.NoError(t, err) + defer func() { + if conn != nil { + _ = conn.Close() + } }() + + requireActiveInboundEventually(t, ctx, connSource, inst, "host->guest TCP connection never appeared in conntrack") + + startAutoStandbyE2EController(t, ctx, mgr, connSource) + + time.Sleep(5 * time.Second) + + current, err := mgr.GetInstance(ctx, instanceID) + require.NoError(t, err) + require.Equal(t, StateRunning, current.State, "instance should remain running while inbound TCP connection is open") + + require.NoError(t, conn.Close()) + conn = nil + + inst, err = waitForInstanceState(ctx, mgr, instanceID, StateStandby, 45*time.Second) + require.NoError(t, err) + require.Equal(t, StateStandby, inst.State) +} + +// TestAutoStandbyCloudHypervisorHalfOpenInboundTCP reproduces the race where a +// client dials the guest but the handshake does not complete for several +// seconds, leaving the host-side conntrack flow in SYN_SENT — what a freshly +// restored guest looks like while it faults its memory back in. That flow must +// keep the VM awake: before SYN_SENT counted as activity, the idle timer fired +// mid-handshake and put the VM into standby with the client's request in +// flight. +func TestAutoStandbyCloudHypervisorHalfOpenInboundTCP(t *testing.T) { + requireAutoStandbyE2EManualRun(t) + requireKVMAccess(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + mgr, connSource, inst := setupAutoStandbyE2EInstance(t, ctx, "auto-standby-e2e-halfopen") + instanceID := inst.Id + + // Confirm the guest serves before quiescing. + probe, err := dialGuestPortWithRetry(inst.IP, 80, 15*time.Second) + require.NoError(t, err) + require.NoError(t, probe.Close()) + + // Drop the guest's replies in the raw table, before conntrack processes + // them: the client's SYN reaches nginx but the SYN-ACK never makes it + // back, so the host-side flow stays in SYN_SENT while the client + // retransmits — the same shape as a claim hitting a freshly restored + // guest that has not answered yet. + matchArgs := []string{"-s", inst.IP, "-p", "tcp", "--sport", "80", "-j", "DROP"} + require.NoError(t, runIptables(append([]string{"-t", "raw", "-I", "PREROUTING", "1"}, matchArgs...)...)) + dropActive := true + removeDrop := func() error { + if !dropActive { + return nil + } + dropActive = false + return runIptables(append([]string{"-t", "raw", "-D", "PREROUTING"}, matchArgs...)...) + } t.Cleanup(func() { - controllerCancel() - select { - case err := <-controllerDone: - if err != nil { - t.Logf("auto-standby controller exited with error during cleanup: %v", err) - } - case <-time.After(2 * time.Second): - t.Log("timed out waiting for auto-standby controller shutdown") + if err := removeDrop(); err != nil { + t.Logf("failed to remove drop rule during cleanup: %v", err) } }) + type dialResult struct { + conn net.Conn + err error + } + dialDone := make(chan dialResult, 1) + go func() { + conn, err := net.DialTimeout("tcp", net.JoinHostPort(inst.IP, "80"), 90*time.Second) + dialDone <- dialResult{conn: conn, err: err} + }() + + requireActiveInboundEventually(t, ctx, connSource, inst, "half-open host->guest TCP flow never counted as inbound activity") + + startAutoStandbyE2EController(t, ctx, mgr, connSource) + + // Well past the 3s idle timeout; only the half-open flow holds the VM up. time.Sleep(5 * time.Second) + select { + case res := <-dialDone: + if res.conn != nil { + _ = res.conn.Close() + } + t.Fatalf("dial completed while guest replies were dropped (err=%v); half-open scenario not established", res.err) + default: + } + current, err := mgr.GetInstance(ctx, instanceID) require.NoError(t, err) - require.Equal(t, StateRunning, current.State, "instance should remain running while inbound TCP connection is open") + require.Equal(t, StateRunning, current.State, "instance must remain running while a client is mid-handshake") + // Let the handshake complete, then confirm the normal idle path still + // reaches standby once the connection closes. + require.NoError(t, removeDrop()) + + var conn net.Conn + select { + case res := <-dialDone: + require.NoError(t, res.err) + conn = res.conn + case <-time.After(45 * time.Second): + t.Fatal("connection never completed after the drop rule was removed") + } require.NoError(t, conn.Close()) - conn = nil inst, err = waitForInstanceState(ctx, mgr, instanceID, StateStandby, 45*time.Second) require.NoError(t, err) require.Equal(t, StateStandby, inst.State) } +func runIptables(args ...string) error { + out, err := exec.Command("iptables", append([]string{"-w", "5"}, args...)...).CombinedOutput() + if err != nil { + return fmt.Errorf("iptables %v: %w: %s", args, err, out) + } + return nil +} + func dialGuestPortWithRetry(ip string, port int, timeout time.Duration) (net.Conn, error) { deadline := time.Now().Add(timeout) address := net.JoinHostPort(ip, fmt.Sprintf("%d", port))