From 3aacde6fa538bc4fe7c730d16d75c587ddf31c3c Mon Sep 17 00:00:00 2001 From: Himanshu Sharma Date: Tue, 28 Dec 2021 13:07:28 +0530 Subject: [PATCH 1/3] trigger orphan collection changes --- pkg/util/provider/machinecontroller/controller.go | 4 +++- .../provider/machinecontroller/machine_safety.go | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/util/provider/machinecontroller/controller.go b/pkg/util/provider/machinecontroller/controller.go index 08214c9bc..d029eb7da 100644 --- a/pkg/util/provider/machinecontroller/controller.go +++ b/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, }) diff --git a/pkg/util/provider/machinecontroller/machine_safety.go b/pkg/util/provider/machinecontroller/machine_safety.go index 0f45597d9..40a61cd83 100644 --- a/pkg/util/provider/machinecontroller/machine_safety.go +++ b/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,6 +308,17 @@ 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 !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) From 5a96db4491076e7894c1d61429f2f1f458161374 Mon Sep 17 00:00:00 2001 From: Himanshu Sharma Date: Wed, 29 Dec 2021 10:34:51 +0530 Subject: [PATCH 2/3] add validation for type assert --- pkg/util/provider/machinecontroller/machine_safety.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/util/provider/machinecontroller/machine_safety.go b/pkg/util/provider/machinecontroller/machine_safety.go index 40a61cd83..1d02a822d 100644 --- a/pkg/util/provider/machinecontroller/machine_safety.go +++ b/pkg/util/provider/machinecontroller/machine_safety.go @@ -313,6 +313,11 @@ 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) @@ -322,6 +327,10 @@ func (c *controller) updateMachineToSafety(oldObj, newObj interface{}) { // 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) } From 01b31ee067622ccaae2fe65e767f6f996f5f05e0 Mon Sep 17 00:00:00 2001 From: Himanshu Sharma Date: Fri, 7 Jan 2022 17:38:51 +0530 Subject: [PATCH 3/3] added unit tests --- .../machinecontroller/machine_safety_test.go | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/pkg/util/provider/machinecontroller/machine_safety_test.go b/pkg/util/provider/machinecontroller/machine_safety_test.go index 8964d0013..eccad9197 100644 --- a/pkg/util/provider/machinecontroller/machine_safety_test.go +++ b/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 (