Skip to content

feat(api): propagate SnapshotJob PodSnapshot metadata #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
julienmancuso merged 1 commit into from
Aug 31, 2026
Merged

feat(api): propagate SnapshotJob PodSnapshot metadata #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 59 additions & 29 deletions api/v1alpha1/crds/embed_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,8 @@ func TestAllReturnsACopy(t *testing.T) {
}

func TestSnapshotJobConditionsUseMapListSchema(t *testing.T) {
manifestJSON, err := utilyaml.ToJSON([]byte(SnapshotJobCRD()))
if err != nil {
t.Fatalf("convert SnapshotJob CRD to JSON: %v", err)
}
var crd map[string]any
if err := json.Unmarshal(manifestJSON, &crd); err != nil {
t.Fatalf("decode SnapshotJob CRD: %v", err)
}

versions := nestedSlice(t, crd, "spec", "versions")
if len(versions) == 0 {
t.Fatal("SnapshotJob CRD has no versions")
}
version, ok := versions[0].(map[string]any)
if !ok {
t.Fatalf("SnapshotJob CRD version has type %T, want object", versions[0])
}
conditions := nestedMap(t, version, "schema", "openAPIV3Schema", "properties", "status", "properties", "conditions")
schema := snapshotJobOpenAPISchema(t)
conditions := nestedMap(t, schema, "properties", "status", "properties", "conditions")

if got := conditions["x-kubernetes-list-type"]; got != "map" {
t.Errorf("conditions x-kubernetes-list-type = %v, want map", got)
Expand All @@ -134,6 +118,57 @@ func TestSnapshotJobConditionsUseMapListSchema(t *testing.T) {
}

func TestSnapshotJobRejectsMissingSpecAndLongNamesAtAdmission(t *testing.T) {
schema := snapshotJobOpenAPISchema(t)

required := nestedSlice(t, schema, "required")
if !slices.Contains(required, any("spec")) {
t.Errorf("SnapshotJob required fields = %v, want spec", required)
}

const nameRule = "size(self.metadata.name) <= 63"
validations := nestedSlice(t, schema, "x-kubernetes-validations")
if !containsValidationRule(validations, nameRule) {
t.Errorf("SnapshotJob root validations = %v, want rule %q", validations, nameRule)
}
}

func TestSnapshotJobIncludesTemplateMetadataAdmissionRules(t *testing.T) {
schema := snapshotJobOpenAPISchema(t)
template := nestedMap(t, schema,
"properties", "spec", "properties", "podSnapshotTemplate")
metadata := nestedMap(t, template, "properties", "metadata")
for _, field := range []string{"labels", "annotations"} {
property := nestedMap(t, metadata, "properties", field)
if got := property["maxProperties"]; got != float64(4096) {
t.Errorf("PodSnapshot template metadata %s maxProperties = %v, want 4096", field, got)
}
}
validations := nestedSlice(t, metadata, "x-kubernetes-validations")
wantRules := []string{
"!has(self.labels) || self.labels.all(k, !format.qualifiedName().validate(k).hasValue())",
"!has(self.labels) || self.labels.all(k, size(self.labels[k]) <= 63)",
"!has(self.annotations) || self.annotations.all(k, !format.qualifiedName().validate(k).hasValue())",
}
for _, rule := range wantRules {
if !containsValidationRule(validations, rule) {
t.Errorf("PodSnapshot template metadata validations = %v, want rule %q", validations, rule)
}
}

templateValidations := nestedSlice(t, template, "x-kubernetes-validations")
wantTemplateRules := []string{
"!has(self.metadata) || !has(self.metadata.labels) || !('nvidia.com/snapshot-job' in self.metadata.labels)",
"!has(self.metadata) || !has(self.metadata.labels) || !('nvidia.com/snapshot-job-uid' in self.metadata.labels)",
}
for _, rule := range wantTemplateRules {
if !containsValidationRule(templateValidations, rule) {
t.Errorf("PodSnapshot template validations = %v, want rule %q", templateValidations, rule)
}
}
}
Comment thread
julienmancuso marked this conversation as resolved.

func snapshotJobOpenAPISchema(t *testing.T) map[string]any {
t.Helper()
manifestJSON, err := utilyaml.ToJSON([]byte(SnapshotJobCRD()))
if err != nil {
t.Fatalf("convert SnapshotJob CRD to JSON: %v", err)
Expand All @@ -151,22 +186,17 @@ func TestSnapshotJobRejectsMissingSpecAndLongNamesAtAdmission(t *testing.T) {
if !ok {
t.Fatalf("SnapshotJob CRD version has type %T, want object", versions[0])
}
schema := nestedMap(t, version, "schema", "openAPIV3Schema")

required := nestedSlice(t, schema, "required")
if !slices.Contains(required, any("spec")) {
t.Errorf("SnapshotJob required fields = %v, want spec", required)
}
return nestedMap(t, version, "schema", "openAPIV3Schema")
}

const nameRule = "size(self.metadata.name) <= 63"
validations := nestedSlice(t, schema, "x-kubernetes-validations")
func containsValidationRule(validations []any, rule string) bool {
for _, validation := range validations {
entry, ok := validation.(map[string]any)
if ok && entry["rule"] == nameRule {
return
if ok && entry["rule"] == rule {
return true
}
}
t.Errorf("SnapshotJob root validations = %v, want rule %q", validations, nameRule)
return false
}

func nestedMap(t *testing.T, object map[string]any, fields ...string) map[string]any {
Expand Down
36 changes: 36 additions & 0 deletions api/v1alpha1/crds/nvidia.com_snapshotjobs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,35 @@ spec:
PodSnapshotTemplate defines the properties of the PodSnapshot produced by
this job. The controller fills in spec.source from the pod it creates.
properties:
metadata:
description: |-
Metadata contains labels and annotations copied to the generated
PodSnapshot. SnapshotJob ownership labels are reserved for the controller
and cannot be supplied here.
properties:
annotations:
additionalProperties:
type: string
description: Annotations are copied to the generated PodSnapshot.
maxProperties: 4096
type: object
labels:
additionalProperties:
type: string
description: Labels are copied to the generated PodSnapshot.
maxProperties: 4096
type: object
type: object
x-kubernetes-validations:
- message: metadata.labels keys must be valid Kubernetes qualified
names
rule: '!has(self.labels) || self.labels.all(k, !format.qualifiedName().validate(k).hasValue())'
- message: metadata.labels values must be no more than 63 characters
rule: '!has(self.labels) || self.labels.all(k, size(self.labels[k])
<= 63)'
- message: metadata.annotations keys must be valid Kubernetes
qualified names
rule: '!has(self.annotations) || self.annotations.all(k, !format.qualifiedName().validate(k).hasValue())'
targetContainers:
default:
- main
Expand All @@ -100,6 +129,13 @@ spec:
minItems: 1
type: array
type: object
x-kubernetes-validations:
- message: metadata.labels must not set controller-owned label nvidia.com/snapshot-job
rule: '!has(self.metadata) || !has(self.metadata.labels) || !(''nvidia.com/snapshot-job''
in self.metadata.labels)'
- message: metadata.labels must not set controller-owned label nvidia.com/snapshot-job-uid
rule: '!has(self.metadata) || !has(self.metadata.labels) || !(''nvidia.com/snapshot-job-uid''
in self.metadata.labels)'
podTemplate:
description: |-
PodTemplate defines the workload to run and capture. The controller injects
Expand Down
32 changes: 30 additions & 2 deletions api/v1alpha1/snapshotjob_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,17 @@ type SnapshotJobSpec struct {
PodSnapshotTemplate PodSnapshotTemplate `json:"podSnapshotTemplate"`
}

// PodSnapshotTemplate mirrors the PodSnapshot spec fields the user controls. The
// controller fills in spec.source (the pod reference) automatically.
// PodSnapshotTemplate mirrors the PodSnapshot fields the user controls. The
// controller fills in identity, ownership, and spec.source automatically.
// +kubebuilder:validation:XValidation:rule="!has(self.metadata) || !has(self.metadata.labels) || !('nvidia.com/snapshot-job' in self.metadata.labels)",message="metadata.labels must not set controller-owned label nvidia.com/snapshot-job"
// +kubebuilder:validation:XValidation:rule="!has(self.metadata) || !has(self.metadata.labels) || !('nvidia.com/snapshot-job-uid' in self.metadata.labels)",message="metadata.labels must not set controller-owned label nvidia.com/snapshot-job-uid"
type PodSnapshotTemplate struct {
// Metadata contains labels and annotations copied to the generated
// PodSnapshot. SnapshotJob ownership labels are reserved for the controller
// and cannot be supplied here.
// +optional
Metadata *PodSnapshotTemplateMetadata `json:"metadata,omitempty"`
Comment thread
julienmancuso marked this conversation as resolved.

// TargetContainers names the container(s) to checkpoint with CRIU. The pod
// may contain any number of additional containers (helpers, sidecars, etc.)
// — this field controls only the CRIU dump target.
Expand All @@ -157,6 +165,26 @@ type PodSnapshotTemplate struct {
TargetContainers []string `json:"targetContainers,omitempty"`
}

// PodSnapshotTemplateMetadata is the caller-owned metadata propagated to a
// SnapshotJob's generated PodSnapshot. It intentionally exposes only labels and
// annotations; identity and ownership remain controller-owned. The SnapshotJob
// spec is immutable, so metadata rejected by the controller as InvalidSpec
// requires deleting and recreating the SnapshotJob.
// +kubebuilder:validation:XValidation:rule="!has(self.labels) || self.labels.all(k, !format.qualifiedName().validate(k).hasValue())",message="metadata.labels keys must be valid Kubernetes qualified names"
// +kubebuilder:validation:XValidation:rule="!has(self.labels) || self.labels.all(k, size(self.labels[k]) <= 63)",message="metadata.labels values must be no more than 63 characters"
// +kubebuilder:validation:XValidation:rule="!has(self.annotations) || self.annotations.all(k, !format.qualifiedName().validate(k).hasValue())",message="metadata.annotations keys must be valid Kubernetes qualified names"
type PodSnapshotTemplateMetadata struct {
// Labels are copied to the generated PodSnapshot.
// +optional
Comment thread
julienmancuso marked this conversation as resolved.
// +kubebuilder:validation:MaxProperties=4096
Labels map[string]string `json:"labels,omitempty"`

// Annotations are copied to the generated PodSnapshot.
// +optional
Comment thread
julienmancuso marked this conversation as resolved.
// +kubebuilder:validation:MaxProperties=4096
Annotations map[string]string `json:"annotations,omitempty"`
}

// SnapshotJobStatus defines the observed state of SnapshotJob.
type SnapshotJobStatus struct {
// SourceJobUID identifies the one source batch/v1 Job incarnation accepted
Expand Down
16 changes: 15 additions & 1 deletion api/v1alpha1/snapshotjob_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ func TestSnapshotJobDeepCopyIsIndependent(t *testing.T) {
Containers: []corev1.Container{{Name: "worker"}},
},
},
PodSnapshotTemplate: PodSnapshotTemplate{TargetContainers: []string{"worker"}},
PodSnapshotTemplate: PodSnapshotTemplate{
Metadata: &PodSnapshotTemplateMetadata{
Labels: map[string]string{"dynamo.nvidia.com/worker-generation": "abc123"},
Annotations: map[string]string{"dynamo.nvidia.com/gms-mode": "enabled"},
},
TargetContainers: []string{"worker"},
},
},
Status: SnapshotJobStatus{
PodSnapshotName: "warm-worker-snapshot",
Expand All @@ -56,13 +62,21 @@ func TestSnapshotJobDeepCopyIsIndependent(t *testing.T) {

clone.Spec.PodTemplate.Spec.Containers[0].Name = "mutated"
clone.Spec.PodSnapshotTemplate.TargetContainers[0] = "mutated"
clone.Spec.PodSnapshotTemplate.Metadata.Labels["dynamo.nvidia.com/worker-generation"] = "changed"
clone.Spec.PodSnapshotTemplate.Metadata.Annotations["dynamo.nvidia.com/gms-mode"] = "disabled"
clone.Status.Conditions[0].Reason = "Changed"
if original.Spec.PodTemplate.Spec.Containers[0].Name != "worker" {
t.Errorf("mutating clone PodTemplate changed original: got %q", original.Spec.PodTemplate.Spec.Containers[0].Name)
}
if original.Spec.PodSnapshotTemplate.TargetContainers[0] != "worker" {
t.Errorf("mutating clone TargetContainers changed original: got %q", original.Spec.PodSnapshotTemplate.TargetContainers[0])
}
if original.Spec.PodSnapshotTemplate.Metadata.Labels["dynamo.nvidia.com/worker-generation"] != "abc123" {
t.Errorf("mutating clone labels changed original: got %q", original.Spec.PodSnapshotTemplate.Metadata.Labels["dynamo.nvidia.com/worker-generation"])
}
if original.Spec.PodSnapshotTemplate.Metadata.Annotations["dynamo.nvidia.com/gms-mode"] != "enabled" {
t.Errorf("mutating clone annotations changed original: got %q", original.Spec.PodSnapshotTemplate.Metadata.Annotations["dynamo.nvidia.com/gms-mode"])
}
if original.Status.Conditions[0].Reason != ReasonPodReady {
t.Errorf("mutating clone condition changed original: got %q", original.Status.Conditions[0].Reason)
}
Expand Down
34 changes: 34 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions charts/snapshot/crds/nvidia.com_snapshotjobs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,35 @@ spec:
PodSnapshotTemplate defines the properties of the PodSnapshot produced by
this job. The controller fills in spec.source from the pod it creates.
properties:
metadata:
description: |-
Metadata contains labels and annotations copied to the generated
PodSnapshot. SnapshotJob ownership labels are reserved for the controller
and cannot be supplied here.
properties:
annotations:
additionalProperties:
type: string
description: Annotations are copied to the generated PodSnapshot.
maxProperties: 4096
type: object
labels:
additionalProperties:
type: string
description: Labels are copied to the generated PodSnapshot.
maxProperties: 4096
type: object
type: object
x-kubernetes-validations:
- message: metadata.labels keys must be valid Kubernetes qualified
names
rule: '!has(self.labels) || self.labels.all(k, !format.qualifiedName().validate(k).hasValue())'
- message: metadata.labels values must be no more than 63 characters
rule: '!has(self.labels) || self.labels.all(k, size(self.labels[k])
<= 63)'
- message: metadata.annotations keys must be valid Kubernetes
qualified names
rule: '!has(self.annotations) || self.annotations.all(k, !format.qualifiedName().validate(k).hasValue())'
targetContainers:
default:
- main
Expand All @@ -100,6 +129,13 @@ spec:
minItems: 1
type: array
type: object
x-kubernetes-validations:
- message: metadata.labels must not set controller-owned label nvidia.com/snapshot-job
rule: '!has(self.metadata) || !has(self.metadata.labels) || !(''nvidia.com/snapshot-job''
in self.metadata.labels)'
- message: metadata.labels must not set controller-owned label nvidia.com/snapshot-job-uid
rule: '!has(self.metadata) || !has(self.metadata.labels) || !(''nvidia.com/snapshot-job-uid''
in self.metadata.labels)'
podTemplate:
description: |-
PodTemplate defines the workload to run and capture. The controller injects
Expand Down
3 changes: 3 additions & 0 deletions operator/internal/controller/snapshotjob_job.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func buildSourceJob(sj *snapshotv1alpha1.SnapshotJob) (*batchv1.Job, error) {
if errs := contentvalidation.IsLabelValue(sj.Name); len(errs) > 0 {
return nil, fmt.Errorf("metadata.name %q is not a valid label value: %s", sj.Name, strings.Join(errs, "; "))
}
if err := validatePodSnapshotTemplateMetadata(sj); err != nil {
return nil, err
}

targetContainer, err := snapshotJobTargetContainer(sj)
if err != nil {
Expand Down
23 changes: 23 additions & 0 deletions operator/internal/controller/snapshotjob_job_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,29 @@ func TestBuildSourceJob(t *testing.T) {
require.Error(t, err)
})

t.Run("invalid PodSnapshot metadata is a terminal spec error", func(t *testing.T) {
tests := map[string]*snapshotv1alpha1.PodSnapshotTemplateMetadata{
"invalid label": {
Labels: map[string]string{"example.com/team": strings.Repeat("x", 64)},
},
"invalid annotation": {
Annotations: map[string]string{"not a qualified annotation key": "value"},
},
"reserved owner label": {
Labels: map[string]string{snapshotv1alpha1.SnapshotJobOwnerLabel: "caller"},
},
}
for name, metadata := range tests {
t.Run(name, func(t *testing.T) {
sj := minimalSnapshotJob()
sj.Spec.PodSnapshotTemplate.Metadata = metadata

_, err := buildSourceJob(sj)
require.Error(t, err)
})
}
})

t.Run("more than one targetContainers entry is a terminal spec error", func(t *testing.T) {
// The CRD caps this at MaxItems=1, but this is defense in depth for an
// object that bypassed CEL validation — v1alpha1 supports exactly one
Expand Down
Loading