Refactor instance options structure and related code

This commit is contained in:
2025-10-16 20:53:24 +02:00
parent a96ed4d797
commit 4b30791be2
16 changed files with 235 additions and 160 deletions

View File

@@ -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
}
}

View File

@@ -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,

View File

@@ -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)

View File

@@ -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 {

View File

@@ -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{