Skip to content

Commit

Permalink
feature(descheduler): add grace_period_seconds for DeschedulerPolicy
Browse files Browse the repository at this point in the history
  • Loading branch information
googs1025 committed Dec 23, 2024
1 parent 29ff28c commit 65ac965
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 5 deletions.
6 changes: 6 additions & 0 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions pkg/api/v1alpha2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/v1alpha2/zz_generated.conversion.go

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

5 changes: 5 additions & 0 deletions pkg/api/v1alpha2/zz_generated.deepcopy.go

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

5 changes: 5 additions & 0 deletions pkg/api/zz_generated.deepcopy.go

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

1 change: 1 addition & 0 deletions pkg/descheduler/descheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down
6 changes: 5 additions & 1 deletion pkg/descheduler/evictions/evictions.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ type PodEvictor struct {
maxPodsToEvictPerNode *uint
maxPodsToEvictPerNamespace *uint
maxPodsToEvictTotal *uint
gracePeriodSeconds *int64
nodePodCount nodePodEvictedCount
namespacePodCount namespacePodEvictCount
totalPodCount uint
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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{
Expand Down
6 changes: 6 additions & 0 deletions pkg/descheduler/evictions/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Options struct {
maxPodsToEvictTotal *uint
evictionFailureEventNotification bool
metricsEnabled bool
gracePeriodSeconds *int64
}

// NewOptions returns an Options with default values.
Expand Down Expand Up @@ -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
Expand Down
35 changes: 31 additions & 4 deletions test/e2e/e2e_toomanyrestarts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -84,6 +84,7 @@ func tooManyRestartsPolicy(targetNamespace string, podRestartThresholds int32, i
},
},
},
GracePeriodSeconds: &gracePeriodSeconds,
}
}

Expand Down Expand Up @@ -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,
},
}
Expand Down Expand Up @@ -197,6 +205,25 @@ 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
t.Logf("Waiting for grace period of %d seconds", 60)
if err := wait.PollUntilContextTimeout(ctx, 1*time.Second, time.Duration(60)*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) {
currentRunNames := sets.NewString(getCurrentPodNames(ctx, clientSet, testNamespace.Name, t)...)
Expand All @@ -210,7 +237,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)
})
Expand Down

0 comments on commit 65ac965

Please sign in to comment.