Add time based drainability rule for non-pdb-assigned system pods

Author: abdelrahman882

cluster-autoscaler/simulator/drain.go

@@ -46,9 +46,10 @@ func GetPodsToMove(nodeInfo *framework.NodeInfo, deleteOptions options.NodeDelet
 		remainingPdbTracker = pdb.NewBasicRemainingPdbTracker()
 	}
 	drainCtx := &drainability.DrainContext{
-		RemainingPdbTracker: remainingPdbTracker,
-		Listers:             listers,
-		Timestamp:           timestamp,
+		RemainingPdbTracker:  remainingPdbTracker,
+		Listers:              listers,
+		Timestamp:            timestamp,
+		BspChosenNodeToEvict: "",
 	}
 	for _, podInfo := range nodeInfo.Pods() {
 		pod := podInfo.Pod

cluster-autoscaler/simulator/drain_test.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/autoscaler/cluster-autoscaler/core/scaledown/pdb"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules"
+	system_rules "k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules/system"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/framework"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/options"
 	"k8s.io/autoscaler/cluster-autoscaler/utils/drain"
@@ -43,7 +44,10 @@ import (
 
 func TestGetPodsToMove(t *testing.T) {
 	var (
-		testTime = time.Date(2020, time.December, 18, 17, 0, 0, 0, time.UTC)
+		testTime                                = time.Date(2020, time.December, 18, 17, 0, 0, 0, time.UTC)
+		creationTimeBeforeBspDisturptionTimeout = testTime.Add(-system_rules.BspDisruptionTimeout).Add(-time.Minute)
+		creationTimeAfterBspDisturptionTimeout  = testTime.Add(-system_rules.BspDisruptionTimeout).Add(time.Minute)
+
 		replicas = int32(5)
 
 		unreplicatedPod = &apiv1.Pod{
@@ -68,6 +72,22 @@ func TestGetPodsToMove(t *testing.T) {
 				OwnerReferences: GenerateOwnerReferences("rs", "ReplicaSet", "extensions/v1beta1", ""),
 			},
 		}
+		drainableBlockingSystemPod = &apiv1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:              "systemPod",
+				Namespace:         "kube-system",
+				OwnerReferences:   GenerateOwnerReferences("rs", "ReplicaSet", "extensions/v1beta1", ""),
+				CreationTimestamp: metav1.Time{Time: creationTimeBeforeBspDisturptionTimeout},
+			},
+		}
+		nonDrainableBlockingSystemPod = &apiv1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:              "systemPod",
+				Namespace:         "kube-system",
+				OwnerReferences:   GenerateOwnerReferences("rs", "ReplicaSet", "extensions/v1beta1", ""),
+				CreationTimestamp: metav1.Time{Time: creationTimeAfterBspDisturptionTimeout},
+			},
+		}
 		localStoragePod = &apiv1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:            "localStoragePod",
@@ -541,6 +561,28 @@ func TestGetPodsToMove(t *testing.T) {
 				Reason: drain.UnmovableKubeSystemPod,
 			},
 		},
+		{
+			desc:    "Kube-system no pdb system pods blocking",
+			pods:    []*apiv1.Pod{nonDrainableBlockingSystemPod},
+			wantErr: true,
+			wantBlocking: &drain.BlockingPod{
+				Pod:    nonDrainableBlockingSystemPod,
+				Reason: drain.UnmovableKubeSystemPod,
+			}},
+		{
+			desc:     "Kube-system no pdb system pods allowing",
+			pods:     []*apiv1.Pod{drainableBlockingSystemPod},
+			wantPods: []*apiv1.Pod{drainableBlockingSystemPod},
+		},
+		{
+			desc:    "Kube-system no pdb system pods blocking",
+			pods:    []*apiv1.Pod{drainableBlockingSystemPod, nonDrainableBlockingSystemPod},
+			wantErr: true,
+			wantBlocking: &drain.BlockingPod{
+				Pod:    nonDrainableBlockingSystemPod,
+				Reason: drain.UnmovableKubeSystemPod,
+			},
+		},
 		{
 			desc:    "Local storage",
 			pods:    []*apiv1.Pod{localStoragePod},

cluster-autoscaler/simulator/drainability/context.go

@@ -25,7 +25,8 @@ import (
 
 // DrainContext contains parameters for drainability rules.
 type DrainContext struct {
-	RemainingPdbTracker pdb.RemainingPdbTracker
-	Listers             kube_util.ListerRegistry
-	Timestamp           time.Time
+	RemainingPdbTracker  pdb.RemainingPdbTracker
+	Listers              kube_util.ListerRegistry
+	Timestamp            time.Time
+	BspChosenNodeToEvict string
 }

cluster-autoscaler/simulator/drainability/rules/system/rule.go

@@ -18,13 +18,20 @@ package system
 
 import (
 	"fmt"
+	"time"
 
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/framework"
 	"k8s.io/autoscaler/cluster-autoscaler/utils/drain"
 )
 
+const (
+	// BspDisruptionTimeout is the timeout afterwhich CA will evict non-pdb-assigned blocking system pods
+	BspDisruptionTimeout = time.Hour
+	KubeSystemNamespace  = "kube-system"
+)
+
 // Rule is a drainability rule on how to handle system pods.
 type Rule struct{}
 
@@ -40,8 +47,25 @@ func (r *Rule) Name() string {
 
 // Drainable decides what to do with system pods on node drain.
 func (r *Rule) Drainable(drainCtx *drainability.DrainContext, pod *apiv1.Pod, _ *framework.NodeInfo) drainability.Status {
-	if pod.Namespace == "kube-system" && len(drainCtx.RemainingPdbTracker.MatchingPdbs(pod)) == 0 {
+	if isBlockingSystemPod(drainCtx, pod) {
+		if isBspPassedDisruptionTimeout(pod, drainCtx.Timestamp) && isChosenNode(pod, drainCtx.BspChosenNodeToEvict) {
+			drainCtx.BspChosenNodeToEvict = pod.Spec.NodeName
+			return drainability.NewDrainableStatus()
+		}
 		return drainability.NewBlockedStatus(drain.UnmovableKubeSystemPod, fmt.Errorf("non-daemonset, non-mirrored, non-pdb-assigned kube-system pod present: %s", pod.Name))
 	}
 	return drainability.NewUndefinedStatus()
 }
+
+func isBlockingSystemPod(drainCtx *drainability.DrainContext, pod *apiv1.Pod) bool {
+	return pod.Namespace == KubeSystemNamespace && len(drainCtx.RemainingPdbTracker.MatchingPdbs(pod)) == 0
+}
+
+func isBspPassedDisruptionTimeout(pod *apiv1.Pod, drainTime time.Time) bool {
+	return !pod.ObjectMeta.CreationTimestamp.IsZero() &&
+		drainTime.After(pod.ObjectMeta.CreationTimestamp.Add(BspDisruptionTimeout))
+}
+
+func isChosenNode(pod *apiv1.Pod, bspChosenNodeToEvict string) bool {
+	return bspChosenNodeToEvict == "" || pod.Spec.NodeName == bspChosenNodeToEvict
+}

cluster-autoscaler/simulator/drainability/rules/system/rule_test.go

@@ -34,7 +34,10 @@ import (
 
 func TestDrainable(t *testing.T) {
 	var (
-		testTime = time.Date(2020, time.December, 18, 17, 0, 0, 0, time.UTC)
+		testTime                                = time.Date(2020, time.December, 18, 17, 0, 0, 0, time.UTC)
+		creationTimeBeforeBspDisturptionTimeout = testTime.Add(-BspDisruptionTimeout).Add(-time.Minute)
+		creationTimeAfterBspDisturptionTimeout  = testTime.Add(-BspDisruptionTimeout).Add(time.Minute)
+
 		replicas = int32(5)
 
 		rc = apiv1.ReplicationController{
@@ -84,6 +87,24 @@ func TestDrainable(t *testing.T) {
 			},
 		}
 
+		drainableBlockingSystemPod = &apiv1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:              "systemPod",
+				Namespace:         "kube-system",
+				OwnerReferences:   test.GenerateOwnerReferences("rs", "ReplicaSet", "extensions/v1beta1", ""),
+				CreationTimestamp: metav1.Time{Time: creationTimeBeforeBspDisturptionTimeout},
+			},
+		}
+
+		nonDrainableBlockingSystemPod = &apiv1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:              "systemPod",
+				Namespace:         "kube-system",
+				OwnerReferences:   test.GenerateOwnerReferences("rs", "ReplicaSet", "extensions/v1beta1", ""),
+				CreationTimestamp: metav1.Time{Time: creationTimeAfterBspDisturptionTimeout},
+			},
+		}
+
 		emptyPDB = &policyv1.PodDisruptionBudget{}
 
 		kubeSystemPDB = &policyv1.PodDisruptionBudget{
@@ -164,6 +185,18 @@ func TestDrainable(t *testing.T) {
 			wantReason: drain.UnmovableKubeSystemPod,
 			wantError:  true,
 		},
+		"block non-pdb system pod existing for less than BspDisruptionTimeout": {
+			pod:        nonDrainableBlockingSystemPod,
+			rcs:        []*apiv1.ReplicationController{&kubeSystemRc},
+			pdbs:       []*policyv1.PodDisruptionBudget{emptyPDB},
+			wantReason: drain.UnmovableKubeSystemPod,
+			wantError:  true,
+		},
+		"allow non-pdb system pod existing for more than BspDisruptionTimeout": {
+			pod:  drainableBlockingSystemPod,
+			rcs:  []*apiv1.ReplicationController{&kubeSystemRc},
+			pdbs: []*policyv1.PodDisruptionBudget{kubeSystemPDB},
+		},
 	} {
 		t.Run(desc, func(t *testing.T) {
 			tracker := pdb.NewBasicRemainingPdbTracker()