Add LRU eviction tests for instance management

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)

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)
+}