Refactor instance node handling to use a map

This commit is contained in:
2025-10-18 00:33:16 +02:00
parent 7bf0809122
commit 113b51eda2
7 changed files with 83 additions and 31 deletions

View File

@@ -193,9 +193,11 @@ func (i *Instance) GetProxy() (*httputil.ReverseProxy, error) {
// Remote instances should not use local proxy - they are handled by RemoteInstanceProxy // Remote instances should not use local proxy - they are handled by RemoteInstanceProxy
opts := i.GetOptions() opts := i.GetOptions()
if opts != nil && len(opts.Nodes) > 0 && opts.Nodes[0] != i.localNodeName { if opts != nil && len(opts.Nodes) > 0 {
if _, isLocal := opts.Nodes[i.localNodeName]; !isLocal {
return nil, fmt.Errorf("instance %s is a remote instance and should not use local proxy", i.Name) return nil, fmt.Errorf("instance %s is a remote instance and should not use local proxy", i.Name)
} }
}
return i.proxy.GetProxy() return i.proxy.GetProxy()
} }
@@ -303,8 +305,8 @@ func (i *Instance) IsRemote() bool {
return false return false
} }
// If the first node is the local node, treat it as a local instance // If the local node is in the nodes map, treat it as a local instance
if opts.Nodes[0] == i.localNodeName { if _, isLocal := opts.Nodes[i.localNodeName]; isLocal {
return false return false
} }

View File

@@ -209,7 +209,7 @@ func TestSetOptions_PreservesNodes(t *testing.T) {
// Create instance with initial nodes // Create instance with initial nodes
initialOptions := &instance.Options{ initialOptions := &instance.Options{
BackendType: backends.BackendTypeLlamaCpp, BackendType: backends.BackendTypeLlamaCpp,
Nodes: []string{"worker1"}, Nodes: map[string]struct{}{"worker1": {}},
LlamaServerOptions: &llamacpp.LlamaServerOptions{ LlamaServerOptions: &llamacpp.LlamaServerOptions{
Model: "/path/to/model.gguf", Model: "/path/to/model.gguf",
Port: 8080, Port: 8080,
@@ -222,7 +222,7 @@ func TestSetOptions_PreservesNodes(t *testing.T) {
// Try to update with different nodes // Try to update with different nodes
updatedOptions := &instance.Options{ updatedOptions := &instance.Options{
BackendType: backends.BackendTypeLlamaCpp, BackendType: backends.BackendTypeLlamaCpp,
Nodes: []string{"worker2"}, // Attempt to change node Nodes: map[string]struct{}{"worker2": {}}, // Attempt to change node
LlamaServerOptions: &llamacpp.LlamaServerOptions{ LlamaServerOptions: &llamacpp.LlamaServerOptions{
Model: "/path/to/new-model.gguf", Model: "/path/to/new-model.gguf",
Port: 8081, Port: 8081,
@@ -233,8 +233,8 @@ func TestSetOptions_PreservesNodes(t *testing.T) {
opts := inst.GetOptions() opts := inst.GetOptions()
// Nodes should remain unchanged // Nodes should remain unchanged
if len(opts.Nodes) != 1 || opts.Nodes[0] != "worker1" { if _, exists := opts.Nodes["worker1"]; len(opts.Nodes) != 1 || !exists {
t.Errorf("Expected nodes to remain ['worker1'], got %v", opts.Nodes) t.Errorf("Expected nodes to contain 'worker1', got %v", opts.Nodes)
} }
// Other options should be updated // Other options should be updated

View File

@@ -10,6 +10,7 @@ import (
"llamactl/pkg/config" "llamactl/pkg/config"
"log" "log"
"maps" "maps"
"slices"
"sync" "sync"
) )
@@ -29,7 +30,7 @@ type Options struct {
BackendType backends.BackendType `json:"backend_type"` BackendType backends.BackendType `json:"backend_type"`
BackendOptions map[string]any `json:"backend_options,omitempty"` BackendOptions map[string]any `json:"backend_options,omitempty"`
Nodes []string `json:"nodes,omitempty"` Nodes map[string]struct{} `json:"-"`
// Backend-specific options // Backend-specific options
LlamaServerOptions *llamacpp.LlamaServerOptions `json:"-"` LlamaServerOptions *llamacpp.LlamaServerOptions `json:"-"`
@@ -87,6 +88,7 @@ func (c *Options) UnmarshalJSON(data []byte) error {
// Use anonymous struct to avoid recursion // Use anonymous struct to avoid recursion
type Alias Options type Alias Options
aux := &struct { aux := &struct {
Nodes []string `json:"nodes,omitempty"` // Accept JSON array
*Alias *Alias
}{ }{
Alias: (*Alias)(c), Alias: (*Alias)(c),
@@ -96,6 +98,14 @@ func (c *Options) UnmarshalJSON(data []byte) error {
return err return err
} }
// Convert nodes array to map
if len(aux.Nodes) > 0 {
c.Nodes = make(map[string]struct{}, len(aux.Nodes))
for _, node := range aux.Nodes {
c.Nodes[node] = struct{}{}
}
}
// Parse backend-specific options // Parse backend-specific options
switch c.BackendType { switch c.BackendType {
case backends.BackendTypeLlamaCpp: case backends.BackendTypeLlamaCpp:
@@ -147,11 +157,22 @@ func (c *Options) MarshalJSON() ([]byte, error) {
// Use anonymous struct to avoid recursion // Use anonymous struct to avoid recursion
type Alias Options type Alias Options
aux := struct { aux := struct {
Nodes []string `json:"nodes,omitempty"` // Output as JSON array
*Alias *Alias
}{ }{
Alias: (*Alias)(c), Alias: (*Alias)(c),
} }
// Convert nodes map to array (sorted for consistency)
if len(c.Nodes) > 0 {
aux.Nodes = make([]string, 0, len(c.Nodes))
for node := range c.Nodes {
aux.Nodes = append(aux.Nodes, node)
}
// Sort for consistent output
slices.Sort(aux.Nodes)
}
// Convert backend-specific options back to BackendOptions map for JSON // Convert backend-specific options back to BackendOptions map for JSON
switch c.BackendType { switch c.BackendType {
case backends.BackendTypeLlamaCpp: case backends.BackendTypeLlamaCpp:

View File

@@ -275,9 +275,19 @@ func (im *instanceManager) loadInstance(name, path string) error {
options := persistedInstance.GetOptions() options := persistedInstance.GetOptions()
// Check if this is a remote instance // Check if this is a remote instance (local node not in the Nodes set)
// An instance is remote if Nodes is specified AND the first node is not the local node var isRemote bool
isRemote := options != nil && len(options.Nodes) > 0 && options.Nodes[0] != im.localNodeName var nodeName string
if options != nil {
if _, isLocal := options.Nodes[im.localNodeName]; !isLocal {
// Get the first node from the set
for node := range options.Nodes {
nodeName = node
isRemote = true
break
}
}
}
var statusCallback func(oldStatus, newStatus instance.Status) var statusCallback func(oldStatus, newStatus instance.Status)
if !isRemote { if !isRemote {
@@ -296,7 +306,6 @@ func (im *instanceManager) loadInstance(name, path string) error {
// Handle remote instance mapping // Handle remote instance mapping
if isRemote { if isRemote {
nodeName := options.Nodes[0]
nodeConfig, exists := im.nodeConfigMap[nodeName] nodeConfig, exists := im.nodeConfigMap[nodeName]
if !exists { if !exists {
return fmt.Errorf("node %s not found for remote instance %s", nodeName, name) return fmt.Errorf("node %s not found for remote instance %s", nodeName, name)

View File

@@ -3,7 +3,6 @@ package manager
import ( import (
"fmt" "fmt"
"llamactl/pkg/backends" "llamactl/pkg/backends"
"llamactl/pkg/config"
"llamactl/pkg/instance" "llamactl/pkg/instance"
"llamactl/pkg/validation" "llamactl/pkg/validation"
"os" "os"
@@ -27,10 +26,12 @@ func (im *instanceManager) updateLocalInstanceFromRemote(localInst *instance.Ins
// Preserve the Nodes field from the local instance // Preserve the Nodes field from the local instance
localOptions := localInst.GetOptions() localOptions := localInst.GetOptions()
var preservedNodes []string var preservedNodes map[string]struct{}
if localOptions != nil && len(localOptions.Nodes) > 0 { if localOptions != nil && len(localOptions.Nodes) > 0 {
preservedNodes = make([]string, len(localOptions.Nodes)) preservedNodes = make(map[string]struct{}, len(localOptions.Nodes))
copy(preservedNodes, localOptions.Nodes) for node := range localOptions.Nodes {
preservedNodes[node] = struct{}{}
}
} }
// Create a copy of remote options and restore the Nodes field // Create a copy of remote options and restore the Nodes field
@@ -98,16 +99,17 @@ func (im *instanceManager) CreateInstance(name string, options *instance.Options
return nil, fmt.Errorf("instance with name %s already exists", name) return nil, fmt.Errorf("instance with name %s already exists", name)
} }
// Check if this is a remote instance // Check if this is a remote instance (local node not in the Nodes set)
// An instance is remote if Nodes is specified AND the first node is not the local node if _, isLocal := options.Nodes[im.localNodeName]; !isLocal && len(options.Nodes) > 0 {
isRemote := len(options.Nodes) > 0 && options.Nodes[0] != im.localNodeName // Get the first node from the set
var nodeConfig *config.NodeConfig var nodeName string
for node := range options.Nodes {
nodeName = node
break
}
if isRemote {
// Validate that the node exists // Validate that the node exists
nodeName := options.Nodes[0] // Use first node for now nodeConfig, exists := im.nodeConfigMap[nodeName]
var exists bool
nodeConfig, exists = im.nodeConfigMap[nodeName]
if !exists { if !exists {
return nil, fmt.Errorf("node %s not found", nodeName) return nil, fmt.Errorf("node %s not found", nodeName)
} }

View File

@@ -394,12 +394,21 @@ func (h *Handler) ProxyToInstance() http.HandlerFunc {
func (h *Handler) RemoteInstanceProxy(w http.ResponseWriter, r *http.Request, name string, inst *instance.Instance) { func (h *Handler) RemoteInstanceProxy(w http.ResponseWriter, r *http.Request, name string, inst *instance.Instance) {
// Get the node name from instance options // Get the node name from instance options
options := inst.GetOptions() options := inst.GetOptions()
if options == nil || len(options.Nodes) == 0 { if options == nil {
http.Error(w, "Instance has no node configured", http.StatusInternalServerError) http.Error(w, "Instance has no options configured", http.StatusInternalServerError)
return return
} }
nodeName := options.Nodes[0] // Get the first node from the set
var nodeName string
for node := range options.Nodes {
nodeName = node
break
}
if nodeName == "" {
http.Error(w, "Instance has no node configured", http.StatusInternalServerError)
return
}
// Check if we have a cached proxy for this node // Check if we have a cached proxy for this node
h.remoteProxiesMu.RLock() h.remoteProxiesMu.RLock()

View File

@@ -155,12 +155,21 @@ func (h *Handler) OpenAIProxy() http.HandlerFunc {
func (h *Handler) RemoteOpenAIProxy(w http.ResponseWriter, r *http.Request, modelName string, inst *instance.Instance) { func (h *Handler) RemoteOpenAIProxy(w http.ResponseWriter, r *http.Request, modelName string, inst *instance.Instance) {
// Get the node name from instance options // Get the node name from instance options
options := inst.GetOptions() options := inst.GetOptions()
if options == nil || len(options.Nodes) == 0 { if options == nil {
http.Error(w, "Instance has no node configured", http.StatusInternalServerError) http.Error(w, "Instance has no options configured", http.StatusInternalServerError)
return return
} }
nodeName := options.Nodes[0] // Get the first node from the set
var nodeName string
for node := range options.Nodes {
nodeName = node
break
}
if nodeName == "" {
http.Error(w, "Instance has no node configured", http.StatusInternalServerError)
return
}
// Check if we have a cached proxy for this node // Check if we have a cached proxy for this node
h.remoteProxiesMu.RLock() h.remoteProxiesMu.RLock()