Merge pull request #28886 from wojtek-t/fix_schedulercache_bug

Automatic merge from submit-queue

Add ForgetPod to SchedulerCache

Fix #28883

@gmarek @davidopp @xiang90

plugin/pkg/scheduler/scheduler.go

@@ -140,6 +140,9 @@ func (s *Scheduler) scheduleOne() {
 		err := s.config.Binder.Bind(b)
 		if err != nil {
 			glog.V(1).Infof("Failed to bind pod: %v/%v", pod.Namespace, pod.Name)
+			if err := s.config.SchedulerCache.ForgetPod(&assumed); err != nil {
+				glog.Errorf("scheduler cache ForgetPod failed: %v", err)
+			}
 			s.config.Error(pod, err)
 			s.config.Recorder.Eventf(pod, api.EventTypeNormal, "FailedScheduling", "Binding rejected: %v", err)
 			s.config.PodConditionUpdater.Update(pod, &api.PodCondition{

plugin/pkg/scheduler/schedulercache/cache.go

@@ -132,6 +132,30 @@ func (cache *schedulerCache) assumePod(pod *api.Pod, now time.Time) error {
 	return nil
 }
 
+func (cache *schedulerCache) ForgetPod(pod *api.Pod) error {
+	key, err := getPodKey(pod)
+	if err != nil {
+		return err
+	}
+
+	cache.mu.Lock()
+	defer cache.mu.Unlock()
+
+	_, ok := cache.podStates[key]
+	switch {
+	// Only assumed pod can be forgotten.
+	case ok && cache.assumedPods[key]:
+		err := cache.removePod(pod)
+		if err != nil {
+			return err
+		}
+		delete(cache.podStates, key)
+	default:
+		return fmt.Errorf("pod state wasn't assumed but get forgotten. Pod key: %v", key)
+	}
+	return nil
+}
+
 func (cache *schedulerCache) AddPod(pod *api.Pod) error {
 	key, err := getPodKey(pod)
 	if err != nil {

plugin/pkg/scheduler/schedulercache/cache_test.go

@@ -106,6 +106,15 @@ func TestAssumePodScheduled(t *testing.T) {
 		}
 		n := cache.nodes[nodeName]
 		deepEqualWithoutGeneration(t, i, n, tt.wNodeInfo)
+
+		for _, pod := range tt.pods {
+			if err := cache.ForgetPod(pod); err != nil {
+				t.Fatalf("ForgetPod failed: %v", err)
+			}
+		}
+		if cache.nodes[nodeName] != nil {
+			t.Errorf("NodeInfo should be cleaned for %s", nodeName)
+		}
 	}
 }
 

plugin/pkg/scheduler/schedulercache/interface.go

@@ -36,13 +36,13 @@ import (
 //   |                                           |  |    | Update
 //   +      Assume                Add            v  v    |
 //Initial +--------> Assumed +------------+---> Added <--+
-//                      +                 |       +
+//   ^                +   +               |       +
-//                      |                 |       |
+//   |                |   |               |       |
-//                      |             Add |       | Remove
+//   |                |   |           Add |       | Remove
-//                      |                 |       |
+//   |                |   |               |       |
-//                      |                 +       |
+//   |                |   |               +       |
-//                      +-------------> Expired   +----> Deleted
+//   +----------------+   +-----------> Expired   +----> Deleted
-//                          Expire
+//         Forget             Expire
 //
 //
 // Note that an assumed pod can expire, because if we haven't received Add event notifying us
@@ -61,6 +61,9 @@ type Cache interface {
 	// After expiration, its information would be subtracted.
 	AssumePod(pod *api.Pod) error
 
+	// ForgetPod removes an assumed pod from cache.
+	ForgetPod(pod *api.Pod) error
+
 	// AddPod either confirms a pod if it's assumed, or adds it back if it's expired.
 	// If added back, the pod's information would be added again.
 	AddPod(pod *api.Pod) error

plugin/pkg/scheduler/testing/fake_cache.go

@@ -32,6 +32,8 @@ func (f *FakeCache) AssumePod(pod *api.Pod) error {
 	return nil
 }
 
+func (f *FakeCache) ForgetPod(pod *api.Pod) error { return nil }
+
 func (f *FakeCache) AddPod(pod *api.Pod) error { return nil }
 
 func (f *FakeCache) UpdatePod(oldPod, newPod *api.Pod) error { return nil }

plugin/pkg/scheduler/testing/pods_to_cache.go

@@ -25,9 +25,9 @@ import (
 // PodsToCache is used for testing
 type PodsToCache []*api.Pod
 
-func (p PodsToCache) AssumePod(pod *api.Pod) error {
+func (p PodsToCache) AssumePod(pod *api.Pod) error { return nil }
-	return nil
+
-}
+func (p PodsToCache) ForgetPod(pod *api.Pod) error { return nil }
 
 func (p PodsToCache) AddPod(pod *api.Pod) error { return nil }