Use one workload status type consistently (#1213)

Prior to this change, the Docker runtime returned Docker status values, the kubernetes code manufactured something which looked like Docker statuses, and the workload manager attempted to convert them to a single consistent enum type.

This code changes the runtime layer to return instances of our WorkloadStatus enum, and removes the translation from the workload manager.

Author: dmjb

docs/server/docs.go

@@ -252,7 +252,7 @@ func (c *Client) ListWorkloads(ctx context.Context) ([]runtime.ContainerInfo, er
 			Name:    name,
 			Image:   c.Image,
 			Status:  c.Status,
-			State:   c.State,
+			State:   dockerToDomainStatus(c.State),
 			Created: created,
 			Labels:  c.Labels,
 			Ports:   ports,
@@ -276,7 +276,7 @@ func (c *Client) StopWorkload(ctx context.Context, workloadID string) error {
 	}
 
 	// If the container is not running, return success
-	if info.State != container.StateRunning {
+	if info.State != runtime.WorkloadStatusRunning {
 		return nil
 	}
 
@@ -441,7 +441,7 @@ func (c *Client) GetWorkloadInfo(ctx context.Context, workloadID string) (runtim
 		Name:    strings.TrimPrefix(info.Name, "/"),
 		Image:   info.Config.Image,
 		Status:  info.State.Status,
-		State:   info.State.Status,
+		State:   dockerToDomainStatus(info.State.Status),
 		Created: created,
 		Labels:  info.Config.Labels,
 		Ports:   ports,
@@ -1487,3 +1487,19 @@ func (c *Client) deleteNetworks(ctx context.Context, containerName string) error
 	}
 	return nil
 }
+
+func dockerToDomainStatus(status string) runtime.WorkloadStatus {
+	// Reference: https://docs.docker.com/reference/cli/docker/container/ls/#status
+	switch status {
+	case "running":
+		return runtime.WorkloadStatusRunning
+	case "created", "restarting":
+		return runtime.WorkloadStatusStarting
+	case "paused", "exited", "dead":
+		return runtime.WorkloadStatusStopped
+	case "removing": // TODO: add handling new workload creation
+		return runtime.WorkloadStatusRemoving
+	}
+	// We should not reach here.
+	return runtime.WorkloadStatusUnknown
+}

docs/server/swagger.json

@@ -343,16 +343,18 @@ func (c *Client) GetWorkloadInfo(ctx context.Context, workloadID string) (runtim
 	}
 
 	// Determine status and state
-	var status, state string
+	var status string
+	var state runtime.WorkloadStatus
 	if statefulset.Status.ReadyReplicas > 0 {
 		status = "Running"
-		state = "running"
+		state = runtime.WorkloadStatusRunning
 	} else if statefulset.Status.Replicas > 0 {
 		status = "Pending"
-		state = "pending"
+		state = runtime.WorkloadStatusStarting
 	} else {
+		// NOTE: Not clear if this is correct since the stop operation is a no-op.
 		status = "Stopped"
-		state = "stopped"
+		state = runtime.WorkloadStatusStopped
 	}
 
 	// Get the image from the pod template
@@ -420,17 +422,17 @@ func (c *Client) ListWorkloads(ctx context.Context) ([]runtime.ContainerInfo, er
 
 		// Get container status
 		status := UnknownStatus
-		state := UnknownStatus
+		state := runtime.WorkloadStatusUnknown
 		if len(pod.Status.ContainerStatuses) > 0 {
 			containerStatus := pod.Status.ContainerStatuses[0]
 			if containerStatus.State.Running != nil {
-				state = "running"
+				state = runtime.WorkloadStatusRunning
 				status = "Running"
 			} else if containerStatus.State.Waiting != nil {
-				state = "waiting"
+				state = runtime.WorkloadStatusStarting
 				status = containerStatus.State.Waiting.Reason
 			} else if containerStatus.State.Terminated != nil {
-				state = "terminated"
+				state = runtime.WorkloadStatusRemoving
 				status = containerStatus.State.Terminated.Reason
 			}
 		}

docs/server/swagger.yaml

@@ -12,6 +12,30 @@ import (
 	"github.com/stacklok/toolhive/pkg/permissions"
 )
 
+// WorkloadStatus is an enum representing the possible statuses of a workload.
+type WorkloadStatus string
+
+const (
+	// WorkloadStatusRunning indicates that the workload is currently running.
+	WorkloadStatusRunning WorkloadStatus = "running"
+	// WorkloadStatusStopped indicates that the workload is stopped.
+	WorkloadStatusStopped WorkloadStatus = "stopped"
+	// WorkloadStatusError indicates that the workload has encountered an error
+	// during creation/stop/restart/delete.
+	WorkloadStatusError WorkloadStatus = "error"
+	// WorkloadStatusStarting indicates that the workload is being started.
+	WorkloadStatusStarting WorkloadStatus = "starting"
+	// WorkloadStatusStopping indicates that the workload is being stopped.
+	WorkloadStatusStopping WorkloadStatus = "stopping"
+	// WorkloadStatusUnhealthy indicates that the workload is running, but is
+	// in an inconsistent state which prevents normal operation.
+	WorkloadStatusUnhealthy WorkloadStatus = "unhealthy"
+	// WorkloadStatusRemoving indicates that the workload is being removed.
+	WorkloadStatusRemoving WorkloadStatus = "removing"
+	// WorkloadStatusUnknown indicates that the workload status is unknown.
+	WorkloadStatusUnknown WorkloadStatus = "unknown"
+)
+
 // ContainerInfo represents information about a container
 type ContainerInfo struct {
 	// ID is the container ID
@@ -21,9 +45,10 @@ type ContainerInfo struct {
 	// Image is the container image
 	Image string
 	// Status is the container status
+	// This is usually some human-readable context.
 	Status string
 	// State is the container state
-	State string
+	State WorkloadStatus
 	// Created is the container creation timestamp
 	Created time.Time
 	// Labels is the container labels

pkg/container/docker/client.go

@@ -12,6 +12,7 @@ import (
 	"github.com/adrg/xdg"
 	"github.com/gofrs/flock"
 
+	"github.com/stacklok/toolhive/pkg/container/runtime"
 	"github.com/stacklok/toolhive/pkg/logger"
 )
 
@@ -50,10 +51,10 @@ type fileStatusManager struct {
 
 // workloadStatusFile represents the JSON structure stored on disk
 type workloadStatusFile struct {
-	Status        WorkloadStatus `json:"status"`
-	StatusContext string         `json:"status_context,omitempty"`
-	CreatedAt     time.Time      `json:"created_at"`
-	UpdatedAt     time.Time      `json:"updated_at"`
+	Status        runtime.WorkloadStatus `json:"status"`
+	StatusContext string                 `json:"status_context,omitempty"`
+	CreatedAt     time.Time              `json:"created_at"`
+	UpdatedAt     time.Time              `json:"updated_at"`
 }
 
 // CreateWorkloadStatus creates the initial `starting` status for a new workload.
@@ -70,7 +71,7 @@ func (f *fileStatusManager) CreateWorkloadStatus(ctx context.Context, workloadNa
 		// Create initial status
 		now := time.Now()
 		statusFile := workloadStatusFile{
-			Status:        WorkloadStatusStarting,
+			Status:        runtime.WorkloadStatusStarting,
 			StatusContext: "",
 			CreatedAt:     now,
 			UpdatedAt:     now,
@@ -86,8 +87,8 @@ func (f *fileStatusManager) CreateWorkloadStatus(ctx context.Context, workloadNa
 }
 
 // GetWorkloadStatus retrieves the status of a workload by its name.
-func (f *fileStatusManager) GetWorkloadStatus(ctx context.Context, workloadName string) (*WorkloadStatus, string, error) {
-	var result *WorkloadStatus
+func (f *fileStatusManager) GetWorkloadStatus(ctx context.Context, workloadName string) (runtime.WorkloadStatus, string, error) {
+	result := runtime.WorkloadStatusUnknown
 	var statusContext string
 
 	err := f.withFileReadLock(ctx, workloadName, func(statusFilePath string) error {
@@ -103,7 +104,7 @@ func (f *fileStatusManager) GetWorkloadStatus(ctx context.Context, workloadName
 			return fmt.Errorf("failed to read status for workload %s: %w", workloadName, err)
 		}
 
-		result = &statusFile.Status
+		result = statusFile.Status
 		statusContext = statusFile.StatusContext
 		return nil
 	})
@@ -114,7 +115,7 @@ func (f *fileStatusManager) GetWorkloadStatus(ctx context.Context, workloadName
 // SetWorkloadStatus sets the status of a workload by its name.
 // This method will do nothing if the workload does not exist, following the interface contract.
 func (f *fileStatusManager) SetWorkloadStatus(
-	ctx context.Context, workloadName string, status WorkloadStatus, contextMsg string,
+	ctx context.Context, workloadName string, status runtime.WorkloadStatus, contextMsg string,
 ) {
 	err := f.withFileLock(ctx, workloadName, func(statusFilePath string) error {
 		// Check if file exists

pkg/container/kubernetes/client.go

@@ -11,6 +11,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/stacklok/toolhive/pkg/container/runtime"
 	"github.com/stacklok/toolhive/pkg/logger"
 )
 
@@ -42,7 +43,7 @@ func TestFileStatusManager_CreateWorkloadStatus(t *testing.T) {
 	err = json.Unmarshal(data, &statusFileData)
 	require.NoError(t, err)
 
-	assert.Equal(t, WorkloadStatusStarting, statusFileData.Status)
+	assert.Equal(t, runtime.WorkloadStatusStarting, statusFileData.Status)
 	assert.Empty(t, statusFileData.StatusContext)
 	assert.False(t, statusFileData.CreatedAt.IsZero())
 	assert.False(t, statusFileData.UpdatedAt.IsZero())
@@ -77,7 +78,7 @@ func TestFileStatusManager_GetWorkloadStatus(t *testing.T) {
 	// Get the status
 	status, statusContext, err := manager.GetWorkloadStatus(ctx, "test-workload")
 	require.NoError(t, err)
-	assert.Equal(t, WorkloadStatusStarting, *status)
+	assert.Equal(t, runtime.WorkloadStatusStarting, status)
 	assert.Empty(t, statusContext)
 }
 
@@ -104,12 +105,12 @@ func TestFileStatusManager_SetWorkloadStatus(t *testing.T) {
 	require.NoError(t, err)
 
 	// Update the status
-	manager.SetWorkloadStatus(ctx, "test-workload", WorkloadStatusRunning, "container started")
+	manager.SetWorkloadStatus(ctx, "test-workload", runtime.WorkloadStatusRunning, "container started")
 
 	// Verify the status was updated
 	status, statusContext, err := manager.GetWorkloadStatus(ctx, "test-workload")
 	require.NoError(t, err)
-	assert.Equal(t, WorkloadStatusRunning, *status)
+	assert.Equal(t, runtime.WorkloadStatusRunning, status)
 	assert.Equal(t, "container started", statusContext)
 
 	// Verify the file on disk
@@ -121,7 +122,7 @@ func TestFileStatusManager_SetWorkloadStatus(t *testing.T) {
 	err = json.Unmarshal(data, &statusFileData)
 	require.NoError(t, err)
 
-	assert.Equal(t, WorkloadStatusRunning, statusFileData.Status)
+	assert.Equal(t, runtime.WorkloadStatusRunning, statusFileData.Status)
 	assert.Equal(t, "container started", statusFileData.StatusContext)
 	// CreatedAt should be preserved, UpdatedAt should be newer
 	assert.False(t, statusFileData.CreatedAt.IsZero())
@@ -137,7 +138,7 @@ func TestFileStatusManager_SetWorkloadStatus_NotFound(t *testing.T) {
 	ctx := context.Background()
 
 	// Try to set status for non-existent workload - should not error but do nothing
-	manager.SetWorkloadStatus(ctx, "non-existent", WorkloadStatusRunning, "test")
+	manager.SetWorkloadStatus(ctx, "non-existent", runtime.WorkloadStatusRunning, "test")
 
 	// Verify no file was created
 	statusFile := filepath.Join(tempDir, "non-existent.json")
@@ -193,7 +194,7 @@ func TestFileStatusManager_ConcurrentAccess(t *testing.T) {
 			defer func() { done <- true }()
 			status, statusContext, err := manager.GetWorkloadStatus(ctx, "test-workload")
 			assert.NoError(t, err)
-			assert.Equal(t, WorkloadStatusStarting, *status)
+			assert.Equal(t, runtime.WorkloadStatusStarting, status)
 			assert.Empty(t, statusContext)
 		}()
 	}
@@ -223,31 +224,31 @@ func TestFileStatusManager_FullLifecycle(t *testing.T) {
 	// 2. Verify initial status
 	status, statusContext, err := manager.GetWorkloadStatus(ctx, workloadName)
 	require.NoError(t, err)
-	assert.Equal(t, WorkloadStatusStarting, *status)
+	assert.Equal(t, runtime.WorkloadStatusStarting, status)
 	assert.Empty(t, statusContext)
 
 	// 3. Update to running
-	manager.SetWorkloadStatus(ctx, workloadName, WorkloadStatusRunning, "started successfully")
+	manager.SetWorkloadStatus(ctx, workloadName, runtime.WorkloadStatusRunning, "started successfully")
 
 	status, statusContext, err = manager.GetWorkloadStatus(ctx, workloadName)
 	require.NoError(t, err)
-	assert.Equal(t, WorkloadStatusRunning, *status)
+	assert.Equal(t, runtime.WorkloadStatusRunning, status)
 	assert.Equal(t, "started successfully", statusContext)
 
 	// 4. Update to stopping
-	manager.SetWorkloadStatus(ctx, workloadName, WorkloadStatusStopping, "shutdown initiated")
+	manager.SetWorkloadStatus(ctx, workloadName, runtime.WorkloadStatusStopping, "shutdown initiated")
 
 	status, statusContext, err = manager.GetWorkloadStatus(ctx, workloadName)
 	require.NoError(t, err)
-	assert.Equal(t, WorkloadStatusStopping, *status)
+	assert.Equal(t, runtime.WorkloadStatusStopping, status)
 	assert.Equal(t, "shutdown initiated", statusContext)
 
 	// 5. Update to stopped
-	manager.SetWorkloadStatus(ctx, workloadName, WorkloadStatusStopped, "shutdown complete")
+	manager.SetWorkloadStatus(ctx, workloadName, runtime.WorkloadStatusStopped, "shutdown complete")
 
 	status, statusContext, err = manager.GetWorkloadStatus(ctx, workloadName)
 	require.NoError(t, err)
-	assert.Equal(t, WorkloadStatusStopped, *status)
+	assert.Equal(t, runtime.WorkloadStatusStopped, status)
 	assert.Equal(t, "shutdown complete", statusContext)
 
 	// 6. Delete workload