Skip to content
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

skip eviction when pod creation time is below minPodAge threshold setting #1475

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ The Default Evictor Plugin is used by default for filtering pods before processi
|`priorityThreshold`|`priorityThreshold`||(see [priority filtering](#priority-filtering))|
|`nodeFit`|`bool`|`false`|(see [node fit filtering](#node-fit-filtering))|
|`minReplicas`|`uint`|`0`| ignore eviction of pods where owner (e.g. `ReplicaSet`) replicas is below this threshold |
|`minPodAge`|`uint`|`0`| ignore eviction of pods with a creation time within this threshold (in minutes) |
|`minPodAge`|`metav1.Duration`|`0`| ignore eviction of pods with a creation time within this threshold (in minutes) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"in minutes" can be dropped

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a8502e4 ty!


### Example policy

Expand Down
1 change: 0 additions & 1 deletion pkg/api/v1alpha1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package v1alpha1
import (
"context"
"fmt"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please undone the new line removal? To keep this file unchanged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 61b7b99

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/apimachinery/pkg/runtime"
Expand Down
3 changes: 0 additions & 3 deletions pkg/api/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ type DeschedulerPolicy struct {

// MaxNoOfPodsToTotal restricts maximum of pods to be evicted total.
MaxNoOfPodsToEvictTotal *uint `json:"maxNoOfPodsToEvictTotal,omitempty"`

// MinPodAge prevents recently created pods (within the specified minutes) from being evicted.
MinPodAge *uint `json:"minPodAge,omitempty"`
}

type (
Expand Down
6 changes: 3 additions & 3 deletions pkg/framework/plugins/defaultevictor/defaultevictor.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,10 @@ func New(args runtime.Object, handle frameworktypes.Handle) (frameworktypes.Plug
})
}

if defaultEvictorArgs.MinPodAge > 0 {
if defaultEvictorArgs.MinPodAge != nil {
ev.constraints = append(ev.constraints, func(pod *v1.Pod) error {
if pod.Status.StartTime == nil || time.Since(pod.Status.StartTime.Time) < time.Duration(defaultEvictorArgs.MinPodAge)*time.Minute {
return fmt.Errorf("pod age is not older than MinPodAge: %d minutes", defaultEvictorArgs.MinPodAge)
if pod.Status.StartTime == nil || time.Since(pod.Status.StartTime.Time) < defaultEvictorArgs.MinPodAge.Duration {
return fmt.Errorf("pod age is not older than MinPodAge: %d seconds", uint(defaultEvictorArgs.MinPodAge.Duration.Seconds()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duration.String() will help to keep the value short and redeable when user chooses high duration. https://pkg.go.dev/time#Duration.String

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9adea9f

}
return nil
})
Expand Down
22 changes: 17 additions & 5 deletions pkg/framework/plugins/defaultevictor/defaultevictor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type testCase struct {
priorityThreshold *int32
nodeFit bool
minReplicas uint
minPodAge uint
minPodAge *metav1.Duration
result bool
}

Expand Down Expand Up @@ -329,6 +329,8 @@ func TestDefaultEvictorFilter(t *testing.T) {
lowPriority := int32(800)
highPriority := int32(900)

minPodAge := metav1.Duration{50 * time.Minute}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkg/framework/plugins/defaultevictor/defaultevictor_test.go:332:15: k8s.io/apimachinery/pkg/apis/meta/v1.Duration struct literal uses unkeyed fields

metav1.Duration{Duration: 50 * time.Minute}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c7de3c4


nodeTaintKey := "hardware"
nodeTaintValue := "gpu"

Expand Down Expand Up @@ -705,15 +707,15 @@ func TestDefaultEvictorFilter(t *testing.T) {
minReplicas: 2,
result: true,
}, {
description: "minPodAge of 50, pod created 1 minute ago, no eviction",
description: "minPodAge of 50, pod created 10 minutes ago, no eviction",
pods: []*v1.Pod{
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
podStartTime := metav1.Now().Add(time.Minute * time.Duration(-1))
podStartTime := metav1.Now().Add(time.Minute * time.Duration(-10))
pod.Status.StartTime = &metav1.Time{Time: podStartTime}
}),
},
minPodAge: 50,
minPodAge: &minPodAge,
result: false,
}, {
description: "minPodAge of 50, pod created 60 minutes ago, evicts",
Expand All @@ -724,8 +726,18 @@ func TestDefaultEvictorFilter(t *testing.T) {
pod.Status.StartTime = &metav1.Time{Time: podStartTime}
}),
},
minPodAge: 50,
minPodAge: &minPodAge,
result: true,
}, {
description: "nil minPodAge, pod created 60 minutes ago, evicts",
pods: []*v1.Pod{
test.BuildTestPod("p1", 1, 1, n1.Name, func(pod *v1.Pod) {
pod.ObjectMeta.OwnerReferences = test.GetNormalPodOwnerRefList()
podStartTime := metav1.Now().Add(time.Minute * time.Duration(-60))
pod.Status.StartTime = &metav1.Time{Time: podStartTime}
}),
},
result: true,
},
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/framework/plugins/defaultevictor/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ type DefaultEvictorArgs struct {
PriorityThreshold *api.PriorityThreshold `json:"priorityThreshold"`
NodeFit bool `json:"nodeFit"`
MinReplicas uint `json:"minReplicas"`
MinPodAge uint `json:"minPodAge"`
MinPodAge *metav1.Duration `json:"minPodAge"`
}
5 changes: 5 additions & 0 deletions pkg/framework/plugins/defaultevictor/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.