mirror of
https://github.com/lordmathis/llamactl.git
synced 2025-12-23 09:34:23 +00:00
Implement SQLite database persistence for instance management
This commit is contained in:
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"llamactl/pkg/config"
|
||||
"llamactl/pkg/database"
|
||||
"llamactl/pkg/instance"
|
||||
"log"
|
||||
"sync"
|
||||
@@ -28,11 +29,11 @@ type InstanceManager interface {
|
||||
|
||||
type instanceManager struct {
|
||||
// Components (each with own synchronization)
|
||||
registry *instanceRegistry
|
||||
ports *portAllocator
|
||||
persistence *instancePersister
|
||||
remote *remoteManager
|
||||
lifecycle *lifecycleManager
|
||||
registry *instanceRegistry
|
||||
ports *portAllocator
|
||||
db database.Database
|
||||
remote *remoteManager
|
||||
lifecycle *lifecycleManager
|
||||
|
||||
// Configuration
|
||||
globalConfig *config.AppConfig
|
||||
@@ -42,8 +43,8 @@ type instanceManager struct {
|
||||
shutdownOnce sync.Once
|
||||
}
|
||||
|
||||
// New creates a new instance of InstanceManager.
|
||||
func New(globalConfig *config.AppConfig) InstanceManager {
|
||||
// New creates a new instance of InstanceManager with dependency injection.
|
||||
func New(globalConfig *config.AppConfig, db database.Database) InstanceManager {
|
||||
|
||||
if globalConfig.Instances.TimeoutCheckInterval <= 0 {
|
||||
globalConfig.Instances.TimeoutCheckInterval = 5 // Default to 5 minutes if not set
|
||||
@@ -56,9 +57,6 @@ func New(globalConfig *config.AppConfig) InstanceManager {
|
||||
portRange := globalConfig.Instances.PortRange
|
||||
ports := newPortAllocator(portRange[0], portRange[1])
|
||||
|
||||
// Initialize persistence
|
||||
persistence := newInstancePersister(globalConfig.Instances.InstancesDir)
|
||||
|
||||
// Initialize remote manager
|
||||
remote := newRemoteManager(globalConfig.Nodes, 30*time.Second)
|
||||
|
||||
@@ -66,7 +64,7 @@ func New(globalConfig *config.AppConfig) InstanceManager {
|
||||
im := &instanceManager{
|
||||
registry: registry,
|
||||
ports: ports,
|
||||
persistence: persistence,
|
||||
db: db,
|
||||
remote: remote,
|
||||
globalConfig: globalConfig,
|
||||
}
|
||||
@@ -86,9 +84,9 @@ func New(globalConfig *config.AppConfig) InstanceManager {
|
||||
return im
|
||||
}
|
||||
|
||||
// persistInstance saves an instance using the persistence component
|
||||
// persistInstance saves an instance using the persistence layer
|
||||
func (im *instanceManager) persistInstance(inst *instance.Instance) error {
|
||||
return im.persistence.save(inst)
|
||||
return im.db.Save(inst)
|
||||
}
|
||||
|
||||
func (im *instanceManager) Shutdown() {
|
||||
@@ -116,13 +114,18 @@ func (im *instanceManager) Shutdown() {
|
||||
}
|
||||
wg.Wait()
|
||||
fmt.Println("All instances stopped.")
|
||||
|
||||
// 4. Close database connection
|
||||
if err := im.db.Close(); err != nil {
|
||||
log.Printf("Error closing database: %v\n", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// loadInstances restores all instances from disk using the persistence component
|
||||
// loadInstances restores all instances from the persistence layer
|
||||
func (im *instanceManager) loadInstances() error {
|
||||
// Load all instances from persistence
|
||||
instances, err := im.persistence.loadAll()
|
||||
instances, err := im.db.LoadAll()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to load instances: %w", err)
|
||||
}
|
||||
|
||||
@@ -4,20 +4,34 @@ import (
|
||||
"fmt"
|
||||
"llamactl/pkg/backends"
|
||||
"llamactl/pkg/config"
|
||||
"llamactl/pkg/database"
|
||||
"llamactl/pkg/instance"
|
||||
"llamactl/pkg/manager"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestManager_PersistsAndLoadsInstances(t *testing.T) {
|
||||
tempDir := t.TempDir()
|
||||
appConfig := createTestAppConfig(tempDir)
|
||||
// Use file-based database for this test since we need to persist across connections
|
||||
appConfig.Database.Path = tempDir + "/test.db"
|
||||
|
||||
// Create instance and check file was created
|
||||
manager1 := manager.New(appConfig)
|
||||
// Create instance and check database was created
|
||||
db1, err := database.Open(&database.Config{
|
||||
Path: appConfig.Database.Path,
|
||||
MaxOpenConnections: appConfig.Database.MaxOpenConnections,
|
||||
MaxIdleConnections: appConfig.Database.MaxIdleConnections,
|
||||
ConnMaxLifetime: appConfig.Database.ConnMaxLifetime,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to open database: %v", err)
|
||||
}
|
||||
if err := database.RunMigrations(db1); err != nil {
|
||||
t.Fatalf("Failed to run migrations: %v", err)
|
||||
}
|
||||
manager1 := manager.New(appConfig, db1)
|
||||
options := &instance.Options{
|
||||
BackendOptions: backends.Options{
|
||||
BackendType: backends.BackendTypeLlamaCpp,
|
||||
@@ -28,18 +42,28 @@ func TestManager_PersistsAndLoadsInstances(t *testing.T) {
|
||||
},
|
||||
}
|
||||
|
||||
_, err := manager1.CreateInstance("test-instance", options)
|
||||
_, err = manager1.CreateInstance("test-instance", options)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateInstance failed: %v", err)
|
||||
}
|
||||
|
||||
expectedPath := filepath.Join(tempDir, "test-instance.json")
|
||||
if _, err := os.Stat(expectedPath); os.IsNotExist(err) {
|
||||
t.Errorf("Expected persistence file %s to exist", expectedPath)
|
||||
}
|
||||
// Shutdown first manager to close database connection
|
||||
manager1.Shutdown()
|
||||
|
||||
// Load instances from disk
|
||||
manager2 := manager.New(appConfig)
|
||||
// Load instances from database
|
||||
db2, err := database.Open(&database.Config{
|
||||
Path: appConfig.Database.Path,
|
||||
MaxOpenConnections: appConfig.Database.MaxOpenConnections,
|
||||
MaxIdleConnections: appConfig.Database.MaxIdleConnections,
|
||||
ConnMaxLifetime: appConfig.Database.ConnMaxLifetime,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to open database: %v", err)
|
||||
}
|
||||
if err := database.RunMigrations(db2); err != nil {
|
||||
t.Fatalf("Failed to run migrations: %v", err)
|
||||
}
|
||||
manager2 := manager.New(appConfig, db2)
|
||||
instances, err := manager2.ListInstances()
|
||||
if err != nil {
|
||||
t.Fatalf("ListInstances failed: %v", err)
|
||||
@@ -50,13 +74,29 @@ func TestManager_PersistsAndLoadsInstances(t *testing.T) {
|
||||
if instances[0].Name != "test-instance" {
|
||||
t.Errorf("Expected loaded instance name 'test-instance', got %q", instances[0].Name)
|
||||
}
|
||||
|
||||
manager2.Shutdown()
|
||||
}
|
||||
|
||||
func TestDeleteInstance_RemovesPersistenceFile(t *testing.T) {
|
||||
func TestDeleteInstance_RemovesFromDatabase(t *testing.T) {
|
||||
tempDir := t.TempDir()
|
||||
appConfig := createTestAppConfig(tempDir)
|
||||
|
||||
mgr := manager.New(appConfig)
|
||||
db, err := database.Open(&database.Config{
|
||||
Path: appConfig.Database.Path,
|
||||
MaxOpenConnections: appConfig.Database.MaxOpenConnections,
|
||||
MaxIdleConnections: appConfig.Database.MaxIdleConnections,
|
||||
ConnMaxLifetime: appConfig.Database.ConnMaxLifetime,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to open database: %v", err)
|
||||
}
|
||||
if err := database.RunMigrations(db); err != nil {
|
||||
t.Fatalf("Failed to run migrations: %v", err)
|
||||
}
|
||||
mgr := manager.New(appConfig, db)
|
||||
defer mgr.Shutdown()
|
||||
|
||||
options := &instance.Options{
|
||||
BackendOptions: backends.Options{
|
||||
BackendType: backends.BackendTypeLlamaCpp,
|
||||
@@ -67,20 +107,33 @@ func TestDeleteInstance_RemovesPersistenceFile(t *testing.T) {
|
||||
},
|
||||
}
|
||||
|
||||
_, err := mgr.CreateInstance("test-instance", options)
|
||||
_, err = mgr.CreateInstance("test-instance", options)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateInstance failed: %v", err)
|
||||
}
|
||||
|
||||
expectedPath := filepath.Join(tempDir, "test-instance.json")
|
||||
// Verify instance exists
|
||||
instances, err := mgr.ListInstances()
|
||||
if err != nil {
|
||||
t.Fatalf("ListInstances failed: %v", err)
|
||||
}
|
||||
if len(instances) != 1 {
|
||||
t.Fatalf("Expected 1 instance, got %d", len(instances))
|
||||
}
|
||||
|
||||
// Delete instance
|
||||
err = mgr.DeleteInstance("test-instance")
|
||||
if err != nil {
|
||||
t.Fatalf("DeleteInstance failed: %v", err)
|
||||
}
|
||||
|
||||
if _, err := os.Stat(expectedPath); !os.IsNotExist(err) {
|
||||
t.Error("Expected persistence file to be deleted")
|
||||
// Verify instance was deleted from database
|
||||
instances, err = mgr.ListInstances()
|
||||
if err != nil {
|
||||
t.Fatalf("ListInstances failed: %v", err)
|
||||
}
|
||||
if len(instances) != 0 {
|
||||
t.Errorf("Expected 0 instances after deletion, got %d", len(instances))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -158,6 +211,12 @@ func createTestAppConfig(instancesDir string) *config.AppConfig {
|
||||
DefaultRestartDelay: 5,
|
||||
TimeoutCheckInterval: 5,
|
||||
},
|
||||
Database: config.DatabaseConfig{
|
||||
Path: ":memory:",
|
||||
MaxOpenConnections: 25,
|
||||
MaxIdleConnections: 5,
|
||||
ConnMaxLifetime: 5 * time.Minute,
|
||||
},
|
||||
LocalNode: "main",
|
||||
Nodes: map[string]config.NodeConfig{},
|
||||
}
|
||||
@@ -166,5 +225,17 @@ func createTestAppConfig(instancesDir string) *config.AppConfig {
|
||||
func createTestManager(t *testing.T) manager.InstanceManager {
|
||||
tempDir := t.TempDir()
|
||||
appConfig := createTestAppConfig(tempDir)
|
||||
return manager.New(appConfig)
|
||||
db, err := database.Open(&database.Config{
|
||||
Path: appConfig.Database.Path,
|
||||
MaxOpenConnections: appConfig.Database.MaxOpenConnections,
|
||||
MaxIdleConnections: appConfig.Database.MaxIdleConnections,
|
||||
ConnMaxLifetime: appConfig.Database.ConnMaxLifetime,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to open database: %v", err)
|
||||
}
|
||||
if err := database.RunMigrations(db); err != nil {
|
||||
t.Fatalf("Failed to run migrations: %v", err)
|
||||
}
|
||||
return manager.New(appConfig, db)
|
||||
}
|
||||
|
||||
@@ -317,9 +317,9 @@ func (im *instanceManager) DeleteInstance(name string) error {
|
||||
im.remote.removeInstance(name)
|
||||
im.registry.remove(name)
|
||||
|
||||
// Delete the instance's persistence file
|
||||
if err := im.persistence.delete(name); err != nil {
|
||||
return fmt.Errorf("failed to delete config file for remote instance %s: %w", name, err)
|
||||
// Delete the instance's persistence
|
||||
if err := im.db.Delete(name); err != nil {
|
||||
return fmt.Errorf("failed to delete remote instance %s: %w", name, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -343,9 +343,9 @@ func (im *instanceManager) DeleteInstance(name string) error {
|
||||
return fmt.Errorf("failed to remove instance from registry: %w", err)
|
||||
}
|
||||
|
||||
// Delete persistence file
|
||||
if err := im.persistence.delete(name); err != nil {
|
||||
return fmt.Errorf("failed to delete config file for instance %s: %w", name, err)
|
||||
// Delete from persistence
|
||||
if err := im.db.Delete(name); err != nil {
|
||||
return fmt.Errorf("failed to delete instance from persistence %s: %w", name, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
|
||||
@@ -3,10 +3,12 @@ package manager_test
|
||||
import (
|
||||
"llamactl/pkg/backends"
|
||||
"llamactl/pkg/config"
|
||||
"llamactl/pkg/database"
|
||||
"llamactl/pkg/instance"
|
||||
"llamactl/pkg/manager"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestCreateInstance_FailsWithDuplicateName(t *testing.T) {
|
||||
@@ -49,10 +51,28 @@ func TestCreateInstance_FailsWhenMaxInstancesReached(t *testing.T) {
|
||||
MaxInstances: 1, // Very low limit for testing
|
||||
TimeoutCheckInterval: 5,
|
||||
},
|
||||
Database: config.DatabaseConfig{
|
||||
Path: ":memory:",
|
||||
MaxOpenConnections: 25,
|
||||
MaxIdleConnections: 5,
|
||||
ConnMaxLifetime: 5 * time.Minute,
|
||||
},
|
||||
LocalNode: "main",
|
||||
Nodes: map[string]config.NodeConfig{},
|
||||
}
|
||||
limitedManager := manager.New(appConfig)
|
||||
db, err := database.Open(&database.Config{
|
||||
Path: appConfig.Database.Path,
|
||||
MaxOpenConnections: appConfig.Database.MaxOpenConnections,
|
||||
MaxIdleConnections: appConfig.Database.MaxIdleConnections,
|
||||
ConnMaxLifetime: appConfig.Database.ConnMaxLifetime,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to open database: %v", err)
|
||||
}
|
||||
if err := database.RunMigrations(db); err != nil {
|
||||
t.Fatalf("Failed to run migrations: %v", err)
|
||||
}
|
||||
limitedManager := manager.New(appConfig, db)
|
||||
|
||||
options := &instance.Options{
|
||||
BackendOptions: backends.Options{
|
||||
@@ -63,7 +83,7 @@ func TestCreateInstance_FailsWhenMaxInstancesReached(t *testing.T) {
|
||||
},
|
||||
}
|
||||
|
||||
_, err := limitedManager.CreateInstance("instance1", options)
|
||||
_, err = limitedManager.CreateInstance("instance1", options)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateInstance 1 failed: %v", err)
|
||||
}
|
||||
|
||||
@@ -1,198 +0,0 @@
|
||||
package manager
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"llamactl/pkg/instance"
|
||||
"log"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
)
|
||||
|
||||
// instancePersister provides atomic file-based persistence with durability guarantees.
|
||||
type instancePersister struct {
|
||||
mu sync.Mutex
|
||||
instancesDir string
|
||||
}
|
||||
|
||||
// newInstancePersister creates a new instance persister.
|
||||
// If instancesDir is empty, persistence is disabled.
|
||||
func newInstancePersister(instancesDir string) *instancePersister {
|
||||
return &instancePersister{
|
||||
instancesDir: instancesDir,
|
||||
}
|
||||
}
|
||||
|
||||
// Save persists an instance to disk with atomic write
|
||||
func (p *instancePersister) save(inst *instance.Instance) error {
|
||||
if inst == nil {
|
||||
return fmt.Errorf("cannot save nil instance")
|
||||
}
|
||||
|
||||
// Validate instance name to prevent path traversal
|
||||
validatedName, err := p.validateInstanceName(inst.Name)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
p.mu.Lock()
|
||||
defer p.mu.Unlock()
|
||||
|
||||
instancePath := filepath.Join(p.instancesDir, validatedName+".json")
|
||||
tempPath := instancePath + ".tmp"
|
||||
|
||||
// Serialize instance to JSON
|
||||
jsonData, err := json.MarshalIndent(inst, "", " ")
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to marshal instance %s: %w", inst.Name, err)
|
||||
}
|
||||
|
||||
// Create temporary file
|
||||
tempFile, err := os.OpenFile(tempPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create temp file for instance %s: %w", inst.Name, err)
|
||||
}
|
||||
|
||||
// Write data to temporary file
|
||||
if _, err := tempFile.Write(jsonData); err != nil {
|
||||
tempFile.Close()
|
||||
os.Remove(tempPath)
|
||||
return fmt.Errorf("failed to write temp file for instance %s: %w", inst.Name, err)
|
||||
}
|
||||
|
||||
// Sync to disk before rename to ensure durability
|
||||
if err := tempFile.Sync(); err != nil {
|
||||
tempFile.Close()
|
||||
os.Remove(tempPath)
|
||||
return fmt.Errorf("failed to sync temp file for instance %s: %w", inst.Name, err)
|
||||
}
|
||||
|
||||
// Close the file
|
||||
if err := tempFile.Close(); err != nil {
|
||||
os.Remove(tempPath)
|
||||
return fmt.Errorf("failed to close temp file for instance %s: %w", inst.Name, err)
|
||||
}
|
||||
|
||||
// Atomic rename (this is atomic on POSIX systems)
|
||||
if err := os.Rename(tempPath, instancePath); err != nil {
|
||||
os.Remove(tempPath)
|
||||
return fmt.Errorf("failed to rename temp file for instance %s: %w", inst.Name, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Delete removes an instance's persistence file from disk.
|
||||
func (p *instancePersister) delete(name string) error {
|
||||
validatedName, err := p.validateInstanceName(name)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
p.mu.Lock()
|
||||
defer p.mu.Unlock()
|
||||
|
||||
instancePath := filepath.Join(p.instancesDir, validatedName+".json")
|
||||
|
||||
if err := os.Remove(instancePath); err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
// Not an error if file doesn't exist
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("failed to delete instance file for %s: %w", name, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// LoadAll loads all persisted instances from disk.
|
||||
// Returns a slice of instances and any errors encountered during loading.
|
||||
func (p *instancePersister) loadAll() ([]*instance.Instance, error) {
|
||||
p.mu.Lock()
|
||||
defer p.mu.Unlock()
|
||||
|
||||
// Check if instances directory exists
|
||||
if _, err := os.Stat(p.instancesDir); os.IsNotExist(err) {
|
||||
return nil, nil // No instances directory, return empty list
|
||||
}
|
||||
|
||||
// Read all JSON files from instances directory
|
||||
files, err := os.ReadDir(p.instancesDir)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to read instances directory: %w", err)
|
||||
}
|
||||
|
||||
instances := make([]*instance.Instance, 0)
|
||||
var loadErrors []string
|
||||
|
||||
for _, file := range files {
|
||||
if file.IsDir() || !strings.HasSuffix(file.Name(), ".json") {
|
||||
continue
|
||||
}
|
||||
|
||||
instanceName := strings.TrimSuffix(file.Name(), ".json")
|
||||
instancePath := filepath.Join(p.instancesDir, file.Name())
|
||||
|
||||
inst, err := p.loadInstanceFile(instanceName, instancePath)
|
||||
if err != nil {
|
||||
log.Printf("Failed to load instance %s: %v", instanceName, err)
|
||||
loadErrors = append(loadErrors, fmt.Sprintf("%s: %v", instanceName, err))
|
||||
continue
|
||||
}
|
||||
|
||||
instances = append(instances, inst)
|
||||
}
|
||||
|
||||
if len(loadErrors) > 0 {
|
||||
log.Printf("Loaded %d instances with %d errors", len(instances), len(loadErrors))
|
||||
} else if len(instances) > 0 {
|
||||
log.Printf("Loaded %d instances from persistence", len(instances))
|
||||
}
|
||||
|
||||
return instances, nil
|
||||
}
|
||||
|
||||
// loadInstanceFile is an internal helper that loads a single instance file.
|
||||
// Note: This assumes the mutex is already held by the caller.
|
||||
func (p *instancePersister) loadInstanceFile(name, path string) (*instance.Instance, error) {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to read instance file: %w", err)
|
||||
}
|
||||
|
||||
var inst instance.Instance
|
||||
if err := json.Unmarshal(data, &inst); err != nil {
|
||||
return nil, fmt.Errorf("failed to unmarshal instance: %w", err)
|
||||
}
|
||||
|
||||
// Validate the instance name matches the filename
|
||||
if inst.Name != name {
|
||||
return nil, fmt.Errorf("instance name mismatch: file=%s, instance.Name=%s", name, inst.Name)
|
||||
}
|
||||
|
||||
return &inst, nil
|
||||
}
|
||||
|
||||
// validateInstanceName ensures the instance name is safe for filesystem operations.
|
||||
// Returns the validated name if valid, or an error if invalid.
|
||||
func (p *instancePersister) validateInstanceName(name string) (string, error) {
|
||||
if name == "" {
|
||||
return "", fmt.Errorf("instance name cannot be empty")
|
||||
}
|
||||
|
||||
// Check for path separators and parent directory references
|
||||
// This prevents path traversal attacks
|
||||
if strings.Contains(name, "/") || strings.Contains(name, "\\") || strings.Contains(name, "..") {
|
||||
return "", fmt.Errorf("invalid instance name: %s (cannot contain path separators or '..')", name)
|
||||
}
|
||||
|
||||
// Additional check: ensure the name doesn't start with a dot (hidden files)
|
||||
// or contain any other suspicious characters
|
||||
if strings.HasPrefix(name, ".") {
|
||||
return "", fmt.Errorf("invalid instance name: %s (cannot start with '.')", name)
|
||||
}
|
||||
|
||||
return name, nil
|
||||
}
|
||||
Reference in New Issue
Block a user