Merge pull request #834 from a7i/podlifetime-container-state

PodLifeTime: support container states PodInitializing and ContainerCreating

README.md

@@ -524,15 +524,17 @@ strategies:
 
 This strategy evicts pods that are older than `maxPodLifeTimeSeconds`.
 
-You can also specify `podStatusPhases` to `only` evict pods with specific `StatusPhases`, currently this parameter is limited
-to `Running` and `Pending`.
+You can also specify `states` parameter to **only** evict pods matching the following conditions:
+  - [Pod Phase](https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-phase) status of: `Running`, `Pending`
+  - [Container State Waiting](https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#container-state-waiting) condition of: `PodInitializing`, `ContainerCreating`
 
 **Parameters:**
 
 |Name|Type|
 |---|---|
 |`maxPodLifeTimeSeconds`|int|
-|`podStatusPhases`|list(string)|
+|`podStatusPhases` (**Deprecated**. Use `states` instead)|list(string)|
+|`states`|list(string)|
 |`thresholdPriority`|int (see [priority filtering](#priority-filtering))|
 |`thresholdPriorityClassName`|string (see [priority filtering](#priority-filtering))|
 |`namespaces`|(see [namespace filtering](#namespace-filtering))|
@@ -549,8 +551,9 @@ strategies:
      params:
        podLifeTime:
          maxPodLifeTimeSeconds: 86400
-         podStatusPhases:
+         states:
          - "Pending"
+         - "PodInitializing"
 ```
 
 ### RemoveFailedPods

examples/pod-life-time.yml

@@ -6,3 +6,6 @@ strategies:
     params:
       podLifeTime:
         maxPodLifeTimeSeconds: 604800 # 7 days
+        states:
+        - "Pending"
+        - "PodInitializing"

pkg/api/types.go

@@ -113,7 +113,10 @@ type RemoveDuplicates struct {
 
 type PodLifeTime struct {
 	MaxPodLifeTimeSeconds *uint
-	PodStatusPhases       []string
+	States                []string
+
+	// Deprecated: Use States instead.
+	PodStatusPhases []string
 }
 
 type FailedPods struct {

pkg/api/v1alpha1/types.go

@@ -111,7 +111,10 @@ type RemoveDuplicates struct {
 
 type PodLifeTime struct {
 	MaxPodLifeTimeSeconds *uint    `json:"maxPodLifeTimeSeconds,omitempty"`
-	PodStatusPhases       []string `json:"podStatusPhases,omitempty"`
+	States                []string `json:"states,omitempty"`
+
+	// Deprecated: Use States instead.
+	PodStatusPhases []string `json:"podStatusPhases,omitempty"`
 }
 
 type FailedPods struct {

pkg/api/v1alpha1/zz_generated.conversion.go

@@ -288,6 +288,7 @@ func Convert_api_NodeResourceUtilizationThresholds_To_v1alpha1_NodeResourceUtili
 
 func autoConvert_v1alpha1_PodLifeTime_To_api_PodLifeTime(in *PodLifeTime, out *api.PodLifeTime, s conversion.Scope) error {
 	out.MaxPodLifeTimeSeconds = (*uint)(unsafe.Pointer(in.MaxPodLifeTimeSeconds))
+	out.States = *(*[]string)(unsafe.Pointer(&in.States))
 	out.PodStatusPhases = *(*[]string)(unsafe.Pointer(&in.PodStatusPhases))
 	return nil
 }
@@ -299,6 +300,7 @@ func Convert_v1alpha1_PodLifeTime_To_api_PodLifeTime(in *PodLifeTime, out *api.P
 
 func autoConvert_api_PodLifeTime_To_v1alpha1_PodLifeTime(in *api.PodLifeTime, out *PodLifeTime, s conversion.Scope) error {
 	out.MaxPodLifeTimeSeconds = (*uint)(unsafe.Pointer(in.MaxPodLifeTimeSeconds))
+	out.States = *(*[]string)(unsafe.Pointer(&in.States))
 	out.PodStatusPhases = *(*[]string)(unsafe.Pointer(&in.PodStatusPhases))
 	return nil
 }

pkg/api/v1alpha1/zz_generated.deepcopy.go

@@ -209,6 +209,11 @@ func (in *PodLifeTime) DeepCopyInto(out *PodLifeTime) {
 		*out = new(uint)
 		**out = **in
 	}
+	if in.States != nil {
+		in, out := &in.States, &out.States
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	if in.PodStatusPhases != nil {
 		in, out := &in.PodStatusPhases, &out.PodStatusPhases
 		*out = make([]string, len(*in))

pkg/api/zz_generated.deepcopy.go

@@ -209,6 +209,11 @@ func (in *PodLifeTime) DeepCopyInto(out *PodLifeTime) {
 		*out = new(uint)
 		**out = **in
 	}
+	if in.States != nil {
+		in, out := &in.States, &out.States
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	if in.PodStatusPhases != nil {
 		in, out := &in.PodStatusPhases, &out.PodStatusPhases
 		*out = make([]string, len(*in))

pkg/descheduler/strategies/pod_lifetime.go

@@ -31,33 +31,48 @@ import (
 	podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
 )
 
-func validatePodLifeTimeParams(params *api.StrategyParameters) error {
+var (
+	podLifeTimeAllowedStates = sets.NewString(
+		string(v1.PodRunning),
+		string(v1.PodPending),
+
+		// Container state reason list: https://github.com/kubernetes/kubernetes/blob/release-1.24/pkg/kubelet/kubelet_pods.go#L76-L79
+		"PodInitializing",
+		"ContainerCreating",
+	)
+)
+
+func validatePodLifeTimeParams(params *api.StrategyParameters) (sets.String, error) {
 	if params == nil || params.PodLifeTime == nil || params.PodLifeTime.MaxPodLifeTimeSeconds == nil {
-		return fmt.Errorf("MaxPodLifeTimeSeconds not set")
+		return nil, fmt.Errorf("MaxPodLifeTimeSeconds not set")
 	}
 
+	var states []string
 	if params.PodLifeTime.PodStatusPhases != nil {
-		for _, phase := range params.PodLifeTime.PodStatusPhases {
-			if phase != string(v1.PodPending) && phase != string(v1.PodRunning) {
-				return fmt.Errorf("only Pending and Running phases are supported in PodLifeTime")
-			}
-		}
+		states = append(states, params.PodLifeTime.PodStatusPhases...)
+	}
+	if params.PodLifeTime.States != nil {
+		states = append(states, params.PodLifeTime.States...)
+	}
+	if !podLifeTimeAllowedStates.HasAll(states...) {
+		return nil, fmt.Errorf("states must be one of %v", podLifeTimeAllowedStates.List())
 	}
 
 	// At most one of include/exclude can be set
 	if params.Namespaces != nil && len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
-		return fmt.Errorf("only one of Include/Exclude namespaces can be set")
+		return nil, fmt.Errorf("only one of Include/Exclude namespaces can be set")
 	}
 	if params.ThresholdPriority != nil && params.ThresholdPriorityClassName != "" {
-		return fmt.Errorf("only one of thresholdPriority and thresholdPriorityClassName can be set")
+		return nil, fmt.Errorf("only one of thresholdPriority and thresholdPriorityClassName can be set")
 	}
 
-	return nil
+	return sets.NewString(states...), nil
 }
 
 // PodLifeTime evicts pods on nodes that were created more than strategy.Params.MaxPodLifeTimeSeconds seconds ago.
 func PodLifeTime(ctx context.Context, client clientset.Interface, strategy api.DeschedulerStrategy, nodes []*v1.Node, podEvictor *evictions.PodEvictor, evictorFilter *evictions.EvictorFilter, getPodsAssignedToNode podutil.GetPodsAssignedToNodeFunc) {
-	if err := validatePodLifeTimeParams(strategy.Params); err != nil {
+	states, err := validatePodLifeTimeParams(strategy.Params)
+	if err != nil {
 		klog.ErrorS(err, "Invalid PodLifeTime parameters")
 		return
 	}
@@ -69,15 +84,20 @@ func PodLifeTime(ctx context.Context, client clientset.Interface, strategy api.D
 	}
 
 	filter := evictorFilter.Filter
-	if strategy.Params.PodLifeTime.PodStatusPhases != nil {
-		filter = func(pod *v1.Pod) bool {
-			for _, phase := range strategy.Params.PodLifeTime.PodStatusPhases {
-				if string(pod.Status.Phase) == phase {
-					return evictorFilter.Filter(pod)
+	if states.Len() > 0 {
+		filter = podutil.WrapFilterFuncs(func(pod *v1.Pod) bool {
+			if states.Has(string(pod.Status.Phase)) {
+				return true
+			}
+
+			for _, containerStatus := range pod.Status.ContainerStatuses {
+				if containerStatus.State.Waiting != nil && states.Has(containerStatus.State.Waiting.Reason) {
+					return true
 				}
 			}
+
 			return false
-		}
+		}, filter)
 	}
 
 	podFilter, err := podutil.NewOptions().

pkg/descheduler/strategies/pod_lifetime_test.go

@@ -197,13 +197,69 @@ func TestPodLifeTime(t *testing.T) {
 			expectedEvictedPodCount: 0,
 		},
 		{
-			description: "Two old pods with different status phases. 1 should be evicted.",
+			description: "Two pods, one with ContainerCreating state. 1 should be evicted.",
 			strategy: api.DeschedulerStrategy{
 				Enabled: true,
 				Params: &api.StrategyParameters{
 					PodLifeTime: &api.PodLifeTime{
 						MaxPodLifeTimeSeconds: &maxLifeTime,
-						PodStatusPhases:       []string{"Pending"},
+						States:                []string{"ContainerCreating"},
+					},
+				},
+			},
+			pods: []*v1.Pod{
+				p9,
+				test.BuildTestPod("container-creating-stuck", 0, 0, node1.Name, func(pod *v1.Pod) {
+					pod.Status.ContainerStatuses = []v1.ContainerStatus{
+						{
+							State: v1.ContainerState{
+								Waiting: &v1.ContainerStateWaiting{Reason: "ContainerCreating"},
+							},
+						},
+					}
+					pod.OwnerReferences = ownerRef1
+					pod.ObjectMeta.CreationTimestamp = olderPodCreationTime
+				}),
+			},
+			nodes:                   []*v1.Node{node1},
+			expectedEvictedPodCount: 1,
+		},
+		{
+			description: "Two pods, one with PodInitializing state. 1 should be evicted.",
+			strategy: api.DeschedulerStrategy{
+				Enabled: true,
+				Params: &api.StrategyParameters{
+					PodLifeTime: &api.PodLifeTime{
+						MaxPodLifeTimeSeconds: &maxLifeTime,
+						States:                []string{"PodInitializing"},
+					},
+				},
+			},
+			pods: []*v1.Pod{
+				p9,
+				test.BuildTestPod("pod-initializing-stuck", 0, 0, node1.Name, func(pod *v1.Pod) {
+					pod.Status.ContainerStatuses = []v1.ContainerStatus{
+						{
+							State: v1.ContainerState{
+								Waiting: &v1.ContainerStateWaiting{Reason: "PodInitializing"},
+							},
+						},
+					}
+					pod.OwnerReferences = ownerRef1
+					pod.ObjectMeta.CreationTimestamp = olderPodCreationTime
+				}),
+			},
+			nodes:                   []*v1.Node{node1},
+			expectedEvictedPodCount: 1,
+		},
+		{
+			description: "Two old pods with different states. 1 should be evicted.",
+			strategy: api.DeschedulerStrategy{
+				Enabled: true,
+				Params: &api.StrategyParameters{
+					PodLifeTime: &api.PodLifeTime{
+						MaxPodLifeTimeSeconds: &maxLifeTime,
+						States:                []string{"Pending"},
 					},
 				},
 			},