Skip to content

Commit 5e9ef2d

Browse files
authored
Stop returning ID outside of the runtime layer, fix k8s ID bug (#1207)
Currently a number of the runtime interface methods expect a container ID to be passed. The format and meaning of this ID is specific to the runtime. Callers of the runtime interface end up having to call the runtime to get an ID and pass it back. This changes all these methods to require a workload name instead of a container ID. The runtime methods will do a lookup if needed to find the container ID. This hides the runtime-specific IDs from the code outside of the runtime, and maintains the abstraction that workloads are uniquely identified by name. While implementing this, I noticed a bug where the k8s runtime code gets mixed up between pod IDs and stateful set names (which are the same as the name of the workload). I changed the code to use the workload name to look up stateful sets, and changed the code which operated on pods to search by name and label instead of pod ID.
1 parent 81a5654 commit 5e9ef2d

File tree

17 files changed

+243
-361
lines changed

17 files changed

+243
-361
lines changed

cmd/thv/app/inspector.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func inspectorCmdFunc(cmd *cobra.Command, args []string) error {
122122
labelsMap := map[string]string{}
123123
labels.AddStandardLabels(labelsMap, "inspector", "inspector", string(types.TransportTypeInspector), inspectorUIPort)
124124
labelsMap["toolhive-auxiliary"] = "true"
125-
_, _, err = rt.DeployWorkload(
125+
_, err = rt.DeployWorkload(
126126
ctx,
127127
processedImage,
128128
"inspector",

cmd/thv/app/logs.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/spf13/cobra"
1313
"github.com/spf13/viper"
1414

15+
rt "github.com/stacklok/toolhive/pkg/container/runtime"
1516
"github.com/stacklok/toolhive/pkg/logger"
1617
"github.com/stacklok/toolhive/pkg/workloads"
1718
)
@@ -71,7 +72,7 @@ func logsCmdFunc(cmd *cobra.Command, args []string) error {
7172

7273
logs, err := manager.GetLogs(ctx, workloadName, follow)
7374
if err != nil {
74-
if errors.Is(err, workloads.ErrWorkloadNotFound) {
75+
if errors.Is(err, rt.ErrWorkloadNotFound) {
7576
logger.Infof("workload %s not found", workloadName)
7677
return nil
7778
}

cmd/thv/app/stop.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/spf13/cobra"
88
"golang.org/x/sync/errgroup"
99

10+
rt "github.com/stacklok/toolhive/pkg/container/runtime"
1011
"github.com/stacklok/toolhive/pkg/workloads"
1112
)
1213

@@ -97,7 +98,7 @@ func stopCmdFunc(cmd *cobra.Command, args []string) error {
9798
group, err = workloadManager.StopWorkloads(ctx, []string{workloadName})
9899
if err != nil {
99100
// If the workload is not found or not running, treat as a non-fatal error.
100-
if errors.Is(err, workloads.ErrWorkloadNotFound) ||
101+
if errors.Is(err, rt.ErrWorkloadNotFound) ||
101102
errors.Is(err, workloads.ErrWorkloadNotRunning) ||
102103
errors.Is(err, workloads.ErrInvalidWorkloadName) {
103104
fmt.Printf("workload %s is not running\n", workloadName)

pkg/api/v1/workloads.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func (s *WorkloadRoutes) getWorkload(w http.ResponseWriter, r *http.Request) {
103103

104104
workload, err := s.workloadManager.GetWorkload(ctx, name)
105105
if err != nil {
106-
if errors.Is(err, workloads.ErrWorkloadNotFound) {
106+
if errors.Is(err, runtime.ErrWorkloadNotFound) {
107107
http.Error(w, "Workload not found", http.StatusNotFound)
108108
return
109109
} else if errors.Is(err, workloads.ErrInvalidWorkloadName) {
@@ -429,7 +429,7 @@ func (s *WorkloadRoutes) getLogsForWorkload(w http.ResponseWriter, r *http.Reque
429429

430430
logs, err := s.workloadManager.GetLogs(ctx, name, false)
431431
if err != nil {
432-
if errors.Is(err, workloads.ErrWorkloadNotFound) {
432+
if errors.Is(err, runtime.ErrWorkloadNotFound) {
433433
http.Error(w, "Workload not found", http.StatusNotFound)
434434
return
435435
}

0 commit comments

Comments
 (0)