diff --git a/README.md b/README.md index e944e5f..65dfb42 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,8 @@ 🌐 **Web Dashboard**: Modern React UI for visual management (unlike CLI-only tools) 🔐 **API Key Authentication**: Separate keys for management vs inference access 📊 **Instance Monitoring**: Health checks, auto-restart, log management -⚡ **Persistent State**: Instances survive server restarts +⏳ **Idle Timeout Management**: Automatically stop idle instances after a configurable period +💾 **State Persistence**: Ensure instances remain intact across server restarts ![Dashboard Screenshot](docs/images/screenshot.png) @@ -172,16 +173,17 @@ server: ```yaml instances: - port_range: [8000, 9000] # Port range for instances (default: [8000, 9000]) - data_dir: "~/.local/share/llamactl" # Directory for all llamactl data (default varies by OS) - configs_dir: "~/.local/share/llamactl/instances" # Directory for instance configs (default: data_dir/instances) - logs_dir: "~/.local/share/llamactl/logs" # Directory for instance logs (default: data_dir/logs) - auto_create_dirs: true # Automatically create data/config/logs directories (default: true) - max_instances: -1 # Maximum instances (-1 = unlimited) - llama_executable: "llama-server" # Path to llama-server executable - default_auto_restart: true # Default auto-restart setting - default_max_restarts: 3 # Default maximum restart attempts - default_restart_delay: 5 # Default restart delay in seconds + port_range: [8000, 9000] # Port range for instances (default: [8000, 9000]) + data_dir: "~/.local/share/llamactl" # Directory for all llamactl data (default varies by OS) + configs_dir: "~/.local/share/llamactl/instances" # Directory for instance configs (default: data_dir/instances) + logs_dir: "~/.local/share/llamactl/logs" # Directory for instance logs (default: data_dir/logs) + auto_create_dirs: true # Automatically create data/config/logs directories (default: true) + max_instances: -1 # Maximum instances (-1 = unlimited) + llama_executable: "llama-server" # Path to llama-server executable + default_auto_restart: true # Default auto-restart setting + default_max_restarts: 3 # Default maximum restart attempts + default_restart_delay: 5 # Default restart delay in seconds + timeout_check_interval: 5 # Default instance timeout check interval in minutes ``` **Environment Variables:** @@ -195,6 +197,7 @@ instances: - `LLAMACTL_DEFAULT_AUTO_RESTART` - Default auto-restart setting (true/false) - `LLAMACTL_DEFAULT_MAX_RESTARTS` - Default maximum restarts - `LLAMACTL_DEFAULT_RESTART_DELAY` - Default restart delay in seconds +- `LLAMACTL_TIMEOUT_CHECK_INTERVAL` - Default instance timeout check interval in minutes #### Authentication Configuration diff --git a/pkg/config/config.go b/pkg/config/config.go index fd33715..386a708 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -66,6 +66,9 @@ type InstancesConfig struct { // Default restart delay for new instances (in seconds) DefaultRestartDelay int `yaml:"default_restart_delay"` + + // Interval for checking instance timeouts (in minutes) + TimeoutCheckInterval int `yaml:"timeout_check_interval"` } // AuthConfig contains authentication settings @@ -98,16 +101,17 @@ func LoadConfig(configPath string) (AppConfig, error) { EnableSwagger: false, }, Instances: InstancesConfig{ - PortRange: [2]int{8000, 9000}, - DataDir: getDefaultDataDirectory(), - InstancesDir: filepath.Join(getDefaultDataDirectory(), "instances"), - LogsDir: filepath.Join(getDefaultDataDirectory(), "logs"), - AutoCreateDirs: true, - MaxInstances: -1, // -1 means unlimited - LlamaExecutable: "llama-server", - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, - DefaultRestartDelay: 5, + PortRange: [2]int{8000, 9000}, + DataDir: getDefaultDataDirectory(), + InstancesDir: filepath.Join(getDefaultDataDirectory(), "instances"), + LogsDir: filepath.Join(getDefaultDataDirectory(), "logs"), + AutoCreateDirs: true, + MaxInstances: -1, // -1 means unlimited + LlamaExecutable: "llama-server", + DefaultAutoRestart: true, + DefaultMaxRestarts: 3, + DefaultRestartDelay: 5, + TimeoutCheckInterval: 5, // Check timeouts every 5 minutes }, Auth: AuthConfig{ RequireInferenceAuth: true, @@ -217,6 +221,11 @@ func loadEnvVars(cfg *AppConfig) { cfg.Instances.DefaultRestartDelay = seconds } } + if timeoutCheckInterval := os.Getenv("LLAMACTL_TIMEOUT_CHECK_INTERVAL"); timeoutCheckInterval != "" { + if minutes, err := strconv.Atoi(timeoutCheckInterval); err == nil { + cfg.Instances.TimeoutCheckInterval = minutes + } + } // Auth config if requireInferenceAuth := os.Getenv("LLAMACTL_REQUIRE_INFERENCE_AUTH"); requireInferenceAuth != "" { if b, err := strconv.ParseBool(requireInferenceAuth); err == nil { diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go index dd4a45f..5ff4bf5 100644 --- a/pkg/instance/instance.go +++ b/pkg/instance/instance.go @@ -13,16 +13,30 @@ import ( "net/url" "os/exec" "sync" + "sync/atomic" "time" ) +// TimeProvider interface allows for testing with mock time +type TimeProvider interface { + Now() time.Time +} + +// realTimeProvider implements TimeProvider using the actual time +type realTimeProvider struct{} + +func (realTimeProvider) Now() time.Time { + return time.Now() +} + type CreateInstanceOptions struct { // Auto restart - AutoRestart *bool `json:"auto_restart,omitempty"` - MaxRestarts *int `json:"max_restarts,omitempty"` - // RestartDelay duration in seconds - RestartDelay *int `json:"restart_delay_seconds,omitempty"` - + AutoRestart *bool `json:"auto_restart,omitempty"` + MaxRestarts *int `json:"max_restarts,omitempty"` + RestartDelay *int `json:"restart_delay,omitempty"` + // Timeout + IdleTimeout *int `json:"idle_timeout,omitempty"` + // LlamaServerOptions contains the options for the llama server llamacpp.LlamaServerOptions `json:",inline"` } @@ -34,7 +48,8 @@ func (c *CreateInstanceOptions) UnmarshalJSON(data []byte) error { type tempCreateOptions struct { AutoRestart *bool `json:"auto_restart,omitempty"` MaxRestarts *int `json:"max_restarts,omitempty"` - RestartDelay *int `json:"restart_delay_seconds,omitempty"` + RestartDelay *int `json:"restart_delay,omitempty"` + IdleTimeout *int `json:"idle_timeout,omitempty"` } var temp tempCreateOptions @@ -46,6 +61,7 @@ func (c *CreateInstanceOptions) UnmarshalJSON(data []byte) error { c.AutoRestart = temp.AutoRestart c.MaxRestarts = temp.MaxRestarts c.RestartDelay = temp.RestartDelay + c.IdleTimeout = temp.IdleTimeout // Now unmarshal the embedded LlamaServerOptions if err := json.Unmarshal(data, &c.LlamaServerOptions); err != nil { @@ -83,6 +99,10 @@ type Process struct { // Restart control restartCancel context.CancelFunc `json:"-"` // Cancel function for pending restarts monitorDone chan struct{} `json:"-"` // Channel to signal monitor goroutine completion + + // Timeout management + lastRequestTime atomic.Int64 // Unix timestamp of last request + timeProvider TimeProvider `json:"-"` // Time provider for testing } // validateAndCopyOptions validates and creates a deep copy of the provided options @@ -117,6 +137,15 @@ func validateAndCopyOptions(name string, options *CreateInstanceOptions) *Create } optionsCopy.RestartDelay = &restartDelay } + + if options.IdleTimeout != nil { + idleTimeout := *options.IdleTimeout + if idleTimeout < 0 { + log.Printf("Instance %s IdleTimeout value (%d) cannot be negative, setting to 0 minutes", name, idleTimeout) + idleTimeout = 0 + } + optionsCopy.IdleTimeout = &idleTimeout + } } return optionsCopy @@ -142,6 +171,11 @@ func applyDefaultOptions(options *CreateInstanceOptions, globalSettings *config. defaultRestartDelay := globalSettings.DefaultRestartDelay options.RestartDelay = &defaultRestartDelay } + + if options.IdleTimeout == nil { + defaultIdleTimeout := 0 + options.IdleTimeout = &defaultIdleTimeout + } } // NewInstance creates a new instance with the given name, log path, and options @@ -158,10 +192,8 @@ func NewInstance(name string, globalSettings *config.InstancesConfig, options *C options: optionsCopy, globalSettings: globalSettings, logger: logger, - - Running: false, - - Created: time.Now().Unix(), + timeProvider: realTimeProvider{}, + Created: time.Now().Unix(), } } @@ -189,6 +221,11 @@ func (i *Process) SetOptions(options *CreateInstanceOptions) { i.proxy = nil } +// SetTimeProvider sets a custom time provider for testing +func (i *Process) SetTimeProvider(tp TimeProvider) { + i.timeProvider = tp +} + // GetProxy returns the reverse proxy for this instance, creating it if needed func (i *Process) GetProxy() (*httputil.ReverseProxy, error) { i.mu.Lock() diff --git a/pkg/instance/instance_test.go b/pkg/instance/instance_test.go index d89bad3..3ed6c08 100644 --- a/pkg/instance/instance_test.go +++ b/pkg/instance/instance_test.go @@ -91,38 +91,6 @@ func TestNewInstance_WithRestartOptions(t *testing.T) { } } -func TestNewInstance_ValidationAndDefaults(t *testing.T) { - globalSettings := &config.InstancesConfig{ - LogsDir: "/tmp/test", - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, - DefaultRestartDelay: 5, - } - - // Test with invalid negative values - invalidMaxRestarts := -5 - invalidRestartDelay := -10 - - options := &instance.CreateInstanceOptions{ - MaxRestarts: &invalidMaxRestarts, - RestartDelay: &invalidRestartDelay, - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - instance := instance.NewInstance("test-instance", globalSettings, options) - opts := instance.GetOptions() - - // Check that negative values were corrected to 0 - if opts.MaxRestarts == nil || *opts.MaxRestarts != 0 { - t.Errorf("Expected MaxRestarts to be corrected to 0, got %v", opts.MaxRestarts) - } - if opts.RestartDelay == nil || *opts.RestartDelay != 0 { - t.Errorf("Expected RestartDelay to be corrected to 0, got %v", opts.RestartDelay) - } -} - func TestSetOptions(t *testing.T) { globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", @@ -164,33 +132,6 @@ func TestSetOptions(t *testing.T) { } } -func TestSetOptions_NilOptions(t *testing.T) { - globalSettings := &config.InstancesConfig{ - LogsDir: "/tmp/test", - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, - DefaultRestartDelay: 5, - } - - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - instance := instance.NewInstance("test-instance", globalSettings, options) - originalOptions := instance.GetOptions() - - // Try to set nil options - instance.SetOptions(nil) - - // Options should remain unchanged - currentOptions := instance.GetOptions() - if currentOptions.Model != originalOptions.Model { - t.Error("Options should not change when setting nil options") - } -} - func TestGetProxy(t *testing.T) { globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", @@ -317,58 +258,6 @@ func TestUnmarshalJSON(t *testing.T) { } } -func TestUnmarshalJSON_PartialOptions(t *testing.T) { - jsonData := `{ - "name": "test-instance", - "running": false, - "options": { - "model": "/path/to/model.gguf" - } - }` - - var inst instance.Process - err := json.Unmarshal([]byte(jsonData), &inst) - if err != nil { - t.Fatalf("JSON unmarshal failed: %v", err) - } - - opts := inst.GetOptions() - if opts.Model != "/path/to/model.gguf" { - t.Errorf("Expected model '/path/to/model.gguf', got %q", opts.Model) - } - - // Note: Defaults are NOT applied during unmarshaling - // They should only be applied by NewInstance or SetOptions - if opts.AutoRestart != nil { - t.Error("Expected AutoRestart to be nil (no defaults applied during unmarshal)") - } -} - -func TestUnmarshalJSON_NoOptions(t *testing.T) { - jsonData := `{ - "name": "test-instance", - "running": false - }` - - var inst instance.Process - err := json.Unmarshal([]byte(jsonData), &inst) - if err != nil { - t.Fatalf("JSON unmarshal failed: %v", err) - } - - if inst.Name != "test-instance" { - t.Errorf("Expected name 'test-instance', got %q", inst.Name) - } - if inst.Running { - t.Error("Expected running to be false") - } - - opts := inst.GetOptions() - if opts != nil { - t.Error("Expected options to be nil when not provided in JSON") - } -} - func TestCreateInstanceOptionsValidation(t *testing.T) { tests := []struct { name string @@ -377,13 +266,6 @@ func TestCreateInstanceOptionsValidation(t *testing.T) { expectedMax int expectedDelay int }{ - { - name: "nil values", - maxRestarts: nil, - restartDelay: nil, - expectedMax: 0, // Should remain nil, but we can't easily test nil in this structure - expectedDelay: 0, - }, { name: "valid positive values", maxRestarts: testutil.IntPtr(10), @@ -424,20 +306,16 @@ func TestCreateInstanceOptionsValidation(t *testing.T) { instance := instance.NewInstance("test", globalSettings, options) opts := instance.GetOptions() - if tt.maxRestarts != nil { - if opts.MaxRestarts == nil { - t.Error("Expected MaxRestarts to be set") - } else if *opts.MaxRestarts != tt.expectedMax { - t.Errorf("Expected MaxRestarts %d, got %d", tt.expectedMax, *opts.MaxRestarts) - } + if opts.MaxRestarts == nil { + t.Error("Expected MaxRestarts to be set") + } else if *opts.MaxRestarts != tt.expectedMax { + t.Errorf("Expected MaxRestarts %d, got %d", tt.expectedMax, *opts.MaxRestarts) } - if tt.restartDelay != nil { - if opts.RestartDelay == nil { - t.Error("Expected RestartDelay to be set") - } else if *opts.RestartDelay != tt.expectedDelay { - t.Errorf("Expected RestartDelay %d, got %d", tt.expectedDelay, *opts.RestartDelay) - } + if opts.RestartDelay == nil { + t.Error("Expected RestartDelay to be set") + } else if *opts.RestartDelay != tt.expectedDelay { + t.Errorf("Expected RestartDelay %d, got %d", tt.expectedDelay, *opts.RestartDelay) } }) } diff --git a/pkg/instance/lifecycle.go b/pkg/instance/lifecycle.go index 1442b6e..3db3ca7 100644 --- a/pkg/instance/lifecycle.go +++ b/pkg/instance/lifecycle.go @@ -30,6 +30,9 @@ func (i *Process) Start() error { i.restarts = 0 } + // Initialize last request time to current time when starting + i.lastRequestTime.Store(i.timeProvider.Now().Unix()) + // Create log files if err := i.logger.Create(); err != nil { return fmt.Errorf("failed to create log files: %w", err) diff --git a/pkg/instance/timeout.go b/pkg/instance/timeout.go new file mode 100644 index 0000000..19344e6 --- /dev/null +++ b/pkg/instance/timeout.go @@ -0,0 +1,28 @@ +package instance + +// UpdateLastRequestTime updates the last request access time for the instance via proxy +func (i *Process) UpdateLastRequestTime() { + i.mu.Lock() + defer i.mu.Unlock() + + lastRequestTime := i.timeProvider.Now().Unix() + i.lastRequestTime.Store(lastRequestTime) +} + +func (i *Process) ShouldTimeout() bool { + i.mu.RLock() + defer i.mu.RUnlock() + + if !i.Running || i.options.IdleTimeout == nil || *i.options.IdleTimeout <= 0 { + return false + } + + // Check if the last request time exceeds the idle timeout + lastRequest := i.lastRequestTime.Load() + idleTimeoutMinutes := *i.options.IdleTimeout + + // Convert timeout from minutes to seconds for comparison + idleTimeoutSeconds := int64(idleTimeoutMinutes * 60) + + return (i.timeProvider.Now().Unix() - lastRequest) > idleTimeoutSeconds +} diff --git a/pkg/instance/timeout_test.go b/pkg/instance/timeout_test.go new file mode 100644 index 0000000..5cd67f2 --- /dev/null +++ b/pkg/instance/timeout_test.go @@ -0,0 +1,195 @@ +package instance_test + +import ( + "llamactl/pkg/backends/llamacpp" + "llamactl/pkg/config" + "llamactl/pkg/instance" + "llamactl/pkg/testutil" + "sync/atomic" + "testing" + "time" +) + +// MockTimeProvider implements TimeProvider for testing +type MockTimeProvider struct { + currentTime atomic.Int64 // Unix timestamp +} + +func NewMockTimeProvider(t time.Time) *MockTimeProvider { + m := &MockTimeProvider{} + m.currentTime.Store(t.Unix()) + return m +} + +func (m *MockTimeProvider) Now() time.Time { + return time.Unix(m.currentTime.Load(), 0) +} + +func (m *MockTimeProvider) SetTime(t time.Time) { + m.currentTime.Store(t.Unix()) +} + +// Timeout-related tests + +func TestUpdateLastRequestTime(t *testing.T) { + globalSettings := &config.InstancesConfig{ + LogsDir: "/tmp/test", + } + + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + } + + inst := instance.NewInstance("test-instance", globalSettings, options) + + // Test that UpdateLastRequestTime doesn't panic + inst.UpdateLastRequestTime() +} + +func TestShouldTimeout_NotRunning(t *testing.T) { + globalSettings := &config.InstancesConfig{ + LogsDir: "/tmp/test", + } + + idleTimeout := 1 // 1 minute + options := &instance.CreateInstanceOptions{ + IdleTimeout: &idleTimeout, + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + } + + inst := instance.NewInstance("test-instance", globalSettings, options) + + // Instance is not running, should not timeout regardless of configuration + if inst.ShouldTimeout() { + t.Error("Non-running instance should never timeout") + } +} + +func TestShouldTimeout_NoTimeoutConfigured(t *testing.T) { + globalSettings := &config.InstancesConfig{ + LogsDir: "/tmp/test", + } + + tests := []struct { + name string + idleTimeout *int + }{ + {"nil timeout", nil}, + {"zero timeout", testutil.IntPtr(0)}, + {"negative timeout", testutil.IntPtr(-5)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + options := &instance.CreateInstanceOptions{ + IdleTimeout: tt.idleTimeout, + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + } + + inst := instance.NewInstance("test-instance", globalSettings, options) + // Simulate running state + inst.Running = true + + if inst.ShouldTimeout() { + t.Errorf("Instance with %s should not timeout", tt.name) + } + }) + } +} + +func TestShouldTimeout_WithinTimeLimit(t *testing.T) { + globalSettings := &config.InstancesConfig{ + LogsDir: "/tmp/test", + } + + idleTimeout := 5 // 5 minutes + options := &instance.CreateInstanceOptions{ + IdleTimeout: &idleTimeout, + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + } + + inst := instance.NewInstance("test-instance", globalSettings, options) + inst.Running = true + + // Update last request time to now + inst.UpdateLastRequestTime() + + // Should not timeout immediately + if inst.ShouldTimeout() { + t.Error("Instance should not timeout when last request was recent") + } +} + +func TestShouldTimeout_ExceedsTimeLimit(t *testing.T) { + globalSettings := &config.InstancesConfig{ + LogsDir: "/tmp/test", + } + + idleTimeout := 1 // 1 minute + options := &instance.CreateInstanceOptions{ + IdleTimeout: &idleTimeout, + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + } + + inst := instance.NewInstance("test-instance", globalSettings, options) + inst.Running = true + + // Use MockTimeProvider to simulate old last request time + mockTime := NewMockTimeProvider(time.Now()) + inst.SetTimeProvider(mockTime) + + // Set last request time to now + inst.UpdateLastRequestTime() + + // Advance time by 2 minutes (exceeds 1 minute timeout) + mockTime.SetTime(time.Now().Add(2 * time.Minute)) + + if !inst.ShouldTimeout() { + t.Error("Instance should timeout when last request exceeds idle timeout") + } +} + +func TestTimeoutConfiguration_Validation(t *testing.T) { + globalSettings := &config.InstancesConfig{ + LogsDir: "/tmp/test", + } + + tests := []struct { + name string + inputTimeout *int + expectedTimeout int + }{ + {"default value when nil", nil, 0}, + {"positive value", testutil.IntPtr(10), 10}, + {"zero value", testutil.IntPtr(0), 0}, + {"negative value gets corrected", testutil.IntPtr(-5), 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + options := &instance.CreateInstanceOptions{ + IdleTimeout: tt.inputTimeout, + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + } + + inst := instance.NewInstance("test-instance", globalSettings, options) + opts := inst.GetOptions() + + if opts.IdleTimeout == nil || *opts.IdleTimeout != tt.expectedTimeout { + t.Errorf("Expected IdleTimeout %d, got %v", tt.expectedTimeout, opts.IdleTimeout) + } + }) + } +} diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 43b1fea..bac7567 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" "sync" + "time" ) // InstanceManager defines the interface for managing instances of the llama server. @@ -31,20 +32,48 @@ type instanceManager struct { instances map[string]*instance.Process ports map[int]bool instancesConfig config.InstancesConfig + + // Timeout checker + timeoutChecker *time.Ticker + shutdownChan chan struct{} + shutdownDone chan struct{} + isShutdown bool } // NewInstanceManager creates a new instance of InstanceManager. func NewInstanceManager(instancesConfig config.InstancesConfig) InstanceManager { + if instancesConfig.TimeoutCheckInterval <= 0 { + instancesConfig.TimeoutCheckInterval = 5 // Default to 5 minutes if not set + } im := &instanceManager{ instances: make(map[string]*instance.Process), ports: make(map[int]bool), instancesConfig: instancesConfig, + + timeoutChecker: time.NewTicker(time.Duration(instancesConfig.TimeoutCheckInterval) * time.Minute), + shutdownChan: make(chan struct{}), + shutdownDone: make(chan struct{}), } // Load existing instances from disk if err := im.loadInstances(); err != nil { log.Printf("Error loading instances: %v", err) } + + // Start the timeout checker goroutine after initialization is complete + go func() { + defer close(im.shutdownDone) + + for { + select { + case <-im.timeoutChecker.C: + im.checkAllTimeouts() + case <-im.shutdownChan: + return // Exit goroutine on shutdown + } + } + }() + return im } @@ -94,6 +123,27 @@ func (im *instanceManager) Shutdown() { im.mu.Lock() defer im.mu.Unlock() + // Check if already shutdown + if im.isShutdown { + return + } + im.isShutdown = true + + // Signal the timeout checker to stop + close(im.shutdownChan) + + // Release lock temporarily to wait for goroutine + im.mu.Unlock() + // Wait for the timeout checker goroutine to actually stop + <-im.shutdownDone + // Reacquire lock + im.mu.Lock() + + // Now stop the ticker + if im.timeoutChecker != nil { + im.timeoutChecker.Stop() + } + var wg sync.WaitGroup wg.Add(len(im.instances)) diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index c2f953c..6c48366 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -1,27 +1,29 @@ package manager_test import ( - "encoding/json" + "fmt" "llamactl/pkg/backends/llamacpp" "llamactl/pkg/config" "llamactl/pkg/instance" "llamactl/pkg/manager" "os" "path/filepath" - "reflect" "strings" + "sync" "testing" + "time" ) func TestNewInstanceManager(t *testing.T) { cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - LogsDir: "/tmp/test", - MaxInstances: 5, - LlamaExecutable: "llama-server", - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, - DefaultRestartDelay: 5, + PortRange: [2]int{8000, 9000}, + LogsDir: "/tmp/test", + MaxInstances: 5, + LlamaExecutable: "llama-server", + DefaultAutoRestart: true, + DefaultMaxRestarts: 3, + DefaultRestartDelay: 5, + TimeoutCheckInterval: 5, } manager := manager.NewInstanceManager(cfg) @@ -65,395 +67,139 @@ func TestCreateInstance_Success(t *testing.T) { } } -func TestCreateInstance_DuplicateName(t *testing.T) { - manager := createTestManager() - - options1 := &instance.CreateInstanceOptions{ +func TestCreateInstance_ValidationAndLimits(t *testing.T) { + // Test duplicate names + mngr := createTestManager() + options := &instance.CreateInstanceOptions{ LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - options2 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - // Create first instance - _, err := manager.CreateInstance("test-instance", options1) + _, err := mngr.CreateInstance("test-instance", options) if err != nil { t.Fatalf("First CreateInstance failed: %v", err) } // Try to create duplicate - _, err = manager.CreateInstance("test-instance", options2) + _, err = mngr.CreateInstance("test-instance", options) if err == nil { t.Error("Expected error for duplicate instance name") } if !strings.Contains(err.Error(), "already exists") { t.Errorf("Expected duplicate name error, got: %v", err) } -} -func TestCreateInstance_MaxInstancesLimit(t *testing.T) { - // Create manager with low max instances limit + // Test max instances limit cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - MaxInstances: 2, // Very low limit for testing + PortRange: [2]int{8000, 9000}, + MaxInstances: 1, // Very low limit for testing + TimeoutCheckInterval: 5, } - manager := manager.NewInstanceManager(cfg) + limitedManager := manager.NewInstanceManager(cfg) - options1 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - options2 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - options3 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - // Create instances up to the limit - _, err := manager.CreateInstance("instance1", options1) + _, err = limitedManager.CreateInstance("instance1", options) if err != nil { t.Fatalf("CreateInstance 1 failed: %v", err) } - _, err = manager.CreateInstance("instance2", options2) - if err != nil { - t.Fatalf("CreateInstance 2 failed: %v", err) - } - // This should fail due to max instances limit - _, err = manager.CreateInstance("instance3", options3) + _, err = limitedManager.CreateInstance("instance2", options) if err == nil { t.Error("Expected error when exceeding max instances limit") } - if !strings.Contains(err.Error(), "maximum number of instances") && !strings.Contains(err.Error(), "limit") { + if !strings.Contains(err.Error(), "maximum number of instances") { t.Errorf("Expected max instances error, got: %v", err) } } -func TestCreateInstance_PortAssignment(t *testing.T) { +func TestPortManagement(t *testing.T) { manager := createTestManager() - // Create instance without specifying port - options := &instance.CreateInstanceOptions{ + // Test auto port assignment + options1 := &instance.CreateInstanceOptions{ LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - inst, err := manager.CreateInstance("test-instance", options) + inst1, err := manager.CreateInstance("instance1", options1) if err != nil { t.Fatalf("CreateInstance failed: %v", err) } - // Should auto-assign a port in the range - port := inst.GetOptions().Port - if port < 8000 || port > 9000 { - t.Errorf("Expected port in range 8000-9000, got %d", port) - } -} - -func TestCreateInstance_PortConflictDetection(t *testing.T) { - manager := createTestManager() - - options1 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - Port: 8080, // Explicit port - }, + port1 := inst1.GetOptions().Port + if port1 < 8000 || port1 > 9000 { + t.Errorf("Expected port in range 8000-9000, got %d", port1) } + // Test port conflict detection options2 := &instance.CreateInstanceOptions{ LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model2.gguf", - Port: 8080, // Same port - should conflict + Port: port1, // Same port - should conflict }, } - // Create first instance - _, err := manager.CreateInstance("instance1", options1) - if err != nil { - t.Fatalf("CreateInstance 1 failed: %v", err) - } - - // Try to create second instance with same port _, err = manager.CreateInstance("instance2", options2) if err == nil { t.Error("Expected error for port conflict") } - if !strings.Contains(err.Error(), "port") && !strings.Contains(err.Error(), "conflict") && !strings.Contains(err.Error(), "in use") { + if !strings.Contains(err.Error(), "port") && !strings.Contains(err.Error(), "in use") { t.Errorf("Expected port conflict error, got: %v", err) } -} - -func TestCreateInstance_MultiplePortAssignment(t *testing.T) { - manager := createTestManager() - - options1 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - options2 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - // Create multiple instances and verify they get different ports - instance1, err := manager.CreateInstance("instance1", options1) - if err != nil { - t.Fatalf("CreateInstance 1 failed: %v", err) - } - - instance2, err := manager.CreateInstance("instance2", options2) - if err != nil { - t.Fatalf("CreateInstance 2 failed: %v", err) - } - - port1 := instance1.GetOptions().Port - port2 := instance2.GetOptions().Port - - if port1 == port2 { - t.Errorf("Expected different ports, both got %d", port1) - } -} - -func TestCreateInstance_PortExhaustion(t *testing.T) { - // Create manager with very small port range - cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 8001}, // Only 2 ports available - MaxInstances: 10, // Higher than available ports - } - manager := manager.NewInstanceManager(cfg) - - options1 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - options2 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } + // Test port release on deletion + specificPort := 8080 options3 := &instance.CreateInstanceOptions{ LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", + Port: specificPort, }, } - // Create instances to exhaust all ports - _, err := manager.CreateInstance("instance1", options1) - if err != nil { - t.Fatalf("CreateInstance 1 failed: %v", err) - } - - _, err = manager.CreateInstance("instance2", options2) - if err != nil { - t.Fatalf("CreateInstance 2 failed: %v", err) - } - - // This should fail due to port exhaustion - _, err = manager.CreateInstance("instance3", options3) - if err == nil { - t.Error("Expected error when ports are exhausted") - } - if !strings.Contains(err.Error(), "port") && !strings.Contains(err.Error(), "available") { - t.Errorf("Expected port exhaustion error, got: %v", err) - } -} - -func TestDeleteInstance_PortRelease(t *testing.T) { - manager := createTestManager() - - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - Port: 8080, - }, - } - - // Create instance with specific port - _, err := manager.CreateInstance("test-instance", options) + _, err = manager.CreateInstance("port-test", options3) if err != nil { t.Fatalf("CreateInstance failed: %v", err) } - // Delete the instance - err = manager.DeleteInstance("test-instance") + err = manager.DeleteInstance("port-test") if err != nil { t.Fatalf("DeleteInstance failed: %v", err) } // Should be able to create new instance with same port - _, err = manager.CreateInstance("new-instance", options) + _, err = manager.CreateInstance("new-port-test", options3) if err != nil { t.Errorf("Expected to reuse port after deletion, got error: %v", err) } } -func TestGetInstance_Success(t *testing.T) { +func TestInstanceOperations(t *testing.T) { manager := createTestManager() - // Create an instance first options := &instance.CreateInstanceOptions{ LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } + + // Create instance created, err := manager.CreateInstance("test-instance", options) if err != nil { t.Fatalf("CreateInstance failed: %v", err) } - // Retrieve it + // Get instance retrieved, err := manager.GetInstance("test-instance") if err != nil { t.Fatalf("GetInstance failed: %v", err) } - if retrieved.Name != created.Name { t.Errorf("Expected name %q, got %q", created.Name, retrieved.Name) } -} -func TestGetInstance_NotFound(t *testing.T) { - manager := createTestManager() - - _, err := manager.GetInstance("nonexistent") - if err == nil { - t.Error("Expected error for nonexistent instance") - } - if !strings.Contains(err.Error(), "not found") { - t.Errorf("Expected 'not found' error, got: %v", err) - } -} - -func TestListInstances(t *testing.T) { - manager := createTestManager() - - // Initially empty - instances, err := manager.ListInstances() - if err != nil { - t.Fatalf("ListInstances failed: %v", err) - } - if len(instances) != 0 { - t.Errorf("Expected 0 instances, got %d", len(instances)) - } - - // Create some instances - options1 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - options2 := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - - _, err = manager.CreateInstance("instance1", options1) - if err != nil { - t.Fatalf("CreateInstance 1 failed: %v", err) - } - - _, err = manager.CreateInstance("instance2", options2) - if err != nil { - t.Fatalf("CreateInstance 2 failed: %v", err) - } - - // List should return both - instances, err = manager.ListInstances() - if err != nil { - t.Fatalf("ListInstances failed: %v", err) - } - if len(instances) != 2 { - t.Errorf("Expected 2 instances, got %d", len(instances)) - } - - // Check names are present - names := make(map[string]bool) - for _, inst := range instances { - names[inst.Name] = true - } - if !names["instance1"] || !names["instance2"] { - t.Error("Expected both instance1 and instance2 in list") - } -} - -func TestDeleteInstance_Success(t *testing.T) { - manager := createTestManager() - - // Create an instance - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - _, err := manager.CreateInstance("test-instance", options) - if err != nil { - t.Fatalf("CreateInstance failed: %v", err) - } - - // Delete it - err = manager.DeleteInstance("test-instance") - if err != nil { - t.Fatalf("DeleteInstance failed: %v", err) - } - - // Should no longer exist - _, err = manager.GetInstance("test-instance") - if err == nil { - t.Error("Instance should not exist after deletion") - } -} - -func TestDeleteInstance_NotFound(t *testing.T) { - manager := createTestManager() - - err := manager.DeleteInstance("nonexistent") - if err == nil { - t.Error("Expected error for deleting nonexistent instance") - } - if !strings.Contains(err.Error(), "not found") { - t.Errorf("Expected 'not found' error, got: %v", err) - } -} - -func TestUpdateInstance_Success(t *testing.T) { - manager := createTestManager() - - // Create an instance - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - Port: 8080, - }, - } - _, err := manager.CreateInstance("test-instance", options) - if err != nil { - t.Fatalf("CreateInstance failed: %v", err) - } - - // Update it + // Update instance newOptions := &instance.CreateInstanceOptions{ LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/new-model.gguf", @@ -465,44 +211,59 @@ func TestUpdateInstance_Success(t *testing.T) { if err != nil { t.Fatalf("UpdateInstance failed: %v", err) } - if updated.GetOptions().Model != "/path/to/new-model.gguf" { t.Errorf("Expected model '/path/to/new-model.gguf', got %q", updated.GetOptions().Model) } - if updated.GetOptions().Port != 8081 { - t.Errorf("Expected port 8081, got %d", updated.GetOptions().Port) + + // List instances + instances, err := manager.ListInstances() + if err != nil { + t.Fatalf("ListInstances failed: %v", err) } -} - -func TestUpdateInstance_NotFound(t *testing.T) { - manager := createTestManager() - - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, + if len(instances) != 1 { + t.Errorf("Expected 1 instance, got %d", len(instances)) } - _, err := manager.UpdateInstance("nonexistent", options) + // Delete instance + err = manager.DeleteInstance("test-instance") + if err != nil { + t.Fatalf("DeleteInstance failed: %v", err) + } + + _, err = manager.GetInstance("test-instance") if err == nil { - t.Error("Expected error for updating nonexistent instance") + t.Error("Instance should not exist after deletion") } - if !strings.Contains(err.Error(), "not found") { + + // Test operations on non-existent instances + _, err = manager.GetInstance("nonexistent") + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Errorf("Expected 'not found' error, got: %v", err) + } + + err = manager.DeleteInstance("nonexistent") + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Errorf("Expected 'not found' error, got: %v", err) + } + + _, err = manager.UpdateInstance("nonexistent", options) + if err == nil || !strings.Contains(err.Error(), "not found") { t.Errorf("Expected 'not found' error, got: %v", err) } } -func TestPersistence_InstancePersistedOnCreation(t *testing.T) { - // Create temporary directory for persistence +func TestPersistence(t *testing.T) { tempDir := t.TempDir() cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - InstancesDir: tempDir, - MaxInstances: 10, + PortRange: [2]int{8000, 9000}, + InstancesDir: tempDir, + MaxInstances: 10, + TimeoutCheckInterval: 5, } - manager := manager.NewInstanceManager(cfg) + // Test instance persistence on creation + manager1 := manager.NewInstanceManager(cfg) options := &instance.CreateInstanceOptions{ LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -510,8 +271,7 @@ func TestPersistence_InstancePersistedOnCreation(t *testing.T) { }, } - // Create instance - _, err := manager.CreateInstance("test-instance", options) + _, err := manager1.CreateInstance("test-instance", options) if err != nil { t.Fatalf("CreateInstance failed: %v", err) } @@ -522,371 +282,228 @@ func TestPersistence_InstancePersistedOnCreation(t *testing.T) { t.Errorf("Expected persistence file %s to exist", expectedPath) } - // Verify file contains correct data - data, err := os.ReadFile(expectedPath) + // Test loading instances from disk + manager2 := manager.NewInstanceManager(cfg) + instances, err := manager2.ListInstances() if err != nil { - t.Fatalf("Failed to read persistence file: %v", err) + t.Fatalf("ListInstances failed: %v", err) + } + if len(instances) != 1 { + t.Fatalf("Expected 1 loaded instance, got %d", len(instances)) + } + if instances[0].Name != "test-instance" { + t.Errorf("Expected loaded instance name 'test-instance', got %q", instances[0].Name) } - var persistedInstance map[string]interface{} - if err := json.Unmarshal(data, &persistedInstance); err != nil { - t.Fatalf("Failed to unmarshal persisted data: %v", err) + // Test port map populated from loaded instances (port conflict should be detected) + _, err = manager2.CreateInstance("new-instance", options) // Same port + if err == nil || !strings.Contains(err.Error(), "port") { + t.Errorf("Expected port conflict error, got: %v", err) } - if persistedInstance["name"] != "test-instance" { - t.Errorf("Expected name 'test-instance', got %v", persistedInstance["name"]) - } -} - -func TestPersistence_InstancePersistedOnUpdate(t *testing.T) { - tempDir := t.TempDir() - - cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - InstancesDir: tempDir, - MaxInstances: 10, - } - manager := manager.NewInstanceManager(cfg) - - // Create instance - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - Port: 8080, - }, - } - _, err := manager.CreateInstance("test-instance", options) - if err != nil { - t.Fatalf("CreateInstance failed: %v", err) - } - - // Update instance - newOptions := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/new-model.gguf", - Port: 8081, - }, - } - _, err = manager.UpdateInstance("test-instance", newOptions) - if err != nil { - t.Fatalf("UpdateInstance failed: %v", err) - } - - // Verify persistence file was updated - expectedPath := filepath.Join(tempDir, "test-instance.json") - data, err := os.ReadFile(expectedPath) - if err != nil { - t.Fatalf("Failed to read persistence file: %v", err) - } - - var persistedInstance map[string]interface{} - if err := json.Unmarshal(data, &persistedInstance); err != nil { - t.Fatalf("Failed to unmarshal persisted data: %v", err) - } - - // Check that the options were updated - options_data, ok := persistedInstance["options"].(map[string]interface{}) - if !ok { - t.Fatal("Expected options to be present in persisted data") - } - - if options_data["model"] != "/path/to/new-model.gguf" { - t.Errorf("Expected updated model '/path/to/new-model.gguf', got %v", options_data["model"]) - } -} - -func TestPersistence_InstanceFileDeletedOnDeletion(t *testing.T) { - tempDir := t.TempDir() - - cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - InstancesDir: tempDir, - MaxInstances: 10, - } - manager := manager.NewInstanceManager(cfg) - - // Create instance - options := &instance.CreateInstanceOptions{ - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - }, - } - _, err := manager.CreateInstance("test-instance", options) - if err != nil { - t.Fatalf("CreateInstance failed: %v", err) - } - - expectedPath := filepath.Join(tempDir, "test-instance.json") - - // Verify file exists - if _, err := os.Stat(expectedPath); os.IsNotExist(err) { - t.Fatal("Expected persistence file to exist before deletion") - } - - // Delete instance - err = manager.DeleteInstance("test-instance") + // Test file deletion on instance deletion + err = manager2.DeleteInstance("test-instance") if err != nil { t.Fatalf("DeleteInstance failed: %v", err) } - // Verify file was deleted if _, err := os.Stat(expectedPath); !os.IsNotExist(err) { t.Error("Expected persistence file to be deleted") } } -func TestPersistence_InstancesLoadedFromDisk(t *testing.T) { - tempDir := t.TempDir() - - // Create JSON files manually (simulating previous run) - instance1JSON := `{ - "name": "instance1", - "running": false, - "options": { - "model": "/path/to/model1.gguf", - "port": 8080 - } - }` - - instance2JSON := `{ - "name": "instance2", - "running": false, - "options": { - "model": "/path/to/model2.gguf", - "port": 8081 - } - }` - - // Write JSON files - err := os.WriteFile(filepath.Join(tempDir, "instance1.json"), []byte(instance1JSON), 0644) - if err != nil { - t.Fatalf("Failed to write test JSON file: %v", err) - } - - err = os.WriteFile(filepath.Join(tempDir, "instance2.json"), []byte(instance2JSON), 0644) - if err != nil { - t.Fatalf("Failed to write test JSON file: %v", err) - } - - // Create manager - should load instances from disk +func TestTimeoutFunctionality(t *testing.T) { + // Test timeout checker initialization cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - InstancesDir: tempDir, - MaxInstances: 10, + PortRange: [2]int{8000, 9000}, + TimeoutCheckInterval: 10, + MaxInstances: 5, } + manager := manager.NewInstanceManager(cfg) - - // Verify instances were loaded - instances, err := manager.ListInstances() - if err != nil { - t.Fatalf("ListInstances failed: %v", err) + if manager == nil { + t.Fatal("Manager should be initialized with timeout checker") } + manager.Shutdown() // Clean up - if len(instances) != 2 { - t.Fatalf("Expected 2 loaded instances, got %d", len(instances)) - } + // Test timeout configuration and logic without starting the actual process + testManager := createTestManager() + defer testManager.Shutdown() - // Check instances by name - instancesByName := make(map[string]*instance.Process) - for _, inst := range instances { - instancesByName[inst.Name] = inst - } - - instance1, exists := instancesByName["instance1"] - if !exists { - t.Error("Expected instance1 to be loaded") - } else { - if instance1.GetOptions().Model != "/path/to/model1.gguf" { - t.Errorf("Expected model '/path/to/model1.gguf', got %q", instance1.GetOptions().Model) - } - if instance1.GetOptions().Port != 8080 { - t.Errorf("Expected port 8080, got %d", instance1.GetOptions().Port) - } - } - - instance2, exists := instancesByName["instance2"] - if !exists { - t.Error("Expected instance2 to be loaded") - } else { - if instance2.GetOptions().Model != "/path/to/model2.gguf" { - t.Errorf("Expected model '/path/to/model2.gguf', got %q", instance2.GetOptions().Model) - } - if instance2.GetOptions().Port != 8081 { - t.Errorf("Expected port 8081, got %d", instance2.GetOptions().Port) - } - } -} - -func TestPersistence_PortMapPopulatedFromLoadedInstances(t *testing.T) { - tempDir := t.TempDir() - - // Create JSON file with specific port - instanceJSON := `{ - "name": "test-instance", - "running": false, - "options": { - "model": "/path/to/model.gguf", - "port": 8080 - } - }` - - err := os.WriteFile(filepath.Join(tempDir, "test-instance.json"), []byte(instanceJSON), 0644) - if err != nil { - t.Fatalf("Failed to write test JSON file: %v", err) - } - - // Create manager - should load instance and register port - cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - InstancesDir: tempDir, - MaxInstances: 10, - } - manager := manager.NewInstanceManager(cfg) - - // Try to create new instance with same port - should fail due to conflict + idleTimeout := 1 // 1 minute options := &instance.CreateInstanceOptions{ + IdleTimeout: &idleTimeout, LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model2.gguf", - Port: 8080, // Same port as loaded instance + Model: "/path/to/model.gguf", }, } - _, err = manager.CreateInstance("new-instance", options) - if err == nil { - t.Error("Expected error for port conflict with loaded instance") - } - if !strings.Contains(err.Error(), "port") || !strings.Contains(err.Error(), "in use") { - t.Errorf("Expected port conflict error, got: %v", err) - } -} - -func TestPersistence_CompleteInstanceDataRoundTrip(t *testing.T) { - tempDir := t.TempDir() - - cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - InstancesDir: tempDir, - MaxInstances: 10, - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, - DefaultRestartDelay: 5, - } - - // Create first manager and instance with comprehensive options - manager1 := manager.NewInstanceManager(cfg) - - autoRestart := false - maxRestarts := 10 - restartDelay := 30 - - originalOptions := &instance.CreateInstanceOptions{ - AutoRestart: &autoRestart, - MaxRestarts: &maxRestarts, - RestartDelay: &restartDelay, - LlamaServerOptions: llamacpp.LlamaServerOptions{ - Model: "/path/to/model.gguf", - Port: 8080, - Host: "localhost", - CtxSize: 4096, - GPULayers: 32, - Temperature: 0.7, - TopK: 40, - TopP: 0.9, - Verbose: true, - FlashAttn: false, - Lora: []string{"adapter1.bin", "adapter2.bin"}, - HFRepo: "microsoft/DialoGPT-medium", - }, - } - - originalInstance, err := manager1.CreateInstance("roundtrip-test", originalOptions) + inst, err := testManager.CreateInstance("timeout-test", options) if err != nil { t.Fatalf("CreateInstance failed: %v", err) } - // Create second manager (simulating restart) - should load the instance - manager2 := manager.NewInstanceManager(cfg) + // Test timeout configuration is properly set + if inst.GetOptions().IdleTimeout == nil { + t.Fatal("Instance should have idle timeout configured") + } + if *inst.GetOptions().IdleTimeout != 1 { + t.Errorf("Expected idle timeout 1 minute, got %d", *inst.GetOptions().IdleTimeout) + } - loadedInstance, err := manager2.GetInstance("roundtrip-test") + // Test timeout logic without actually starting the process + // Create a mock time provider to simulate timeout + mockTime := NewMockTimeProvider(time.Now()) + inst.SetTimeProvider(mockTime) + + // Set instance to running state so timeout logic can work + inst.Running = true + + // Simulate instance being "running" for timeout check (without actual process) + // We'll test the ShouldTimeout logic directly + inst.UpdateLastRequestTime() + + // Initially should not timeout (just updated) + if inst.ShouldTimeout() { + t.Error("Instance should not timeout immediately after request") + } + + // Advance time to trigger timeout + mockTime.SetTime(time.Now().Add(2 * time.Minute)) + + // Now it should timeout + if !inst.ShouldTimeout() { + t.Error("Instance should timeout after idle period") + } + + // Reset running state to avoid shutdown issues + inst.Running = false + + // Test that instance without timeout doesn't timeout + noTimeoutOptions := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + // No IdleTimeout set + } + + noTimeoutInst, err := testManager.CreateInstance("no-timeout-test", noTimeoutOptions) if err != nil { - t.Fatalf("GetInstance failed after reload: %v", err) + t.Fatalf("CreateInstance failed: %v", err) } - // Compare all data - if loadedInstance.Name != originalInstance.Name { - t.Errorf("Name mismatch: original=%q, loaded=%q", originalInstance.Name, loadedInstance.Name) + noTimeoutInst.SetTimeProvider(mockTime) + noTimeoutInst.Running = true // Set to running for timeout check + noTimeoutInst.UpdateLastRequestTime() + + // Even with time advanced, should not timeout + if noTimeoutInst.ShouldTimeout() { + t.Error("Instance without timeout configuration should never timeout") } - originalOpts := originalInstance.GetOptions() - loadedOpts := loadedInstance.GetOptions() + // Reset running state to avoid shutdown issues + noTimeoutInst.Running = false +} - // Compare restart options - if *loadedOpts.AutoRestart != *originalOpts.AutoRestart { - t.Errorf("AutoRestart mismatch: original=%v, loaded=%v", *originalOpts.AutoRestart, *loadedOpts.AutoRestart) - } - if *loadedOpts.MaxRestarts != *originalOpts.MaxRestarts { - t.Errorf("MaxRestarts mismatch: original=%v, loaded=%v", *originalOpts.MaxRestarts, *loadedOpts.MaxRestarts) - } - if *loadedOpts.RestartDelay != *originalOpts.RestartDelay { - t.Errorf("RestartDelay mismatch: original=%v, loaded=%v", *originalOpts.RestartDelay, *loadedOpts.RestartDelay) +func TestConcurrentAccess(t *testing.T) { + manager := createTestManager() + defer manager.Shutdown() + + // Test concurrent operations + var wg sync.WaitGroup + errChan := make(chan error, 10) + + // Concurrent instance creation + for i := 0; i < 5; i++ { + wg.Add(1) + go func(index int) { + defer wg.Done() + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + } + instanceName := fmt.Sprintf("concurrent-test-%d", index) + if _, err := manager.CreateInstance(instanceName, options); err != nil { + errChan <- err + } + }(i) } - // Compare llama server options - if loadedOpts.Model != originalOpts.Model { - t.Errorf("Model mismatch: original=%q, loaded=%q", originalOpts.Model, loadedOpts.Model) - } - if loadedOpts.Port != originalOpts.Port { - t.Errorf("Port mismatch: original=%d, loaded=%d", originalOpts.Port, loadedOpts.Port) - } - if loadedOpts.Host != originalOpts.Host { - t.Errorf("Host mismatch: original=%q, loaded=%q", originalOpts.Host, loadedOpts.Host) - } - if loadedOpts.CtxSize != originalOpts.CtxSize { - t.Errorf("CtxSize mismatch: original=%d, loaded=%d", originalOpts.CtxSize, loadedOpts.CtxSize) - } - if loadedOpts.GPULayers != originalOpts.GPULayers { - t.Errorf("GPULayers mismatch: original=%d, loaded=%d", originalOpts.GPULayers, loadedOpts.GPULayers) - } - if loadedOpts.Temperature != originalOpts.Temperature { - t.Errorf("Temperature mismatch: original=%f, loaded=%f", originalOpts.Temperature, loadedOpts.Temperature) - } - if loadedOpts.TopK != originalOpts.TopK { - t.Errorf("TopK mismatch: original=%d, loaded=%d", originalOpts.TopK, loadedOpts.TopK) - } - if loadedOpts.TopP != originalOpts.TopP { - t.Errorf("TopP mismatch: original=%f, loaded=%f", originalOpts.TopP, loadedOpts.TopP) - } - if loadedOpts.Verbose != originalOpts.Verbose { - t.Errorf("Verbose mismatch: original=%v, loaded=%v", originalOpts.Verbose, loadedOpts.Verbose) - } - if loadedOpts.FlashAttn != originalOpts.FlashAttn { - t.Errorf("FlashAttn mismatch: original=%v, loaded=%v", originalOpts.FlashAttn, loadedOpts.FlashAttn) - } - if loadedOpts.HFRepo != originalOpts.HFRepo { - t.Errorf("HFRepo mismatch: original=%q, loaded=%q", originalOpts.HFRepo, loadedOpts.HFRepo) + // Concurrent list operations + for i := 0; i < 3; i++ { + wg.Add(1) + go func() { + defer wg.Done() + if _, err := manager.ListInstances(); err != nil { + errChan <- err + } + }() } - // Compare slice fields - if !reflect.DeepEqual(loadedOpts.Lora, originalOpts.Lora) { - t.Errorf("Lora mismatch: original=%v, loaded=%v", originalOpts.Lora, loadedOpts.Lora) + wg.Wait() + close(errChan) + + // Check for any errors during concurrent access + for err := range errChan { + t.Errorf("Concurrent access error: %v", err) + } +} + +func TestShutdown(t *testing.T) { + manager := createTestManager() + + // Create test instance + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ + Model: "/path/to/model.gguf", + }, + } + _, err := manager.CreateInstance("test-instance", options) + if err != nil { + t.Fatalf("CreateInstance failed: %v", err) } - // Verify created timestamp is preserved - if loadedInstance.Created != originalInstance.Created { - t.Errorf("Created timestamp mismatch: original=%d, loaded=%d", originalInstance.Created, loadedInstance.Created) - } + // Shutdown should not panic + manager.Shutdown() + + // Multiple shutdowns should not panic + manager.Shutdown() } // Helper function to create a test manager with standard config func createTestManager() manager.InstanceManager { cfg := config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - LogsDir: "/tmp/test", - MaxInstances: 10, - LlamaExecutable: "llama-server", - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, - DefaultRestartDelay: 5, + PortRange: [2]int{8000, 9000}, + LogsDir: "/tmp/test", + MaxInstances: 10, + LlamaExecutable: "llama-server", + DefaultAutoRestart: true, + DefaultMaxRestarts: 3, + DefaultRestartDelay: 5, + TimeoutCheckInterval: 5, } return manager.NewInstanceManager(cfg) } + +// Helper for timeout tests +type MockTimeProvider struct { + currentTime time.Time + mu sync.RWMutex +} + +func NewMockTimeProvider(t time.Time) *MockTimeProvider { + return &MockTimeProvider{currentTime: t} +} + +func (m *MockTimeProvider) Now() time.Time { + m.mu.RLock() + defer m.mu.RUnlock() + return m.currentTime +} + +func (m *MockTimeProvider) SetTime(t time.Time) { + m.mu.Lock() + defer m.mu.Unlock() + m.currentTime = t +} diff --git a/pkg/manager/operations.go b/pkg/manager/operations.go index baa4eb7..7849152 100644 --- a/pkg/manager/operations.go +++ b/pkg/manager/operations.go @@ -27,10 +27,6 @@ func (im *instanceManager) CreateInstance(name string, options *instance.CreateI return nil, fmt.Errorf("instance options cannot be nil") } - if len(im.instances) >= im.instancesConfig.MaxInstances && im.instancesConfig.MaxInstances != -1 { - return nil, fmt.Errorf("maximum number of instances (%d) reached", im.instancesConfig.MaxInstances) - } - name, err := validation.ValidateInstanceName(name) if err != nil { return nil, err @@ -44,6 +40,11 @@ func (im *instanceManager) CreateInstance(name string, options *instance.CreateI im.mu.Lock() defer im.mu.Unlock() + // Check max instances limit after acquiring the lock + if len(im.instances) >= im.instancesConfig.MaxInstances && im.instancesConfig.MaxInstances != -1 { + return nil, fmt.Errorf("maximum number of instances (%d) reached", im.instancesConfig.MaxInstances) + } + // Check if instance with this name already exists if im.instances[name] != nil { return nil, fmt.Errorf("instance with name %s already exists", name) diff --git a/pkg/manager/timeout.go b/pkg/manager/timeout.go new file mode 100644 index 0000000..a28bc67 --- /dev/null +++ b/pkg/manager/timeout.go @@ -0,0 +1,26 @@ +package manager + +import "log" + +func (im *instanceManager) checkAllTimeouts() { + im.mu.RLock() + var timeoutInstances []string + + // Identify instances that should timeout + for _, inst := range im.instances { + if inst.ShouldTimeout() { + timeoutInstances = append(timeoutInstances, inst.Name) + } + } + im.mu.RUnlock() // Release read lock before calling StopInstance + + // Stop the timed-out instances + for _, name := range timeoutInstances { + log.Printf("Instance %s has timed out, stopping it", name) + if _, err := im.StopInstance(name); err != nil { + log.Printf("Error stopping instance %s: %v", name, err) + } else { + log.Printf("Instance %s stopped successfully", name) + } + } +} diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index 0bb5285..0d473e7 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -472,6 +472,9 @@ func (h *Handler) ProxyToInstance() http.HandlerFunc { proxyPath = "/" + proxyPath } + // Update the last request time for the instance + inst.UpdateLastRequestTime() + // Modify the request to remove the proxy prefix originalPath := r.URL.Path r.URL.Path = proxyPath @@ -582,6 +585,9 @@ func (h *Handler) OpenAIProxy() http.HandlerFunc { return } + // Update last request time for the instance + inst.UpdateLastRequestTime() + // Recreate the request body from the bytes we read r.Body = io.NopCloser(bytes.NewReader(bodyBytes)) r.ContentLength = int64(len(bodyBytes)) diff --git a/webui/src/lib/zodFormUtils.ts b/webui/src/lib/zodFormUtils.ts index e5400d3..5a964bb 100644 --- a/webui/src/lib/zodFormUtils.ts +++ b/webui/src/lib/zodFormUtils.ts @@ -1,7 +1,7 @@ import { type CreateInstanceOptions, getAllFieldKeys } from '@/schemas/instanceOptions' // Only define the basic fields we want to show by default -export const basicFieldsConfig: Record