Merge pull request #411 from yibozhuang/release-1.23

Cherry-pick coscheduling: remove lastDeniedPG cache and lastDeniedPGExpirationTime

apis/config/scheme/scheme_test.go

@@ -103,8 +103,7 @@ profiles:
 						{
 							Name: coscheduling.Name,
 							Args: &config.CoschedulingArgs{
-								PermitWaitingTimeSeconds:      10,
-								DeniedPGExpirationTimeSeconds: 3,
+								PermitWaitingTimeSeconds: 10,
 							},
 						},
 						{
@@ -214,8 +213,7 @@ profiles:
 						{
 							Name: coscheduling.Name,
 							Args: &config.CoschedulingArgs{
-								PermitWaitingTimeSeconds:      60,
-								DeniedPGExpirationTimeSeconds: 20,
+								PermitWaitingTimeSeconds: 60,
 							},
 						},
 						{
@@ -356,8 +354,7 @@ func TestCodecsEncodePluginConfig(t *testing.T) {
 							{
 								Name: coscheduling.Name,
 								Args: &config.CoschedulingArgs{
-									PermitWaitingTimeSeconds:      10,
-									DeniedPGExpirationTimeSeconds: 3,
+									PermitWaitingTimeSeconds: 10,
 								},
 							},
 							{
@@ -430,7 +427,6 @@ profiles:
 - pluginConfig:
   - args:
       apiVersion: kubescheduler.config.k8s.io/v1beta2
-      deniedPGExpirationTimeSeconds: 3
       kind: CoschedulingArgs
       permitWaitingTimeSeconds: 10
     name: Coscheduling

apis/config/types.go

@@ -30,8 +30,6 @@ type CoschedulingArgs struct {
 
 	// PermitWaitingTimeSeconds is the waiting timeout in seconds.
 	PermitWaitingTimeSeconds int64
-	// DeniedPGExpirationTimeSeconds is the expiration time of the denied podgroup.
-	DeniedPGExpirationTimeSeconds int64
 }
 
 // ModeType is a "string" type.

apis/config/v1beta3/defaults.go

@@ -26,8 +26,7 @@ import (
 )
 
 var (
-	defaultPermitWaitingTimeSeconds      int64 = 60
-	defaultDeniedPGExpirationTimeSeconds int64 = 20
+	defaultPermitWaitingTimeSeconds int64 = 60
 
 	defaultNodeResourcesAllocatableMode = Least
 
@@ -78,9 +77,6 @@ func SetDefaults_CoschedulingArgs(obj *CoschedulingArgs) {
 	if obj.PermitWaitingTimeSeconds == nil {
 		obj.PermitWaitingTimeSeconds = &defaultPermitWaitingTimeSeconds
 	}
-	if obj.DeniedPGExpirationTimeSeconds == nil {
-		obj.DeniedPGExpirationTimeSeconds = &defaultDeniedPGExpirationTimeSeconds
-	}
 }
 
 // SetDefaults_NodeResourcesAllocatableArgs sets the defaults parameters for NodeResourceAllocatable.

apis/config/v1beta3/defaults_test.go

@@ -39,19 +39,16 @@ func TestSchedulingDefaults(t *testing.T) {
 			name:   "empty config CoschedulingArgs",
 			config: &CoschedulingArgs{},
 			expect: &CoschedulingArgs{
-				PermitWaitingTimeSeconds:      pointer.Int64Ptr(60),
-				DeniedPGExpirationTimeSeconds: pointer.Int64Ptr(20),
+				PermitWaitingTimeSeconds: pointer.Int64Ptr(60),
 			},
 		},
 		{
 			name: "set non default CoschedulingArgs",
 			config: &CoschedulingArgs{
-				PermitWaitingTimeSeconds:      pointer.Int64Ptr(60),
-				DeniedPGExpirationTimeSeconds: pointer.Int64Ptr(10),
+				PermitWaitingTimeSeconds: pointer.Int64Ptr(60),
 			},
 			expect: &CoschedulingArgs{
-				PermitWaitingTimeSeconds:      pointer.Int64Ptr(60),
-				DeniedPGExpirationTimeSeconds: pointer.Int64Ptr(10),
+				PermitWaitingTimeSeconds: pointer.Int64Ptr(60),
 			},
 		},
 		{

apis/config/v1beta3/types.go

@@ -30,9 +30,6 @@ type CoschedulingArgs struct {
 
 	// PermitWaitingTimeSeconds is the waiting timeout in seconds.
 	PermitWaitingTimeSeconds *int64 `json:"permitWaitingTimeSeconds,omitempty"`
-
-	// DeniedPGExpirationTimeSeconds is the expiration time of the denied podgroup store.
-	DeniedPGExpirationTimeSeconds *int64 `json:"deniedPGExpirationTimeSeconds,omitempty"`
 }
 
 // ModeType is a type "string".

pkg/coscheduling/core/core.go

@@ -59,7 +59,6 @@ type Manager interface {
 	PostBind(context.Context, *corev1.Pod, string)
 	GetPodGroup(*corev1.Pod) (string, *v1alpha1.PodGroup)
 	GetCreationTimestamp(*corev1.Pod, time.Time) time.Time
-	AddDeniedPodGroup(string)
 	DeletePermittedPodGroup(string)
 	CalculateAssignedPods(string, string) int
 	ActivateSiblings(pod *corev1.Pod, state *framework.CycleState)
@@ -74,13 +73,8 @@ type PodGroupManager struct {
 	// scheduleTimeout is the default timeout for podgroup scheduling.
 	// If podgroup's scheduleTimeoutSeconds is set, it will be used.
 	scheduleTimeout *time.Duration
-	// lastDeniedPG stores the podgroup name if a pod can not pass pre-filer,
-	// or times out
-	lastDeniedPG *gochache.Cache
 	// permittedPG stores the podgroup name which has passed the pre resource check.
 	permittedPG *gochache.Cache
-	// deniedCacheExpirationTime is the expiration time that the podgroup remains in lastDeniedPG store.
-	lastDeniedPGExpirationTime *time.Duration
 	// pgLister is podgroup lister
 	pgLister pglister.PodGroupLister
 	// podLister is pod lister
@@ -91,17 +85,15 @@ type PodGroupManager struct {
 }
 
 // NewPodGroupManager creates a new operation object.
-func NewPodGroupManager(pgClient pgclientset.Interface, snapshotSharedLister framework.SharedLister, scheduleTimeout, deniedPGExpirationTime *time.Duration,
+func NewPodGroupManager(pgClient pgclientset.Interface, snapshotSharedLister framework.SharedLister, scheduleTimeout *time.Duration,
 	pgInformer pginformer.PodGroupInformer, podInformer informerv1.PodInformer) *PodGroupManager {
 	pgMgr := &PodGroupManager{
-		pgClient:                   pgClient,
-		snapshotSharedLister:       snapshotSharedLister,
-		scheduleTimeout:            scheduleTimeout,
-		lastDeniedPGExpirationTime: deniedPGExpirationTime,
-		pgLister:                   pgInformer.Lister(),
-		podLister:                  podInformer.Lister(),
-		lastDeniedPG:               gochache.New(3*time.Second, 3*time.Second),
-		permittedPG:                gochache.New(3*time.Second, 3*time.Second),
+		pgClient:             pgClient,
+		snapshotSharedLister: snapshotSharedLister,
+		scheduleTimeout:      scheduleTimeout,
+		pgLister:             pgInformer.Lister(),
+		podLister:            podInformer.Lister(),
+		permittedPG:          gochache.New(3*time.Second, 3*time.Second),
 	}
 	return pgMgr
 }
@@ -152,9 +144,7 @@ func (pgMgr *PodGroupManager) PreFilter(ctx context.Context, pod *corev1.Pod) er
 	if pg == nil {
 		return nil
 	}
-	if _, ok := pgMgr.lastDeniedPG.Get(pgFullName); ok {
-		return fmt.Errorf("pod with pgName: %v last failed in %v, deny", pgFullName, *pgMgr.lastDeniedPGExpirationTime)
-	}
+
 	pods, err := pgMgr.podLister.Pods(pod.Namespace).List(
 		labels.SelectorFromSet(labels.Set{v1alpha1.PodGroupLabel: util.GetPodGroupLabel(pod)}),
 	)
@@ -188,7 +178,6 @@ func (pgMgr *PodGroupManager) PreFilter(ctx context.Context, pod *corev1.Pod) er
 	err = CheckClusterResource(nodes, minResources, pgFullName)
 	if err != nil {
 		klog.ErrorS(err, "Failed to PreFilter", "podGroup", klog.KObj(pg))
-		pgMgr.AddDeniedPodGroup(pgFullName)
 		return err
 	}
 	pgMgr.permittedPG.Add(pgFullName, pgFullName, *pgMgr.scheduleTimeout)
@@ -264,11 +253,6 @@ func (pgMgr *PodGroupManager) GetCreationTimestamp(pod *corev1.Pod, ts time.Time
 	return pg.CreationTimestamp.Time
 }
 
-// AddDeniedPodGroup adds a podGroup that fails to be scheduled to a PodGroup cache with expiration time.
-func (pgMgr *PodGroupManager) AddDeniedPodGroup(pgFullName string) {
-	pgMgr.lastDeniedPG.Add(pgFullName, "", *pgMgr.lastDeniedPGExpirationTime)
-}
-
 // DeletePermittedPodGroup deletes a podGroup that passes Pre-Filter but reaches PostFilter.
 func (pgMgr *PodGroupManager) DeletePermittedPodGroup(pgFullName string) {
 	pgMgr.permittedPG.Delete(pgFullName)

pkg/coscheduling/core/core_test.go

@@ -56,14 +56,11 @@ func TestPreFilter(t *testing.T) {
 	pgInformer.Informer().GetStore().Add(pg2)
 	pgInformer.Informer().GetStore().Add(pg3)
 	pgLister := pgInformer.Lister()
-	denyCache := newCache()
-	denyCache.SetDefault("ns1/pg1", "ns1/pg1")
 
 	tests := []struct {
 		name            string
 		pod             *corev1.Pod
 		pods            []*corev1.Pod
-		lastDeniedPG    *gochache.Cache
 		expectedSuccess bool
 	}{
 		{
@@ -73,19 +70,11 @@ func TestPreFilter(t *testing.T) {
 				st.MakePod().Name("pg1-1").UID("pg1-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg1").Obj(),
 				st.MakePod().Name("pg2-1").UID("pg2-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg2").Obj(),
 			},
-			lastDeniedPG:    newCache(),
 			expectedSuccess: true,
 		},
-		{
-			name:            "pg was previously denied",
-			pod:             st.MakePod().Name("p1").UID("p1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg1").Obj(),
-			lastDeniedPG:    denyCache,
-			expectedSuccess: false,
-		},
 		{
 			name:            "pod belongs to a non-existing pg",
 			pod:             st.MakePod().Name("p2").UID("p2").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg-notexisting").Obj(),
-			lastDeniedPG:    newCache(),
 			expectedSuccess: true,
 		},
 		{
@@ -94,7 +83,6 @@ func TestPreFilter(t *testing.T) {
 			pods: []*corev1.Pod{
 				st.MakePod().Name("pg2-1").UID("pg2-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg2").Obj(),
 			},
-			lastDeniedPG:    newCache(),
 			expectedSuccess: false,
 		},
 		{
@@ -104,7 +92,6 @@ func TestPreFilter(t *testing.T) {
 				st.MakePod().Name("pg1-1").UID("pg1-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg1").Obj(),
 				st.MakePod().Name("pg2-1").UID("pg2-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg1").Obj(),
 			},
-			lastDeniedPG:    newCache(),
 			expectedSuccess: true,
 		},
 		{
@@ -115,7 +102,6 @@ func TestPreFilter(t *testing.T) {
 				st.MakePod().Name("pg2-1").UID("pg2-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg1").Obj(),
 				st.MakePod().Name("pg3-1").UID("pg3-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg1").Obj(),
 			},
-			lastDeniedPG:    newCache(),
 			expectedSuccess: true,
 		},
 		{
@@ -126,7 +112,6 @@ func TestPreFilter(t *testing.T) {
 				st.MakePod().Name("pg1-1").UID("pg1-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg2").Obj(),
 				st.MakePod().Name("pg2-1").UID("pg2-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg2").Obj(),
 			},
-			lastDeniedPG:    newCache(),
 			expectedSuccess: true,
 		},
 		{
@@ -137,7 +122,6 @@ func TestPreFilter(t *testing.T) {
 				st.MakePod().Name("pg1-1").UID("pg1-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg3").Obj(),
 				st.MakePod().Name("pg2-1").UID("pg2-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg3").Obj(),
 			},
-			lastDeniedPG:    newCache(),
 			expectedSuccess: false,
 		},
 		{
@@ -147,7 +131,6 @@ func TestPreFilter(t *testing.T) {
 				st.MakePod().Name("pg1-1").UID("pg1-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg1").Obj(),
 				st.MakePod().Name("pg2-1").UID("pg2-1").Namespace("ns1").Label(v1alpha1.PodGroupLabel, "pg1").Obj(),
 			},
-			lastDeniedPG:    newCache(),
 			expectedSuccess: true,
 		},
 	}
@@ -158,8 +141,8 @@ func TestPreFilter(t *testing.T) {
 			podInformer := informerFactory.Core().V1().Pods()
 			existingPods, allNodes := testutil.MakeNodesAndPods(map[string]string{"test": "a"}, 60, 30)
 			snapshot := testutil.NewFakeSharedLister(existingPods, allNodes)
-			pgMgr := &PodGroupManager{pgLister: pgLister, lastDeniedPG: tt.lastDeniedPG, permittedPG: newCache(),
-				snapshotSharedLister: snapshot, podLister: podInformer.Lister(), scheduleTimeout: &scheduleTimeout, lastDeniedPGExpirationTime: &scheduleTimeout}
+			pgMgr := &PodGroupManager{pgLister: pgLister, permittedPG: newCache(),
+				snapshotSharedLister: snapshot, podLister: podInformer.Lister(), scheduleTimeout: &scheduleTimeout}
 			informerFactory.Start(ctx.Done())
 			if !clicache.WaitForCacheSync(ctx.Done(), podInformer.Informer().HasSynced) {
 				t.Fatal("WaitForCacheSync failed")

pkg/coscheduling/coscheduling.go

@@ -71,11 +71,10 @@ func New(obj runtime.Object, handle framework.Handle) (framework.Plugin, error)
 	podInformer := handle.SharedInformerFactory().Core().V1().Pods()
 
 	scheduleTimeDuration := time.Duration(args.PermitWaitingTimeSeconds) * time.Second
-	deniedPGExpirationTime := time.Duration(args.DeniedPGExpirationTimeSeconds) * time.Second
 
 	ctx := context.TODO()
 
-	pgMgr := core.NewPodGroupManager(pgClient, handle.SnapshotSharedLister(), &scheduleTimeDuration, &deniedPGExpirationTime, pgInformer, podInformer)
+	pgMgr := core.NewPodGroupManager(pgClient, handle.SnapshotSharedLister(), &scheduleTimeDuration, pgInformer, podInformer)
 	plugin := &Coscheduling{
 		frameworkHandler: handle,
 		pgMgr:            pgMgr,
@@ -169,7 +168,6 @@ func (cs *Coscheduling) PostFilter(ctx context.Context, state *framework.CycleSt
 			waitingPod.Reject(cs.Name(), "optimistic rejection in PostFilter")
 		}
 	})
-	cs.pgMgr.AddDeniedPodGroup(pgName)
 	cs.pgMgr.DeletePermittedPodGroup(pgName)
 	return &framework.PostFilterResult{}, framework.NewStatus(framework.Unschedulable,
 		fmt.Sprintf("PodGroup %v gets rejected due to Pod %v is unschedulable even after PostFilter", pgName, pod.Name))
@@ -232,7 +230,6 @@ func (cs *Coscheduling) Unreserve(ctx context.Context, state *framework.CycleSta
 			waitingPod.Reject(cs.Name(), "rejection in Unreserve")
 		}
 	})
-	cs.pgMgr.AddDeniedPodGroup(pgName)
 	cs.pgMgr.DeletePermittedPodGroup(pgName)
 }