Skip to content

Commit accf167

Browse files
eleftheriaseleftherias
authored
Remove groups dependency on runner package (#1327)
1 parent ddbfbec commit accf167

26 files changed

+726
-444
lines changed

‎cmd/thv/app/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func registerClientsWithGroups(
293293
return fmt.Errorf("failed to register clients with groups: %w", err)
294294
}
295295

296-
filteredWorkloads, err := workloads.FilterByGroups(ctx, runningWorkloads, groupNames)
296+
filteredWorkloads, err := workloads.FilterByGroups(runningWorkloads, groupNames)
297297
if err != nil {
298298
return fmt.Errorf("failed to filter workloads by groups: %w", err)
299299
}

‎cmd/thv/app/group.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,14 @@ func groupRmCmdFunc(cmd *cobra.Command, args []string) error {
135135
return fmt.Errorf("group '%s' does not exist", groupName)
136136
}
137137

138+
// Create workloads manager
139+
workloadsManager, err := workloads.NewManager(ctx)
140+
if err != nil {
141+
return fmt.Errorf("failed to create workloads manager: %w", err)
142+
}
143+
138144
// Get all workloads in the group
139-
groupWorkloads, err := manager.ListWorkloadsInGroup(ctx, groupName)
145+
groupWorkloads, err := workloadsManager.ListWorkloadsInGroup(ctx, groupName)
140146
if err != nil {
141147
return fmt.Errorf("failed to list workloads in group: %w", err)
142148
}

‎cmd/thv/app/list.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func listCmdFunc(cmd *cobra.Command, _ []string) error {
5353

5454
// Apply group filtering if specified
5555
if listGroupFilter != "" {
56-
workloadList, err = workloads.FilterByGroup(ctx, workloadList, listGroupFilter)
56+
workloadList, err = workloads.FilterByGroup(workloadList, listGroupFilter)
5757
if err != nil {
5858
return fmt.Errorf("failed to filter workloads by group: %v", err)
5959
}

‎cmd/thv/app/restart.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func restartWorkloadsByGroup(ctx context.Context, workloadManager workloads.Mana
119119
}
120120

121121
// Get all workload names in the group
122-
workloadNames, err := groupManager.ListWorkloadsInGroup(ctx, groupName)
122+
workloadNames, err := workloadManager.ListWorkloadsInGroup(ctx, groupName)
123123
if err != nil {
124124
return fmt.Errorf("failed to list workloads in group '%s': %v", groupName, err)
125125
}

‎cmd/thv/app/rm.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,14 @@ func deleteAllWorkloadsInGroup(ctx context.Context, groupName string) error {
8989
return fmt.Errorf("group '%s' does not exist", groupName)
9090
}
9191

92+
// Create workload manager
93+
workloadManager, err := workloads.NewManager(ctx)
94+
if err != nil {
95+
return fmt.Errorf("failed to create workload manager: %v", err)
96+
}
97+
9298
// Get all workloads in the group
93-
groupWorkloads, err := groupManager.ListWorkloadsInGroup(ctx, groupName)
99+
groupWorkloads, err := workloadManager.ListWorkloadsInGroup(ctx, groupName)
94100
if err != nil {
95101
return fmt.Errorf("failed to list workloads in group: %v", err)
96102
}
@@ -100,12 +106,6 @@ func deleteAllWorkloadsInGroup(ctx context.Context, groupName string) error {
100106
return nil
101107
}
102108

103-
// Create workload manager
104-
workloadManager, err := workloads.NewManager(ctx)
105-
if err != nil {
106-
return fmt.Errorf("failed to create workload manager: %v", err)
107-
}
108-
109109
// Delete all workloads in the group
110110
group, err := workloadManager.DeleteWorkloads(ctx, groupWorkloads)
111111
if err != nil {

‎cmd/thv/app/run.go

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package app
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"net"
78
"os"
@@ -12,6 +13,7 @@ import (
1213
"github.com/spf13/cobra"
1314

1415
"github.com/stacklok/toolhive/pkg/container"
16+
"github.com/stacklok/toolhive/pkg/container/runtime"
1517
"github.com/stacklok/toolhive/pkg/groups"
1618
"github.com/stacklok/toolhive/pkg/logger"
1719
"github.com/stacklok/toolhive/pkg/process"
@@ -137,23 +139,23 @@ func runCmdFunc(cmd *cobra.Command, args []string) error {
137139
// Get debug mode flag
138140
debugMode, _ := cmd.Flags().GetBool("debug")
139141

140-
err := validateGroup(ctx, serverOrImage)
142+
// Create container runtime
143+
rt, err := container.NewFactory().Create(ctx)
141144
if err != nil {
142-
return err
145+
return fmt.Errorf("failed to create container runtime: %v", err)
143146
}
147+
workloadManager := workloads.NewManagerFromRuntime(rt)
144148

145-
// Build the run configuration
146-
runnerConfig, err := BuildRunnerConfig(ctx, &runFlags, serverOrImage, cmdArgs, debugMode, cmd)
149+
err = validateGroup(ctx, workloadManager, serverOrImage)
147150
if err != nil {
148151
return err
149152
}
150153

151-
// Create container runtime
152-
rt, err := container.NewFactory().Create(ctx)
154+
// Build the run configuration
155+
runnerConfig, err := BuildRunnerConfig(ctx, &runFlags, serverOrImage, cmdArgs, debugMode, cmd)
153156
if err != nil {
154-
return fmt.Errorf("failed to create container runtime: %v", err)
157+
return err
155158
}
156-
workloadManager := workloads.NewManagerFromRuntime(rt)
157159

158160
// Always save the run config to disk before starting (both foreground and detached modes)
159161
// NOTE: Save before secrets processing to avoid storing secrets in the state store
@@ -193,7 +195,7 @@ func runForeground(ctx context.Context, workloadManager workloads.Manager, runne
193195
}
194196
}
195197

196-
func validateGroup(ctx context.Context, serverOrImage string) error {
198+
func validateGroup(ctx context.Context, workloadsManager workloads.Manager, serverOrImage string) error {
197199
workloadName := runFlags.Name
198200
if workloadName == "" {
199201
workloadName = serverOrImage
@@ -206,12 +208,14 @@ func validateGroup(ctx context.Context, serverOrImage string) error {
206208
}
207209

208210
// Check if the workload is already in a group
209-
group, err := groupManager.GetWorkloadGroup(ctx, workloadName)
211+
workload, err := workloadsManager.GetWorkload(ctx, workloadName)
210212
if err != nil {
211-
return fmt.Errorf("failed to get workload group: %v", err)
212-
}
213-
if group != nil && group.Name != runFlags.Group {
214-
return fmt.Errorf("workload '%s' is already in group '%s'", workloadName, group.Name)
213+
// If the workload does not exist, we can proceed to create it
214+
if !errors.Is(err, runtime.ErrWorkloadNotFound) {
215+
return fmt.Errorf("failed to get workload: %v", err)
216+
}
217+
} else if workload.Group != "" && workload.Group != runFlags.Group {
218+
return fmt.Errorf("workload '%s' is already in group '%s'", workloadName, workload.Group)
215219
}
216220

217221
if runFlags.Group != "" {

‎cmd/thv/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func main() {
2121
// Check and perform default group migration if needed
2222
// Migrates existing workloads to the default group, only executes once
2323
// TODO: Re-enable when group functionality is complete
24-
// groups.CheckAndPerformDefaultGroupMigration()
24+
//migration.CheckAndPerformDefaultGroupMigration()
2525

2626
// Skip update check for completion command or if we are running in kubernetes
2727
if err := app.NewRootCmd(!app.IsCompletionCommand(os.Args) && !runtime.IsKubernetesRuntime()).Execute(); err != nil {

‎pkg/api/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ func Serve(
187187
"/api/v1beta/discovery": v1.DiscoveryRouter(),
188188
"/api/v1beta/clients": v1.ClientRouter(clientManager, workloadManager, groupManager),
189189
"/api/v1beta/secrets": v1.SecretsRouter(),
190-
"/api/v1beta/groups": v1.GroupsRouter(groupManager),
190+
"/api/v1beta/groups": v1.GroupsRouter(groupManager, workloadManager),
191191
}
192192

193193
// Only mount docs router if enabled

‎pkg/api/v1/clients.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func (c *ClientRoutes) performClientRegistration(ctx context.Context, clients []
260260
if len(groupNames) > 0 {
261261
logger.Infof("Filtering workloads to groups: %v", groupNames)
262262

263-
filteredWorkloads, err := workloads.FilterByGroups(ctx, runningWorkloads, groupNames)
263+
filteredWorkloads, err := workloads.FilterByGroups(runningWorkloads, groupNames)
264264
if err != nil {
265265
return fmt.Errorf("failed to filter workloads by groups: %w", err)
266266
}

‎pkg/api/v1/groups.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ import (
1616

1717
// GroupsRoutes defines the routes for group management.
1818
type GroupsRoutes struct {
19-
groupManager groups.Manager
19+
groupManager groups.Manager
20+
workloadManager workloads.Manager
2021
}
2122

2223
// GroupsRouter creates a new GroupsRoutes instance.
23-
func GroupsRouter(groupManager groups.Manager) http.Handler {
24+
func GroupsRouter(groupManager groups.Manager, workloadManager workloads.Manager) http.Handler {
2425
routes := GroupsRoutes{
25-
groupManager: groupManager,
26+
groupManager: groupManager,
27+
workloadManager: workloadManager,
2628
}
2729

2830
r := chi.NewRouter()
@@ -201,7 +203,7 @@ func (s *GroupsRoutes) deleteGroup(w http.ResponseWriter, r *http.Request) {
201203
withWorkloads := r.URL.Query().Get("with-workloads") == "true"
202204

203205
// Get all workloads in the group
204-
groupWorkloads, err := s.groupManager.ListWorkloadsInGroup(ctx, name)
206+
groupWorkloads, err := s.workloadManager.ListWorkloadsInGroup(ctx, name)
205207
if err != nil {
206208
logger.Errorf("Failed to list workloads in group %s: %v", name, err)
207209
http.Error(w, "Failed to list workloads in group", http.StatusInternalServerError)
@@ -210,16 +212,9 @@ func (s *GroupsRoutes) deleteGroup(w http.ResponseWriter, r *http.Request) {
210212

211213
// Handle workloads if any exist
212214
if len(groupWorkloads) > 0 {
213-
workloadManager, err := workloads.NewManager(ctx)
214-
if err != nil {
215-
logger.Errorf("Failed to create workload manager: %v", err)
216-
http.Error(w, "Failed to create workload manager", http.StatusInternalServerError)
217-
return
218-
}
219-
220215
if withWorkloads {
221216
// Delete all workloads in the group
222-
group, err := workloadManager.DeleteWorkloads(ctx, groupWorkloads)
217+
group, err := s.workloadManager.DeleteWorkloads(ctx, groupWorkloads)
223218
if err != nil {
224219
logger.Errorf("Failed to delete workloads in group %s: %v", name, err)
225220
http.Error(w, "Failed to delete workloads in group", http.StatusInternalServerError)
@@ -236,7 +231,7 @@ func (s *GroupsRoutes) deleteGroup(w http.ResponseWriter, r *http.Request) {
236231
logger.Infof("Deleted %d workload(s) from group '%s'", len(groupWorkloads), name)
237232
} else {
238233
// Move workloads to default group
239-
if err := workloadManager.MoveToDefaultGroup(ctx, groupWorkloads, name); err != nil {
234+
if err := s.workloadManager.MoveToDefaultGroup(ctx, groupWorkloads, name); err != nil {
240235
logger.Errorf("Failed to move workloads to default group: %v", err)
241236
http.Error(w, "Failed to move workloads to default group", http.StatusInternalServerError)
242237
return

0 commit comments

Comments
 (0)