Migrate legacy workloads to stamp runtime ownership on healthy reconciliation

JAORMX · claude · JAORMX · commit 094fb1167deb · 2026-05-29T11:07:21.000Z
Legacy workloads created before runtime ownership tracking have an empty
RuntimeName field. Rather than leaving them permanently in the legacy
state, opportunistically stamp them with the active runtime's name when
reconciliation confirms the workload is healthy (container running,
proxy OK). This ensures the cross-runtime protection from the previous
commit covers existing workloads progressively.

Also adds a clarifying comment on the Remote guard in handleRuntimeMissing
per reviewer feedback, and improves diagnostics with a DEBUG log when
GetReader fails during migration.

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/pkg/workloads/statuses/file_status.go b/pkg/workloads/statuses/file_status.go
@@ -106,6 +106,61 @@ func (f *fileStatusManager) isOwnedByActiveRuntime(ctx context.Context, workload
 	return config.RuntimeName == f.runtime.Name()
 }
 
+// migrateRuntimeName stamps a legacy workload (RuntimeName == "") with the
+// active runtime's name. This is called only after the runtime has confirmed the
+// workload is healthy, so we know with certainty it belongs to the active
+// runtime. Save failures are non-fatal — the migration will be retried on the
+// next reconciliation cycle.
+func (f *fileStatusManager) migrateRuntimeName(ctx context.Context, workloadName string) {
+	reader, err := f.runConfigStore.GetReader(ctx, workloadName)
+	if err != nil {
+		slog.Debug("skipping runtime name migration: cannot read config", "workload", workloadName, "error", err)
+		return
+	}
+
+	var raw map[string]json.RawMessage
+	decodeErr := json.NewDecoder(reader).Decode(&raw)
+	// Always close the reader before acting on the decode result.
+	if closeErr := reader.Close(); closeErr != nil {
+		slog.Warn("failed to close reader for runtime name migration", "workload", workloadName, "error", closeErr)
+	}
+	if decodeErr != nil {
+		return
+	}
+
+	// Check if runtime_name is already set
+	if rn, ok := raw["runtime_name"]; ok {
+		var name string
+		if err := json.Unmarshal(rn, &name); err == nil && name != "" {
+			return // Already migrated
+		}
+	}
+
+	// Stamp with the active runtime name
+	runtimeBytes, err := json.Marshal(f.runtime.Name())
+	if err != nil {
+		return
+	}
+	raw["runtime_name"] = runtimeBytes
+
+	writer, err := f.runConfigStore.GetWriter(ctx, workloadName)
+	if err != nil {
+		slog.Warn("failed to open writer for runtime name migration", "workload", workloadName, "error", err)
+		return
+	}
+	defer func() {
+		if err := writer.Close(); err != nil {
+			slog.Warn("failed to close writer for runtime name migration", "workload", workloadName, "error", err)
+		}
+	}()
+
+	encoder := json.NewEncoder(writer)
+	encoder.SetIndent("", "  ")
+	if err := encoder.Encode(raw); err != nil {
+		slog.Warn("failed to write migrated RunConfig", "workload", workloadName, "error", err)
+	}
+}
+
 // isRemoteWorkload checks if a workload is remote by attempting to load its run configuration
 // and checking if it has a RemoteURL field set.
 // TODO: This is a temporary solution to check if a workload is remote
@@ -964,7 +1019,12 @@ func (f *fileStatusManager) validateRunningWorkload(
 		return unhealthyWorkload, nil
 	}
 
-	// Runtime and proxy confirm workload is healthy - merge runtime data with file status
+	// Runtime and proxy confirm workload is healthy — opportunistically migrate
+	// legacy workloads that predate the runtime_name field so they are stamped
+	// with the owning runtime for future reconciliation cycles.
+	f.migrateRuntimeName(ctx, workloadName)
+
+	// Merge runtime data with file status
 	return f.mergeHealthyWorkloadData(containerInfo, result)
 }
 
@@ -995,10 +1055,10 @@ func (f *fileStatusManager) handleRuntimeMismatch(
 func (f *fileStatusManager) handleRuntimeMissing(
 	ctx context.Context, workloadName string, fileWorkload core.Workload,
 ) (core.Workload, error) {
-	// Check if this is a remote workload using the Remote field
+	// Remote workloads don't exist in the container runtime, so it's normal
+	// for them to be missing. This also means they bypass the runtime-ownership
+	// check below — remote workloads have no owning runtime.
 	if fileWorkload.Remote {
-		// Remote workloads don't exist in the container runtime, so it's normal for them to be missing
-		// Don't mark them as unhealthy
 		return fileWorkload, nil
 	}
 
diff --git a/pkg/workloads/statuses/file_status_test.go b/pkg/workloads/statuses/file_status_test.go
@@ -4,6 +4,7 @@
 package statuses
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -233,6 +234,14 @@ func TestFileStatusManager_GetWorkload_FileAndRuntimeCombination(t *testing.T) {
 		return io.NopCloser(strings.NewReader(`{"name": "running-workload", "transport": "sse"}`)), nil
 	}).AnyTimes()
 
+	// Mock runtime name for isOwnedByActiveRuntime and migrateRuntimeName
+	mockRuntime.EXPECT().Name().Return("docker").AnyTimes()
+
+	// Mock GetWriter for migrateRuntimeName (legacy workload with no runtime_name gets migrated)
+	mockRunConfigStore.EXPECT().GetWriter(gomock.Any(), "running-workload").DoAndReturn(func(context.Context, string) (io.WriteCloser, error) {
+		return nopWriteCloser{&bytes.Buffer{}}, nil
+	}).AnyTimes()
+
 	// Create a workload status file and set it to running
 	err := manager.SetWorkloadStatus(ctx, "running-workload", rt.WorkloadStatusStarting, "")
 	require.NoError(t, err)
@@ -915,6 +924,14 @@ func TestFileStatusManager_GetWorkload_HealthyRunningWorkload(t *testing.T) {
 		return io.NopCloser(strings.NewReader(`{"name": "healthy-workload", "transport": "sse"}`)), nil
 	}).AnyTimes()
 
+	// Mock runtime name for isOwnedByActiveRuntime and migrateRuntimeName
+	mockRuntime.EXPECT().Name().Return("docker").AnyTimes()
+
+	// Mock GetWriter for migrateRuntimeName (legacy workload with no runtime_name gets migrated)
+	mockRunConfigStore.EXPECT().GetWriter(gomock.Any(), "healthy-workload").DoAndReturn(func(context.Context, string) (io.WriteCloser, error) {
+		return nopWriteCloser{&bytes.Buffer{}}, nil
+	}).AnyTimes()
+
 	// Set the workload status to running in the file
 	err := manager.SetWorkloadStatus(ctx, "healthy-workload", rt.WorkloadStatusRunning, "container started")
 	require.NoError(t, err)
@@ -964,6 +981,9 @@ func TestFileStatusManager_GetWorkload_ProxyNotRunning(t *testing.T) {
 		return io.NopCloser(strings.NewReader(`{"name": "proxy-down-workload", "transport": "sse"}`)), nil
 	}).AnyTimes()
 
+	// Mock runtime name for isOwnedByActiveRuntime
+	mockRuntime.EXPECT().Name().Return("docker").AnyTimes()
+
 	// First, create a status file manually to ensure file is found
 	statusFile := workloadStatusFile{
 		Status:        rt.WorkloadStatusRunning,
@@ -1041,6 +1061,14 @@ func TestFileStatusManager_GetWorkload_HealthyWithProxy(t *testing.T) {
 		return io.NopCloser(strings.NewReader(`{"name": "healthy-with-proxy", "transport": "sse"}`)), nil
 	}).AnyTimes()
 
+	// Mock runtime name for isOwnedByActiveRuntime and migrateRuntimeName
+	mockRuntime.EXPECT().Name().Return("docker").AnyTimes()
+
+	// Mock GetWriter for migrateRuntimeName (legacy workload with no runtime_name gets migrated)
+	mockRunConfigStore.EXPECT().GetWriter(gomock.Any(), "healthy-with-proxy").DoAndReturn(func(context.Context, string) (io.WriteCloser, error) {
+		return nopWriteCloser{&bytes.Buffer{}}, nil
+	}).AnyTimes()
+
 	// Set the workload status to running in the file
 	err := manager.SetWorkloadStatus(ctx, "healthy-with-proxy", rt.WorkloadStatusRunning, "container started")
 	require.NoError(t, err)
@@ -2353,3 +2381,135 @@ func TestFileStatusManager_CrossRuntimeProtection(t *testing.T) {
 		})
 	}
 }
+
+// nopWriteCloser wraps a bytes.Buffer with a no-op Close.
+type nopWriteCloser struct {
+	*bytes.Buffer
+}
+
+func (nopWriteCloser) Close() error { return nil }
+
+func TestMigrateRuntimeName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name            string
+		runtimeName     string
+		runConfigJSON   string
+		expectWrite     bool
+		expectedRuntime string
+		getReaderErr    bool
+		getWriterErr    bool
+	}{
+		{
+			name:            "migrates empty runtime_name",
+			runtimeName:     "docker",
+			runConfigJSON:   `{"name": "my-workload", "image": "test:latest"}`,
+			expectWrite:     true,
+			expectedRuntime: "docker",
+		},
+		{
+			name:            "migrates explicit empty runtime_name",
+			runtimeName:     "docker",
+			runConfigJSON:   `{"name": "my-workload", "runtime_name": "", "image": "test:latest"}`,
+			expectWrite:     true,
+			expectedRuntime: "docker",
+		},
+		{
+			name:          "no-op when runtime_name already set",
+			runtimeName:   "docker",
+			runConfigJSON: `{"name": "my-workload", "runtime_name": "docker", "image": "test:latest"}`,
+			expectWrite:   false,
+		},
+		{
+			name:          "no-op when runtime_name set to different runtime",
+			runtimeName:   "docker",
+			runConfigJSON: `{"name": "my-workload", "runtime_name": "go-microvm", "image": "test:latest"}`,
+			expectWrite:   false,
+		},
+		{
+			name:         "graceful on GetReader error",
+			runtimeName:  "docker",
+			getReaderErr: true,
+			expectWrite:  false,
+		},
+		{
+			name:          "graceful on GetWriter error",
+			runtimeName:   "docker",
+			runConfigJSON: `{"name": "my-workload"}`,
+			getWriterErr:  true,
+			expectWrite:   false,
+		},
+		{
+			name:          "graceful on malformed JSON",
+			runtimeName:   "docker",
+			runConfigJSON: `{bad json`,
+			expectWrite:   false,
+		},
+		{
+			name:            "migrates non-string runtime_name",
+			runtimeName:     "docker",
+			runConfigJSON:   `{"name": "my-workload", "runtime_name": 42}`,
+			expectWrite:     true,
+			expectedRuntime: "docker",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			manager, mockRuntime, mockRunConfigStore := newTestFileStatusManager(t, ctrl)
+			ctx := t.Context()
+
+			workloadName := "test-workload"
+			mockRuntime.EXPECT().Name().Return(tt.runtimeName).AnyTimes()
+
+			if tt.getReaderErr {
+				mockRunConfigStore.EXPECT().GetReader(gomock.Any(), workloadName).
+					Return(nil, errors.New("store error"))
+			} else {
+				mockRunConfigStore.EXPECT().GetReader(gomock.Any(), workloadName).DoAndReturn(
+					func(context.Context, string) (io.ReadCloser, error) {
+						return io.NopCloser(strings.NewReader(tt.runConfigJSON)), nil
+					},
+				)
+			}
+
+			var writeBuf nopWriteCloser
+			if tt.expectWrite {
+				if tt.getWriterErr {
+					mockRunConfigStore.EXPECT().GetWriter(gomock.Any(), workloadName).
+						Return(nil, errors.New("write error"))
+				} else {
+					writeBuf = nopWriteCloser{&bytes.Buffer{}}
+					mockRunConfigStore.EXPECT().GetWriter(gomock.Any(), workloadName).
+						Return(writeBuf, nil)
+				}
+			} else if tt.getWriterErr {
+				mockRunConfigStore.EXPECT().GetWriter(gomock.Any(), workloadName).
+					Return(nil, errors.New("write error"))
+			}
+
+			manager.migrateRuntimeName(ctx, workloadName)
+
+			if tt.expectWrite && !tt.getWriterErr {
+				var written map[string]json.RawMessage
+				require.NoError(t, json.Unmarshal(writeBuf.Bytes(), &written))
+
+				var runtimeName string
+				require.NoError(t, json.Unmarshal(written["runtime_name"], &runtimeName))
+				assert.Equal(t, tt.expectedRuntime, runtimeName,
+					"migrated runtime_name should match active runtime")
+
+				// Verify other fields are preserved
+				var name string
+				require.NoError(t, json.Unmarshal(written["name"], &name))
+				assert.Equal(t, "my-workload", name, "other fields should be preserved")
+			}
+		})
+	}
+}
