Refactor instance status management: replace Running boolean with InstanceStatus enum and update related methods

This commit is contained in:
2025-08-27 19:44:38 +02:00
parent 615c2ac54e
commit 1443746add
10 changed files with 111 additions and 45 deletions

View File

@@ -82,7 +82,7 @@ type Process struct {
globalSettings *config.InstancesConfig globalSettings *config.InstancesConfig
// Status // Status
Running bool `json:"running"` Status InstanceStatus `json:"status"`
// Creation time // Creation time
Created int64 `json:"created,omitempty"` // Unix timestamp when the instance was created Created int64 `json:"created,omitempty"` // Unix timestamp when the instance was created
@@ -287,12 +287,12 @@ func (i *Process) MarshalJSON() ([]byte, error) {
temp := struct { temp := struct {
Name string `json:"name"` Name string `json:"name"`
Options *CreateInstanceOptions `json:"options,omitempty"` Options *CreateInstanceOptions `json:"options,omitempty"`
Running bool `json:"running"` Status InstanceStatus `json:"status"`
Created int64 `json:"created,omitempty"` Created int64 `json:"created,omitempty"`
}{ }{
Name: i.Name, Name: i.Name,
Options: i.options, Options: i.options,
Running: i.Running, Status: i.Status,
Created: i.Created, Created: i.Created,
} }
@@ -305,7 +305,7 @@ func (i *Process) UnmarshalJSON(data []byte) error {
temp := struct { temp := struct {
Name string `json:"name"` Name string `json:"name"`
Options *CreateInstanceOptions `json:"options,omitempty"` Options *CreateInstanceOptions `json:"options,omitempty"`
Running bool `json:"running"` Status InstanceStatus `json:"status"`
Created int64 `json:"created,omitempty"` Created int64 `json:"created,omitempty"`
}{} }{}
@@ -315,7 +315,7 @@ func (i *Process) UnmarshalJSON(data []byte) error {
// Set the fields // Set the fields
i.Name = temp.Name i.Name = temp.Name
i.Running = temp.Running i.Status = temp.Status
i.Created = temp.Created i.Created = temp.Created
// Handle options with validation but no defaults // Handle options with validation but no defaults

View File

@@ -29,7 +29,7 @@ func TestNewInstance(t *testing.T) {
if instance.Name != "test-instance" { if instance.Name != "test-instance" {
t.Errorf("Expected name 'test-instance', got %q", instance.Name) t.Errorf("Expected name 'test-instance', got %q", instance.Name)
} }
if instance.Running { if instance.IsRunning() {
t.Error("New instance should not be running") t.Error("New instance should not be running")
} }
@@ -188,7 +188,7 @@ func TestMarshalJSON(t *testing.T) {
} }
// Check that JSON contains expected fields // Check that JSON contains expected fields
var result map[string]interface{} var result map[string]any
err = json.Unmarshal(data, &result) err = json.Unmarshal(data, &result)
if err != nil { if err != nil {
t.Fatalf("JSON unmarshal failed: %v", err) t.Fatalf("JSON unmarshal failed: %v", err)
@@ -197,8 +197,8 @@ func TestMarshalJSON(t *testing.T) {
if result["name"] != "test-instance" { if result["name"] != "test-instance" {
t.Errorf("Expected name 'test-instance', got %v", result["name"]) t.Errorf("Expected name 'test-instance', got %v", result["name"])
} }
if result["running"] != false { if result["status"] != "stopped" {
t.Errorf("Expected running false, got %v", result["running"]) t.Errorf("Expected status 'stopped', got %v", result["status"])
} }
// Check that options are included // Check that options are included
@@ -218,7 +218,7 @@ func TestMarshalJSON(t *testing.T) {
func TestUnmarshalJSON(t *testing.T) { func TestUnmarshalJSON(t *testing.T) {
jsonData := `{ jsonData := `{
"name": "test-instance", "name": "test-instance",
"running": true, "status": "running",
"options": { "options": {
"model": "/path/to/model.gguf", "model": "/path/to/model.gguf",
"port": 8080, "port": 8080,
@@ -236,8 +236,8 @@ func TestUnmarshalJSON(t *testing.T) {
if inst.Name != "test-instance" { if inst.Name != "test-instance" {
t.Errorf("Expected name 'test-instance', got %q", inst.Name) t.Errorf("Expected name 'test-instance', got %q", inst.Name)
} }
if !inst.Running { if !inst.IsRunning() {
t.Error("Expected running to be true") t.Error("Expected status to be running")
} }
opts := inst.GetOptions() opts := inst.GetOptions()

View File

@@ -11,18 +11,12 @@ import (
"time" "time"
) )
func (i *Process) IsRunning() bool {
i.mu.RLock()
defer i.mu.RUnlock()
return i.Running
}
// Start starts the llama server instance and returns an error if it fails. // Start starts the llama server instance and returns an error if it fails.
func (i *Process) Start() error { func (i *Process) Start() error {
i.mu.Lock() i.mu.Lock()
defer i.mu.Unlock() defer i.mu.Unlock()
if i.Running { if i.IsRunning() {
return fmt.Errorf("instance %s is already running", i.Name) return fmt.Errorf("instance %s is already running", i.Name)
} }
@@ -71,7 +65,7 @@ func (i *Process) Start() error {
return fmt.Errorf("failed to start instance %s: %w", i.Name, err) return fmt.Errorf("failed to start instance %s: %w", i.Name, err)
} }
i.Running = true i.SetStatus(Running)
// Create channel for monitor completion signaling // Create channel for monitor completion signaling
i.monitorDone = make(chan struct{}) i.monitorDone = make(chan struct{})
@@ -88,7 +82,7 @@ func (i *Process) Start() error {
func (i *Process) Stop() error { func (i *Process) Stop() error {
i.mu.Lock() i.mu.Lock()
if !i.Running { if !i.IsRunning() {
// Even if not running, cancel any pending restart // Even if not running, cancel any pending restart
if i.restartCancel != nil { if i.restartCancel != nil {
i.restartCancel() i.restartCancel()
@@ -105,8 +99,8 @@ func (i *Process) Stop() error {
i.restartCancel = nil i.restartCancel = nil
} }
// Set running to false first to signal intentional stop // Set status to stopped first to signal intentional stop
i.Running = false i.SetStatus(Stopped)
// Clean up the proxy // Clean up the proxy
i.proxy = nil i.proxy = nil
@@ -151,7 +145,7 @@ func (i *Process) Stop() error {
} }
func (i *Process) WaitForHealthy(timeout int) error { func (i *Process) WaitForHealthy(timeout int) error {
if !i.Running { if !i.IsRunning() {
return fmt.Errorf("instance %s is not running", i.Name) return fmt.Errorf("instance %s is not running", i.Name)
} }
@@ -233,12 +227,12 @@ func (i *Process) monitorProcess() {
i.mu.Lock() i.mu.Lock()
// Check if the instance was intentionally stopped // Check if the instance was intentionally stopped
if !i.Running { if !i.IsRunning() {
i.mu.Unlock() i.mu.Unlock()
return return
} }
i.Running = false i.SetStatus(Stopped)
i.logger.Close() i.logger.Close()
// Cancel any existing restart context since we're handling a new exit // Cancel any existing restart context since we're handling a new exit

72
pkg/instance/status.go Normal file
View File

@@ -0,0 +1,72 @@
package instance
import (
"encoding/json"
"log"
)
// Enum for instance status
type InstanceStatus int
const (
Stopped InstanceStatus = iota
Running
Failed
)
var nameToStatus = map[string]InstanceStatus{
"stopped": Stopped,
"running": Running,
"failed": Failed,
}
var statusToName = map[InstanceStatus]string{
Stopped: "stopped",
Running: "running",
Failed: "failed",
}
func (p *Process) SetStatus(status InstanceStatus) {
p.mu.Lock()
defer p.mu.Unlock()
p.Status = status
}
func (p *Process) GetStatus() InstanceStatus {
p.mu.RLock()
defer p.mu.RUnlock()
return p.Status
}
// IsRunning returns true if the status is Running
func (p *Process) IsRunning() bool {
p.mu.RLock()
defer p.mu.RUnlock()
return p.Status == Running
}
func (s InstanceStatus) MarshalJSON() ([]byte, error) {
name, ok := statusToName[s]
if !ok {
name = "stopped" // Default to "stopped" for unknown status
}
return json.Marshal(name)
}
// UnmarshalJSON implements json.Unmarshaler
func (s *InstanceStatus) UnmarshalJSON(data []byte) error {
var str string
if err := json.Unmarshal(data, &str); err != nil {
return err
}
status, ok := nameToStatus[str]
if !ok {
log.Printf("Unknown instance status: %s", str)
status = Stopped // Default to Stopped on unknown status
}
*s = status
return nil
}

View File

@@ -13,7 +13,7 @@ func (i *Process) ShouldTimeout() bool {
i.mu.RLock() i.mu.RLock()
defer i.mu.RUnlock() defer i.mu.RUnlock()
if !i.Running || i.options.IdleTimeout == nil || *i.options.IdleTimeout <= 0 { if !i.IsRunning() || i.options.IdleTimeout == nil || *i.options.IdleTimeout <= 0 {
return false return false
} }

View File

@@ -94,7 +94,7 @@ func TestShouldTimeout_NoTimeoutConfigured(t *testing.T) {
inst := instance.NewInstance("test-instance", globalSettings, options) inst := instance.NewInstance("test-instance", globalSettings, options)
// Simulate running state // Simulate running state
inst.Running = true inst.SetStatus(instance.Running)
if inst.ShouldTimeout() { if inst.ShouldTimeout() {
t.Errorf("Instance with %s should not timeout", tt.name) t.Errorf("Instance with %s should not timeout", tt.name)
@@ -117,7 +117,7 @@ func TestShouldTimeout_WithinTimeLimit(t *testing.T) {
} }
inst := instance.NewInstance("test-instance", globalSettings, options) inst := instance.NewInstance("test-instance", globalSettings, options)
inst.Running = true inst.SetStatus(instance.Running)
// Update last request time to now // Update last request time to now
inst.UpdateLastRequestTime() inst.UpdateLastRequestTime()
@@ -142,7 +142,7 @@ func TestShouldTimeout_ExceedsTimeLimit(t *testing.T) {
} }
inst := instance.NewInstance("test-instance", globalSettings, options) inst := instance.NewInstance("test-instance", globalSettings, options)
inst.Running = true inst.SetStatus(instance.Running)
// Use MockTimeProvider to simulate old last request time // Use MockTimeProvider to simulate old last request time
mockTime := NewMockTimeProvider(time.Now()) mockTime := NewMockTimeProvider(time.Now())

View File

@@ -150,7 +150,7 @@ func (im *instanceManager) Shutdown() {
wg.Add(len(im.instances)) wg.Add(len(im.instances))
for name, inst := range im.instances { for name, inst := range im.instances {
if !inst.Running { if !inst.IsRunning() {
wg.Done() // If instance is not running, just mark it as done wg.Done() // If instance is not running, just mark it as done
continue continue
} }
@@ -234,7 +234,7 @@ func (im *instanceManager) loadInstance(name, path string) error {
// Restore persisted fields that NewInstance doesn't set // Restore persisted fields that NewInstance doesn't set
inst.Created = persistedInstance.Created inst.Created = persistedInstance.Created
inst.Running = persistedInstance.Running inst.SetStatus(persistedInstance.Status)
// Check for port conflicts and add to maps // Check for port conflicts and add to maps
if inst.GetOptions() != nil && inst.GetOptions().Port > 0 { if inst.GetOptions() != nil && inst.GetOptions().Port > 0 {
@@ -254,7 +254,7 @@ func (im *instanceManager) autoStartInstances() {
im.mu.RLock() im.mu.RLock()
var instancesToStart []*instance.Process var instancesToStart []*instance.Process
for _, inst := range im.instances { for _, inst := range im.instances {
if inst.Running && // Was running when persisted if inst.IsRunning() && // Was running when persisted
inst.GetOptions() != nil && inst.GetOptions() != nil &&
inst.GetOptions().AutoRestart != nil && inst.GetOptions().AutoRestart != nil &&
*inst.GetOptions().AutoRestart { *inst.GetOptions().AutoRestart {
@@ -266,7 +266,7 @@ func (im *instanceManager) autoStartInstances() {
for _, inst := range instancesToStart { for _, inst := range instancesToStart {
log.Printf("Auto-starting instance %s", inst.Name) log.Printf("Auto-starting instance %s", inst.Name)
// Reset running state before starting (since Start() expects stopped instance) // Reset running state before starting (since Start() expects stopped instance)
inst.Running = false inst.SetStatus(instance.Stopped)
if err := inst.Start(); err != nil { if err := inst.Start(); err != nil {
log.Printf("Failed to auto-start instance %s: %v", inst.Name, err) log.Printf("Failed to auto-start instance %s: %v", inst.Name, err)
} }

View File

@@ -59,7 +59,7 @@ func TestCreateInstance_Success(t *testing.T) {
if inst.Name != "test-instance" { if inst.Name != "test-instance" {
t.Errorf("Expected instance name 'test-instance', got %q", inst.Name) t.Errorf("Expected instance name 'test-instance', got %q", inst.Name)
} }
if inst.Running { if inst.GetStatus() != instance.Stopped {
t.Error("New instance should not be running") t.Error("New instance should not be running")
} }
if inst.GetOptions().Port != 8080 { if inst.GetOptions().Port != 8080 {
@@ -357,7 +357,7 @@ func TestTimeoutFunctionality(t *testing.T) {
inst.SetTimeProvider(mockTime) inst.SetTimeProvider(mockTime)
// Set instance to running state so timeout logic can work // Set instance to running state so timeout logic can work
inst.Running = true inst.SetStatus(instance.Running)
// Simulate instance being "running" for timeout check (without actual process) // Simulate instance being "running" for timeout check (without actual process)
// We'll test the ShouldTimeout logic directly // We'll test the ShouldTimeout logic directly
@@ -377,7 +377,7 @@ func TestTimeoutFunctionality(t *testing.T) {
} }
// Reset running state to avoid shutdown issues // Reset running state to avoid shutdown issues
inst.Running = false inst.SetStatus(instance.Stopped)
// Test that instance without timeout doesn't timeout // Test that instance without timeout doesn't timeout
noTimeoutOptions := &instance.CreateInstanceOptions{ noTimeoutOptions := &instance.CreateInstanceOptions{
@@ -393,7 +393,7 @@ func TestTimeoutFunctionality(t *testing.T) {
} }
noTimeoutInst.SetTimeProvider(mockTime) noTimeoutInst.SetTimeProvider(mockTime)
noTimeoutInst.Running = true // Set to running for timeout check noTimeoutInst.SetStatus(instance.Running) // Set to running for timeout check
noTimeoutInst.UpdateLastRequestTime() noTimeoutInst.UpdateLastRequestTime()
// Even with time advanced, should not timeout // Even with time advanced, should not timeout
@@ -402,7 +402,7 @@ func TestTimeoutFunctionality(t *testing.T) {
} }
// Reset running state to avoid shutdown issues // Reset running state to avoid shutdown issues
noTimeoutInst.Running = false noTimeoutInst.SetStatus(instance.Stopped)
} }
func TestConcurrentAccess(t *testing.T) { func TestConcurrentAccess(t *testing.T) {

View File

@@ -109,7 +109,7 @@ func (im *instanceManager) UpdateInstance(name string, options *instance.CreateI
} }
// Check if instance is running before updating options // Check if instance is running before updating options
wasRunning := instance.Running wasRunning := instance.IsRunning()
// If the instance is running, stop it first // If the instance is running, stop it first
if wasRunning { if wasRunning {
@@ -147,7 +147,7 @@ func (im *instanceManager) DeleteInstance(name string) error {
return fmt.Errorf("instance with name %s not found", name) return fmt.Errorf("instance with name %s not found", name)
} }
if instance.Running { if instance.IsRunning() {
return fmt.Errorf("instance with name %s is still running, stop it before deleting", name) return fmt.Errorf("instance with name %s is still running, stop it before deleting", name)
} }
@@ -173,7 +173,7 @@ func (im *instanceManager) StartInstance(name string) (*instance.Process, error)
if !exists { if !exists {
return nil, fmt.Errorf("instance with name %s not found", name) return nil, fmt.Errorf("instance with name %s not found", name)
} }
if instance.Running { if instance.IsRunning() {
return instance, fmt.Errorf("instance with name %s is already running", name) return instance, fmt.Errorf("instance with name %s is already running", name)
} }
@@ -200,7 +200,7 @@ func (im *instanceManager) StopInstance(name string) (*instance.Process, error)
if !exists { if !exists {
return nil, fmt.Errorf("instance with name %s not found", name) return nil, fmt.Errorf("instance with name %s not found", name)
} }
if !instance.Running { if !instance.IsRunning() {
return instance, fmt.Errorf("instance with name %s is already stopped", name) return instance, fmt.Errorf("instance with name %s is already stopped", name)
} }

View File

@@ -451,7 +451,7 @@ func (h *Handler) ProxyToInstance() http.HandlerFunc {
return return
} }
if !inst.Running { if !inst.IsRunning() {
http.Error(w, "Instance is not running", http.StatusServiceUnavailable) http.Error(w, "Instance is not running", http.StatusServiceUnavailable)
return return
} }
@@ -574,7 +574,7 @@ func (h *Handler) OpenAIProxy() http.HandlerFunc {
return return
} }
if !inst.Running { if !inst.IsRunning() {
if inst.GetOptions().OnDemandStart != nil && *inst.GetOptions().OnDemandStart { if inst.GetOptions().OnDemandStart != nil && *inst.GetOptions().OnDemandStart {
// If on-demand start is enabled, start the instance // If on-demand start is enabled, start the instance
if _, err := h.InstanceManager.StartInstance(modelName); err != nil { if _, err := h.InstanceManager.StartInstance(modelName); err != nil {