diff --git a/cmd/server/main.go b/cmd/server/main.go index 579b924..770e3ea 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -63,8 +63,8 @@ func main() { } // Create logs directory - if err := os.MkdirAll(cfg.Instances.LogsDir, 0755); err != nil { - log.Printf("Error creating log directory %s: %v\nInstance logs will not be available.", cfg.Instances.LogsDir, err) + if err := os.MkdirAll(cfg.Instances.Logging.LogsDir, 0755); err != nil { + log.Printf("Error creating log directory %s: %v\nInstance logs will not be available.", cfg.Instances.Logging.LogsDir, err) } } diff --git a/go.mod b/go.mod index 4bbcc21..100d144 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module llamactl go 1.24.5 require ( + github.com/DeRuina/timberjack v1.3.9 github.com/go-chi/chi/v5 v5.2.2 github.com/go-chi/cors v1.2.2 github.com/golang-migrate/migrate/v4 v4.19.1 @@ -20,6 +21,7 @@ require ( github.com/go-openapi/spec v0.21.0 // indirect github.com/go-openapi/swag v0.23.1 // indirect github.com/josharian/intern v1.0.0 // indirect + github.com/klauspost/compress v1.17.11 // indirect github.com/mailru/easyjson v0.9.0 // indirect github.com/swaggo/files v1.0.1 // indirect golang.org/x/mod v0.29.0 // indirect diff --git a/go.sum b/go.sum index 589e961..9de8889 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,11 @@ +github.com/DeRuina/timberjack v1.3.9 h1:6UXZ1I7ExPGTX/1UNYawR58LlOJUHKBPiYC7WQ91eBo= +github.com/DeRuina/timberjack v1.3.9/go.mod h1:RLoeQrwrCGIEF8gO5nV5b/gMD0QIy7bzQhBUgpp1EqE= github.com/KyleBanks/depth v1.2.1 h1:5h8fQADFrWtarTdtDudMmGsC7GPbOAu6RVB3ffsVFHc= github.com/KyleBanks/depth v1.2.1/go.mod h1:jzSb9d0L43HxTQfT+oSA1EEp2q+ne2uh6XgeJcm8brE= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/fortytw2/leaktest v1.3.0 h1:u8491cBMTQ8ft8aeV+adlcytMZylmA5nnwwkRZjI8vw= +github.com/fortytw2/leaktest v1.3.0/go.mod h1:jDsjWgpAGjm2CA7WthBh/CdZYEPF31XHquHwclZch5g= github.com/go-chi/chi/v5 v5.2.2 h1:CMwsvRVTbXVytCk1Wd72Zy1LAsAh9GxMmSNWLHCG618= github.com/go-chi/chi/v5 v5.2.2/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops= github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE= @@ -20,6 +24,8 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= +github.com/klauspost/compress v1.17.11 h1:In6xLpyWOi1+C7tXUUWv2ot1QvBjxevKAaI6IXrJmUc= +github.com/klauspost/compress v1.17.11/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= diff --git a/pkg/config/config.go b/pkg/config/config.go index 0f49f58..6a06098 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -88,6 +88,23 @@ type DatabaseConfig struct { ConnMaxLifetime time.Duration `yaml:"connection_max_lifetime" json:"connection_max_lifetime" swaggertype:"string" example:"1h"` } +// LoggingConfig contains all logging-related configuration for instances +type LoggingConfig struct { + // Logs directory override (relative to data_dir if not absolute) + LogsDir string `yaml:"logs_dir" json:"logs_dir"` + + // Log rotation configuration + LogRotation LogRotationConfig `yaml:"log_rotation" json:"log_rotation"` +} + +// LogRotationConfig contains log rotation settings for instances +type LogRotationConfig struct { + Enabled bool `yaml:"enabled" default:"true"` + MaxSizeMB int `yaml:"max_size_mb" default:"100"` // MB + MaxBackups int `yaml:"max_backups" default:"3"` + Compress bool `yaml:"compress" default:"false"` +} + // InstancesConfig contains instance management configuration type InstancesConfig struct { // Port range for instances (e.g., 8000,9000) @@ -96,9 +113,6 @@ type InstancesConfig struct { // Instance config directory override (relative to data_dir if not absolute) InstancesDir string `yaml:"configs_dir" json:"configs_dir"` - // Logs directory override (relative to data_dir if not absolute) - LogsDir string `yaml:"logs_dir" json:"logs_dir"` - // Automatically create the data directory if it doesn't exist AutoCreateDirs bool `yaml:"auto_create_dirs" json:"auto_create_dirs"` @@ -128,6 +142,9 @@ type InstancesConfig struct { // Interval for checking instance timeouts (in minutes) TimeoutCheckInterval int `yaml:"timeout_check_interval" json:"timeout_check_interval"` + + // Logging configuration + Logging LoggingConfig `yaml:"logging" json:"logging"` } // AuthConfig contains authentication settings @@ -205,10 +222,9 @@ func LoadConfig(configPath string) (AppConfig, error) { }, Instances: InstancesConfig{ PortRange: [2]int{8000, 9000}, - // NOTE: empty strings are set as placeholder values since InstancesDir and LogsDir + // NOTE: empty string is set as placeholder value since InstancesDir // should be relative path to DataDir if not explicitly set. InstancesDir: "", - LogsDir: "", AutoCreateDirs: true, MaxInstances: -1, // -1 means unlimited MaxRunningInstances: -1, // -1 means unlimited @@ -219,6 +235,15 @@ func LoadConfig(configPath string) (AppConfig, error) { DefaultOnDemandStart: true, OnDemandStartTimeout: 120, // 2 minutes TimeoutCheckInterval: 5, // Check timeouts every 5 minutes + Logging: LoggingConfig{ + LogsDir: "", // Will be set to data_dir/logs if empty + LogRotation: LogRotationConfig{ + Enabled: true, + MaxSizeMB: 100, + MaxBackups: 3, + Compress: false, + }, + }, }, Database: DatabaseConfig{ Path: "", // Will be set to data_dir/llamactl.db if empty @@ -260,8 +285,8 @@ func LoadConfig(configPath string) (AppConfig, error) { // Log deprecation warning if using custom instances dir log.Println("⚠️ Instances directory is deprecated and will be removed in future versions. Instances are persisted in the database.") } - if cfg.Instances.LogsDir == "" { - cfg.Instances.LogsDir = filepath.Join(cfg.DataDir, "logs") + if cfg.Instances.Logging.LogsDir == "" { + cfg.Instances.Logging.LogsDir = filepath.Join(cfg.DataDir, "logs") } if cfg.Database.Path == "" { cfg.Database.Path = filepath.Join(cfg.DataDir, "llamactl.db") @@ -328,7 +353,7 @@ func loadEnvVars(cfg *AppConfig) { cfg.Instances.InstancesDir = instancesDir } if logsDir := os.Getenv("LLAMACTL_LOGS_DIR"); logsDir != "" { - cfg.Instances.LogsDir = logsDir + cfg.Instances.Logging.LogsDir = logsDir } if autoCreate := os.Getenv("LLAMACTL_AUTO_CREATE_DATA_DIR"); autoCreate != "" { if b, err := strconv.ParseBool(autoCreate); err == nil { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 5be2199..71fc6ae 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -44,8 +44,8 @@ func TestLoadConfig_Defaults(t *testing.T) { if cfg.Instances.InstancesDir != filepath.Join(homedir, ".local", "share", "llamactl", "instances") { t.Errorf("Expected default instances directory '%s', got %q", filepath.Join(homedir, ".local", "share", "llamactl", "instances"), cfg.Instances.InstancesDir) } - if cfg.Instances.LogsDir != filepath.Join(homedir, ".local", "share", "llamactl", "logs") { - t.Errorf("Expected default logs directory '%s', got %q", filepath.Join(homedir, ".local", "share", "llamactl", "logs"), cfg.Instances.LogsDir) + if cfg.Instances.Logging.LogsDir != filepath.Join(homedir, ".local", "share", "llamactl", "logs") { + t.Errorf("Expected default logs directory '%s', got %q", filepath.Join(homedir, ".local", "share", "llamactl", "logs"), cfg.Instances.Logging.LogsDir) } if !cfg.Instances.AutoCreateDirs { t.Error("Expected default instances auto-create to be true") @@ -78,8 +78,9 @@ server: port: 9090 instances: port_range: [7000, 8000] - logs_dir: "/custom/logs" max_instances: 5 + logging: + logs_dir: "/custom/logs" llama_executable: "/usr/bin/llama-server" default_auto_restart: false default_max_restarts: 10 @@ -106,8 +107,8 @@ instances: if cfg.Instances.PortRange != [2]int{7000, 8000} { t.Errorf("Expected port range [7000, 8000], got %v", cfg.Instances.PortRange) } - if cfg.Instances.LogsDir != "/custom/logs" { - t.Errorf("Expected logs directory '/custom/logs', got %q", cfg.Instances.LogsDir) + if cfg.Instances.Logging.LogsDir != "/custom/logs" { + t.Errorf("Expected logs directory '/custom/logs', got %q", cfg.Instances.Logging.LogsDir) } if cfg.Instances.MaxInstances != 5 { t.Errorf("Expected max instances 5, got %d", cfg.Instances.MaxInstances) @@ -157,8 +158,8 @@ func TestLoadConfig_EnvironmentOverrides(t *testing.T) { if cfg.Instances.PortRange != [2]int{5000, 6000} { t.Errorf("Expected port range [5000, 6000], got %v", cfg.Instances.PortRange) } - if cfg.Instances.LogsDir != "/env/logs" { - t.Errorf("Expected logs directory '/env/logs', got %q", cfg.Instances.LogsDir) + if cfg.Instances.Logging.LogsDir != "/env/logs" { + t.Errorf("Expected logs directory '/env/logs', got %q", cfg.Instances.Logging.LogsDir) } if cfg.Instances.MaxInstances != 20 { t.Errorf("Expected max instances 20, got %d", cfg.Instances.MaxInstances) @@ -219,7 +220,6 @@ instances: } } - func TestParsePortRange(t *testing.T) { tests := []struct { name string @@ -248,7 +248,6 @@ func TestParsePortRange(t *testing.T) { } } - func TestGetBackendSettings_NewStructuredConfig(t *testing.T) { bc := &config.BackendConfig{ LlamaCpp: config.BackendSettings{ @@ -305,7 +304,6 @@ func TestGetBackendSettings_NewStructuredConfig(t *testing.T) { } } - func TestLoadConfig_BackendEnvironmentVariables(t *testing.T) { // Test that backend environment variables work correctly envVars := map[string]string{ @@ -375,7 +373,6 @@ func TestLoadConfig_BackendEnvironmentVariables(t *testing.T) { } } - func TestLoadConfig_LocalNode(t *testing.T) { t.Run("default local node", func(t *testing.T) { cfg, err := config.LoadConfig("nonexistent-file.yaml") diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go index 465cd5e..6b3a259 100644 --- a/pkg/instance/instance.go +++ b/pkg/instance/instance.go @@ -68,7 +68,11 @@ func New(name string, globalConfig *config.AppConfig, opts *Options, onStatusCha // Only create logger, proxy, and process for local instances if !instance.IsRemote() { - instance.logger = newLogger(name, globalInstanceSettings.LogsDir) + instance.logger = newLogger( + name, + globalInstanceSettings.Logging.LogsDir, + &globalInstanceSettings.Logging.LogRotation, + ) instance.process = newProcess(instance) } diff --git a/pkg/instance/instance_test.go b/pkg/instance/instance_test.go index 122cc96..351feed 100644 --- a/pkg/instance/instance_test.go +++ b/pkg/instance/instance_test.go @@ -27,8 +27,10 @@ func TestNewInstance(t *testing.T) { }, }, Instances: config.InstancesConfig{ - LogsDir: "/tmp/test", - DefaultAutoRestart: true, + DefaultAutoRestart: true, + Logging: config.LoggingConfig{ + LogsDir: "/tmp/test", + }, DefaultMaxRestarts: 3, DefaultRestartDelay: 5, }, @@ -120,8 +122,10 @@ func TestSetOptions(t *testing.T) { }, }, Instances: config.InstancesConfig{ - LogsDir: "/tmp/test", - DefaultAutoRestart: true, + DefaultAutoRestart: true, + Logging: config.LoggingConfig{ + LogsDir: "/tmp/test", + }, DefaultMaxRestarts: 3, DefaultRestartDelay: 5, }, @@ -176,7 +180,7 @@ func TestMarshalJSON(t *testing.T) { Backends: config.BackendConfig{ LlamaCpp: config.BackendSettings{Command: "llama-server"}, }, - Instances: config.InstancesConfig{LogsDir: "/tmp/test"}, + Instances: config.InstancesConfig{Logging: config.LoggingConfig{LogsDir: "/tmp/test"}}, Nodes: map[string]config.NodeConfig{}, LocalNode: "main", } @@ -313,7 +317,9 @@ func TestCreateOptionsValidation(t *testing.T) { }, }, Instances: config.InstancesConfig{ - LogsDir: "/tmp/test", + Logging: config.LoggingConfig{ + LogsDir: "/tmp/test", + }, }, Nodes: map[string]config.NodeConfig{}, LocalNode: "main", @@ -358,7 +364,7 @@ func TestStatusChangeCallback(t *testing.T) { Backends: config.BackendConfig{ LlamaCpp: config.BackendSettings{Command: "llama-server"}, }, - Instances: config.InstancesConfig{LogsDir: "/tmp/test"}, + Instances: config.InstancesConfig{Logging: config.LoggingConfig{LogsDir: "/tmp/test"}}, Nodes: map[string]config.NodeConfig{}, LocalNode: "main", } @@ -400,7 +406,7 @@ func TestSetOptions_NodesPreserved(t *testing.T) { Backends: config.BackendConfig{ LlamaCpp: config.BackendSettings{Command: "llama-server"}, }, - Instances: config.InstancesConfig{LogsDir: "/tmp/test"}, + Instances: config.InstancesConfig{Logging: config.LoggingConfig{LogsDir: "/tmp/test"}}, Nodes: map[string]config.NodeConfig{}, LocalNode: "main", } @@ -482,7 +488,7 @@ func TestProcessErrorCases(t *testing.T) { Backends: config.BackendConfig{ LlamaCpp: config.BackendSettings{Command: "llama-server"}, }, - Instances: config.InstancesConfig{LogsDir: "/tmp/test"}, + Instances: config.InstancesConfig{Logging: config.LoggingConfig{LogsDir: "/tmp/test"}}, Nodes: map[string]config.NodeConfig{}, LocalNode: "main", } @@ -518,7 +524,7 @@ func TestRemoteInstanceOperations(t *testing.T) { Backends: config.BackendConfig{ LlamaCpp: config.BackendSettings{Command: "llama-server"}, }, - Instances: config.InstancesConfig{LogsDir: "/tmp/test"}, + Instances: config.InstancesConfig{Logging: config.LoggingConfig{LogsDir: "/tmp/test"}}, Nodes: map[string]config.NodeConfig{ "remote-node": {Address: "http://remote-node:8080"}, }, @@ -566,7 +572,7 @@ func TestIdleTimeout(t *testing.T) { Backends: config.BackendConfig{ LlamaCpp: config.BackendSettings{Command: "llama-server"}, }, - Instances: config.InstancesConfig{LogsDir: "/tmp/test"}, + Instances: config.InstancesConfig{Logging: config.LoggingConfig{LogsDir: "/tmp/test"}}, Nodes: map[string]config.NodeConfig{}, LocalNode: "main", } diff --git a/pkg/instance/logger.go b/pkg/instance/logger.go index 681875b..bca3fee 100644 --- a/pkg/instance/logger.go +++ b/pkg/instance/logger.go @@ -7,66 +7,111 @@ import ( "os" "strings" "sync" - "sync/atomic" "time" + + timber "github.com/DeRuina/timberjack" + "llamactl/pkg/config" ) type logger struct { name string logDir string - logFile atomic.Pointer[os.File] + logFile *timber.Logger logFilePath string mu sync.RWMutex + cfg *config.LogRotationConfig } -func newLogger(name string, logDir string) *logger { +func newLogger(name, logDir string, cfg *config.LogRotationConfig) *logger { return &logger{ name: name, logDir: logDir, + cfg: cfg, } } -// create creates and opens the log files for stdout and stderr -func (i *logger) create() error { - i.mu.Lock() - defer i.mu.Unlock() +func (l *logger) create() error { + l.mu.Lock() + defer l.mu.Unlock() - if i.logDir == "" { - return fmt.Errorf("logDir is empty for instance %s", i.name) + if l.logDir == "" { + return fmt.Errorf("logDir empty for instance %s", l.name) } - // Set up instance logs - logPath := i.logDir + "/" + i.name + ".log" - - i.logFilePath = logPath - if err := os.MkdirAll(i.logDir, 0755); err != nil { + if err := os.MkdirAll(l.logDir, 0755); err != nil { return fmt.Errorf("failed to create log directory: %w", err) } - logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644) - if err != nil { - return fmt.Errorf("failed to create stdout log file: %w", err) + logPath := fmt.Sprintf("%s/%s.log", l.logDir, l.name) + l.logFilePath = logPath + + // Build the timber logger + t := &timber.Logger{ + Filename: logPath, + MaxSize: l.cfg.MaxSizeMB, + MaxBackups: l.cfg.MaxBackups, + // Compression: "gzip" if Compress is true, else "none" + Compression: func() string { + if l.cfg.Compress { + return "gzip" + } + return "none" + }(), + FileMode: 0644, // default; timberjack uses 640 if 0 + LocalTime: true, // use local time for consistency with lumberjack } - i.logFile.Store(logFile) + // If rotation is disabled, set MaxSize to 0 so no rotation occurs + if !l.cfg.Enabled { + t.MaxSize = 0 + } - // Write a startup marker to both files - timestamp := time.Now().Format("2006-01-02 15:04:05") - fmt.Fprintf(logFile, "\n=== Instance %s started at %s ===\n", i.name, timestamp) + l.logFile = t + + // Write a startup marker + ts := time.Now().Format("2006-01-02 15:04:05") + fmt.Fprintf(t, "\n=== Instance %s started at %s ===\n", l.name, ts) return nil } -// getLogs retrieves the last n lines of logs from the instance -func (i *logger) getLogs(num_lines int) (string, error) { - i.mu.RLock() - defer i.mu.RUnlock() +func (l *logger) readOutput(rc io.ReadCloser) { + defer rc.Close() + scanner := bufio.NewScanner(rc) + for scanner.Scan() { + line := scanner.Text() + if lg := l.logFile; lg != nil { + fmt.Fprintln(lg, line) // timber.Logger implements io.Writer + } + } +} - if i.logFilePath == "" { - return "", fmt.Errorf("log file not created for instance %s", i.name) +func (l *logger) close() { + l.mu.Lock() + defer l.mu.Unlock() + + lg := l.logFile + if lg == nil { + return } - file, err := os.Open(i.logFilePath) + ts := time.Now().Format("2006-01-02 15:04:05") + fmt.Fprintf(lg, "=== Instance %s stopped at %s ===\n\n", l.name, ts) + + _ = lg.Close() // shuts down any background goroutines (none in this config) + l.logFile = nil +} + +// getLogs retrieves the last n lines of logs from the instance +func (l *logger) getLogs(num_lines int) (string, error) { + l.mu.RLock() + defer l.mu.RUnlock() + + if l.logFilePath == "" { + return "", fmt.Errorf("log file not created for instance %s", l.name) + } + + file, err := os.Open(l.logFilePath) if err != nil { return "", fmt.Errorf("failed to open log file: %w", err) } @@ -97,31 +142,3 @@ func (i *logger) getLogs(num_lines int) (string, error) { return strings.Join(lines[start:], "\n"), nil } - -// close closes the log files -func (i *logger) close() { - i.mu.Lock() - defer i.mu.Unlock() - - logFile := i.logFile.Swap(nil) - if logFile != nil { - timestamp := time.Now().Format("2006-01-02 15:04:05") - fmt.Fprintf(logFile, "=== Instance %s stopped at %s ===\n\n", i.name, timestamp) - logFile.Sync() // Ensure all buffered data is written to disk - logFile.Close() - } -} - -// readOutput reads from the given reader and writes lines to the log file -func (i *logger) readOutput(reader io.ReadCloser) { - defer reader.Close() - - scanner := bufio.NewScanner(reader) - for scanner.Scan() { - line := scanner.Text() - // Use atomic load to avoid lock contention on every line - if logFile := i.logFile.Load(); logFile != nil { - fmt.Fprintln(logFile, line) - } - } -} diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index ba6a109..a4dadd8 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -201,13 +201,15 @@ func createTestAppConfig(instancesDir string) *config.AppConfig { }, }, Instances: config.InstancesConfig{ - PortRange: [2]int{8000, 9000}, - InstancesDir: instancesDir, - LogsDir: instancesDir, - MaxInstances: 10, - MaxRunningInstances: 10, - DefaultAutoRestart: true, - DefaultMaxRestarts: 3, + PortRange: [2]int{8000, 9000}, + InstancesDir: instancesDir, + MaxInstances: 10, + MaxRunningInstances: 10, + DefaultAutoRestart: true, + DefaultMaxRestarts: 3, + Logging: config.LoggingConfig{ + LogsDir: instancesDir, + }, DefaultRestartDelay: 5, TimeoutCheckInterval: 5, },