diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 4f4e322d8d..15e4b56f46 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -32,6 +32,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" @@ -53,10 +54,60 @@ import ( ) const ( - defaultResync = 1 * time.Hour - syncedPollPeriod = 100 * time.Millisecond + defaultResync = 1 * time.Hour + syncedPollPeriod = 100 * time.Millisecond + unaccountedPodAllocationReason = "node has an unaccounted pod device allocation" ) +// podAllocationDecodeFailures tracks bound pods whose device allocations +// cannot be reconstructed. Its zero value is ready for use so Scheduler +// values constructed directly by tests remain valid. +type podAllocationDecodeFailures struct { + mutex sync.RWMutex + pods map[k8stypes.UID]string +} + +func (f *podAllocationDecodeFailures) record(uid k8stypes.UID, nodeID string) { + if uid == "" || nodeID == "" { + return + } + f.mutex.Lock() + defer f.mutex.Unlock() + if f.pods == nil { + f.pods = make(map[k8stypes.UID]string) + } + f.pods[uid] = nodeID +} + +func (f *podAllocationDecodeFailures) clearPod(uid k8stypes.UID) { + f.mutex.Lock() + defer f.mutex.Unlock() + delete(f.pods, uid) +} + +func (f *podAllocationDecodeFailures) clearNode(nodeID string) { + f.mutex.Lock() + defer f.mutex.Unlock() + for uid, failedNodeID := range f.pods { + if failedNodeID == nodeID { + delete(f.pods, uid) + } + } +} + +func (f *podAllocationDecodeFailures) nodes() map[string]struct{} { + f.mutex.RLock() + defer f.mutex.RUnlock() + if len(f.pods) == 0 { + return nil + } + nodes := make(map[string]struct{}, len(f.pods)) + for _, nodeID := range f.pods { + nodes[nodeID] = struct{}{} + } + return nodes +} + type Scheduler struct { *nodeManager podManager *device.PodManager @@ -86,6 +137,8 @@ type Scheduler struct { // cycle, so in the common path this adds no contention; it exists so these // paths cannot observe or produce half-applied accounting. allocLock sync.Mutex + + allocationDecodeFailures podAllocationDecodeFailures } func NewScheduler() *Scheduler { @@ -138,6 +191,35 @@ func (s *Scheduler) doNodeNotify() { } } +func (s *Scheduler) recordAllocationDecodeFailure(pod *corev1.Pod, nodeID string) bool { + if pod.Spec.NodeName == "" || pod.Spec.NodeName != nodeID { + return false + } + + // A Pod author can set spec.nodeName and HAMi annotations directly. Only + // quarantine the node when Kubernetes confirms that the Pod was scheduled + // and the Pod actually requests a device managed by this scheduler. + scheduled := false + for _, condition := range pod.Status.Conditions { + if condition.Type == corev1.PodScheduled && + condition.Status == corev1.ConditionTrue { + scheduled = true + break + } + } + if !scheduled { + return false + } + for _, dev := range device.GetDevices() { + if device.PodRequiresDevice(dev, pod) { + s.allocationDecodeFailures.record(pod.UID, nodeID) + return pod.UID != "" + } + } + + return false +} + func (s *Scheduler) onAddPod(obj any) { pod, ok := obj.(*corev1.Pod) if !ok { @@ -150,6 +232,7 @@ func (s *Scheduler) onAddPod(obj any) { return } if util.IsPodInTerminatedState(pod) { + s.allocationDecodeFailures.clearPod(pod.UID) if pi, ok := s.podManager.TakeAndDeletePod(pod); ok { s.quotaManager.RmUsage(pod, pi.Devices) } @@ -169,7 +252,12 @@ func (s *Scheduler) onAddPod(obj any) { rawDevices, err := device.DecodePodDevices(device.SupportDevices, pod.Annotations) if err != nil { - klog.ErrorS(err, "failed to decode pod devices", "pod", klog.KObj(pod)) + _, cached := s.podManager.GetPod(pod) + blocked := false + if !cached { + blocked = s.recordAllocationDecodeFailure(pod, nodeID) + } + klog.ErrorS(err, "failed to decode pod devices", "pod", klog.KObj(pod), "node", nodeID, "nodeBlocked", blocked) return } @@ -178,6 +266,7 @@ func (s *Scheduler) onAddPod(obj any) { if s.podManager.AddPod(pod, nodeID, effectiveDevices) { s.quotaManager.AddUsage(pod, effectiveDevices) } + s.allocationDecodeFailures.clearPod(pod.UID) } func (s *Scheduler) onUpdatePod(oldObj, newObj any) { @@ -188,17 +277,22 @@ func (s *Scheduler) onUpdatePod(oldObj, newObj any) { klog.V(5).InfoS("Pod updated", "pod", klog.KObj(newPod)) - if _, ok := newPod.Annotations[util.AssignedNodeAnnotations]; !ok { - return - } - if util.IsPodInTerminatedState(newPod) { + s.allocationDecodeFailures.clearPod(newPod.UID) + if _, ok := newPod.Annotations[util.AssignedNodeAnnotations]; !ok { + return + } if pi, ok := s.podManager.TakeAndDeletePod(newPod); ok { s.quotaManager.RmUsage(newPod, pi.Devices) } return } + if _, ok := newPod.Annotations[util.AssignedNodeAnnotations]; !ok { + s.allocationDecodeFailures.clearPod(newPod.UID) + return + } + if util.IsPodTerminating(newPod) { // Same as onAddPod: a resync update for a terminating pod that is // missing from the cache must be accounted, not dropped. @@ -266,6 +360,8 @@ func (s *Scheduler) onDelPod(obj any) { return } + s.allocationDecodeFailures.clearPod(pod.UID) + // Delete notifications can contain incomplete Pod objects. The cached // allocation, keyed by the immutable UID, is the cleanup source of truth. if pi, ok := s.podManager.TakeAndDeletePod(pod); ok { @@ -299,6 +395,7 @@ func (s *Scheduler) onDelNode(obj any) { } nodelockutil.CleanupNodeLock(nodeName) + s.allocationDecodeFailures.clearNode(nodeName) s.rmNode(nodeName) s.cleanupNodeUsage(nodeName) // Clear per-device health bookkeeping for the deleted node. @@ -870,7 +967,12 @@ func (s *Scheduler) getNodesUsage(nodes *[]string, task *corev1.Pod) (*map[strin if nodes == nil { return &cachenodeMap, &overallnodeMap, failedNodes, nil } + blockedNodes := s.allocationDecodeFailures.nodes() for _, nodeID := range *nodes { + if _, blocked := blockedNodes[nodeID]; blocked { + failedNodes[nodeID] = unaccountedPodAllocationReason + continue + } node, err := s.GetNode(nodeID) if err != nil { // The identified node does not have a gpu device, so the log here has no practical meaning,increase log priority. diff --git a/pkg/scheduler/scheduler_pod_lifecycle_test.go b/pkg/scheduler/scheduler_pod_lifecycle_test.go index 1521ce84a9..65da21a86e 100644 --- a/pkg/scheduler/scheduler_pod_lifecycle_test.go +++ b/pkg/scheduler/scheduler_pod_lifecycle_test.go @@ -18,10 +18,13 @@ package scheduler import ( "maps" + "strconv" + "sync" "testing" "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/cache" @@ -81,6 +84,87 @@ func newTerminatingAllocatedPod(uid, name, namespace string) *corev1.Pod { return pod } +func addReplayNode(s *Scheduler, nodeName string) { + s.addNode(nodeName, &device.NodeInfo{ + ID: nodeName, + Node: &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: nodeName}}, + Devices: map[string][]device.DeviceInfo{ + nvidia.NvidiaGPUDevice: {{ + ID: "GPU0", + Index: 0, + Count: 10, + Devmem: 40000, + Devcore: 100, + Mode: "hami", + Health: true, + }}, + }, + }) +} + +func newMalformedAllocatedPod(uid, name, namespace, nodeName string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: k8stypes.UID(uid), + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + util.AssignedNodeAnnotations: nodeName, + device.SupportDevices[nvidia.NvidiaGPUDevice]: "GPU0,NVIDIA,20000:;", + }, + }, + Spec: corev1.PodSpec{ + NodeName: nodeName, + Containers: []corev1.Container{{ + Name: "app", + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{"hami.io/gpu": resource.MustParse("1")}, + }, + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{ + Type: corev1.PodScheduled, + Status: corev1.ConditionTrue, + }}, + }, + } +} + +func validAllocatedAnnotations() map[string]string { + return device.EncodePodDevices(device.SupportDevices, device.PodDevices{ + nvidia.NvidiaGPUDevice: device.PodSingleDevice{ + device.ContainerDevices{{ + UUID: "GPU0", + Type: nvidia.NvidiaGPUDevice, + Usedmem: 20000, + Usedcores: 50, + }}, + }, + }) +} + +func TestPodAllocationDecodeFailuresConcurrentAccess(t *testing.T) { + var failures podAllocationDecodeFailures + var wg sync.WaitGroup + for i := range 100 { + uid := k8stypes.UID("pod-" + strconv.Itoa(i)) + wg.Add(2) + go func() { + defer wg.Done() + failures.record(uid, "node1") + failures.clearPod(uid) + }() + go func() { + defer wg.Done() + _ = failures.nodes() + }() + } + wg.Wait() + assert.Equal(t, len(failures.nodes()), 0) +} + // After a scheduler restart the informer's initial sync replays every pod as // an add. A pod that is terminating with a long grace period still runs and // holds its devices, so it must land in the cache; dropping it made its GPU @@ -209,3 +293,168 @@ func Test_onDelPod_AnnotationlessOldUIDDoesNotDeleteReplacement(t *testing.T) { assert.Equal(t, replayQuotaUsage(s, oldPod.Namespace, "hami.io/gpumem"), int64(20000)) assert.Equal(t, replayQuotaUsage(s, oldPod.Namespace, "hami.io/gpucores"), int64(100)) } + +func Test_onUpdatePod_ValidAllocationClearsDecodeFailure(t *testing.T) { + initReplayDevices(t) + s := NewScheduler() + addReplayNode(s, "node1") + pod := newMalformedAllocatedPod("decode-recovery-uid", "decode-recovery", "decode-recovery-ns", "node1") + + s.onAddPod(pod) + + nodes := []string{"node1"} + candidates, _, failedNodes, err := s.getNodesUsage(&nodes, nil) + assert.NilError(t, err) + assert.Equal(t, len(*candidates), 0) + assert.Equal(t, failedNodes["node1"], unaccountedPodAllocationReason) + + updated := pod.DeepCopy() + maps.Copy(updated.Annotations, validAllocatedAnnotations()) + s.onUpdatePod(pod, updated) + t.Cleanup(func() { s.onDelPod(updated) }) + + candidates, _, failedNodes, err = s.getNodesUsage(&nodes, nil) + assert.NilError(t, err) + assert.Equal(t, len(failedNodes), 0) + usage, ok := (*candidates)["node1"] + assert.Equal(t, ok, true) + assert.Equal(t, usage.Devices.DeviceLists[0].Device.Usedmem, int32(20000)) + assert.Equal(t, usage.Devices.DeviceLists[0].Device.Usedcores, int32(50)) +} + +func Test_onDelPod_ClearsOnlyDeletedPodDecodeFailure(t *testing.T) { + initReplayDevices(t) + s := NewScheduler() + addReplayNode(s, "node1") + first := newMalformedAllocatedPod("decode-delete-first", "decode-delete-first", "decode-delete-ns", "node1") + second := newMalformedAllocatedPod("decode-delete-second", "decode-delete-second", "decode-delete-ns", "node1") + s.onAddPod(first) + s.onAddPod(second) + nodes := []string{"node1"} + + deletedFirst := first.DeepCopy() + deletedFirst.Annotations = nil + s.onDelPod(deletedFirst) + candidates, _, failedNodes, err := s.getNodesUsage(&nodes, nil) + assert.NilError(t, err) + assert.Equal(t, len(*candidates), 0) + assert.Equal(t, failedNodes["node1"], unaccountedPodAllocationReason) + + deletedSecond := second.DeepCopy() + deletedSecond.Annotations = nil + s.onDelPod(deletedSecond) + candidates, _, failedNodes, err = s.getNodesUsage(&nodes, nil) + assert.NilError(t, err) + assert.Equal(t, len(failedNodes), 0) + _, ok := (*candidates)["node1"] + assert.Equal(t, ok, true) +} + +func Test_onAddPod_DecodeFailureDoesNotBlockFromAnnotationAlone(t *testing.T) { + initReplayDevices(t) + s := NewScheduler() + addReplayNode(s, "node1") + pod := newMalformedAllocatedPod("decode-unbound-uid", "decode-unbound", "decode-unbound-ns", "node1") + pod.Spec.NodeName = "" + + s.onAddPod(pod) + + nodes := []string{"node1"} + candidates, _, failedNodes, err := s.getNodesUsage(&nodes, nil) + assert.NilError(t, err) + assert.Equal(t, len(failedNodes), 0) + _, ok := (*candidates)["node1"] + assert.Equal(t, ok, true) +} + +func Test_onAddPod_DecodeFailureRequiresScheduledHAMiPod(t *testing.T) { + initReplayDevices(t) + tests := []struct { + name string + mutate func(*corev1.Pod) + }{ + { + name: "manually bound pod", + mutate: func(pod *corev1.Pod) { + pod.Status.Conditions = nil + }, + }, + { + name: "scheduled pod without HAMi resource", + mutate: func(pod *corev1.Pod) { + pod.Spec.Containers[0].Resources = corev1.ResourceRequirements{} + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s := NewScheduler() + addReplayNode(s, "node1") + pod := newMalformedAllocatedPod("decode-untrusted-uid", "decode-untrusted", "decode-untrusted-ns", "node1") + test.mutate(pod) + + s.onAddPod(pod) + + nodes := []string{"node1"} + candidates, _, failedNodes, err := s.getNodesUsage(&nodes, nil) + assert.NilError(t, err) + assert.Equal(t, len(failedNodes), 0) + _, ok := (*candidates)["node1"] + assert.Equal(t, ok, true) + }) + } +} + +func Test_onUpdatePod_RemovedAssignmentClearsDecodeFailure(t *testing.T) { + initReplayDevices(t) + s := NewScheduler() + addReplayNode(s, "node1") + pod := newMalformedAllocatedPod("decode-unassigned-uid", "decode-unassigned", "decode-unassigned-ns", "node1") + s.onAddPod(pod) + + updated := pod.DeepCopy() + delete(updated.Annotations, util.AssignedNodeAnnotations) + s.onUpdatePod(pod, updated) + + nodes := []string{"node1"} + candidates, _, failedNodes, err := s.getNodesUsage(&nodes, nil) + assert.NilError(t, err) + assert.Equal(t, len(failedNodes), 0) + _, ok := (*candidates)["node1"] + assert.Equal(t, ok, true) +} + +func Test_onAddPod_BadResyncKeepsCachedAllocation(t *testing.T) { + initReplayDevices(t) + s := NewScheduler() + addReplayNode(s, "node1") + pod := newMalformedAllocatedPod("decode-cached-uid", "decode-cached", "decode-cached-ns", "node1") + maps.Copy(pod.Annotations, validAllocatedAnnotations()) + s.onAddPod(pod) + t.Cleanup(func() { s.onDelPod(pod) }) + + malformed := pod.DeepCopy() + malformed.Annotations[device.SupportDevices[nvidia.NvidiaGPUDevice]] = "GPU0,NVIDIA,20000:;" + s.onAddPod(malformed) + + nodes := []string{"node1"} + candidates, _, failedNodes, err := s.getNodesUsage(&nodes, nil) + assert.NilError(t, err) + assert.Equal(t, len(failedNodes), 0) + usage, ok := (*candidates)["node1"] + assert.Equal(t, ok, true) + assert.Equal(t, usage.Devices.DeviceLists[0].Device.Usedmem, int32(20000)) +} + +func Test_onDelNode_ClearsAllocationDecodeFailures(t *testing.T) { + initReplayDevices(t) + s := NewScheduler() + addReplayNode(s, "node1") + pod := newMalformedAllocatedPod("decode-node-delete-uid", "decode-node-delete", "decode-node-delete-ns", "node1") + s.onAddPod(pod) + + s.onDelNode(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}}) + _, blocked := s.allocationDecodeFailures.nodes()["node1"] + assert.Equal(t, blocked, false) +}