Skip to content

Commit

Permalink
skip eviction when pod creation time is below minPodAge threshold set…
Browse files Browse the repository at this point in the history
…ting (#1475)

* skip eviction when pod creation time is below minPodAge threshold setting

In the default initialization phase of the descheduler, add a new
constraint to not evict pods that creation time is below minPodAge
threshold.

Added value:

- Avoid crazy pod movement when the autoscaler scales up and down.

- Avoid evicting pods when they are warming up.

- Decreases the overall cost of eviction as no pod will be evicted
  before doing significant amount of work.

- Guard against scheduling. Descheduling loops in situations where
  the descheduler has a different node fit logic from scheduler,
  like not considering topology spread constraints.

* Use *time.Duration instead of uint for MinPodAge type

* Remove '(in minutes)' from default evictor configuration table

* make fmt

* Add explicit name for Duration field

* Use Duration.String()
  • Loading branch information
victorgs authored Jul 26, 2024
1 parent f3569b5 commit 55a0812
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +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`|`metav1.Duration`|`0`| ignore eviction of pods with a creation time within this threshold |

### Example policy

Expand Down
10 changes: 10 additions & 0 deletions pkg/framework/plugins/defaultevictor/defaultevictor.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"errors"
"fmt"
"time"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -64,6 +65,7 @@ func HaveEvictAnnotation(pod *v1.Pod) bool {
}

// New builds plugin from its arguments while passing a handle
// nolint: gocyclo
func New(args runtime.Object, handle frameworktypes.Handle) (frameworktypes.Plugin, error) {
defaultEvictorArgs, ok := args.(*DefaultEvictorArgs)
if !ok {
Expand Down Expand Up @@ -185,6 +187,14 @@ func New(args runtime.Object, handle frameworktypes.Handle) (frameworktypes.Plug
})
}

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) < defaultEvictorArgs.MinPodAge.Duration {
return fmt.Errorf("pod age is not older than MinPodAge: %s seconds", defaultEvictorArgs.MinPodAge.String())
}
return nil
})
}
return ev, nil
}

Expand Down
39 changes: 39 additions & 0 deletions pkg/framework/plugins/defaultevictor/defaultevictor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import (
"context"
"fmt"
"testing"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand All @@ -42,6 +45,7 @@ type testCase struct {
priorityThreshold *int32
nodeFit bool
minReplicas uint
minPodAge *metav1.Duration
result bool
}

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

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

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

Expand Down Expand Up @@ -701,6 +707,38 @@ func TestDefaultEvictorFilter(t *testing.T) {
},
minReplicas: 2,
result: true,
}, {
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(-10))
pod.Status.StartTime = &metav1.Time{Time: podStartTime}
}),
},
minPodAge: &minPodAge,
result: false,
}, {
description: "minPodAge of 50, 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}
}),
},
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 Expand Up @@ -797,6 +835,7 @@ func initializePlugin(ctx context.Context, test testCase) (frameworktypes.Plugin
},
NodeFit: test.nodeFit,
MinReplicas: test.minReplicas,
MinPodAge: test.minPodAge,
}

evictorPlugin, err := New(
Expand Down
1 change: 1 addition & 0 deletions pkg/framework/plugins/defaultevictor/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@ type DefaultEvictorArgs struct {
PriorityThreshold *api.PriorityThreshold `json:"priorityThreshold"`
NodeFit bool `json:"nodeFit"`
MinReplicas uint `json:"minReplicas"`
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.

0 comments on commit 55a0812

Please sign in to comment.