Skip to content

Commit b6c69ab

Browse files
JAORMXclaude
andcommitted
Track runtime ownership to prevent cross-runtime status corruption
When multiple runtimes coexist (e.g., podman and go-microvm), running CLI commands with one runtime active permanently corrupts the status files of workloads managed by the other runtime, marking them as "unhealthy." This happens because the reconciliation logic assumes all workloads belong to the single active runtime. Add a RuntimeName field to RunConfig that records which runtime created a workload, and a Name() method to the Runtime interface so the active runtime can identify itself. During reconciliation, skip workloads whose owning runtime differs from the active one. Legacy workloads without a RuntimeName are conservatively treated as owned by the active runtime. Fixes #4432 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 6f63ac0 commit b6c69ab

11 files changed

Lines changed: 221 additions & 1 deletion

File tree

cmd/thv/app/run.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,10 @@ func runSingleServer(ctx context.Context, runFlags *RunFlags, serverOrImage stri
243243
return err
244244
}
245245

246+
// Record which runtime owns this workload so that reconciliation logic
247+
// does not corrupt its status file when a different runtime is active.
248+
runnerConfig.RuntimeName = rt.Name()
249+
246250
// Enforce policy in the main process before saving state or spawning a
247251
// detached worker, so violations surface synchronously with a non-zero
248252
// exit code rather than silently failing in the background log.
@@ -469,6 +473,7 @@ func runFromConfigFile(ctx context.Context) error {
469473

470474
// Set the runtime in the config
471475
runConfig.Deployer = rt
476+
runConfig.RuntimeName = rt.Name()
472477

473478
// Create workload manager
474479
workloadManager, err := workloads.NewManagerFromRuntime(rt)

pkg/api/v1/workload_service.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ func (s *WorkloadService) CreateWorkloadFromRequest(ctx context.Context, req *cr
100100
return nil, err
101101
}
102102

103+
// Record which runtime owns this workload for cross-runtime reconciliation.
104+
runConfig.RuntimeName = s.containerRuntime.Name()
105+
103106
// Enforce policy before saving state or starting the workload, so
104107
// violations are returned as API errors rather than creating the server
105108
// in a broken state.

pkg/api/v1/workloads_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ func TestCreateWorkload(t *testing.T) {
261261
mockRuntime := runtimemocks.NewMockRuntime(ctrl)
262262
mockGroupManager := groupsmocks.NewMockManager(ctrl)
263263

264+
mockRuntime.EXPECT().Name().Return("docker").AnyTimes()
264265
tt.setupMock(t, mockWorkloadManager, mockRuntime, mockGroupManager)
265266
expectedServerOrImage := tt.expectedServerOrImage
266267
if expectedServerOrImage == "" {
@@ -281,6 +282,7 @@ func TestCreateWorkload(t *testing.T) {
281282
workloadService: &WorkloadService{
282283
groupManager: mockGroupManager,
283284
workloadManager: mockWorkloadManager,
285+
containerRuntime: mockRuntime,
284286
imageRetriever: mockRetriever,
285287
imagePuller: func(_ context.Context, _ string) error { return nil },
286288
configProvider: config.NewDefaultProvider(),
@@ -502,6 +504,7 @@ func TestUpdateWorkload(t *testing.T) {
502504
workloadService: &WorkloadService{
503505
groupManager: mockGroupManager,
504506
workloadManager: mockWorkloadManager,
507+
containerRuntime: mockRuntime,
505508
imageRetriever: mockRetriever,
506509
imagePuller: func(_ context.Context, _ string) error { return nil },
507510
configProvider: config.NewDefaultProvider(),

pkg/container/docker/client.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,11 @@ func (c *Client) AttachToWorkload(ctx context.Context, workloadName string) (io.
651651
return resp.Conn, stdoutReader, nil
652652
}
653653

654+
// Name returns the registered name of this runtime.
655+
func (*Client) Name() string {
656+
return RuntimeName
657+
}
658+
654659
// IsRunning checks the health of the container runtime.
655660
// This is used to verify that the runtime is operational and can manage workloads.
656661
func (c *Client) IsRunning(ctx context.Context) error {

pkg/container/kubernetes/client.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,11 @@ func (*Client) StopWorkload(_ context.Context, _ string) error {
787787
return nil
788788
}
789789

790+
// Name returns the registered name of this runtime.
791+
func (*Client) Name() string {
792+
return RuntimeName
793+
}
794+
790795
// IsRunning checks the health of the container runtime.
791796
// This is used to verify that the runtime is operational and can manage workloads.
792797
func (c *Client) IsRunning(ctx context.Context) error {

pkg/container/runtime/mocks/mock_runtime.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/container/runtime/types.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,11 @@ type Runtime interface {
180180
// IsRunning checks the health of the container runtime.
181181
// This is used to verify that the runtime is operational and can manage workloads.
182182
IsRunning(ctx context.Context) error
183+
184+
// Name returns the registered name of this runtime (e.g., "docker", "kubernetes").
185+
// Used to track which runtime owns a workload so that reconciliation logic
186+
// does not corrupt status files of workloads managed by a different runtime.
187+
Name() string
183188
}
184189

185190
// Monitor defines the interface for container monitoring

pkg/mcp/server/run_server.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,14 @@ func buildServerConfig(
169169

170170
// Build the configuration
171171
envVarValidator := &runner.DetachedEnvVarValidator{}
172-
return runner.NewRunConfigBuilder(ctx, imageMetadata, envVars, envVarValidator, opts...)
172+
cfg, err := runner.NewRunConfigBuilder(ctx, imageMetadata, envVars, envVarValidator, opts...)
173+
if err != nil {
174+
return nil, err
175+
}
176+
177+
// Record which runtime owns this workload for cross-runtime reconciliation.
178+
cfg.RuntimeName = rt.Name()
179+
return cfg, nil
173180
}
174181

175182
// configureTransport sets up transport configuration from metadata

pkg/runner/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ type RunConfig struct {
5555
// means unversioned (backward-compat with older operators, or non-operator callers).
5656
MCPServerGeneration int64 `json:"mcpserver_generation,omitempty" yaml:"mcpserver_generation,omitempty"`
5757

58+
// RuntimeName is the registered name of the container runtime that owns this
59+
// workload (e.g., "docker", "kubernetes"). Used during reconciliation to avoid
60+
// corrupting status files of workloads managed by a different runtime.
61+
RuntimeName string `json:"runtime_name,omitempty" yaml:"runtime_name,omitempty"`
62+
5863
// Image is the Docker image to run
5964
Image string `json:"image" yaml:"image"`
6065

pkg/workloads/statuses/file_status.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,38 @@ type fileStatusManager struct {
7474
runConfigStore state.Store
7575
}
7676

77+
// isOwnedByActiveRuntime checks whether the named workload was created by the
78+
// currently active runtime. It loads the runtime_name field from the persisted
79+
// RunConfig and compares it against f.runtime.Name(). Legacy workloads that
80+
// predate the runtime_name field (empty string) are conservatively treated as
81+
// owned by the active runtime so that existing validation behaviour is preserved.
82+
func (f *fileStatusManager) isOwnedByActiveRuntime(ctx context.Context, workloadName string) bool {
83+
reader, err := f.runConfigStore.GetReader(ctx, workloadName)
84+
if err != nil {
85+
// RunConfig missing or unreadable -- assume ours so we don't silently
86+
// skip validation for workloads we should be checking.
87+
return true
88+
}
89+
defer func() {
90+
if err := reader.Close(); err != nil {
91+
slog.Warn("failed to close reader", "error", err)
92+
}
93+
}()
94+
95+
var config struct {
96+
RuntimeName string `json:"runtime_name"`
97+
}
98+
if err := json.NewDecoder(reader).Decode(&config); err != nil {
99+
return true // can't determine ownership -- assume ours
100+
}
101+
102+
// Empty means legacy workload; treat as owned by active runtime.
103+
if config.RuntimeName == "" {
104+
return true
105+
}
106+
return config.RuntimeName == f.runtime.Name()
107+
}
108+
77109
// isRemoteWorkload checks if a workload is remote by attempting to load its run configuration
78110
// and checking if it has a RemoteURL field set.
79111
// TODO: This is a temporary solution to check if a workload is remote
@@ -910,6 +942,12 @@ func (f *fileStatusManager) validateRunningWorkload(
910942
return result, nil
911943
}
912944

945+
// Skip validation for workloads owned by a different runtime to avoid
946+
// corrupting their status files (see #4432).
947+
if !f.isOwnedByActiveRuntime(ctx, workloadName) {
948+
return result, nil
949+
}
950+
913951
// Get raw container info from runtime (before label filtering)
914952
containerInfo, err := f.runtime.GetWorkloadInfo(ctx, workloadName)
915953
if err != nil {
@@ -964,6 +1002,12 @@ func (f *fileStatusManager) handleRuntimeMissing(
9641002
return fileWorkload, nil
9651003
}
9661004

1005+
// Skip reconciliation for workloads owned by a different runtime to avoid
1006+
// corrupting their status files (see #4432).
1007+
if !f.isOwnedByActiveRuntime(ctx, workloadName) {
1008+
return fileWorkload, nil
1009+
}
1010+
9671011
if fileWorkload.Status == rt.WorkloadStatusRunning || fileWorkload.Status == rt.WorkloadStatusStopped {
9681012
// The workload cannot be running or stopped if the runtime container is not found
9691013
contextMsg := fmt.Sprintf("workload %s not found in runtime, marking as unhealthy", workloadName)

0 commit comments

Comments
 (0)