Trigger orphan collection on OutOfRange machine status

pkg/util/provider/machinecontroller/controller.go

@@ -197,7 +197,9 @@ func NewController(
 	controller.machineSafetyOrphanVMsQueue.Add("")
 	controller.machineSafetyAPIServerQueue.Add("")
 	machineInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
-		// deleteMachineToSafety makes sure that orphan VM handler is invoked
+		// updateMachineToSafety makes sure that orphan VM handler is invoked on some specific machine obj updates
+		UpdateFunc: controller.updateMachineToSafety,
+		// deleteMachineToSafety makes sure that orphan VM handler is invoked on any machine deletion
 		DeleteFunc: controller.deleteMachineToSafety,
 	})
 

pkg/util/provider/machinecontroller/machine_safety.go

@@ -19,11 +19,13 @@ package controller
 
 import (
 	"context"
+	"strings"
 	"time"
 
 	"github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
 	"github.com/gardener/machine-controller-manager/pkg/util/provider/cache"
 	"github.com/gardener/machine-controller-manager/pkg/util/provider/driver"
+	"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes"
 	"github.com/gardener/machine-controller-manager/pkg/util/provider/machineutils"
 
 	corev1 "k8s.io/api/core/v1"
@@ -306,9 +308,29 @@ func (c *controller) checkMachineClass(ctx context.Context, machineClass *v1alph
 	return machineutils.LongRetry, nil
 }
 
+// updateMachineToSafety enqueues into machineSafetyQueue when a machine is updated to particular status
+func (c *controller) updateMachineToSafety(oldObj, newObj interface{}) {
+	oldMachine := oldObj.(*v1alpha1.Machine)
+	newMachine := newObj.(*v1alpha1.Machine)
+
+	if oldMachine == nil || newMachine == nil {
+		klog.Errorf("Couldn't convert to machine resource from object")
+		return
+	}
+
+	if !strings.Contains(oldMachine.Status.LastOperation.Description, codes.OutOfRange.String()) && strings.Contains(newMachine.Status.LastOperation.Description, codes.OutOfRange.String()) {
+		klog.Warningf("Multiple VMs backing machine obj %q found, triggering orphan collection.", newMachine.Name)
+		c.enqueueMachineSafetyOrphanVMsKey(newMachine)
+	}
+}
+
 // deleteMachineToSafety enqueues into machineSafetyQueue when a new machine is deleted
 func (c *controller) deleteMachineToSafety(obj interface{}) {
 	machine := obj.(*v1alpha1.Machine)
+	if machine == nil {
+		klog.Errorf("Couldn't convert to machine resource from object")
+		return
+	}
 	c.enqueueMachineSafetyOrphanVMsKey(machine)
 }
 

pkg/util/provider/machinecontroller/machine_safety_test.go

@@ -32,6 +32,120 @@ import (
 
 var _ = Describe("safety_logic", func() {
 
+	Describe("#Event Handling functions", func() {
+		type setup struct {
+			machineObject, newMachineObject *v1alpha1.Machine
+		}
+		type expect struct {
+			expectedQueueSize int
+		}
+		type data struct {
+			setup  setup
+			expect expect
+		}
+
+		DescribeTable("##deleteMachineToSafety", func(data *data) {
+			stop := make(chan struct{})
+			defer close(stop)
+
+			objects := []runtime.Object{}
+
+			c, trackers := createController(stop, testNamespace, objects, nil, nil, nil)
+
+			defer trackers.Stop()
+			//waitForCacheSync(stop, c)
+			c.deleteMachineToSafety(data.setup.machineObject)
+
+			//waitForCacheSync(stop, c)
+			Expect(c.machineSafetyOrphanVMsQueue.Len()).To(Equal(data.expect.expectedQueueSize))
+		},
+			Entry("should enqueue the machine key", &data{
+				setup: setup{
+					machineObject: &v1alpha1.Machine{},
+				},
+				expect: expect{
+					expectedQueueSize: 1,
+				},
+			}),
+		)
+
+		DescribeTable("##updateMachineToSafety", func(data *data) {
+			stop := make(chan struct{})
+			defer close(stop)
+
+			objects := []runtime.Object{}
+
+			c, trackers := createController(stop, testNamespace, objects, nil, nil, nil)
+
+			defer trackers.Stop()
+			//waitForCacheSync(stop, c)
+			c.updateMachineToSafety(data.setup.machineObject, data.setup.newMachineObject)
+
+			//waitForCacheSync(stop, c)
+			Expect(c.machineSafetyOrphanVMsQueue.Len()).To(Equal(data.expect.expectedQueueSize))
+		},
+			Entry("shouldn't enqueue machine key if OutOfRange error not there in last reconciliation", &data{
+				setup: setup{
+					machineObject:    &v1alpha1.Machine{},
+					newMachineObject: &v1alpha1.Machine{},
+				},
+				expect: expect{
+					expectedQueueSize: 0,
+				},
+			}),
+			Entry("shouldn't enqueue machine key if no OutOfRange error in last reconciliation", &data{
+				setup: setup{
+					machineObject: &v1alpha1.Machine{},
+					newMachineObject: &v1alpha1.Machine{
+						Status: v1alpha1.MachineStatus{
+							LastOperation: v1alpha1.LastOperation{
+								Description: "Cloud provider message - machine codes error: code = [NotFound] message = [AWS plugin is returning multiple VM instances backing this machine object. IDs for all backing VMs - [i-1234abcd i-5678efgh]",
+							},
+						},
+					},
+				},
+				expect: expect{
+					expectedQueueSize: 0,
+				},
+			}),
+			Entry("should enqueue machine key if OutOfRange error in last reconciliation", &data{
+				setup: setup{
+					machineObject: &v1alpha1.Machine{},
+					newMachineObject: &v1alpha1.Machine{
+						Status: v1alpha1.MachineStatus{
+							LastOperation: v1alpha1.LastOperation{
+								Description: "Cloud provider message - machine codes error: code = [OutOfRange] message = [AWS plugin is returning multiple VM instances backing this machine object. IDs for all backing VMs - [i-1234abcd i-5678efgh]",
+							},
+						},
+					},
+				},
+				expect: expect{
+					expectedQueueSize: 1,
+				},
+			}),
+			Entry("shouldn't enqueue if new machine obj doesn't have OutOfRange error which old machine obj had", &data{
+				setup: setup{
+					machineObject: &v1alpha1.Machine{
+						Status: v1alpha1.MachineStatus{
+							LastOperation: v1alpha1.LastOperation{
+								Description: "Cloud provider message - machine codes error: code = [OutOfRange] message = [AWS plugin is returning multiple VM instances backing this machine object. IDs for all backing VMs - [i-1234abcd i-5678efgh]",
+							},
+						},
+					},
+					newMachineObject: &v1alpha1.Machine{
+						Status: v1alpha1.MachineStatus{
+							LastOperation: v1alpha1.LastOperation{
+								Description: "Machine abc successfully joined the cluster",
+							},
+						},
+					},
+				},
+				expect: expect{
+					expectedQueueSize: 0,
+				},
+			}),
+		)
+	})
 	Describe("#machine_safety", func() {
 
 		const (