-
Notifications
You must be signed in to change notification settings - Fork 11
feat: refuse incompatible restores before CRIU #135
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.
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
Closed
leeZardNav
wants to merge
30
commits into
main
from
lirane/RUN-42197-restore-compatibility-checker-v2
Closed
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
09ea6ba
feat(api): add restore compatibility policy table with no rules
leeZardNav 8d990c6
feat(api): render a compatibility mismatch as one stable reason
leeZardNav b8adf63
feat(agent): compare compatibility before claiming a restore
leeZardNav c325502
feat(agent): compare compatibility on the restore node
leeZardNav 7c466c7
feat(agent): log a refused restore with a stable reason field
leeZardNav b59b9ad
feat: let one restore ask for the compatibility gates to be skipped
leeZardNav 87cc4c8
feat: let a node turn the restore compatibility gate off
leeZardNav 7b4d77d
feat(agent): read the node compatibility switch per restore
leeZardNav 1f8af79
feat(agent): describe the GPUs a container can see, on every path
leeZardNav 0fef190
feat(agent): bound every nvidia-smi call with a deadline
leeZardNav da5b311
feat(agent): record in the manifest what the GPUs were
leeZardNav c73af49
feat(agent): read the node kernel version once at startup
leeZardNav d1d21ed
feat(agent): record the capture node's kernel and arch
leeZardNav 35dd5f6
feat(agent): record the captured container's image and its limits
leeZardNav 94db5b6
feat(agent): fill image and limit facts from authoritative sources
leeZardNav 99db2e1
feat(compat): refuse a restore onto a different CPU architecture
leeZardNav d801f0a
feat(compat): refuse a restore onto a different or too-old kernel
leeZardNav 8f90a01
feat(compat): refuse a restore into another build of the same image
leeZardNav 9439590
feat(compat): refuse a restore into less memory than was captured
leeZardNav 8c7149a
feat(compat): refuse a restore into less CPU than was captured
leeZardNav 12d1095
feat(compat): refuse a restore when a mounted path is not in the pod
leeZardNav 4c9bbcb
feat(compat): refuse a restore onto a different GPU model
leeZardNav 8824f1a
feat(compat): refuse a restore onto a different number of GPUs
leeZardNav bf4850a
feat(compat): refuse a restore on a different or too-old GPU driver
leeZardNav a516d35
feat(agent): report a refused restore on its pod
leeZardNav 478a2ac
feat(agent): stop re-checking a restore that was already refused
leeZardNav 0c1414b
test(e2e): assert a real artifact records the machine it came from
leeZardNav fc01ebd
test(e2e): refuse a restore into a smaller memory limit
leeZardNav 9a55d8d
test(e2e): let a refused restore through with each kill switch
leeZardNav 995d159
fix(cuda): ignore discovery rows without GPU UUIDs
leeZardNav File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package main | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/go-logr/logr" | ||
| ) | ||
|
|
||
| func writeConfig(t *testing.T, path, document string) { | ||
| t.Helper() | ||
| if err := os.WriteFile(path, []byte(document), 0o600); err != nil { | ||
| t.Fatalf("write config: %v", err) | ||
| } | ||
| } | ||
|
|
||
| // The point of re-reading is that flipping the ConfigMap is enough: the kubelet | ||
| // updates the mounted file on its own, and the next restore sees the new value | ||
| // without the DaemonSet being rolled. | ||
| func TestNewSkipCompatCheckFnFollowsTheFile(t *testing.T) { | ||
| path := filepath.Join(t.TempDir(), "config.yaml") | ||
| writeConfig(t, path, "restore:\n skipCompatCheck: false\n") | ||
| skip := NewSkipCompatCheckFn(path, false, logr.Discard()) | ||
|
|
||
| if skip() { | ||
| t.Fatal("switch read true from a file that says false") | ||
| } | ||
|
|
||
| writeConfig(t, path, "restore:\n skipCompatCheck: true\n") | ||
| if !skip() { | ||
| t.Fatal("switch did not follow the file being flipped on") | ||
| } | ||
|
|
||
| writeConfig(t, path, "restore:\n skipCompatCheck: false\n") | ||
| if skip() { | ||
| t.Fatal("switch did not follow the file being flipped back off") | ||
| } | ||
| } | ||
|
|
||
| // A restore is never failed, and never quietly checked differently, because a | ||
| // config read went wrong. | ||
| func TestNewSkipCompatCheckFnKeepsTheLastGoodValue(t *testing.T) { | ||
| dir := t.TempDir() | ||
| path := filepath.Join(dir, "config.yaml") | ||
|
|
||
| t.Run("missing file keeps what the agent started with", func(t *testing.T) { | ||
| absent := filepath.Join(dir, "absent.yaml") | ||
| if !NewSkipCompatCheckFn(absent, true, logr.Discard())() { | ||
| t.Fatal("switch lost the startup value when the file was missing") | ||
| } | ||
| if NewSkipCompatCheckFn(absent, false, logr.Discard())() { | ||
| t.Fatal("switch invented a value when the file was missing") | ||
| } | ||
| }) | ||
|
|
||
| t.Run("malformed file keeps the last value read", func(t *testing.T) { | ||
| writeConfig(t, path, "restore:\n skipCompatCheck: true\n") | ||
| skip := NewSkipCompatCheckFn(path, false, logr.Discard()) | ||
| if !skip() { | ||
| t.Fatal("switch read false from a file that says true") | ||
| } | ||
|
|
||
| writeConfig(t, path, "restore:\n\tskipCompatCheck: not-a-bool\n") | ||
| if !skip() { | ||
| t.Fatal("switch dropped the last known value on an unparseable file") | ||
| } | ||
| }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package controller | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "runtime" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
|
|
||
| "github.com/ai-dynamo/snapshot/agent/internal/types" | ||
| "github.com/ai-dynamo/snapshot/api/compat" | ||
| snapshotv1alpha1 "github.com/ai-dynamo/snapshot/api/v1alpha1" | ||
| ) | ||
|
|
||
| // refuseRestore records a restore this node will not attempt. It is terminal | ||
| // like any other restore failure and reports through the same condition, with | ||
| // its own reason so an operator can tell a checkpoint that cannot run here from | ||
| // one that tried and broke. | ||
| func (w *NodeController) refuseRestore(ctx context.Context, pod *corev1.Pod, incompatible *compat.IncompatibleError) bool { | ||
| reason := compat.Reasons(incompatible.Mismatches) | ||
| w.logRestoreRefusal(pod, incompatible, reason) | ||
| return w.finishRestore( | ||
| ctx, | ||
| pod, | ||
| corev1.ConditionFalse, | ||
| restoreIncompatibleReason, | ||
| reason, | ||
| ) != nil | ||
| } | ||
|
|
||
| func (w *NodeController) logRestoreRefusal(pod *corev1.Pod, incompatible *compat.IncompatibleError, reason string) { | ||
| w.log.Info("Refusing restore; this node cannot run the checkpoint", | ||
| "pod", fmt.Sprintf("%s/%s", pod.Namespace, pod.Name), | ||
| "gate", string(incompatible.Gate), | ||
| "reason", reason, | ||
| ) | ||
| } | ||
|
|
||
| // reopenedAfterRefusal reports a pod that the gates turned down and that has | ||
| // since asked for them to be skipped. Nothing else reopens a terminal restore, | ||
| // which is what makes the skip request an escape hatch and not a retry. | ||
| func (w *NodeController) reopenedAfterRefusal(pod *corev1.Pod) bool { | ||
| condition := findRestoredCondition(pod) | ||
| if condition == nil || condition.Status != corev1.ConditionFalse || condition.Reason != restoreIncompatibleReason { | ||
| return false | ||
| } | ||
| return w.skipCompatCheckRequested(pod) | ||
| } | ||
|
|
||
| // podFacts reads what one container of a pod runs as and is allowed. It serves | ||
| // both sides of a comparison: what a capture records about the source pod, and | ||
| // what a restore target offers. | ||
| // | ||
| // A container that is not in the pod leaves its facts unknown. | ||
| func podFacts(pod *corev1.Pod, containerName string) compat.Facts { | ||
| if pod == nil { | ||
| return compat.Facts{} | ||
| } | ||
|
|
||
| facts := compat.Facts{} | ||
| for _, container := range pod.Spec.Containers { | ||
| if container.Name != containerName { | ||
| continue | ||
| } | ||
| facts.Image = container.Image | ||
| facts.CPULimit = limitString(container.Resources.Limits, corev1.ResourceCPU) | ||
| facts.MemoryLimit = limitString(container.Resources.Limits, corev1.ResourceMemory) | ||
| } | ||
| return facts | ||
| } | ||
|
|
||
| // limitString keeps an unset limit unset. A missing quantity formats as "0", | ||
| // which would otherwise read as a container limited to nothing. | ||
| func limitString(limits corev1.ResourceList, name corev1.ResourceName) string { | ||
| quantity, ok := limits[name] | ||
| if !ok { | ||
| return "" | ||
| } | ||
| return quantity.String() | ||
| } | ||
|
|
||
| // skipCompatCheckRequested reports whether this restore was asked to skip | ||
| // the compatibility gates, by the pod that is being restored or by the node | ||
| // it landed on. | ||
| func (w *NodeController) skipCompatCheckRequested(pod *corev1.Pod) bool { | ||
| return w.skipCompatCheckFn() || | ||
| snapshotv1alpha1.SkipCompatCheckFromAnnotations(pod.Annotations) | ||
| } | ||
|
|
||
| // preflightCompatibility runs the pre-flight compatibility gate for one restore. | ||
| // A nil error means the restore may be attempted. | ||
| func (w *NodeController) preflightCompatibility( | ||
| pod *corev1.Pod, | ||
| artifact *restoreArtifact, | ||
| mappings []snapshotv1alpha1.RestoreContainerMapping, | ||
| ) error { | ||
| log := w.log.WithValues("pod", fmt.Sprintf("%s/%s", pod.Namespace, pod.Name), "container", artifact.SourceContainerName) | ||
| if artifact.SkipCompatCheck { | ||
| log.Info("Restore compatibility check skipped by request", "gate", string(compat.GatePreflight)) | ||
| return nil | ||
| } | ||
|
|
||
| manifest, err := types.ReadManifest(artifact.Path) | ||
| if err != nil { | ||
| // An unreadable manifest is not an incompatibility. The restore path | ||
| // reads it again and reports the real error from there, so refusing here | ||
| // would relabel a broken artifact as an incompatible one. | ||
| log.V(1).Info("Skipping restore compatibility gate; checkpoint manifest is unreadable", | ||
| "artifact_path", artifact.Path, | ||
| "error", err.Error(), | ||
| ) | ||
| return nil | ||
| } | ||
|
|
||
| sourceFacts := manifest.CompatFacts() | ||
| for _, mapping := range mappings { | ||
| mismatches := w.compareFn( | ||
| compat.GatePreflight, | ||
| sourceFacts, | ||
| w.preflightTargetFacts(pod, mapping.Destination), | ||
| ) | ||
| if len(mismatches) != 0 { | ||
| return compat.NewIncompatibleError(compat.GatePreflight, mismatches) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // preflightTargetFacts describes what this node and this pod offer a restore, as | ||
| // far as it is knowable before the placeholder container exists. It is assembled | ||
| // per restore from facts the agent already holds, so the gate costs no syscalls | ||
| // and no API reads. | ||
| func (w *NodeController) preflightTargetFacts(pod *corev1.Pod, containerName string) compat.Facts { | ||
| facts := podFacts(pod, containerName) | ||
| // The agent's own architecture, which is the node's: this binary could not | ||
| // be running here otherwise. | ||
| facts.CPUArch = runtime.GOARCH | ||
| facts.KernelVersion = w.config.HostKernelVersion | ||
| return facts | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.