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..1d02a822d 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,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) } 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 (