diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go index fba8b8c..6a86fcd 100644 --- a/pkg/instance/instance.go +++ b/pkg/instance/instance.go @@ -21,8 +21,8 @@ type Instance struct { Created int64 `json:"created,omitempty"` // Unix timestamp when the instance was created // Mutable state - each owns its own lock - status *status `json:"-"` // unexported - status owns its lock - options *CreateInstanceOptions `json:"-"` + status *status `json:"-"` // unexported - status owns its lock + options *options `json:"-"` // unexported - options owns its lock // Global configuration (read-only, no lock needed) globalInstanceSettings *config.InstancesConfig @@ -47,9 +47,9 @@ type Instance struct { } // NewInstance creates a new instance with the given name, log path, and options -func NewInstance(name string, globalBackendSettings *config.BackendConfig, globalInstanceSettings *config.InstancesConfig, options *CreateInstanceOptions, onStatusChange func(oldStatus, newStatus Status)) *Instance { +func NewInstance(name string, globalBackendSettings *config.BackendConfig, globalInstanceSettings *config.InstancesConfig, opts *Options, onStatusChange func(oldStatus, newStatus Status)) *Instance { // Validate and copy options - options.ValidateAndApplyDefaults(name, globalInstanceSettings) + opts.ValidateAndApplyDefaults(name, globalInstanceSettings) // Create the instance logger logger := NewLogger(name, globalInstanceSettings.LogsDir) @@ -58,6 +58,9 @@ func NewInstance(name string, globalBackendSettings *config.BackendConfig, globa status := newStatus(Stopped) status.onStatusChange = onStatusChange + // Create options wrapper + options := newOptions(opts) + instance := &Instance{ Name: name, options: options, @@ -74,10 +77,12 @@ func NewInstance(name string, globalBackendSettings *config.BackendConfig, globa return instance } -func (i *Instance) GetOptions() *CreateInstanceOptions { - i.mu.RLock() - defer i.mu.RUnlock() - return i.options +// GetOptions returns the current options, delegating to options component +func (i *Instance) GetOptions() *Options { + if i.options == nil { + return nil + } + return i.options.Get() } // GetStatus returns the current status, delegating to status component @@ -104,21 +109,20 @@ func (i *Instance) IsRunning() bool { } func (i *Instance) GetPort() int { - i.mu.RLock() - defer i.mu.RUnlock() - if i.options != nil { - switch i.options.BackendType { + opts := i.GetOptions() + if opts != nil { + switch opts.BackendType { case backends.BackendTypeLlamaCpp: - if i.options.LlamaServerOptions != nil { - return i.options.LlamaServerOptions.Port + if opts.LlamaServerOptions != nil { + return opts.LlamaServerOptions.Port } case backends.BackendTypeMlxLm: - if i.options.MlxServerOptions != nil { - return i.options.MlxServerOptions.Port + if opts.MlxServerOptions != nil { + return opts.MlxServerOptions.Port } case backends.BackendTypeVllm: - if i.options.VllmServerOptions != nil { - return i.options.VllmServerOptions.Port + if opts.VllmServerOptions != nil { + return opts.VllmServerOptions.Port } } } @@ -126,40 +130,39 @@ func (i *Instance) GetPort() int { } func (i *Instance) GetHost() string { - i.mu.RLock() - defer i.mu.RUnlock() - if i.options != nil { - switch i.options.BackendType { + opts := i.GetOptions() + if opts != nil { + switch opts.BackendType { case backends.BackendTypeLlamaCpp: - if i.options.LlamaServerOptions != nil { - return i.options.LlamaServerOptions.Host + if opts.LlamaServerOptions != nil { + return opts.LlamaServerOptions.Host } case backends.BackendTypeMlxLm: - if i.options.MlxServerOptions != nil { - return i.options.MlxServerOptions.Host + if opts.MlxServerOptions != nil { + return opts.MlxServerOptions.Host } case backends.BackendTypeVllm: - if i.options.VllmServerOptions != nil { - return i.options.VllmServerOptions.Host + if opts.VllmServerOptions != nil { + return opts.VllmServerOptions.Host } } } return "" } -func (i *Instance) SetOptions(options *CreateInstanceOptions) { - i.mu.Lock() - defer i.mu.Unlock() - - if options == nil { +// SetOptions sets the options, delegating to options component +func (i *Instance) SetOptions(opts *Options) { + if opts == nil { log.Println("Warning: Attempted to set nil options on instance", i.Name) return } // Validate and copy options - options.ValidateAndApplyDefaults(i.Name, i.globalInstanceSettings) + opts.ValidateAndApplyDefaults(i.Name, i.globalInstanceSettings) - i.options = options + if i.options != nil { + i.options.Set(opts) + } // Clear the proxy so it gets recreated with new options if i.proxy != nil { @@ -189,10 +192,13 @@ func (i *Instance) MarshalJSON() ([]byte, error) { i.mu.RLock() defer i.mu.RUnlock() + // Get options + opts := i.GetOptions() + // Determine if docker is enabled for this instance's backend var dockerEnabled bool - if i.options != nil { - switch i.options.BackendType { + if opts != nil { + switch opts.BackendType { case backends.BackendTypeLlamaCpp: if i.globalBackendSettings != nil && i.globalBackendSettings.LlamaCpp.Docker != nil && i.globalBackendSettings.LlamaCpp.Docker.Enabled { dockerEnabled = true @@ -208,11 +214,11 @@ func (i *Instance) MarshalJSON() ([]byte, error) { // Explicitly serialize to maintain backward compatible JSON format return json.Marshal(&struct { - Name string `json:"name"` - Status *status `json:"status"` - Created int64 `json:"created,omitempty"` - Options *CreateInstanceOptions `json:"options,omitempty"` - DockerEnabled bool `json:"docker_enabled,omitempty"` + Name string `json:"name"` + Status *status `json:"status"` + Created int64 `json:"created,omitempty"` + Options *options `json:"options,omitempty"` + DockerEnabled bool `json:"docker_enabled,omitempty"` }{ Name: i.Name, Status: i.status, @@ -226,10 +232,10 @@ func (i *Instance) MarshalJSON() ([]byte, error) { func (i *Instance) UnmarshalJSON(data []byte) error { // Explicitly deserialize to match MarshalJSON format aux := &struct { - Name string `json:"name"` - Status *status `json:"status"` - Created int64 `json:"created,omitempty"` - Options *CreateInstanceOptions `json:"options,omitempty"` + Name string `json:"name"` + Status *status `json:"status"` + Created int64 `json:"created,omitempty"` + Options *options `json:"options,omitempty"` }{} if err := json.Unmarshal(data, aux); err != nil { @@ -240,69 +246,79 @@ func (i *Instance) UnmarshalJSON(data []byte) error { i.Name = aux.Name i.Created = aux.Created i.status = aux.Status + i.options = aux.Options // Handle options with validation and defaults - if aux.Options != nil { - aux.Options.ValidateAndApplyDefaults(i.Name, i.globalInstanceSettings) - i.options = aux.Options + if i.options != nil { + opts := i.options.Get() + if opts != nil { + opts.ValidateAndApplyDefaults(i.Name, i.globalInstanceSettings) + } } // Initialize fields that are not serialized or may be nil if i.status == nil { i.status = newStatus(Stopped) } - if i.logger == nil && i.globalInstanceSettings != nil { - i.logger = NewLogger(i.Name, i.globalInstanceSettings.LogsDir) + if i.options == nil { + i.options = newOptions(&Options{}) } - if i.proxy == nil { - i.proxy = NewProxy(i) + + // Only create logger and proxy for non-remote instances + // Remote instances are metadata only (no logger, proxy, or process) + if !i.IsRemote() { + if i.logger == nil && i.globalInstanceSettings != nil { + i.logger = NewLogger(i.Name, i.globalInstanceSettings.LogsDir) + } + if i.proxy == nil { + i.proxy = NewProxy(i) + } } return nil } func (i *Instance) IsRemote() bool { - i.mu.RLock() - defer i.mu.RUnlock() - - if i.options == nil { + opts := i.GetOptions() + if opts == nil { return false } - return len(i.options.Nodes) > 0 + return len(opts.Nodes) > 0 } func (i *Instance) GetLogs(num_lines int) (string, error) { + if i.logger == nil { + return "", fmt.Errorf("instance %s has no logger (remote instances don't have logs)", i.Name) + } return i.logger.GetLogs(num_lines) } // getBackendHostPort extracts the host and port from instance options // Returns the configured host and port for the backend func (i *Instance) getBackendHostPort() (string, int) { - i.mu.RLock() - defer i.mu.RUnlock() - - if i.options == nil { + opts := i.GetOptions() + if opts == nil { return "localhost", 0 } var host string var port int - switch i.options.BackendType { + switch opts.BackendType { case backends.BackendTypeLlamaCpp: - if i.options.LlamaServerOptions != nil { - host = i.options.LlamaServerOptions.Host - port = i.options.LlamaServerOptions.Port + if opts.LlamaServerOptions != nil { + host = opts.LlamaServerOptions.Host + port = opts.LlamaServerOptions.Port } case backends.BackendTypeMlxLm: - if i.options.MlxServerOptions != nil { - host = i.options.MlxServerOptions.Host - port = i.options.MlxServerOptions.Port + if opts.MlxServerOptions != nil { + host = opts.MlxServerOptions.Host + port = opts.MlxServerOptions.Port } case backends.BackendTypeVllm: - if i.options.VllmServerOptions != nil { - host = i.options.VllmServerOptions.Host - port = i.options.VllmServerOptions.Port + if opts.VllmServerOptions != nil { + host = opts.VllmServerOptions.Host + port = opts.VllmServerOptions.Port } } diff --git a/pkg/instance/instance_test.go b/pkg/instance/instance_test.go index 9940633..d2c31ce 100644 --- a/pkg/instance/instance_test.go +++ b/pkg/instance/instance_test.go @@ -33,7 +33,7 @@ func TestNewInstance(t *testing.T) { DefaultRestartDelay: 5, } - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -102,7 +102,7 @@ func TestNewInstance_WithRestartOptions(t *testing.T) { maxRestarts := 10 restartDelay := 15 - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ AutoRestart: &autoRestart, MaxRestarts: &maxRestarts, RestartDelay: &restartDelay, @@ -153,7 +153,7 @@ func TestSetOptions(t *testing.T) { DefaultRestartDelay: 5, } - initialOptions := &instance.CreateInstanceOptions{ + initialOptions := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -167,7 +167,7 @@ func TestSetOptions(t *testing.T) { inst := instance.NewInstance("test-instance", backendConfig, globalSettings, initialOptions, mockOnStatusChange) // Update options - newOptions := &instance.CreateInstanceOptions{ + newOptions := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/new-model.gguf", @@ -211,7 +211,7 @@ func TestGetProxy(t *testing.T) { LogsDir: "/tmp/test", } - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Host: "localhost", @@ -266,7 +266,7 @@ func TestMarshalJSON(t *testing.T) { DefaultRestartDelay: 5, } - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -434,7 +434,7 @@ func TestCreateInstanceOptionsValidation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ MaxRestarts: tt.maxRestarts, RestartDelay: tt.restartDelay, BackendType: backends.BackendTypeLlamaCpp, diff --git a/pkg/instance/lifecycle.go b/pkg/instance/lifecycle.go index 23bd385..86f73db 100644 --- a/pkg/instance/lifecycle.go +++ b/pkg/instance/lifecycle.go @@ -311,29 +311,30 @@ func (i *Instance) handleRestart() { // validateRestartConditions checks if the instance should be restarted and returns the parameters func (i *Instance) validateRestartConditions() (shouldRestart bool, maxRestarts int, restartDelay int) { - if i.options == nil { + opts := i.GetOptions() + if opts == nil { log.Printf("Instance %s not restarting: options are nil", i.Name) return false, 0, 0 } - if i.options.AutoRestart == nil || !*i.options.AutoRestart { + if opts.AutoRestart == nil || !*opts.AutoRestart { log.Printf("Instance %s not restarting: AutoRestart is disabled", i.Name) return false, 0, 0 } - if i.options.MaxRestarts == nil { + if opts.MaxRestarts == nil { log.Printf("Instance %s not restarting: MaxRestarts is nil", i.Name) return false, 0, 0 } - if i.options.RestartDelay == nil { + if opts.RestartDelay == nil { log.Printf("Instance %s not restarting: RestartDelay is nil", i.Name) return false, 0, 0 } // Values are already validated during unmarshaling/SetOptions - maxRestarts = *i.options.MaxRestarts - restartDelay = *i.options.RestartDelay + maxRestarts = *opts.MaxRestarts + restartDelay = *opts.RestartDelay if i.restarts >= maxRestarts { log.Printf("Instance %s exceeded max restart attempts (%d)", i.Name, maxRestarts) @@ -345,6 +346,12 @@ func (i *Instance) validateRestartConditions() (shouldRestart bool, maxRestarts // buildCommand builds the command to execute using backend-specific logic func (i *Instance) buildCommand() (*exec.Cmd, error) { + // Get options + opts := i.GetOptions() + if opts == nil { + return nil, fmt.Errorf("instance options are nil") + } + // Get backend configuration backendConfig, err := i.getBackendConfig() if err != nil { @@ -352,13 +359,13 @@ func (i *Instance) buildCommand() (*exec.Cmd, error) { } // Build the environment variables - env := i.options.BuildEnvironment(backendConfig) + env := opts.BuildEnvironment(backendConfig) // Get the command to execute - command := i.options.GetCommand(backendConfig) + command := opts.GetCommand(backendConfig) // Build command arguments - args := i.options.BuildCommandArgs(backendConfig) + args := opts.BuildCommandArgs(backendConfig) // Create the exec.Cmd cmd := exec.CommandContext(i.ctx, command, args...) @@ -376,9 +383,14 @@ func (i *Instance) buildCommand() (*exec.Cmd, error) { // getBackendConfig resolves the backend configuration for the current instance func (i *Instance) getBackendConfig() (*config.BackendSettings, error) { + opts := i.GetOptions() + if opts == nil { + return nil, fmt.Errorf("instance options are nil") + } + var backendTypeStr string - switch i.options.BackendType { + switch opts.BackendType { case backends.BackendTypeLlamaCpp: backendTypeStr = "llama-cpp" case backends.BackendTypeMlxLm: @@ -386,7 +398,7 @@ func (i *Instance) getBackendConfig() (*config.BackendSettings, error) { case backends.BackendTypeVllm: backendTypeStr = "vllm" default: - return nil, fmt.Errorf("unsupported backend type: %s", i.options.BackendType) + return nil, fmt.Errorf("unsupported backend type: %s", opts.BackendType) } settings := i.globalBackendSettings.GetBackendSettings(backendTypeStr) diff --git a/pkg/instance/options.go b/pkg/instance/options.go index 439f426..c5db58b 100644 --- a/pkg/instance/options.go +++ b/pkg/instance/options.go @@ -10,9 +10,11 @@ import ( "llamactl/pkg/config" "log" "maps" + "sync" ) -type CreateInstanceOptions struct { +// Options contains the actual configuration (exported - this is the public API). +type Options struct { // Auto restart AutoRestart *bool `json:"auto_restart,omitempty"` MaxRestarts *int `json:"max_restarts,omitempty"` @@ -35,10 +37,55 @@ type CreateInstanceOptions struct { VllmServerOptions *vllm.VllmServerOptions `json:"-"` } -// UnmarshalJSON implements custom JSON unmarshaling for CreateInstanceOptions -func (c *CreateInstanceOptions) UnmarshalJSON(data []byte) error { +// options wraps Options with thread-safe access (unexported). +type options struct { + mu sync.RWMutex + opts *Options +} + +// newOptions creates a new options wrapper with the given Options +func newOptions(opts *Options) *options { + return &options{ + opts: opts, + } +} + +// Get returns a copy of the current options +func (o *options) Get() *Options { + o.mu.RLock() + defer o.mu.RUnlock() + return o.opts +} + +// Set updates the options +func (o *options) Set(opts *Options) { + o.mu.Lock() + defer o.mu.Unlock() + o.opts = opts +} + +// MarshalJSON implements json.Marshaler for options wrapper +func (o *options) MarshalJSON() ([]byte, error) { + o.mu.RLock() + defer o.mu.RUnlock() + return o.opts.MarshalJSON() +} + +// UnmarshalJSON implements json.Unmarshaler for options wrapper +func (o *options) UnmarshalJSON(data []byte) error { + o.mu.Lock() + defer o.mu.Unlock() + + if o.opts == nil { + o.opts = &Options{} + } + return o.opts.UnmarshalJSON(data) +} + +// UnmarshalJSON implements custom JSON unmarshaling for Options +func (c *Options) UnmarshalJSON(data []byte) error { // Use anonymous struct to avoid recursion - type Alias CreateInstanceOptions + type Alias Options aux := &struct { *Alias }{ @@ -95,10 +142,10 @@ func (c *CreateInstanceOptions) UnmarshalJSON(data []byte) error { return nil } -// MarshalJSON implements custom JSON marshaling for CreateInstanceOptions -func (c *CreateInstanceOptions) MarshalJSON() ([]byte, error) { +// MarshalJSON implements custom JSON marshaling for Options +func (c *Options) MarshalJSON() ([]byte, error) { // Use anonymous struct to avoid recursion - type Alias CreateInstanceOptions + type Alias Options aux := struct { *Alias }{ @@ -155,7 +202,7 @@ func (c *CreateInstanceOptions) MarshalJSON() ([]byte, error) { } // ValidateAndApplyDefaults validates the instance options and applies constraints -func (c *CreateInstanceOptions) ValidateAndApplyDefaults(name string, globalSettings *config.InstancesConfig) { +func (c *Options) ValidateAndApplyDefaults(name string, globalSettings *config.InstancesConfig) { // Validate and apply constraints if c.MaxRestarts != nil && *c.MaxRestarts < 0 { log.Printf("Instance %s MaxRestarts value (%d) cannot be negative, setting to 0", name, *c.MaxRestarts) @@ -193,7 +240,7 @@ func (c *CreateInstanceOptions) ValidateAndApplyDefaults(name string, globalSett } } -func (c *CreateInstanceOptions) GetCommand(backendConfig *config.BackendSettings) string { +func (c *Options) GetCommand(backendConfig *config.BackendSettings) string { if backendConfig.Docker != nil && backendConfig.Docker.Enabled && c.BackendType != backends.BackendTypeMlxLm { return "docker" @@ -203,7 +250,7 @@ func (c *CreateInstanceOptions) GetCommand(backendConfig *config.BackendSettings } // BuildCommandArgs builds command line arguments for the backend -func (c *CreateInstanceOptions) BuildCommandArgs(backendConfig *config.BackendSettings) []string { +func (c *Options) BuildCommandArgs(backendConfig *config.BackendSettings) []string { var args []string @@ -246,7 +293,7 @@ func (c *CreateInstanceOptions) BuildCommandArgs(backendConfig *config.BackendSe return args } -func (c *CreateInstanceOptions) BuildEnvironment(backendConfig *config.BackendSettings) map[string]string { +func (c *Options) BuildEnvironment(backendConfig *config.BackendSettings) map[string]string { env := map[string]string{} if backendConfig.Environment != nil { diff --git a/pkg/instance/timeout_test.go b/pkg/instance/timeout_test.go index c602131..f91a8a8 100644 --- a/pkg/instance/timeout_test.go +++ b/pkg/instance/timeout_test.go @@ -46,7 +46,7 @@ func TestUpdateLastRequestTime(t *testing.T) { LogsDir: "/tmp/test", } - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -77,7 +77,7 @@ func TestShouldTimeout_NotRunning(t *testing.T) { } idleTimeout := 1 // 1 minute - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ IdleTimeout: &idleTimeout, BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ @@ -124,7 +124,7 @@ func TestShouldTimeout_NoTimeoutConfigured(t *testing.T) { // Mock onStatusChange function mockOnStatusChange := func(oldStatus, newStatus instance.Status) {} - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ IdleTimeout: tt.idleTimeout, BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ @@ -158,7 +158,7 @@ func TestShouldTimeout_WithinTimeLimit(t *testing.T) { } idleTimeout := 5 // 5 minutes - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ IdleTimeout: &idleTimeout, BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ @@ -196,7 +196,7 @@ func TestShouldTimeout_ExceedsTimeLimit(t *testing.T) { } idleTimeout := 1 // 1 minute - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ IdleTimeout: &idleTimeout, BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ @@ -252,7 +252,7 @@ func TestTimeoutConfiguration_Validation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ IdleTimeout: tt.inputTimeout, BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index da5a900..a30c084 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -17,9 +17,9 @@ import ( // InstanceManager defines the interface for managing instances of the llama server. type InstanceManager interface { ListInstances() ([]*instance.Instance, error) - CreateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) + CreateInstance(name string, options *instance.Options) (*instance.Instance, error) GetInstance(name string) (*instance.Instance, error) - UpdateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) + UpdateInstance(name string, options *instance.Options) (*instance.Instance, error) DeleteInstance(name string) error StartInstance(name string) (*instance.Instance, error) IsMaxRunningInstancesReached() bool @@ -32,9 +32,9 @@ type InstanceManager interface { type RemoteManager interface { ListRemoteInstances(node *config.NodeConfig) ([]*instance.Instance, error) - CreateRemoteInstance(node *config.NodeConfig, name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) + CreateRemoteInstance(node *config.NodeConfig, name string, options *instance.Options) (*instance.Instance, error) GetRemoteInstance(node *config.NodeConfig, name string) (*instance.Instance, error) - UpdateRemoteInstance(node *config.NodeConfig, name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) + UpdateRemoteInstance(node *config.NodeConfig, name string, options *instance.Options) (*instance.Instance, error) DeleteRemoteInstance(node *config.NodeConfig, name string) error StartRemoteInstance(node *config.NodeConfig, name string) (*instance.Instance, error) StopRemoteInstance(node *config.NodeConfig, name string) (*instance.Instance, error) diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index e59e2eb..850c8ca 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -70,7 +70,7 @@ func TestPersistence(t *testing.T) { // Test instance persistence on creation manager1 := manager.NewInstanceManager(backendConfig, cfg, map[string]config.NodeConfig{}) - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -132,7 +132,7 @@ func TestConcurrentAccess(t *testing.T) { wg.Add(1) go func(index int) { defer wg.Done() - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -169,7 +169,7 @@ func TestShutdown(t *testing.T) { mgr := createTestManager() // Create test instance - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -230,7 +230,7 @@ func TestAutoRestartDisabledInstanceStatus(t *testing.T) { manager1 := manager.NewInstanceManager(backendConfig, cfg, map[string]config.NodeConfig{}) autoRestart := false - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, AutoRestart: &autoRestart, LlamaServerOptions: &llamacpp.LlamaServerOptions{ diff --git a/pkg/manager/operations.go b/pkg/manager/operations.go index cc2dba4..a48ebcb 100644 --- a/pkg/manager/operations.go +++ b/pkg/manager/operations.go @@ -75,7 +75,7 @@ func (im *instanceManager) ListInstances() ([]*instance.Instance, error) { // CreateInstance creates a new instance with the given options and returns it. // The instance is initially in a "stopped" state. -func (im *instanceManager) CreateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) { +func (im *instanceManager) CreateInstance(name string, options *instance.Options) (*instance.Instance, error) { if options == nil { return nil, fmt.Errorf("instance options cannot be nil") } @@ -194,7 +194,7 @@ func (im *instanceManager) GetInstance(name string) (*instance.Instance, error) // UpdateInstance updates the options of an existing instance and returns it. // If the instance is running, it will be restarted to apply the new options. -func (im *instanceManager) UpdateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) { +func (im *instanceManager) UpdateInstance(name string, options *instance.Options) (*instance.Instance, error) { im.mu.RLock() inst, exists := im.instances[name] im.mu.RUnlock() @@ -489,7 +489,7 @@ func (im *instanceManager) GetInstanceLogs(name string, numLines int) (string, e } // getPortFromOptions extracts the port from backend-specific options -func (im *instanceManager) getPortFromOptions(options *instance.CreateInstanceOptions) int { +func (im *instanceManager) getPortFromOptions(options *instance.Options) int { switch options.BackendType { case backends.BackendTypeLlamaCpp: if options.LlamaServerOptions != nil { @@ -508,7 +508,7 @@ func (im *instanceManager) getPortFromOptions(options *instance.CreateInstanceOp } // setPortInOptions sets the port in backend-specific options -func (im *instanceManager) setPortInOptions(options *instance.CreateInstanceOptions, port int) { +func (im *instanceManager) setPortInOptions(options *instance.Options, port int) { switch options.BackendType { case backends.BackendTypeLlamaCpp: if options.LlamaServerOptions != nil { @@ -526,7 +526,7 @@ func (im *instanceManager) setPortInOptions(options *instance.CreateInstanceOpti } // assignAndValidatePort assigns a port if not specified and validates it's not in use -func (im *instanceManager) assignAndValidatePort(options *instance.CreateInstanceOptions) error { +func (im *instanceManager) assignAndValidatePort(options *instance.Options) error { currentPort := im.getPortFromOptions(options) if currentPort == 0 { diff --git a/pkg/manager/operations_test.go b/pkg/manager/operations_test.go index fdeb44f..ff9d68d 100644 --- a/pkg/manager/operations_test.go +++ b/pkg/manager/operations_test.go @@ -13,7 +13,7 @@ import ( func TestCreateInstance_Success(t *testing.T) { manager := createTestManager() - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -40,7 +40,7 @@ func TestCreateInstance_Success(t *testing.T) { func TestCreateInstance_ValidationAndLimits(t *testing.T) { // Test duplicate names mngr := createTestManager() - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -96,7 +96,7 @@ func TestPortManagement(t *testing.T) { manager := createTestManager() // Test auto port assignment - options1 := &instance.CreateInstanceOptions{ + options1 := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -114,7 +114,7 @@ func TestPortManagement(t *testing.T) { } // Test port conflict detection - options2 := &instance.CreateInstanceOptions{ + options2 := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model2.gguf", @@ -132,7 +132,7 @@ func TestPortManagement(t *testing.T) { // Test port release on deletion specificPort := 8080 - options3 := &instance.CreateInstanceOptions{ + options3 := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -160,7 +160,7 @@ func TestPortManagement(t *testing.T) { func TestInstanceOperations(t *testing.T) { manager := createTestManager() - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -183,7 +183,7 @@ func TestInstanceOperations(t *testing.T) { } // Update instance - newOptions := &instance.CreateInstanceOptions{ + newOptions := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/new-model.gguf", diff --git a/pkg/manager/remote_ops.go b/pkg/manager/remote_ops.go index 9e25623..b68e479 100644 --- a/pkg/manager/remote_ops.go +++ b/pkg/manager/remote_ops.go @@ -12,7 +12,7 @@ import ( // stripNodesFromOptions creates a copy of the instance options without the Nodes field // to prevent routing loops when sending requests to remote nodes -func (im *instanceManager) stripNodesFromOptions(options *instance.CreateInstanceOptions) *instance.CreateInstanceOptions { +func (im *instanceManager) stripNodesFromOptions(options *instance.Options) *instance.Options { if options == nil { return nil } @@ -31,7 +31,7 @@ func (im *instanceManager) makeRemoteRequest(nodeConfig *config.NodeConfig, meth var reqBody io.Reader if body != nil { // Strip nodes from CreateInstanceOptions to prevent routing loops - if options, ok := body.(*instance.CreateInstanceOptions); ok { + if options, ok := body.(*instance.Options); ok { body = im.stripNodesFromOptions(options) } @@ -102,7 +102,7 @@ func (im *instanceManager) ListRemoteInstances(nodeConfig *config.NodeConfig) ([ } // CreateRemoteInstance creates a new instance on the remote node -func (im *instanceManager) CreateRemoteInstance(nodeConfig *config.NodeConfig, name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) { +func (im *instanceManager) CreateRemoteInstance(nodeConfig *config.NodeConfig, name string, options *instance.Options) (*instance.Instance, error) { path := fmt.Sprintf("/api/v1/instances/%s/", name) resp, err := im.makeRemoteRequest(nodeConfig, "POST", path, options) @@ -135,7 +135,7 @@ func (im *instanceManager) GetRemoteInstance(nodeConfig *config.NodeConfig, name } // UpdateRemoteInstance updates an existing instance on the remote node -func (im *instanceManager) UpdateRemoteInstance(nodeConfig *config.NodeConfig, name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) { +func (im *instanceManager) UpdateRemoteInstance(nodeConfig *config.NodeConfig, name string, options *instance.Options) (*instance.Instance, error) { path := fmt.Sprintf("/api/v1/instances/%s/", name) resp, err := im.makeRemoteRequest(nodeConfig, "PUT", path, options) diff --git a/pkg/manager/remote_ops_test.go b/pkg/manager/remote_ops_test.go index 94db40b..65fabe6 100644 --- a/pkg/manager/remote_ops_test.go +++ b/pkg/manager/remote_ops_test.go @@ -15,7 +15,7 @@ func TestStripNodesFromOptions(t *testing.T) { } // Test main case: nodes should be stripped, other fields preserved - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, Nodes: []string{"node1", "node2"}, Environment: map[string]string{"TEST": "value"}, diff --git a/pkg/manager/timeout_test.go b/pkg/manager/timeout_test.go index 214a488..7ca3a3e 100644 --- a/pkg/manager/timeout_test.go +++ b/pkg/manager/timeout_test.go @@ -34,7 +34,7 @@ func TestTimeoutFunctionality(t *testing.T) { defer testManager.Shutdown() idleTimeout := 1 // 1 minute - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ IdleTimeout: &idleTimeout, BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ @@ -84,7 +84,7 @@ func TestTimeoutFunctionality(t *testing.T) { inst.SetStatus(instance.Stopped) // Test that instance without timeout doesn't timeout - noTimeoutOptions := &instance.CreateInstanceOptions{ + noTimeoutOptions := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -115,21 +115,21 @@ func TestEvictLRUInstance_Success(t *testing.T) { // 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{ + options1 := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model1.gguf", }, IdleTimeout: func() *int { timeout := 1; return &timeout }(), // Any value > 0 } - options2 := &instance.CreateInstanceOptions{ + options2 := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model2.gguf", }, IdleTimeout: func() *int { timeout := 1; return &timeout }(), // Any value > 0 } - options3 := &instance.CreateInstanceOptions{ + options3 := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model3.gguf", @@ -197,7 +197,7 @@ func TestEvictLRUInstance_Success(t *testing.T) { func TestEvictLRUInstance_NoEligibleInstances(t *testing.T) { // Helper function to create instances with different timeout configurations createInstanceWithTimeout := func(manager manager.InstanceManager, name, model string, timeout *int) *instance.Instance { - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: model, diff --git a/pkg/server/handlers_backends.go b/pkg/server/handlers_backends.go index 7d6cab0..6fa833c 100644 --- a/pkg/server/handlers_backends.go +++ b/pkg/server/handlers_backends.go @@ -106,7 +106,7 @@ func (h *Handler) LlamaCppProxy(onDemandStart bool) http.HandlerFunc { // @Accept json // @Produce json // @Param request body ParseCommandRequest true "Command to parse" -// @Success 200 {object} instance.CreateInstanceOptions "Parsed options" +// @Success 200 {object} instance.Options "Parsed options" // @Failure 400 {object} map[string]string "Invalid request or command" // @Failure 500 {object} map[string]string "Internal Server Error" // @Router /backends/llama-cpp/parse-command [post] @@ -135,7 +135,7 @@ func (h *Handler) ParseLlamaCommand() http.HandlerFunc { writeError(w, http.StatusBadRequest, "parse_error", err.Error()) return } - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: llamaOptions, } @@ -154,7 +154,7 @@ func (h *Handler) ParseLlamaCommand() http.HandlerFunc { // @Accept json // @Produce json // @Param request body ParseCommandRequest true "Command to parse" -// @Success 200 {object} instance.CreateInstanceOptions "Parsed options" +// @Success 200 {object} instance.Options "Parsed options" // @Failure 400 {object} map[string]string "Invalid request or command" // @Router /backends/mlx/parse-command [post] func (h *Handler) ParseMlxCommand() http.HandlerFunc { @@ -188,7 +188,7 @@ func (h *Handler) ParseMlxCommand() http.HandlerFunc { // Currently only support mlx_lm backend type backendType := backends.BackendTypeMlxLm - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backendType, MlxServerOptions: mlxOptions, } @@ -208,7 +208,7 @@ func (h *Handler) ParseMlxCommand() http.HandlerFunc { // @Accept json // @Produce json // @Param request body ParseCommandRequest true "Command to parse" -// @Success 200 {object} instance.CreateInstanceOptions "Parsed options" +// @Success 200 {object} instance.Options "Parsed options" // @Failure 400 {object} map[string]string "Invalid request or command" // @Router /backends/vllm/parse-command [post] func (h *Handler) ParseVllmCommand() http.HandlerFunc { @@ -241,7 +241,7 @@ func (h *Handler) ParseVllmCommand() http.HandlerFunc { backendType := backends.BackendTypeVllm - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backendType, VllmServerOptions: vllmOptions, } diff --git a/pkg/server/handlers_instances.go b/pkg/server/handlers_instances.go index 080952c..9cb887e 100644 --- a/pkg/server/handlers_instances.go +++ b/pkg/server/handlers_instances.go @@ -47,7 +47,7 @@ func (h *Handler) ListInstances() http.HandlerFunc { // @Accept json // @Produces json // @Param name path string true "Instance Name" -// @Param options body instance.CreateInstanceOptions true "Instance configuration options" +// @Param options body instance.Options true "Instance configuration options" // @Success 201 {object} instance.Process "Created instance details" // @Failure 400 {string} string "Invalid request body" // @Failure 500 {string} string "Internal Server Error" @@ -60,7 +60,7 @@ func (h *Handler) CreateInstance() http.HandlerFunc { return } - var options instance.CreateInstanceOptions + var options instance.Options if err := json.NewDecoder(r.Body).Decode(&options); err != nil { http.Error(w, "Invalid request body", http.StatusBadRequest) return @@ -122,7 +122,7 @@ func (h *Handler) GetInstance() http.HandlerFunc { // @Accept json // @Produces json // @Param name path string true "Instance Name" -// @Param options body instance.CreateInstanceOptions true "Instance configuration options" +// @Param options body instance.Options true "Instance configuration options" // @Success 200 {object} instance.Process "Updated instance details" // @Failure 400 {string} string "Invalid name format" // @Failure 500 {string} string "Internal Server Error" @@ -135,7 +135,7 @@ func (h *Handler) UpdateInstance() http.HandlerFunc { return } - var options instance.CreateInstanceOptions + var options instance.Options if err := json.NewDecoder(r.Body).Decode(&options); err != nil { http.Error(w, "Invalid request body", http.StatusBadRequest) return diff --git a/pkg/validation/validation.go b/pkg/validation/validation.go index 638e5d2..6d6638d 100644 --- a/pkg/validation/validation.go +++ b/pkg/validation/validation.go @@ -35,7 +35,7 @@ func validateStringForInjection(value string) error { } // ValidateInstanceOptions performs validation based on backend type -func ValidateInstanceOptions(options *instance.CreateInstanceOptions) error { +func ValidateInstanceOptions(options *instance.Options) error { if options == nil { return ValidationError(fmt.Errorf("options cannot be nil")) } @@ -54,7 +54,7 @@ func ValidateInstanceOptions(options *instance.CreateInstanceOptions) error { } // validateLlamaCppOptions validates llama.cpp specific options -func validateLlamaCppOptions(options *instance.CreateInstanceOptions) error { +func validateLlamaCppOptions(options *instance.Options) error { if options.LlamaServerOptions == nil { return ValidationError(fmt.Errorf("llama server options cannot be nil for llama.cpp backend")) } @@ -73,7 +73,7 @@ func validateLlamaCppOptions(options *instance.CreateInstanceOptions) error { } // validateMlxOptions validates MLX backend specific options -func validateMlxOptions(options *instance.CreateInstanceOptions) error { +func validateMlxOptions(options *instance.Options) error { if options.MlxServerOptions == nil { return ValidationError(fmt.Errorf("MLX server options cannot be nil for MLX backend")) } @@ -91,7 +91,7 @@ func validateMlxOptions(options *instance.CreateInstanceOptions) error { } // validateVllmOptions validates vLLM backend specific options -func validateVllmOptions(options *instance.CreateInstanceOptions) error { +func validateVllmOptions(options *instance.Options) error { if options.VllmServerOptions == nil { return ValidationError(fmt.Errorf("vLLM server options cannot be nil for vLLM backend")) } diff --git a/pkg/validation/validation_test.go b/pkg/validation/validation_test.go index 8d8c49e..759ebc3 100644 --- a/pkg/validation/validation_test.go +++ b/pkg/validation/validation_test.go @@ -83,7 +83,7 @@ func TestValidateInstanceOptions_PortValidation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Port: tt.port, @@ -137,7 +137,7 @@ func TestValidateInstanceOptions_StringInjection(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Test with Model field (string field) - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: tt.value, @@ -175,7 +175,7 @@ func TestValidateInstanceOptions_ArrayInjection(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Test with Lora field (array field) - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Lora: tt.array, @@ -194,12 +194,12 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { // Test that injection in any field is caught tests := []struct { name string - options *instance.CreateInstanceOptions + options *instance.Options wantErr bool }{ { name: "injection in model field", - options: &instance.CreateInstanceOptions{ + options: &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "safe.gguf", @@ -210,7 +210,7 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { }, { name: "injection in log file", - options: &instance.CreateInstanceOptions{ + options: &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "safe.gguf", @@ -221,7 +221,7 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { }, { name: "all safe fields", - options: &instance.CreateInstanceOptions{ + options: &instance.Options{ BackendType: backends.BackendTypeLlamaCpp, LlamaServerOptions: &llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", @@ -247,7 +247,7 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { func TestValidateInstanceOptions_NonStringFields(t *testing.T) { // Test that non-string fields don't interfere with validation - options := &instance.CreateInstanceOptions{ + options := &instance.Options{ AutoRestart: testutil.BoolPtr(true), MaxRestarts: testutil.IntPtr(5), RestartDelay: testutil.IntPtr(10),