PodLifeTime defaulting + moving arguments to its corresponding plugin

pkg/framework/plugins/podlifetime/pod_lifetime.go

@@ -25,7 +25,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
-	"sigs.k8s.io/descheduler/pkg/apis/componentconfig"
 	"sigs.k8s.io/descheduler/pkg/framework"
 
 	"sigs.k8s.io/descheduler/pkg/descheduler/evictions"
@@ -39,13 +38,13 @@ var _ framework.DeschedulePlugin = &PodLifeTime{}
 // PodLifeTime evicts pods on the node that violate the max pod lifetime threshold
 type PodLifeTime struct {
 	handle    framework.Handle
-	args      *componentconfig.PodLifeTimeArgs
+	args      *PodLifeTimeArgs
 	podFilter podutil.FilterFunc
 }
 
 // New builds plugin from its arguments while passing a handle
 func New(args runtime.Object, handle framework.Handle) (framework.Plugin, error) {
-	podLifeTimeArgs, ok := args.(*componentconfig.PodLifeTimeArgs)
+	podLifeTimeArgs, ok := args.(*PodLifeTimeArgs)
 	if !ok {
 		return nil, fmt.Errorf("want args to be of type PodLifeTimeArgs, got %T", args)
 	}

pkg/framework/plugins/podlifetime/pod_lifetime_test.go

@@ -28,7 +28,6 @@ import (
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/events"
-	"sigs.k8s.io/descheduler/pkg/apis/componentconfig"
 	"sigs.k8s.io/descheduler/pkg/descheduler/evictions"
 	podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
 	"sigs.k8s.io/descheduler/pkg/framework"
@@ -143,7 +142,7 @@ func TestPodLifeTime(t *testing.T) {
 	var maxLifeTime uint = 600
 	testCases := []struct {
 		description                string
-		args                       *componentconfig.PodLifeTimeArgs
+		args                       *PodLifeTimeArgs
 		pods                       []*v1.Pod
 		nodes                      []*v1.Node
 		expectedEvictedPodCount    uint
@@ -153,7 +152,7 @@ func TestPodLifeTime(t *testing.T) {
 	}{
 		{
 			description: "Two pods in the `dev` Namespace, 1 is new and 1 very is old. 1 should be evicted.",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                    []*v1.Pod{p1, p2},
@@ -162,7 +161,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "Two pods in the `dev` Namespace, 2 are new and 0 are old. 0 should be evicted.",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                    []*v1.Pod{p3, p4},
@@ -171,7 +170,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "Two pods in the `dev` Namespace, 1 created 605 seconds ago. 1 should be evicted.",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                    []*v1.Pod{p5, p6},
@@ -180,7 +179,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "Two pods in the `dev` Namespace, 1 created 595 seconds ago. 0 should be evicted.",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                    []*v1.Pod{p7, p8},
@@ -189,7 +188,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "Two pods, one with ContainerCreating state. 1 should be evicted.",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 				States:                []string{"ContainerCreating"},
 			},
@@ -212,7 +211,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "Two pods, one with PodInitializing state. 1 should be evicted.",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 				States:                []string{"PodInitializing"},
 			},
@@ -235,7 +234,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "Two old pods with different states. 1 should be evicted.",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 				States:                []string{"Pending"},
 			},
@@ -245,7 +244,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "does not evict pvc pods with ignorePvcPods set to true",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                    []*v1.Pod{p11},
@@ -255,7 +254,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "evicts pvc pods with ignorePvcPods set to false (or unset)",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                    []*v1.Pod{p11},
@@ -264,7 +263,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "No pod to evicted since all pod terminating",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 				LabelSelector: &metav1.LabelSelector{
 					MatchLabels: map[string]string{"foo": "bar"},
@@ -276,7 +275,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "No pod should be evicted since pod terminating",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 				LabelSelector: &metav1.LabelSelector{
 					MatchLabels: map[string]string{"foo": "bar"},
@@ -288,7 +287,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "2 Oldest pods should be evicted when maxPodsToEvictPerNode and maxPodsToEvictPerNamespace are not set",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                       []*v1.Pod{p1, p2, p9},
@@ -299,7 +298,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "1 Oldest pod should be evicted when maxPodsToEvictPerNamespace is set to 1",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                       []*v1.Pod{p1, p2, p9},
@@ -309,7 +308,7 @@ func TestPodLifeTime(t *testing.T) {
 		},
 		{
 			description: "1 Oldest pod should be evicted when maxPodsToEvictPerNode is set to 1",
-			args: &componentconfig.PodLifeTimeArgs{
+			args: &PodLifeTimeArgs{
 				MaxPodLifeTimeSeconds: &maxLifeTime,
 			},
 			pods:                    []*v1.Pod{p1, p2, p9},

pkg/framework/plugins/podlifetime/types.go

@@ -0,0 +1,32 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package podlifetime
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/descheduler/pkg/api"
+)
+
+// +k8s:deepcopy-gen=true
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// PodLifeTimeArgs holds arguments used to configure PodLifeTime plugin.
+type PodLifeTimeArgs struct {
+	metav1.TypeMeta
+
+	Namespaces            *api.Namespaces
+	LabelSelector         *metav1.LabelSelector
+	MaxPodLifeTimeSeconds *uint
+	States                []string
+}

pkg/framework/plugins/podlifetime/validation.go

@@ -0,0 +1,56 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package podlifetime
+
+import (
+	"fmt"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+// ValidatePodLifeTimeArgs validates PodLifeTime arguments
+func ValidatePodLifeTimeArgs(args *PodLifeTimeArgs) error {
+	if args.MaxPodLifeTimeSeconds == nil {
+		return fmt.Errorf("MaxPodLifeTimeSeconds not set")
+	}
+
+	// At most one of include/exclude can be set
+	if args.Namespaces != nil && len(args.Namespaces.Include) > 0 && len(args.Namespaces.Exclude) > 0 {
+		return fmt.Errorf("only one of Include/Exclude namespaces can be set")
+	}
+
+	if args.LabelSelector != nil {
+		if _, err := metav1.LabelSelectorAsSelector(args.LabelSelector); err != nil {
+			return fmt.Errorf("failed to get label selectors from strategy's params: %+v", err)
+		}
+	}
+	podLifeTimeAllowedStates := sets.NewString(
+		string(v1.PodRunning),
+		string(v1.PodPending),
+
+		// Container state reasons: https://github.com/kubernetes/kubernetes/blob/release-1.24/pkg/kubelet/kubelet_pods.go#L76-L79
+		"PodInitializing",
+		"ContainerCreating",
+	)
+
+	if !podLifeTimeAllowedStates.HasAll(args.States...) {
+		return fmt.Errorf("states must be one of %v", podLifeTimeAllowedStates.List())
+	}
+
+	return nil
+}

pkg/framework/plugins/podlifetime/validation_test.go

@@ -0,0 +1,64 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package podlifetime
+
+import (
+	v1 "k8s.io/api/core/v1"
+	"testing"
+)
+
+func TestValidateRemovePodLifeTimeArgs(t *testing.T) {
+	testCases := []struct {
+		description string
+		args        *PodLifeTimeArgs
+		expectError bool
+	}{
+		{
+			description: "valid arg, no errors",
+			args: &PodLifeTimeArgs{
+				MaxPodLifeTimeSeconds: func(i uint) *uint { return &i }(1),
+				States:                []string{string(v1.PodRunning)},
+			},
+			expectError: false,
+		},
+		{
+			description: "nil MaxPodLifeTimeSeconds arg, expects errors",
+			args: &PodLifeTimeArgs{
+				MaxPodLifeTimeSeconds: nil,
+			},
+			expectError: true,
+		},
+		{
+			description: "invalid pod state arg, expects errors",
+			args: &PodLifeTimeArgs{
+				States: []string{string(v1.NodeRunning)},
+			},
+			expectError: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			err := ValidatePodLifeTimeArgs(tc.args)
+
+			hasError := err != nil
+			if tc.expectError != hasError {
+				t.Error("unexpected arg validation behavior")
+			}
+		})
+	}
+}

pkg/framework/plugins/podlifetime/zz_generated.deepcopy.go

@@ -0,0 +1,73 @@
+//go:build !ignore_autogenerated
+// +build !ignore_autogenerated
+
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Code generated by deepcopy-gen. DO NOT EDIT.
+
+package podlifetime
+
+import (
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	runtime "k8s.io/apimachinery/pkg/runtime"
+	api "sigs.k8s.io/descheduler/pkg/api"
+)
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *PodLifeTimeArgs) DeepCopyInto(out *PodLifeTimeArgs) {
+	*out = *in
+	out.TypeMeta = in.TypeMeta
+	if in.Namespaces != nil {
+		in, out := &in.Namespaces, &out.Namespaces
+		*out = new(api.Namespaces)
+		(*in).DeepCopyInto(*out)
+	}
+	if in.LabelSelector != nil {
+		in, out := &in.LabelSelector, &out.LabelSelector
+		*out = new(v1.LabelSelector)
+		(*in).DeepCopyInto(*out)
+	}
+	if in.MaxPodLifeTimeSeconds != nil {
+		in, out := &in.MaxPodLifeTimeSeconds, &out.MaxPodLifeTimeSeconds
+		*out = new(uint)
+		**out = **in
+	}
+	if in.States != nil {
+		in, out := &in.States, &out.States
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodLifeTimeArgs.
+func (in *PodLifeTimeArgs) DeepCopy() *PodLifeTimeArgs {
+	if in == nil {
+		return nil
+	}
+	out := new(PodLifeTimeArgs)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
+func (in *PodLifeTimeArgs) DeepCopyObject() runtime.Object {
+	if c := in.DeepCopy(); c != nil {
+		return c
+	}
+	return nil
+}