Skip to content

pod write-back silently corrupts workloads #346

Description

@AviadHayumi

the bug

changing one pod field through karta can silently destroy the workload. the write reports success , the corruption is discovered later.

mutation today reads the pod template through the definition path , converts it to a typed corev1 struct , and writes the WHOLE struct back through the path. that write-back fails three ways , all reproduced below with unit tests that build everything in code - no recorded data , no files. both tests pass on main ( verified at 3b82723 ).

the write-back today , and why paths are not all addresses

what happens vs what we expect

the intent : add ONE label , team: ml , to a running pod ( the catalog core-pod-v1 definition , podTemplateSpecPath: "." ).

what happens - real output :

UpdatePodTemplateSpec returned: <nil>     <- the write says it worked
GetResource: invalid Kubernetes object: missing apiVersion

the pod was replaced by its own template. a template has metadata and spec - no apiVersion , no kind , no status. all deleted , and the write returned nil.

what we expect : the same pod , byte for byte , with exactly one difference - the new label. status intact , everything intact.

reproduce it : core pod ( unit test , runs on main , zero external inputs )

the pod and the definition are both built in code. the test asserts the object is VALID before the write ( apiVersion , kind , status all present ) , so the corruption cannot be blamed on the input.

// pkg/resource/writeback_repro_test.go
package resource

import (
	"context"
	"strings"
	"testing"

	corev1 "k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
	"k8s.io/utils/ptr"

	"github.com/run-ai/karta/pkg/api/runai/v1alpha1"
)

func TestPodTemplateWriteBackDestroysWholeObjectPath(t *testing.T) {
	ctx := context.Background()

	pod := &unstructured.Unstructured{Object: map[string]any{
		"apiVersion": "v1", "kind": "Pod",
		"metadata": map[string]any{"name": "web-1", "labels": map[string]any{"app": "web"}},
		"spec": map[string]any{
			"nodeName":   "node-7",
			"containers": []any{map[string]any{"name": "main", "image": "nginx:1.25"}},
		},
		"status": map[string]any{"phase": "Running", "podIP": "10.0.0.12"},
	}}

	definition := &v1alpha1.Karta{
		ObjectMeta: metav1.ObjectMeta{Name: "core-pod"},
		Spec: v1alpha1.KartaSpec{
			StructureDefinition: v1alpha1.StructureDefinition{
				RootComponent: v1alpha1.ComponentDefinition{
					Name: "pod",
					Kind: &v1alpha1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"},
					SpecDefinition: &v1alpha1.SpecDefinition{
						PodTemplateSpecPath: ptr.To("."),
					},
				},
			},
		},
	}

	factory := NewComponentFactoryFromObject(definition, pod)

	// before the write : the object is valid and carries apiVersion
	before, err := factory.GetResource()
	if err != nil {
		t.Fatalf("the input object is already invalid before any write: %v", err)
	}
	beforeMap := before.(*unstructured.Unstructured).Object
	if beforeMap["apiVersion"] != "v1" || beforeMap["kind"] != "Pod" || beforeMap["status"] == nil {
		t.Fatalf("the input object is missing fields before any write: %v", beforeMap)
	}

	// the write : add ONE label through the old path
	component, err := factory.GetComponent("pod")
	if err != nil {
		t.Fatal(err)
	}
	templates, err := component.GetPodTemplateSpec(ctx)
	if err != nil {
		t.Fatal(err)
	}
	for id, template := range templates {
		if template.Labels == nil {
			template.Labels = map[string]string{}
		}
		template.Labels["team"] = "ml"
		templates[id] = template
	}
	if err := component.UpdatePodTemplateSpec(ctx, map[string]corev1.PodTemplateSpec(templates)); err != nil {
		t.Fatalf("the write itself failed - the bug is that it does NOT: %v", err)
	}
	t.Log("UpdatePodTemplateSpec returned nil - the caller believes the label was added")

	// after the write : the SAME factory no longer holds a valid object
	_, err = factory.GetResource()
	if err == nil {
		t.Fatal("expected the corruption, got a valid object")
	}
	if !strings.Contains(err.Error(), "missing apiVersion") {
		t.Fatalf("expected 'missing apiVersion', got: %v", err)
	}
	t.Logf("after the write, GetResource: %v", err)
}

output :

--- PASS: TestPodTemplateWriteBackDestroysWholeObjectPath
    UpdatePodTemplateSpec returned nil - the caller believes the label was added
    after the write, GetResource: invalid Kubernetes object: missing apiVersion

why this is dangerous

put karta where it is meant to live : a controller that mutates workloads through it. say the controller sets schedulerName: kai-scheduler on every workload it admits , then applies the object back to the cluster.

  • on a kserve inference service , the apply SUCCEEDS - the corrupted object is still a valid k8s object. so production now runs a service whose pod labels are gone ( selectors , monitoring , quota attribution - whatever read them just broke ) and whose minReplicas is gone ( the autoscaling floor a human configured is silently deleted ). the audit log for this change says "set schedulerName".
  • on a pod , the apply FAILS ( no apiVersion ) - so the controllers reconcile is now stuck erroring forever on every pod it touches. mutation through karta simply cannot work on pods , and you find out in production.
  • in gitops , the corrupted fields show up as unexplained drift - argo diffs that nobody can trace back , because the write that caused them reported success and touched "one field".

the common thread : nothing fails at the moment of damage. no error , no crash , no page. the write returns nil , the damage rides the next apply , and it is discovered by whoever depended on the deleted fields - autoscaling , scheduling , monitoring - with a diff that points at an innocent one-field change.

the second victim : kserve , two paths on one object

the shipped kserve definition points podSpecPath and metadataPath at the SAME object ( .spec.transformer ). change one field through one path and the whole-struct write wipes what the other path owns - plus every field the typed struct does not know.

kserve : the write stomps what the other path owns

reproduce it : kserve shape ( unit test , real output - three failures in one write )

the intent is ONE field , schedulerName. the real result : labels gone ( the metadata path owns them ) , minReplicas gone ( corev1.PodSpec has no such member ) , and resources: {} INJECTED - a zero-value field that was never in the object.

func TestPodSpecWriteBackStompsSharedMetadataPath(t *testing.T) {
	ctx := context.Background()

	inferenceService := &unstructured.Unstructured{Object: map[string]any{
		"apiVersion": "serving.kserve.io/v1beta1", "kind": "InferenceService",
		"metadata": map[string]any{"name": "sklearn-iris"},
		"spec": map[string]any{
			"transformer": map[string]any{
				"minReplicas": int64(1),
				"labels":      map[string]any{"team": "ml"},
				"containers": []any{
					map[string]any{"name": "transformer", "image": "transformer:v1"},
				},
			},
		},
	}}

	definition := &v1alpha1.Karta{
		ObjectMeta: metav1.ObjectMeta{Name: "kserve"},
		Spec: v1alpha1.KartaSpec{
			StructureDefinition: v1alpha1.StructureDefinition{
				RootComponent: v1alpha1.ComponentDefinition{
					Name: "transformer",
					Kind: &v1alpha1.GroupVersionKind{Group: "serving.kserve.io", Version: "v1beta1", Kind: "InferenceService"},
					SpecDefinition: &v1alpha1.SpecDefinition{
						PodSpecPath:  ptr.To(".spec.transformer"),
						MetadataPath: ptr.To(".spec.transformer"),
					},
				},
			},
		},
	}

	factory := NewComponentFactoryFromObject(definition, inferenceService)
	component, err := factory.GetComponent("transformer")
	if err != nil {
		t.Fatal(err)
	}

	// before the write : the shared object holds labels AND minReplicas
	metadata, err := component.GetPodMetadata(ctx)
	if err != nil {
		t.Fatal(err)
	}
	if metadata[""].Labels["team"] != "ml" {
		t.Fatalf("labels not readable before the write: %v", metadata)
	}

	// the write : change ONE thing on the pod spec - the scheduler name
	specs, err := component.GetPodSpec(ctx)
	if err != nil {
		t.Fatal(err)
	}
	for id, spec := range specs {
		spec.SchedulerName = "kai-scheduler"
		specs[id] = spec
	}
	if err := component.UpdatePodSpec(ctx, map[string]corev1.PodSpec(specs)); err != nil {
		t.Fatalf("the write itself failed - the bug is that it does NOT: %v", err)
	}
	t.Log("UpdatePodSpec returned nil - the caller believes only schedulerName changed")

	// after the write : labels gone , minReplicas gone
	object, err := factory.GetResource()
	if err != nil {
		t.Fatal(err)
	}
	transformer := object.(*unstructured.Unstructured).Object["spec"].(map[string]any)["transformer"].(map[string]any)
	if _, ok := transformer["labels"]; ok {
		t.Fatalf("expected the labels stomp, transformer still has labels: %v", transformer)
	}
	if _, ok := transformer["minReplicas"]; ok {
		t.Fatalf("expected minReplicas to be dropped, still present: %v", transformer)
	}
	t.Logf("after the write: %v", transformer)
}

output :

--- PASS: TestPodSpecWriteBackStompsSharedMetadataPath
    UpdatePodSpec returned nil - the caller believes only schedulerName changed
    after the write: map[containers:[map[image:transformer:v1 name:transformer resources:map[]]]
                     schedulerName:kai-scheduler]

labels gone , minReplicas gone , resources injected. one field was asked for.

why it happens

two independent causes , one write strategy :

  1. the write is always WHOLE-OBJECT. UpdatePodTemplateSpec / UpdatePodSpec assign the entire rebuilt struct at the path - not the one field that changed. when the path is "." ( core pod ) the object is replaced by its own template ; when two paths share an object ( kserve ) each write stomps the other.
  2. the round trip goes through typed corev1 structs. fields the struct does not know are dropped ( minReplicas , vendor extensions , newer api fields ) , and zero-value members it does know appear from nowhere ( resources: {} ).
the deeper problem : 51 of the 283 catalog paths are not writable at all

a definition path can be an ADDRESS ( .spec.template.spec.schedulerName - a place you can read and write ) or a FORMULA ( .spec.replicatedJobs[] | .replicas * .parallelism - a calculation. reading computes a number , a write has nowhere to land ). jq does not always fail cleanly on the second kind - a write can land where you never intended and return success.

swept the shipped catalog : 283 paths = 232 addresses + 51 formulas / filters ( kserves select(.storageUri) pipelines , every // 1 default ). today nothing in the code distinguishes them before writing.

already acknowledged around the repo

possible fix

two parts , both prototyped in #345 :

  1. write FIELDS , not objects : each changed field becomes its own small jq assignment. a labels patch touches ...labels and nothing else. core pod safe ( nothing assigns to "." ) , kserve safe ( a spec write never visits the labels ) , unknown fields safe ( nothing round-trips through a struct ).
  2. check the path BEFORE writing : parse it and answer "is this a plain address ?". formulas and filters return a typed "not supported" error before anything runs , instead of a corrupting write.

expected behavior after the fix , same examples : the pod comes back byte-identical plus one label ; the transformer changes schedulerName and keeps labels , minReplicas and nothing injected.

wdyt ?

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Type

Fields

Priority

None yet

Effort

None yet

Target date

None yet

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions