Repository: gardener/machine-controller-manager (go)

Issue:
Reduce load on kube-apiserver / ETCD on pod eviction for termination machines
**How to categorize this issue?**
<!--
Please select area, kind, and priority for this issue. This helps the community categorizing it.
Replace below TODOs or exchange the existing identifiers with those that fit best in your opinion.
If multiple identifiers make sense you can also state the commands multiple times, e.g.
  /area control-plane
  /area auto-scaling
  ...

"/area" identifiers:     audit-logging|auto-scaling|backup|certification|control-plane-migration|control-plane|cost|delivery|dev-productivity|disaster-recovery|documentation|high-availability|logging|metering|monitoring|networking|open-source|ops-productivity|os|performance|quality|robustness|scalability|security|storage|testing|usability|user-management
"/kind" identifiers:     api-change|bug|cleanup|discussion|enhancement|epic|impediment|poc|post-mortem|question|regression|task|technical-debt|test
"/priority" identifiers: 1|2|3|4|5 (ordered from greatest to least)
-->
/area robustness
/kind bug
/priority 3

**What happened**:

On rolling nodes of a large cluster with about 100 nodes, the max surge was limited to 10%. The termination of a node was waiting for workload to end. Therefore the rolling took more than 12 hours. This means during this time there have been about 10 machines in state `Terminating`. The machine controller manager tried to evict pods on these nodes with high frequency and produced a lot of load on the kube-apiservers and the ETCD.

![image](https://user-images.githubusercontent.com/3703277/163144060-ceb43863-0b3b-49f7-bc3f-79e30704b90b.png)

In this image you see the client out traffic of the ETCD. Between 15:20 and 15:30 we scaled down the machine-controller-manager and the traffic was reduced immediately.

In the logs of the machine-controller-manager we found lots of throttling messages. Here I show only the eviction call for a single pod (name redacted):

```txt
I0412 15:29:36.845890       1 request.go:591] Throttling request took 542.507996ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:29:48.512932       1 request.go:591] Throttling request took 290.637231ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:29:59.562728       1 request.go:591] Throttling request took 338.986248ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:30:09.012570       1 request.go:591] Throttling request took 419.676173ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:30:19.270585       1 request.go:591] Throttling request took 355.943286ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:30:40.369073       1 request.go:591] Throttling request took 251.448724ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:30:51.019605       1 request.go:591] Throttling request took 329.512125ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:31:00.169268       1 request.go:591] Throttling request took 596.340432ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:31:05.519530       1 request.go:591] Throttling request took 181.186876ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:31:14.868701       1 request.go:591] Throttling request took 545.271603ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:31:23.119681       1 request.go:591] Throttling request took 388.854376ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:31:31.669300       1 request.go:591] Throttling request took 246.905561ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:31:40.369208       1 request.go:591] Throttling request took 374.552037ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:31:44.791522       1 request.go:591] Throttling request took 93.770177ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
I0412 15:31:59.091661       1 request.go:591] Throttling request took 495.377239ms, request: POST:https://kube-apiserver/api/v1/namespaces/ws-xxxxx/pods/xxxx-deployment-78864894dd-2xdfd/eviction
```

The eviction request is repeated every 5 to 10 seconds.

**What you expected to happen**:

The machine controller manager should reduce the frequency of pod eviction calls if the termination of a machine takes a long time.

**How to reproduce it (as minimally and precisely as possible)**:

**Anything else we need to know?**:

Internal reference:  see live issue #1570

**Environment**:

- Kubernetes version (use `kubectl version`):
- Cloud provider or hardware configuration:
- Others:

/cc @kon-angelo 

Pull request:
Reduce load on etcd/kube-apiserver on pod eviction
**What this PR does / why we need it**:

The PR changes the way pods are listed before eviction, by using a `PodInformer` which uses a local cache rather than directly querying the kube-apiserver/etcd. The pods are listed only after the cache has synced. 

**Which issue(s) this PR fixes**:
Fixes #703 


**Special notes for your reviewer**:

Impact of changes tested by running pods  configuring a PDB with `maxUnavailable` set to 0, on a cluster with 20 machines, and deleting all the machines, thereby initiating drain. Without change, peak traffic recorded was 8.51 MB/s. With change, peak traffic recorded was 1.48 MB/s. 

<img width="1628" alt="Screenshot 2024-10-21 at 1 10 05 PM" src="https://github.com/user-attachments/assets/89766b6f-4b2c-4efb-8740-8a08fdd3537d">

Integration tests run for providers AWS and Azure completed successfully.

Additionally, to manually check if cache for `podInformer` syncs successfully before the `podLister.List()` call,`time.Sleep( 30 * time.Second)` was introduced before calling `RunCordonOrUncordon()`, and a new pod (default/nginx-pod2 ) was deployed during the sleep period. Logs were added to print the names of all pods on the node returned by the podLister.  After triggering the deletion of the machine, the machine entered the drain flow, and after the sleep the new pod's name was logged.

```
I1023 16:13:08.044770   91923 machine_util.go:1182] (drainNode) Invoking RunDrain, forceDeleteMachine: false, forceDeletePods: false, timeOutDuration: 5m0s
I1023 16:13:08.044894   91923 drain.go:238] ABOUT TO SLEEP for 30s
I1023 16:13:43.861554   91923 drain.go:369] Found pod default/nginx-pod2 
```

**Release note**:
<!--  Write your release note:
1. Enter your release note in the below block.
2. If no release note is required, just write "NONE" within the block.

Format of block header: <category> <target_group>
Possible values:
- category:       breaking|feature|bugfix|doc|other
- target_group:   user|operator|developer|dependency
-->
```other operator
MCM will use an `informer` instead of the`clientset` to list pods in the drain logic. This will reduce the load on etcd/kube-apiserver.
```


Patch:
diff --git a/pkg/util/provider/app/app.go b/pkg/util/provider/app/app.go
index b9781634e..904a986c5 100644
--- a/pkg/util/provider/app/app.go
+++ b/pkg/util/provider/app/app.go
@@ -275,6 +275,7 @@ func StartControllers(s *options.MCServer,
 		targetCoreInformerFactory.Core().V1().PersistentVolumes(),
 		controlCoreInformerFactory.Core().V1().Secrets(),
 		targetCoreInformerFactory.Core().V1().Nodes(),
+		targetCoreInformerFactory.Core().V1().Pods(),
 		pdbInformer,
 		targetCoreInformerFactory.Storage().V1().VolumeAttachments(),
 		machineSharedInformers.MachineClasses(),
diff --git a/pkg/util/provider/drain/drain.go b/pkg/util/provider/drain/drain.go
index a711fdcff..dcf660851 100644
--- a/pkg/util/provider/drain/drain.go
+++ b/pkg/util/provider/drain/drain.go
@@ -27,6 +27,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/client-go/tools/cache"
 	"regexp"
 	"sort"
 	"strings"
@@ -39,7 +41,6 @@ import (
 	storagev1 "k8s.io/api/storage/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/fields"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
@@ -72,8 +73,10 @@ type Options struct {
 	pvLister                     corelisters.PersistentVolumeLister
 	pdbLister                    policyv1listers.PodDisruptionBudgetLister
 	nodeLister                   corelisters.NodeLister
+	podLister                    corelisters.PodLister
 	volumeAttachmentHandler      *VolumeAttachmentHandler
 	Timeout                      time.Duration
+	podSynced                    cache.InformerSynced
 }
 
 // Takes a pod and returns a bool indicating whether or not to operate on the
@@ -181,7 +184,9 @@ func NewDrainOptions(
 	pvLister corelisters.PersistentVolumeLister,
 	pdbLister policyv1listers.PodDisruptionBudgetLister,
 	nodeLister corelisters.NodeLister,
+	podLister corelisters.PodLister,
 	volumeAttachmentHandler *VolumeAttachmentHandler,
+	podSynced cache.InformerSynced,
 ) *Options {
 	return &Options{
 		client:                       client,
@@ -203,7 +208,9 @@ func NewDrainOptions(
 		pvLister:                     pvLister,
 		pdbLister:                    pdbLister,
 		nodeLister:                   nodeLister,
+		podLister:                    podLister,
 		volumeAttachmentHandler:      volumeAttachmentHandler,
+		podSynced:                    podSynced,
 	}
 }
 
@@ -232,20 +239,24 @@ func (o *Options) RunDrain(ctx context.Context) error {
 		klog.Errorf("Drain Error: Cordoning of node failed with error: %v", err)
 		return err
 	}
+	if !cache.WaitForCacheSync(drainContext.Done(), o.podSynced) {
+		err := fmt.Errorf("timed out waiting for pod cache to sync")
+		return err
+	}
 
 	err := o.deleteOrEvictPodsSimple(drainContext)
 	return err
 }
 
 func (o *Options) deleteOrEvictPodsSimple(ctx context.Context) error {
-	pods, err := o.getPodsForDeletion(ctx)
+	pods, err := o.getPodsForDeletion()
 	if err != nil {
 		return err
 	}
 
 	err = o.deleteOrEvictPods(ctx, pods)
 	if err != nil {
-		pendingPods, newErr := o.getPodsForDeletion(ctx)
+		pendingPods, newErr := o.getPodsForDeletion()
 		if newErr != nil {
 			return newErr
 		}
@@ -337,21 +348,25 @@ func (ps podStatuses) Message() string {
 
 // getPodsForDeletion returns all the pods we're going to delete.  If there are
 // any pods preventing us from deleting, we return that list in an error.
-func (o *Options) getPodsForDeletion(ctx context.Context) (pods []corev1.Pod, err error) {
-	podList, err := o.client.CoreV1().Pods(metav1.NamespaceAll).List(ctx, metav1.ListOptions{
-		FieldSelector: fields.SelectorFromSet(fields.Set{"spec.nodeName": o.nodeName}).String()})
+func (o *Options) getPodsForDeletion() (pods []corev1.Pod, err error) {
+	podList, err := o.podLister.List(labels.Everything())
 	if err != nil {
-		return pods, err
+		return
+	}
+	if len(podList) == 0 {
+		klog.Infof("no pods found in store")
+		return
 	}
-
 	ws := podStatuses{}
 	fs := podStatuses{}
 
-	for _, pod := range podList.Items {
+	for _, pod := range podList {
+		if pod.Spec.NodeName != o.nodeName {
+			continue
+		}
 		podOk := true
 		for _, filt := range []podFilter{mirrorPodFilter, o.localStorageFilter, o.unreplicatedFilter, o.daemonsetFilter} {
-			filterOk, w, f := filt(pod)
-
+			filterOk, w, f := filt(*pod)
 			podOk = podOk && filterOk
 			if w != nil {
 				ws[w.string] = append(ws[w.string], pod.Name)
@@ -361,7 +376,7 @@ func (o *Options) getPodsForDeletion(ctx context.Context) (pods []corev1.Pod, er
 			}
 		}
 		if podOk {
-			pods = append(pods, pod)
+			pods = append(pods, *pod)
 		}
 	}
 
diff --git a/pkg/util/provider/drain/fake_controller.go b/pkg/util/provider/drain/fake_controller.go
index 550dc775a..44f82e13e 100644
--- a/pkg/util/provider/drain/fake_controller.go
+++ b/pkg/util/provider/drain/fake_controller.go
@@ -24,6 +24,8 @@ func createFakeController(
 	corelisters.PersistentVolumeLister,
 	corelisters.PersistentVolumeClaimLister,
 	corelisters.NodeLister,
+	corelisters.PodLister,
+	func() bool,
 	func() bool,
 	func() bool,
 	func() bool,
@@ -45,14 +47,17 @@ func createFakeController(
 	pvcs := coreTargetSharedInformers.PersistentVolumeClaims()
 	pvs := coreTargetSharedInformers.PersistentVolumes()
 	nodes := coreTargetSharedInformers.Nodes()
+	pods := coreTargetSharedInformers.Pods()
 
 	pvcLister := pvcs.Lister()
 	pvLister := pvs.Lister()
 	nodeLister := nodes.Lister()
+	podLister := pods.Lister()
 
 	pvcSynced := pvcs.Informer().HasSynced
 	pvSynced := pvs.Informer().HasSynced
 	nodeSynced := nodes.Informer().HasSynced
+	podSynced := pods.Informer().HasSynced
 
-	return fakeTargetCoreClient, pvLister, pvcLister, nodeLister, pvcSynced, pvSynced, nodeSynced, targetCoreObjectTracker
+	return fakeTargetCoreClient, pvLister, pvcLister, nodeLister, podLister, pvcSynced, pvSynced, nodeSynced, podSynced, targetCoreObjectTracker
 }
diff --git a/pkg/util/provider/machinecontroller/controller.go b/pkg/util/provider/machinecontroller/controller.go
index 97fa3e8db..7eb26d22f 100644
--- a/pkg/util/provider/machinecontroller/controller.go
+++ b/pkg/util/provider/machinecontroller/controller.go
@@ -65,6 +65,7 @@ func NewController(
 	pvInformer coreinformers.PersistentVolumeInformer,
 	secretInformer coreinformers.SecretInformer,
 	nodeInformer coreinformers.NodeInformer,
+	podInformer coreinformers.PodInformer,
 	pdbInformer policyv1informers.PodDisruptionBudgetInformer,
 	volumeAttachmentInformer storageinformers.VolumeAttachmentInformer,
 	machineClassInformer machineinformers.MachineClassInformer,
@@ -124,6 +125,7 @@ func NewController(
 	controller.machineClassLister = machineClassInformer.Lister()
 	controller.nodeLister = nodeInformer.Lister()
 	controller.machineLister = machineInformer.Lister()
+	controller.podLister = podInformer.Lister()
 
 	// Controller syncs
 	controller.pvcSynced = pvcInformer.Informer().HasSynced
@@ -133,6 +135,7 @@ func NewController(
 	controller.machineClassSynced = machineClassInformer.Informer().HasSynced
 	controller.nodeSynced = nodeInformer.Informer().HasSynced
 	controller.machineSynced = machineInformer.Informer().HasSynced
+	controller.podSynced = podInformer.Informer().HasSynced
 
 	controller.pdbLister = pdbInformer.Lister()
 	controller.pdbSynced = pdbInformer.Informer().HasSynced
@@ -244,6 +247,7 @@ type controller struct {
 	volumeAttachementLister storagelisters.VolumeAttachmentLister
 	machineClassLister      machinelisters.MachineClassLister
 	machineLister           machinelisters.MachineLister
+	podLister               corelisters.PodLister
 	// queues
 	secretQueue                 workqueue.RateLimitingInterface
 	nodeQueue                   workqueue.RateLimitingInterface
@@ -260,6 +264,7 @@ type controller struct {
 	nodeSynced              cache.InformerSynced
 	machineClassSynced      cache.InformerSynced
 	machineSynced           cache.InformerSynced
+	podSynced               cache.InformerSynced
 }
 
 func (c *controller) Run(workers int, stopCh <-chan struct{}) {
diff --git a/pkg/util/provider/machinecontroller/machine_util.go b/pkg/util/provider/machinecontroller/machine_util.go
index b0af4dbdd..a0c10645c 100644
--- a/pkg/util/provider/machinecontroller/machine_util.go
+++ b/pkg/util/provider/machinecontroller/machine_util.go
@@ -1175,7 +1175,9 @@ func (c *controller) drainNode(ctx context.Context, deleteMachineRequest *driver
 				c.pvLister,
 				c.pdbLister,
 				c.nodeLister,
+				c.podLister,
 				c.volumeAttachmentHandler,
+				c.podSynced,
 			)
 			klog.V(3).Infof("(drainNode) Invoking RunDrain, forceDeleteMachine: %t, forceDeletePods: %t, timeOutDuration: %s", forceDeletePods, forceDeleteMachine, timeOutDuration)
 			err = drainOptions.RunDrain(ctx)


Test patch:
diff --git a/pkg/util/provider/drain/drain_test.go b/pkg/util/provider/drain/drain_test.go
index a66aba237..0161cc7fd 100644
--- a/pkg/util/provider/drain/drain_test.go
+++ b/pkg/util/provider/drain/drain_test.go
@@ -119,7 +119,7 @@ var _ = Describe("drain", func() {
 			targetCoreObjects = appendVolumeAttachments(targetCoreObjects, volumeAttachments)
 		}
 
-		fakeTargetCoreClient, fakePVLister, fakePVCLister, fakeNodeLister, pvcSynced, pvSynced, nodeSynced, tracker := createFakeController(
+		fakeTargetCoreClient, fakePVLister, fakePVCLister, fakeNodeLister, fakePodLister, pvcSynced, pvSynced, nodeSynced, podSynced, tracker := createFakeController(
 			stop, testNamespace, targetCoreObjects,
 		)
 		defer tracker.Stop()
@@ -159,8 +159,10 @@ var _ = Describe("drain", func() {
 			pvLister:                     fakePVLister,
 			pdbLister:                    nil,
 			nodeLister:                   fakeNodeLister,
+			podLister:                    fakePodLister,
 			Timeout:                      2 * time.Minute,
 			volumeAttachmentHandler:      volumeAttachmentHandler,
+			podSynced:                    podSynced,
 		}
 
 		// Get the pod directly from the ObjectTracker to avoid locking issues in the Fake object.
diff --git a/pkg/util/provider/machinecontroller/controller_suite_test.go b/pkg/util/provider/machinecontroller/controller_suite_test.go
index 0dd97f5fd..43cfb8e24 100644
--- a/pkg/util/provider/machinecontroller/controller_suite_test.go
+++ b/pkg/util/provider/machinecontroller/controller_suite_test.go
@@ -539,6 +539,7 @@ func createController(
 	nodes := coreTargetSharedInformers.Nodes()
 	pvcs := coreTargetSharedInformers.PersistentVolumeClaims()
 	pvs := coreTargetSharedInformers.PersistentVolumes()
+	pods := coreTargetSharedInformers.Pods()
 
 	coreControlInformerFactory := coreinformers.NewFilteredSharedInformerFactory(
 		fakeControlCoreClient,
@@ -593,9 +594,11 @@ func createController(
 		secretLister:                secrets.Lister(),
 		pvLister:                    pvs.Lister(),
 		machineLister:               machines.Lister(),
+		podLister:                   pods.Lister(),
 		machineSynced:               machines.Informer().HasSynced,
 		nodeSynced:                  nodes.Informer().HasSynced,
 		secretSynced:                secrets.Informer().HasSynced,
+		podSynced:                   pods.Informer().HasSynced,
 		machineClassQueue:           workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "machineclass"),
 		secretQueue:                 workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "secret"),
 		nodeQueue:                   workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "node"),