Repository: gardener/machine-controller-manager
Pull request #300 (2019-08-02 10:22:54)

Split force deletion
**What this PR does / why we need it**:

**Which issue(s) this PR fixes**:
Fixes #

**Special notes for your reviewer**:

**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:       improvement|noteworthy
- target_group:   user|operator|developer
-->
```improvement operator
Bugfix: Allows force deletion of pods who have no controllers backing them
```



Patch:
diff --git a/pkg/controller/drain.go b/pkg/controller/drain.go
index 905157d78..c0f8c9087 100644
--- a/pkg/controller/drain.go
+++ b/pkg/controller/drain.go
@@ -48,22 +48,23 @@ import (
 
 // DrainOptions are configurable options while draining a node before deletion
 type DrainOptions struct {
-	client             kubernetes.Interface
-	Force              bool
-	GracePeriodSeconds int
-	IgnoreDaemonsets   bool
-	Timeout            time.Duration
-	MaxEvictRetries    int32
-	PvDetachTimeout    time.Duration
-	DeleteLocalData    bool
-	nodeName           string
-	Out                io.Writer
-	ErrOut             io.Writer
-	Driver             driver.Driver
-	pvcLister          corelisters.PersistentVolumeClaimLister
-	pvLister           corelisters.PersistentVolumeLister
-	drainStartedOn     time.Time
-	drainEndedOn       time.Time
+	client                       kubernetes.Interface
+	ForceDeletePods              bool
+	IgnorePodsWithoutControllers bool
+	GracePeriodSeconds           int
+	IgnoreDaemonsets             bool
+	Timeout                      time.Duration
+	MaxEvictRetries              int32
+	PvDetachTimeout              time.Duration
+	DeleteLocalData              bool
+	nodeName                     string
+	Out                          io.Writer
+	ErrOut                       io.Writer
+	Driver                       driver.Driver
+	pvcLister                    corelisters.PersistentVolumeClaimLister
+	pvLister                     corelisters.PersistentVolumeLister
+	drainStartedOn               time.Time
+	drainEndedOn                 time.Time
 }
 
 // Takes a pod and returns a bool indicating whether or not to operate on the
@@ -120,7 +121,8 @@ func NewDrainOptions(
 	pvDetachTimeout time.Duration,
 	nodeName string,
 	gracePeriodSeconds int,
-	force bool,
+	forceDeletePods bool,
+	ignorePodsWithoutControllers bool,
 	ignoreDaemonsets bool,
 	deleteLocalData bool,
 	out io.Writer,
@@ -131,20 +133,21 @@ func NewDrainOptions(
 ) *DrainOptions {
 
 	return &DrainOptions{
-		client:             client,
-		Force:              force,
-		GracePeriodSeconds: gracePeriodSeconds,
-		IgnoreDaemonsets:   ignoreDaemonsets,
-		MaxEvictRetries:    maxEvictRetries,
-		Timeout:            timeout,
-		PvDetachTimeout:    pvDetachTimeout,
-		DeleteLocalData:    deleteLocalData,
-		nodeName:           nodeName,
-		Out:                out,
-		ErrOut:             errOut,
-		Driver:             driver,
-		pvcLister:          pvcLister,
-		pvLister:           pvLister,
+		client:                       client,
+		ForceDeletePods:              forceDeletePods,
+		IgnorePodsWithoutControllers: ignorePodsWithoutControllers,
+		GracePeriodSeconds:           gracePeriodSeconds,
+		IgnoreDaemonsets:             ignoreDaemonsets,
+		MaxEvictRetries:              maxEvictRetries,
+		Timeout:                      timeout,
+		PvDetachTimeout:              pvDetachTimeout,
+		DeleteLocalData:              deleteLocalData,
+		nodeName:                     nodeName,
+		Out:                          out,
+		ErrOut:                       errOut,
+		Driver:                       driver,
+		pvcLister:                    pvcLister,
+		pvLister:                     pvLister,
 	}
 
 }
@@ -200,7 +203,7 @@ func (o *DrainOptions) unreplicatedFilter(pod api.Pod) (bool, *warning, *fatal)
 	if controllerRef != nil {
 		return true, nil, nil
 	}
-	if !o.Force {
+	if !o.IgnorePodsWithoutControllers {
 		return false, nil, &fatal{unmanagedFatal}
 	}
 	return true, &warning{unmanagedWarning}, nil
@@ -402,7 +405,7 @@ func (o *DrainOptions) getGlobalTimeoutForPodsWithoutPV(pods []*api.Pod) time.Du
 func (o *DrainOptions) evictPods(attemptEvict bool, pods []api.Pod, policyGroupVersion string, getPodFn func(namespace, name string) (*api.Pod, error)) error {
 	returnCh := make(chan error, len(pods))
 
-	if o.Force {
+	if o.ForceDeletePods {
 		podsToDrain := make([]*api.Pod, len(pods))
 		for i := range pods {
 			podsToDrain[i] = &pods[i]
@@ -794,7 +797,7 @@ func (o *DrainOptions) evictPodWithoutPVInternal(attemptEvict bool, pod *corev1.
 		}
 	}
 
-	if o.Force {
+	if o.ForceDeletePods {
 		// Skip waiting for pod termination in case of forced drain
 		if err == nil {
 			returnCh <- nil
diff --git a/pkg/controller/machine.go b/pkg/controller/machine.go
index d9ef7de3b..cbf040969 100644
--- a/pkg/controller/machine.go
+++ b/pkg/controller/machine.go
@@ -488,13 +488,13 @@ func (c *controller) machineDelete(machine *v1alpha1.Machine, driver driver.Driv
 			timeOutDuration := c.safetyOptions.MachineDrainTimeout.Duration
 			// Timeout value obtained by subtracting last operation with expected time out period
 			timeOut := metav1.Now().Add(-timeOutDuration).Sub(machine.Status.CurrentStatus.LastUpdateTime.Time)
-			force := false
+			forceDeletePods := false
 
 			// To perform forceful drain either one of the below conditions must be satified
 			// 1. force-deletion: "True" label must be present
 			// 2. Deletion operation is more than drain-timeout minutes old
 			if machine.Labels["force-deletion"] == "True" || timeOut > 0 {
-				force = true
+				forceDeletePods = true
 				timeOutDuration = 30 * time.Second
 				maxEvictRetries = 1
 				glog.V(2).Infof("Force deletion has been triggerred for machine %q", machine.Name)
@@ -511,7 +511,8 @@ func (c *controller) machineDelete(machine *v1alpha1.Machine, driver driver.Driv
 				pvDetachTimeOut,
 				nodeName,
 				-1,
-				force,
+				forceDeletePods,
+				true,
 				true,
 				true,
 				buf,


Test patch:
diff --git a/pkg/controller/drain_test.go b/pkg/controller/drain_test.go
index 58a8b2550..1d3d7ffbb 100644
--- a/pkg/controller/drain_test.go
+++ b/pkg/controller/drain_test.go
@@ -118,20 +118,21 @@ var _ = Describe("drain", func() {
 			maxEvictRetries = 3
 		}
 		d := &DrainOptions{
-			DeleteLocalData:    true,
-			Driver:             &drainDriver{},
-			ErrOut:             GinkgoWriter,
-			Force:              setup.force,
-			GracePeriodSeconds: 30,
-			IgnoreDaemonsets:   true,
-			MaxEvictRetries:    maxEvictRetries,
-			Out:                GinkgoWriter,
-			PvDetachTimeout:    3 * time.Minute,
-			Timeout:            time.Minute,
-			client:             c.targetCoreClient,
-			nodeName:           testNodeName,
-			pvcLister:          c.pvcLister,
-			pvLister:           c.pvLister,
+			DeleteLocalData:              true,
+			Driver:                       &drainDriver{},
+			ErrOut:                       GinkgoWriter,
+			ForceDeletePods:              setup.force,
+			IgnorePodsWithoutControllers: true,
+			GracePeriodSeconds:           30,
+			IgnoreDaemonsets:             true,
+			MaxEvictRetries:              maxEvictRetries,
+			Out:                          GinkgoWriter,
+			PvDetachTimeout:              3 * time.Minute,
+			Timeout:                      time.Minute,
+			client:                       c.targetCoreClient,
+			nodeName:                     testNodeName,
+			pvcLister:                    c.pvcLister,
+			pvLister:                     c.pvLister,
 		}
 
 		// Get the pod directly from the ObjectTracker to avoid locking issues in the Fake object.