diff --git a/pkg/instance/lifecycle.go b/pkg/instance/lifecycle.go index ef92f97..91bea6c 100644 --- a/pkg/instance/lifecycle.go +++ b/pkg/instance/lifecycle.go @@ -110,19 +110,25 @@ func (i *Process) Stop() error { i.mu.Unlock() - // Stop the process with SIGINT - if i.cmd.Process != nil { + // Stop the process with SIGINT if cmd exists + if i.cmd != nil && i.cmd.Process != nil { if err := i.cmd.Process.Signal(syscall.SIGINT); err != nil { log.Printf("Failed to send SIGINT to instance %s: %v", i.Name, err) } } + // If no process exists, we can return immediately + if i.cmd == nil || monitorDone == nil { + i.logger.Close() + return nil + } + select { case <-monitorDone: // Process exited normally case <-time.After(30 * time.Second): // Force kill if it doesn't exit within 30 seconds - if i.cmd.Process != nil { + if i.cmd != nil && i.cmd.Process != nil { killErr := i.cmd.Process.Kill() if killErr != nil { log.Printf("Failed to force kill instance %s: %v", i.Name, killErr) diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index df675dc..e6d4116 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -507,3 +507,230 @@ func (m *MockTimeProvider) SetTime(t time.Time) { defer m.mu.Unlock() m.currentTime = t } + +func TestEvictLRUInstance_Success(t *testing.T) { + manager := createTestManager() + // Don't defer manager.Shutdown() - we'll handle cleanup manually + + // Create 3 instances with idle timeout enabled (value doesn't matter for LRU logic) + options1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model1.gguf", + }, + IdleTimeout: func() *int { timeout := 1; return &timeout }(), // Any value > 0 + } + options2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model2.gguf", + }, + IdleTimeout: func() *int { timeout := 1; return &timeout }(), // Any value > 0 + } + options3 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model3.gguf", + }, + IdleTimeout: func() *int { timeout := 1; return &timeout }(), // Any value > 0 + } + + inst1, err := manager.CreateInstance("instance-1", options1) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + inst2, err := manager.CreateInstance("instance-2", options2) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + inst3, err := manager.CreateInstance("instance-3", options3) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + + // Set up mock time and set instances to running + mockTime := NewMockTimeProvider(time.Now()) + inst1.SetTimeProvider(mockTime) + inst2.SetTimeProvider(mockTime) + inst3.SetTimeProvider(mockTime) + + inst1.SetStatus(instance.Running) + inst2.SetStatus(instance.Running) + inst3.SetStatus(instance.Running) + + // Set different last request times (oldest to newest) + // inst1: oldest (will be evicted) + inst1.UpdateLastRequestTime() + + mockTime.SetTime(mockTime.Now().Add(1 * time.Minute)) + inst2.UpdateLastRequestTime() + + mockTime.SetTime(mockTime.Now().Add(1 * time.Minute)) + inst3.UpdateLastRequestTime() + + // Evict LRU instance (should be inst1) + err = manager.EvictLRUInstance() + if err != nil { + t.Fatalf("EvictLRUInstance failed: %v", err) + } + + // Verify inst1 is stopped + if inst1.IsRunning() { + t.Error("Expected instance-1 to be stopped after eviction") + } + + // Verify inst2 and inst3 are still running + if !inst2.IsRunning() { + t.Error("Expected instance-2 to still be running") + } + if !inst3.IsRunning() { + t.Error("Expected instance-3 to still be running") + } + + // Clean up manually - set all to stopped and then shutdown + inst2.SetStatus(instance.Stopped) + inst3.SetStatus(instance.Stopped) +} + +func TestEvictLRUInstance_NoEligibleInstances(t *testing.T) { + manager := createTestManager() + defer manager.Shutdown() + + // Test 1: No running instances + err := manager.EvictLRUInstance() + if err == nil { + t.Error("Expected error when no running instances exist") + } + if err.Error() != "failed to find lru instance" { + t.Errorf("Expected 'failed to find lru instance' error, got: %v", err) + } + + // Test 2: Only instances with IdleTimeout <= 0 + options1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model1.gguf", + }, + IdleTimeout: func() *int { timeout := 0; return &timeout }(), // 0 = no timeout + } + options2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model2.gguf", + }, + IdleTimeout: func() *int { timeout := -1; return &timeout }(), // negative = no timeout + } + options3 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model3.gguf", + }, + // No IdleTimeout set (nil) + } + + inst1, err := manager.CreateInstance("no-timeout-1", options1) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + inst2, err := manager.CreateInstance("no-timeout-2", options2) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + inst3, err := manager.CreateInstance("no-timeout-3", options3) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + + // Set instances to running + inst1.SetStatus(instance.Running) + inst2.SetStatus(instance.Running) + inst3.SetStatus(instance.Running) + + // Try to evict - should fail because no eligible instances + err = manager.EvictLRUInstance() + if err == nil { + t.Error("Expected error when no eligible instances exist") + } + if err.Error() != "failed to find lru instance" { + t.Errorf("Expected 'failed to find lru instance' error, got: %v", err) + } + + // Verify all instances are still running + if !inst1.IsRunning() { + t.Error("Expected no-timeout-1 to still be running") + } + if !inst2.IsRunning() { + t.Error("Expected no-timeout-2 to still be running") + } + if !inst3.IsRunning() { + t.Error("Expected no-timeout-3 to still be running") + } + + // Reset instances to stopped to avoid shutdown panics + inst1.SetStatus(instance.Stopped) + inst2.SetStatus(instance.Stopped) + inst3.SetStatus(instance.Stopped) +} + +func TestEvictLRUInstance_SkipsInstancesWithoutTimeout(t *testing.T) { + manager := createTestManager() + defer manager.Shutdown() + + // Create mix of instances: some with timeout enabled, some disabled + optionsWithTimeout := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model-with-timeout.gguf", + }, + IdleTimeout: func() *int { timeout := 1; return &timeout }(), // Any value > 0 + } + optionsWithoutTimeout1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model-no-timeout1.gguf", + }, + IdleTimeout: func() *int { timeout := 0; return &timeout }(), // 0 = no timeout + } + optionsWithoutTimeout2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model-no-timeout2.gguf", + }, + // No IdleTimeout set (nil) + } + + instWithTimeout, err := manager.CreateInstance("with-timeout", optionsWithTimeout) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + instNoTimeout1, err := manager.CreateInstance("no-timeout-1", optionsWithoutTimeout1) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + instNoTimeout2, err := manager.CreateInstance("no-timeout-2", optionsWithoutTimeout2) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) + } + + // Set all instances to running + instWithTimeout.SetStatus(instance.Running) + instNoTimeout1.SetStatus(instance.Running) + instNoTimeout2.SetStatus(instance.Running) + + // Update request times + instWithTimeout.UpdateLastRequestTime() + instNoTimeout1.UpdateLastRequestTime() + instNoTimeout2.UpdateLastRequestTime() + + // Evict LRU instance - should only consider the one with timeout + err = manager.EvictLRUInstance() + if err != nil { + t.Fatalf("EvictLRUInstance failed: %v", err) + } + + // Verify only the instance with timeout was evicted + if instWithTimeout.IsRunning() { + t.Error("Expected with-timeout instance to be stopped after eviction") + } + if !instNoTimeout1.IsRunning() { + t.Error("Expected no-timeout-1 instance to still be running") + } + if !instNoTimeout2.IsRunning() { + t.Error("Expected no-timeout-2 instance to still be running") + } + + // Reset instances to stopped to avoid shutdown panics + instNoTimeout1.SetStatus(instance.Stopped) + instNoTimeout2.SetStatus(instance.Stopped) +}