diff --git a/apis/config/scheme/scheme_test.go b/apis/config/scheme/scheme_test.go index 2aefb0998..c126e2788 100644 --- a/apis/config/scheme/scheme_test.go +++ b/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 diff --git a/apis/config/types.go b/apis/config/types.go index 605d4b329..7ae1ce40d 100644 --- a/apis/config/types.go +++ b/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. diff --git a/apis/config/v1beta2/zz_generated.conversion.go b/apis/config/v1beta2/zz_generated.conversion.go index 5b1a19258..7a6fe93fe 100644 --- a/apis/config/v1beta2/zz_generated.conversion.go +++ b/apis/config/v1beta2/zz_generated.conversion.go @@ -127,9 +127,6 @@ func autoConvert_v1beta2_CoschedulingArgs_To_config_CoschedulingArgs(in *Cosched if err := v1.Convert_Pointer_int64_To_int64(&in.PermitWaitingTimeSeconds, &out.PermitWaitingTimeSeconds, s); err != nil { return err } - if err := v1.Convert_Pointer_int64_To_int64(&in.DeniedPGExpirationTimeSeconds, &out.DeniedPGExpirationTimeSeconds, s); err != nil { - return err - } return nil } @@ -142,9 +139,6 @@ func autoConvert_config_CoschedulingArgs_To_v1beta2_CoschedulingArgs(in *config. if err := v1.Convert_int64_To_Pointer_int64(&in.PermitWaitingTimeSeconds, &out.PermitWaitingTimeSeconds, s); err != nil { return err } - if err := v1.Convert_int64_To_Pointer_int64(&in.DeniedPGExpirationTimeSeconds, &out.DeniedPGExpirationTimeSeconds, s); err != nil { - return err - } return nil } diff --git a/apis/config/v1beta3/defaults.go b/apis/config/v1beta3/defaults.go index 0125f0093..0e81326a6 100644 --- a/apis/config/v1beta3/defaults.go +++ b/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. diff --git a/apis/config/v1beta3/defaults_test.go b/apis/config/v1beta3/defaults_test.go index 5e956f09b..8c7bb4c2a 100644 --- a/apis/config/v1beta3/defaults_test.go +++ b/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), }, }, { diff --git a/apis/config/v1beta3/types.go b/apis/config/v1beta3/types.go index 6317a962e..6eda44881 100644 --- a/apis/config/v1beta3/types.go +++ b/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". diff --git a/apis/config/v1beta3/zz_generated.conversion.go b/apis/config/v1beta3/zz_generated.conversion.go index 7f7901aca..7b7150326 100644 --- a/apis/config/v1beta3/zz_generated.conversion.go +++ b/apis/config/v1beta3/zz_generated.conversion.go @@ -127,9 +127,6 @@ func autoConvert_v1beta3_CoschedulingArgs_To_config_CoschedulingArgs(in *Cosched if err := v1.Convert_Pointer_int64_To_int64(&in.PermitWaitingTimeSeconds, &out.PermitWaitingTimeSeconds, s); err != nil { return err } - if err := v1.Convert_Pointer_int64_To_int64(&in.DeniedPGExpirationTimeSeconds, &out.DeniedPGExpirationTimeSeconds, s); err != nil { - return err - } return nil } @@ -142,9 +139,6 @@ func autoConvert_config_CoschedulingArgs_To_v1beta3_CoschedulingArgs(in *config. if err := v1.Convert_int64_To_Pointer_int64(&in.PermitWaitingTimeSeconds, &out.PermitWaitingTimeSeconds, s); err != nil { return err } - if err := v1.Convert_int64_To_Pointer_int64(&in.DeniedPGExpirationTimeSeconds, &out.DeniedPGExpirationTimeSeconds, s); err != nil { - return err - } return nil } diff --git a/apis/config/v1beta3/zz_generated.deepcopy.go b/apis/config/v1beta3/zz_generated.deepcopy.go index 2f61f5f24..867595d4b 100644 --- a/apis/config/v1beta3/zz_generated.deepcopy.go +++ b/apis/config/v1beta3/zz_generated.deepcopy.go @@ -36,11 +36,6 @@ func (in *CoschedulingArgs) DeepCopyInto(out *CoschedulingArgs) { *out = new(int64) **out = **in } - if in.DeniedPGExpirationTimeSeconds != nil { - in, out := &in.DeniedPGExpirationTimeSeconds, &out.DeniedPGExpirationTimeSeconds - *out = new(int64) - **out = **in - } return } diff --git a/pkg/coscheduling/core/core.go b/pkg/coscheduling/core/core.go index c86ca9ab2..d4b94d1a6 100644 --- a/pkg/coscheduling/core/core.go +++ b/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) diff --git a/pkg/coscheduling/core/core_test.go b/pkg/coscheduling/core/core_test.go index 38a1b651b..55e68dfae 100644 --- a/pkg/coscheduling/core/core_test.go +++ b/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") diff --git a/pkg/coscheduling/coscheduling.go b/pkg/coscheduling/coscheduling.go index 6e72e7ad1..a1f9aa9c7 100644 --- a/pkg/coscheduling/coscheduling.go +++ b/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) } diff --git a/pkg/coscheduling/coscheduling_test.go b/pkg/coscheduling/coscheduling_test.go index 7b86d9c9d..f7a95087a 100644 --- a/pkg/coscheduling/coscheduling_test.go +++ b/pkg/coscheduling/coscheduling_test.go @@ -88,8 +88,7 @@ func TestLess(t *testing.T) { existingPods, allNodes := testutil.MakeNodesAndPods(map[string]string{"test": "a"}, 60, 30) snapshot := testutil.NewFakeSharedLister(existingPods, allNodes) - scheudleDuration := 10 * time.Second - deniedPGExpirationTime := 3 * time.Second + scheduleDuration := 10 * time.Second var lowPriority, highPriority = int32(10), int32(100) ns1, ns2 := "namespace1", "namespace2" for _, tt := range []struct { @@ -257,7 +256,7 @@ func TestLess(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - pgMgr := core.NewPodGroupManager(cs, snapshot, &scheudleDuration, &deniedPGExpirationTime, pgInformer, podInformer) + pgMgr := core.NewPodGroupManager(cs, snapshot, &scheduleDuration, pgInformer, podInformer) coscheduling := &Coscheduling{pgMgr: pgMgr} if got := coscheduling.Less(tt.p1, tt.p2); got != tt.expected { t.Errorf("expected %v, got %v", tt.expected, got) @@ -318,12 +317,11 @@ func TestPermit(t *testing.T) { if err != nil { t.Fatal(err) } - scheudleDuration := 10 * time.Second - deniedPGExpirationTime := 3 * time.Second + scheduleDuration := 10 * time.Second for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - pgMgr := core.NewPodGroupManager(cs, snapshot, &scheudleDuration, &deniedPGExpirationTime, pgInformer, podInformer) - coscheduling := &Coscheduling{pgMgr: pgMgr, frameworkHandler: f, scheduleTimeout: &scheudleDuration} + pgMgr := core.NewPodGroupManager(cs, snapshot, &scheduleDuration, pgInformer, podInformer) + coscheduling := &Coscheduling{pgMgr: pgMgr, frameworkHandler: f, scheduleTimeout: &scheduleDuration} code, _ := coscheduling.Permit(context.Background(), framework.NewCycleState(), tt.pod, "test") if code.Code() != tt.expected { t.Errorf("expected %v, got %v", tt.expected, code.Code()) @@ -369,7 +367,6 @@ func TestPostFilter(t *testing.T) { } groupPodSnapshot := testutil.NewFakeSharedLister(existingPods, allNodes) scheduleDuration := 10 * time.Second - deniedPGExpirationTime := 3 * time.Second tests := []struct { name string pod *v1.Pod @@ -402,7 +399,7 @@ func TestPostFilter(t *testing.T) { mgrSnapShot = tt.snapshotSharedLister } - pgMgr := core.NewPodGroupManager(cs, mgrSnapShot, &scheduleDuration, &deniedPGExpirationTime, pgInformer, podInformer) + pgMgr := core.NewPodGroupManager(cs, mgrSnapShot, &scheduleDuration, pgInformer, podInformer) coscheduling := &Coscheduling{pgMgr: pgMgr, frameworkHandler: f, scheduleTimeout: &scheduleDuration} _, code := coscheduling.PostFilter(context.Background(), cycleState, tt.pod, nodeStatusMap) if code.Message() == "" != tt.expectedEmptyMsg {