feature(descheduler): add grace_period_seconds for DeschedulerPolicy

Author: googs1025

pkg/api/types.go

@@ -48,6 +48,12 @@ type DeschedulerPolicy struct {
 
 	// MetricsCollector configures collection of metrics about actual resource utilization
 	MetricsCollector MetricsCollector
+
+	// GracePeriodSeconds The duration in seconds before the object should be deleted. Value must be non-negative integer.
+	// The value zero indicates delete immediately. If this value is nil, the default grace period for the
+	// specified type will be used.
+	// Defaults to a per object value if not specified. zero means delete immediately.
+	GracePeriodSeconds *int64
 }
 
 // Namespaces carries a list of included/excluded namespaces

pkg/api/v1alpha2/types.go

@@ -47,6 +47,12 @@ type DeschedulerPolicy struct {
 
 	// MetricsCollector configures collection of metrics for actual resource utilization
 	MetricsCollector MetricsCollector `json:"metricsCollector,omitempty"`
+
+	// GracePeriodSeconds The duration in seconds before the object should be deleted. Value must be non-negative integer.
+	// The value zero indicates delete immediately. If this value is nil, the default grace period for the
+	// specified type will be used.
+	// Defaults to a per object value if not specified. zero means delete immediately.
+	GracePeriodSeconds *int64
 }
 
 type DeschedulerProfile struct {

pkg/api/v1alpha2/zz_generated.conversion.go

@@ -157,6 +157,7 @@ func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedu
 			WithMaxPodsToEvictPerNamespace(deschedulerPolicy.MaxNoOfPodsToEvictPerNamespace).
 			WithMaxPodsToEvictTotal(deschedulerPolicy.MaxNoOfPodsToEvictTotal).
 			WithEvictionFailureEventNotification(deschedulerPolicy.EvictionFailureEventNotification).
+			WithGracePeriodSeconds(deschedulerPolicy.GracePeriodSeconds).
 			WithDryRun(rs.DryRun).
 			WithMetricsEnabled(!rs.DisableMetrics),
 	)

pkg/api/v1alpha2/zz_generated.deepcopy.go

@@ -214,6 +214,7 @@ type PodEvictor struct {
 	maxPodsToEvictPerNode            *uint
 	maxPodsToEvictPerNamespace       *uint
 	maxPodsToEvictTotal              *uint
+	gracePeriodSeconds               *int64
 	nodePodCount                     nodePodEvictedCount
 	namespacePodCount                namespacePodEvictCount
 	totalPodCount                    uint
@@ -247,6 +248,7 @@ func NewPodEvictor(
 		maxPodsToEvictPerNode:            options.maxPodsToEvictPerNode,
 		maxPodsToEvictPerNamespace:       options.maxPodsToEvictPerNamespace,
 		maxPodsToEvictTotal:              options.maxPodsToEvictTotal,
+		gracePeriodSeconds:               options.gracePeriodSeconds,
 		metricsEnabled:                   options.metricsEnabled,
 		nodePodCount:                     make(nodePodEvictedCount),
 		namespacePodCount:                make(namespacePodEvictCount),
@@ -563,7 +565,9 @@ func (pe *PodEvictor) EvictPod(ctx context.Context, pod *v1.Pod, opts EvictOptio
 
 // return (ignore, err)
 func (pe *PodEvictor) evictPod(ctx context.Context, pod *v1.Pod) (bool, error) {
-	deleteOptions := &metav1.DeleteOptions{}
+	deleteOptions := &metav1.DeleteOptions{
+		GracePeriodSeconds: pe.gracePeriodSeconds,
+	}
 	// GracePeriodSeconds ?
 	eviction := &policy.Eviction{
 		TypeMeta: metav1.TypeMeta{

pkg/api/zz_generated.deepcopy.go

@@ -12,6 +12,7 @@ type Options struct {
 	maxPodsToEvictTotal              *uint
 	evictionFailureEventNotification bool
 	metricsEnabled                   bool
+	gracePeriodSeconds               *int64
 }
 
 // NewOptions returns an Options with default values.
@@ -46,6 +47,11 @@ func (o *Options) WithMaxPodsToEvictTotal(maxPodsToEvictTotal *uint) *Options {
 	return o
 }
 
+func (o *Options) WithGracePeriodSeconds(gracePeriodSeconds *int64) *Options {
+	o.gracePeriodSeconds = gracePeriodSeconds
+	return o
+}
+
 func (o *Options) WithMetricsEnabled(metricsEnabled bool) *Options {
 	o.metricsEnabled = metricsEnabled
 	return o

pkg/descheduler/descheduler.go

@@ -43,7 +43,7 @@ import (
 
 const deploymentReplicas = 4
 
-func tooManyRestartsPolicy(targetNamespace string, podRestartThresholds int32, includingInitContainers bool) *apiv1alpha2.DeschedulerPolicy {
+func tooManyRestartsPolicy(targetNamespace string, podRestartThresholds int32, includingInitContainers bool, gracePeriodSeconds int64) *apiv1alpha2.DeschedulerPolicy {
 	return &apiv1alpha2.DeschedulerPolicy{
 		Profiles: []apiv1alpha2.DeschedulerProfile{
 			{
@@ -84,6 +84,7 @@ func tooManyRestartsPolicy(targetNamespace string, podRestartThresholds int32, i
 				},
 			},
 		},
+		GracePeriodSeconds: &gracePeriodSeconds,
 	}
 }
 
@@ -127,16 +128,23 @@ func TestTooManyRestarts(t *testing.T) {
 	tests := []struct {
 		name                    string
 		policy                  *apiv1alpha2.DeschedulerPolicy
+		enableGracePeriod       bool
 		expectedEvictedPodCount uint
 	}{
 		{
 			name:                    "test-no-evictions",
-			policy:                  tooManyRestartsPolicy(testNamespace.Name, 10000, true),
+			policy:                  tooManyRestartsPolicy(testNamespace.Name, 10000, true, 0),
 			expectedEvictedPodCount: 0,
 		},
 		{
 			name:                    "test-one-evictions",
-			policy:                  tooManyRestartsPolicy(testNamespace.Name, 3, true),
+			policy:                  tooManyRestartsPolicy(testNamespace.Name, 3, true, 0),
+			expectedEvictedPodCount: 4,
+		},
+		{
+			name:                    "test-one-evictions-use-gracePeriodSeconds",
+			policy:                  tooManyRestartsPolicy(testNamespace.Name, 3, true, 60),
+			enableGracePeriod:       true,
 			expectedEvictedPodCount: 4,
 		},
 	}
@@ -197,8 +205,32 @@ func TestTooManyRestarts(t *testing.T) {
 				deschedulerPodName = deschedulerPods[0].Name
 			}
 
+			// Check if grace period is enabled and wait accordingly
+			if tc.enableGracePeriod {
+				// Ensure no pods are evicted during the grace period
+				// Wait for 55 seconds to ensure that the pods are not evicted during the grace period
+				// We do not want to use an extreme waiting time, such as 59 seconds,
+				// because the grace period is set to 60 seconds.
+				// In order to avoid unnecessary flake failures,
+				// we only need to make sure that the pod is not evicted within a certain range.
+				t.Logf("Waiting for grace period of %d seconds", 55)
+				if err := wait.PollUntilContextTimeout(ctx, 1*time.Second, time.Duration(55)*time.Second, true, func(ctx context.Context) (bool, error) {
+					currentRunNames := sets.NewString(getCurrentPodNames(ctx, clientSet, testNamespace.Name, t)...)
+					actualEvictedPod := preRunNames.Difference(currentRunNames)
+					actualEvictedPodCount := uint(actualEvictedPod.Len())
+					t.Logf("preRunNames: %v, currentRunNames: %v, actualEvictedPodCount: %v\n", preRunNames.List(), currentRunNames.List(), actualEvictedPodCount)
+					if actualEvictedPodCount > 0 {
+						t.Fatalf("Pods were evicted during grace period; expected 0, got %v", actualEvictedPodCount)
+						return false, nil
+					}
+					return true, nil
+				}); err != nil {
+					t.Fatalf("Error waiting during grace period: %v", err)
+				}
+			}
+
 			// Run RemovePodsHavingTooManyRestarts strategy
-			if err := wait.PollUntilContextTimeout(ctx, 1*time.Second, 20*time.Second, true, func(ctx context.Context) (bool, error) {
+			if err := wait.PollUntilContextTimeout(ctx, 1*time.Second, 30*time.Second, true, func(ctx context.Context) (bool, error) {
 				currentRunNames := sets.NewString(getCurrentPodNames(ctx, clientSet, testNamespace.Name, t)...)
 				actualEvictedPod := preRunNames.Difference(currentRunNames)
 				actualEvictedPodCount := uint(actualEvictedPod.Len())
@@ -210,7 +242,7 @@ func TestTooManyRestarts(t *testing.T) {
 
 				return true, nil
 			}); err != nil {
-				t.Errorf("Error waiting for descheduler running: %v", err)
+				t.Fatalf("Error waiting for descheduler running: %v", err)
 			}
 			waitForTerminatingPodsToDisappear(ctx, t, clientSet, testNamespace.Name)
 		})