From ae337fb176c5e82a44878af42739fb0c4ab20674 Mon Sep 17 00:00:00 2001 From: prashanth26 Date: Fri, 13 Dec 2019 14:42:09 +0530 Subject: [PATCH] Refactored code to fix unit tests --- pkg/cmiclient/cmi-client.go | 2 +- pkg/cmiclient/fake-cmi-client.go | 42 +- pkg/controller/controller_suite_test.go | 18 +- pkg/controller/deployment_rollback_test.go | 1 + pkg/controller/deployment_rolling_test.go | 1 + pkg/controller/machine.go | 37 +- pkg/controller/machine_safety_test.go | 1 + pkg/controller/machine_test.go | 1891 +++++++++++++++----- pkg/controller/machine_util.go | 99 +- pkg/controller/machine_util_test.go | 50 +- 10 files changed, 1588 insertions(+), 554 deletions(-) diff --git a/pkg/cmiclient/cmi-client.go b/pkg/cmiclient/cmi-client.go index 26dad073f..4a6f95002 100644 --- a/pkg/cmiclient/cmi-client.go +++ b/pkg/cmiclient/cmi-client.go @@ -42,8 +42,8 @@ type VMs map[string]string // CMIClient is the client used to communicate with the CMIServer type CMIClient interface { CreateMachine() (string, string, string, error) - GetMachineStatus() (string, string, string, error) DeleteMachine() (string, error) + GetMachineStatus() (string, string, string, error) ListMachines() (map[string]string, error) ShutDownMachine() error GetProviderID() string diff --git a/pkg/cmiclient/fake-cmi-client.go b/pkg/cmiclient/fake-cmi-client.go index 236fbe083..49dabc306 100644 --- a/pkg/cmiclient/fake-cmi-client.go +++ b/pkg/cmiclient/fake-cmi-client.go @@ -18,42 +18,54 @@ limitations under the License. package cmiclient import ( + "fmt" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" corev1 "k8s.io/api/core/v1" ) // FakeCMIClient is a fake driver returned when none of the actual drivers match type FakeCMIClient struct { - MachineID string - NodeName string - Err error + VMExists bool + ProviderID string + NodeName string + LastKnownState string + Err error } // NewFakeCMIClient returns a new fakedriver object -func NewFakeCMIClient(machineID string, nodeName string, err error) *FakeCMIClient { - return &FakeCMIClient{ - MachineID: machineID, - NodeName: nodeName, - Err: err, - } +func NewFakeCMIClient(fakeCMIClient *FakeCMIClient) *FakeCMIClient { + return fakeCMIClient } // CreateMachine makes a gRPC call to the driver to create the machine. func (c *FakeCMIClient) CreateMachine() (string, string, string, error) { - return c.MachineID, c.NodeName, "", c.Err + if c.Err == nil { + c.VMExists = true + return c.ProviderID, c.NodeName, c.LastKnownState, c.Err + } + + return "", "", "", c.Err } // DeleteMachine make a grpc call to the driver to delete the machine. func (c *FakeCMIClient) DeleteMachine() (string, error) { - return "", c.Err + c.VMExists = false + return c.LastKnownState, c.Err } // GetMachineStatus makes a gRPC call to the driver to check existance of machine func (c *FakeCMIClient) GetMachineStatus() (string, string, string, error) { - if c.Err == nil { - return "", "", "", nil + switch { + case !c.VMExists: + errMessage := fmt.Sprintf("Fake plugin is returning no VM instances backing this machine object") + return "", "", "", status.Error(codes.NotFound, errMessage) + case c.Err != nil: + return "", "", "", c.Err } - return "", "", "", c.Err + return c.ProviderID, c.NodeName, c.LastKnownState, nil } // ListMachines have to list machines @@ -69,7 +81,7 @@ func (c *FakeCMIClient) ShutDownMachine() error { // GetProviderID returns the GetProviderID func (c *FakeCMIClient) GetProviderID() string { - return c.MachineID + return c.ProviderID } // GetVolumeIDs returns a list of VolumeIDs for the PV spec list supplied diff --git a/pkg/controller/controller_suite_test.go b/pkg/controller/controller_suite_test.go index 4ec6bcda4..38509b14c 100644 --- a/pkg/controller/controller_suite_test.go +++ b/pkg/controller/controller_suite_test.go @@ -35,6 +35,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/watch" coreinformers "k8s.io/client-go/informers" v1core "k8s.io/client-go/kubernetes/typed/core/v1" @@ -245,8 +246,9 @@ func newMachineFromMachineSet( statusTemplate *v1alpha1.MachineStatus, annotations map[string]string, labels map[string]string, + addFinalizer bool, ) *v1alpha1.Machine { - return newMachinesFromMachineSet(1, machineSet, statusTemplate, annotations, labels)[0] + return newMachinesFromMachineSet(1, machineSet, statusTemplate, annotations, labels, addFinalizer)[0] } func newMachinesFromMachineSet( @@ -255,6 +257,7 @@ func newMachinesFromMachineSet( statusTemplate *v1alpha1.MachineStatus, annotations map[string]string, labels map[string]string, + addFinalizer bool, ) []*v1alpha1.Machine { t := &machineSet.TypeMeta @@ -280,6 +283,7 @@ func newMachinesFromMachineSet( }, annotations, finalLabels, + addFinalizer, ) } @@ -289,8 +293,9 @@ func newMachine( owner *metav1.OwnerReference, annotations map[string]string, labels map[string]string, + addFinalizer bool, ) *v1alpha1.Machine { - return newMachines(1, specTemplate, statusTemplate, owner, annotations, labels)[0] + return newMachines(1, specTemplate, statusTemplate, owner, annotations, labels, addFinalizer)[0] } func newMachines( @@ -300,6 +305,7 @@ func newMachines( owner *metav1.OwnerReference, annotations map[string]string, labels map[string]string, + addFinalizer bool, ) []*v1alpha1.Machine { machines := make([]*v1alpha1.Machine, machineCount) @@ -324,6 +330,12 @@ func newMachines( }, Spec: *newMachineSpec(&specTemplate.Spec, i), } + finalizers := sets.NewString(m.Finalizers...) + + if addFinalizer { + finalizers.Insert(DeleteFinalizerName) + } + m.Finalizers = finalizers.List() if statusTemplate != nil { m.Status = *newMachineStatus(statusTemplate, i) @@ -571,7 +583,7 @@ var _ = Describe("#createController", func() { It("success", func() { machine0 := newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *objMeta, - }, nil, nil, nil, nil) + }, nil, nil, nil, nil, false) stop := make(chan struct{}) defer close(stop) diff --git a/pkg/controller/deployment_rollback_test.go b/pkg/controller/deployment_rollback_test.go index 5a9aa6844..95b52db12 100644 --- a/pkg/controller/deployment_rollback_test.go +++ b/pkg/controller/deployment_rollback_test.go @@ -133,6 +133,7 @@ var _ = Describe("deployment_rollback", func() { }, nil, nil, + true, ), nodes: newNodes( 1, diff --git a/pkg/controller/deployment_rolling_test.go b/pkg/controller/deployment_rolling_test.go index 6de91a41b..425724a8e 100644 --- a/pkg/controller/deployment_rolling_test.go +++ b/pkg/controller/deployment_rolling_test.go @@ -131,6 +131,7 @@ var _ = Describe("deployment_rolling", func() { }, nil, nil, + true, ), nodes: newNodes( 1, diff --git a/pkg/controller/machine.go b/pkg/controller/machine.go index b1b13f42a..62fbc8530 100644 --- a/pkg/controller/machine.go +++ b/pkg/controller/machine.go @@ -185,12 +185,6 @@ func (c *controller) reconcileClusterMachine(machine *v1alpha1.Machine) (bool, e return c.triggerDeletionFlow(machine, driver) } - // Add finalizers if not present - retry, err := c.addMachineFinalizers(machine) - if err != nil { - return retry, err - } - if machine.Status.Node != "" { // If reference to node object exists execute the below @@ -313,6 +307,13 @@ func (c *controller) triggerCreationFlow(machine *v1alpha1.Machine, driver cmicl err error ) + // Add finalizers if not present + retry, err := c.addMachineFinalizers(machine) + if err != nil { + return retry, err + } + + // providerID, nodeName, lastKnownState, err = driver.GetMachineStatus() if err == nil { // Found VM with required machine name @@ -387,7 +388,7 @@ func (c *controller) triggerCreationFlow(machine *v1alpha1.Machine, driver cmicl glog.V(2).Infof("Machine labels/annotations UPDATE for %q", machine.Name) // Return error even when machine object is updated - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine creation in process. Machine UPDATE successful") } return RetryOp, err } @@ -416,7 +417,7 @@ func (c *controller) triggerCreationFlow(machine *v1alpha1.Machine, driver cmicl glog.V(2).Infof("Machine/status UPDATE for %q during creation", machine.Name) // Return error even when machine object is updated - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine creation in process. Machine/Status UPDATE successful") } return RetryOp, err @@ -464,18 +465,18 @@ func (c *controller) triggerUpdationFlow(machine *v1alpha1.Machine, actualProvid } func (c *controller) triggerDeletionFlow(machine *v1alpha1.Machine, driver cmiclient.CMIClient) (bool, error) { - if finalizers := sets.NewString(machine.Finalizers...); !finalizers.Has(DeleteFinalizerName) { + finalizers := sets.NewString(machine.Finalizers...) + + switch { + case !finalizers.Has(DeleteFinalizerName): // If Finalizers are not present on machine err := fmt.Errorf("Machine %q is missing finalizers. Deletion cannot proceed", machine.Name) - glog.Error(err) return DoNotRetryOp, err - } - switch { case machine.Status.CurrentStatus.Phase != v1alpha1.MachineTerminating: return c.setMachineTerminationStatus(machine, driver) - case machine.Status.LastOperation.Description == getVMStatus: + case strings.Contains(machine.Status.LastOperation.Description, getVMStatus): return c.getVMStatus(machine, driver) case strings.Contains(machine.Status.LastOperation.Description, initiateDrain): @@ -495,19 +496,15 @@ func (c *controller) triggerDeletionFlow(machine *v1alpha1.Machine, driver cmicl return RetryOp, err } - case strings.Contains(machine.Status.LastOperation.Description, permanentError): - err := fmt.Errorf("Permanent Error has occurred for machine %q: \t%s", machine.Name, machine.Status.LastOperation.Description) + default: + err := fmt.Errorf("Unable to decode deletion flow state for machine %s", machine.Name) glog.Error(err) return DoNotRetryOp, err - - default: - glog.Errorf("Unable to decode deletion flow state for machine %q", machine.Name) - return DoNotRetryOp, nil } /* // Delete machine object - err = c.controlMachineClient.Machines(machine.Namespace).Delete(machine.Name, &metav1.DeleteOptions{}) + err := c.controlMachineClient.Machines(machine.Namespace).Delete(machine.Name, &metav1.DeleteOptions{}) if err != nil && !apierrors.IsNotFound(err) { // If its an error, and anyother error than object not found glog.Errorf("Deletion of Machine Object %q failed due to error: %s", machine.Name, err) diff --git a/pkg/controller/machine_safety_test.go b/pkg/controller/machine_safety_test.go index 25cae16ce..58fbf4aa0 100644 --- a/pkg/controller/machine_safety_test.go +++ b/pkg/controller/machine_safety_test.go @@ -206,6 +206,7 @@ var _ = Describe("#machine_safety", func() { &v1alpha1.MachineStatus{}, nil, nil, + true, ) controlMachineObjects := []runtime.Object{} diff --git a/pkg/controller/machine_test.go b/pkg/controller/machine_test.go index 0ddc70c4c..dc826033b 100644 --- a/pkg/controller/machine_test.go +++ b/pkg/controller/machine_test.go @@ -16,12 +16,12 @@ limitations under the License. package controller import ( + "fmt" "math" "time" machineapi "github.com/gardener/machine-controller-manager/pkg/apis/machine" "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1" - machinev1 "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1" "github.com/gardener/machine-controller-manager/pkg/apis/machine/validation" fakemachineapi "github.com/gardener/machine-controller-manager/pkg/client/clientset/versioned/typed/machine/v1alpha1/fake" "github.com/gardener/machine-controller-manager/pkg/cmiclient" @@ -30,7 +30,6 @@ import ( . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" @@ -45,95 +44,6 @@ var _ = Describe("machine", func() { c *controller ) - /* - Describe("#updateMachineStatus", func() { - var ( - machine *machinev1.Machine - lastOperation machinev1.LastOperation - currentStatus machinev1.CurrentStatus - ) - - BeforeEach(func() { - fakeMachineClient = &fakemachineapi.FakeMachineV1alpha1{ - Fake: &k8stesting.Fake{}, - } - c = &controller{ - controlMachineClient: fakeMachineClient, - } - machine = &machinev1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "machine-1", - Namespace: testNamespace, - }, - } - lastOperation = machinev1.LastOperation{ - Description: "test operation", - LastUpdateTime: metav1.Now(), - State: machinev1.MachineStateProcessing, - Type: "Create", - } - currentStatus = machinev1.CurrentStatus{ - LastUpdateTime: lastOperation.LastUpdateTime, - Phase: machinev1.MachinePending, - TimeoutActive: true, - } - }) - - It("should return error", func() { - err := errors.New("test error") - - fakeMachineClient.AddReactor("get", "machines", func(action k8stesting.Action) (bool, runtime.Object, error) { - return true, nil, err - }) - - machineRet, errRet := c.updateMachineStatus(machine, lastOperation, currentStatus) - Expect(errRet).Should(Not(BeNil())) - Expect(errRet).Should(BeIdenticalTo(err)) - Expect(machineRet).Should(Not(BeNil())) - Expect(machineRet).Should(BeIdenticalTo(machine)) - }) - - It("should return success", func() { - fakeMachineClient.AddReactor("get", "machines", func(action k8stesting.Action) (bool, runtime.Object, error) { - if action.(k8stesting.GetAction).GetName() == machine.GetName() { - return true, machine, nil - } - return false, nil, nil - }) - - var machineUpdated *machinev1.Machine - fakeMachineClient.AddReactor("update", "machines", func(action k8stesting.Action) (bool, runtime.Object, error) { - o := action.(k8stesting.UpdateAction).GetObject() - if o == nil { - return false, nil, nil - } - - m := o.(*machinev1.Machine) - if m.GetName() == machine.GetName() { - machineUpdated = m - return true, m, nil - } - - return false, nil, nil - - }) - - _, errRet := c.updateMachineStatus(machine, lastOperation, currentStatus) - Expect(errRet).Should(BeNil()) - Expect(machineUpdated).Should(Not(BeNil())) - - machineRet, err := c.controlMachineClient.Machines(c.namespace).Get(machine.Name, metav1.GetOptions{}) - Expect(err).Should(BeNil()) - - Expect(machineUpdated).Should(Not(BeIdenticalTo(machine))) - Expect(machineRet).Should(Not(BeIdenticalTo(machine))) - Expect(machineRet).Should(BeIdenticalTo(machineUpdated)) - Expect(machineRet.Status.CurrentStatus).Should(BeIdenticalTo(currentStatus)) - Expect(machineRet.Status.LastOperation).Should(BeIdenticalTo(lastOperation)) - }) - }) - */ - Describe("#isHealthy", func() { BeforeEach(func() { fakeMachineClient = &fakemachineapi.FakeMachineV1alpha1{ @@ -145,12 +55,12 @@ var _ = Describe("machine", func() { } }) - testMachine := machinev1.Machine{ + testMachine := v1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "testmachine", Namespace: testNamespace, }, - Status: machinev1.MachineStatus{ + Status: v1alpha1.MachineStatus{ Conditions: []corev1.NodeCondition{}, }, } @@ -227,14 +137,14 @@ var _ = Describe("machine", func() { c, trackers := createController(stop, testNamespace, objects, nil, nil) defer trackers.Stop() - testMachine := &machinev1.Machine{ + testMachine := &v1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "testmachine", Namespace: testNamespace, }, - Status: machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineTerminating, + Status: v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, }, }, } @@ -244,17 +154,17 @@ var _ = Describe("machine", func() { }) }) DescribeTable("Update conditions of an existing machine", - func(phase machinev1.MachinePhase, conditions []corev1.NodeCondition, expectedPhase machinev1.MachinePhase) { + func(phase v1alpha1.MachinePhase, conditions []corev1.NodeCondition, expectedPhase v1alpha1.MachinePhase) { stop := make(chan struct{}) defer close(stop) - testMachine := &machinev1.Machine{ + testMachine := &v1alpha1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "testmachine", Namespace: testNamespace, }, - Status: machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ + Status: v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ Phase: phase, }, }, @@ -270,24 +180,24 @@ var _ = Describe("machine", func() { Expect(updatedMachine.Status.CurrentStatus.Phase).Should(BeIdenticalTo(expectedPhase)) Expect(err).Should(BeNil()) }, - Entry("healthy status but machine terminating", machinev1.MachineTerminating, []corev1.NodeCondition{ + Entry("healthy status but machine terminating", v1alpha1.MachineTerminating, []corev1.NodeCondition{ { Type: corev1.NodeReady, Status: corev1.ConditionTrue, }, - }, machinev1.MachineTerminating), - Entry("unhealthy status but machine running", machinev1.MachineRunning, []corev1.NodeCondition{ + }, v1alpha1.MachineTerminating), + Entry("unhealthy status but machine running", v1alpha1.MachineRunning, []corev1.NodeCondition{ { Type: corev1.NodeReady, Status: corev1.ConditionFalse, }, - }, machinev1.MachineUnknown), - Entry("healthy status but machine not running", machinev1.MachineAvailable, []corev1.NodeCondition{ + }, v1alpha1.MachineUnknown), + Entry("healthy status but machine not running", v1alpha1.MachineAvailable, []corev1.NodeCondition{ { Type: corev1.NodeReady, Status: corev1.ConditionTrue, }, - }, machinev1.MachineRunning), + }, v1alpha1.MachineRunning), ) }) */ @@ -318,7 +228,7 @@ var _ = Describe("machine", func() { Describe("#ValidateMachineClass", func() { type setup struct { - aws []*machinev1.MachineClass + aws []*v1alpha1.MachineClass secrets []*corev1.Secret } type expect struct { @@ -328,7 +238,7 @@ var _ = Describe("machine", func() { } type data struct { setup setup - action *machinev1.ClassSpec + action *v1alpha1.ClassSpec expect expect } @@ -376,13 +286,13 @@ var _ = Describe("machine", func() { }, Entry("non-existing machine class", &data{ setup: setup{ - aws: []*machinev1.MachineClass{ - &machinev1.MachineClass{ + aws: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), }, }, }, - action: &machinev1.ClassSpec{ + action: &v1alpha1.ClassSpec{ Kind: "MachineClass", Name: "non-existing", }, @@ -397,19 +307,19 @@ var _ = Describe("machine", func() { ObjectMeta: *newObjectMeta(objMeta, 0), }, }, - aws: []*machinev1.MachineClass{ - &machinev1.MachineClass{ + aws: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, }, }, - action: &machinev1.ClassSpec{ + action: &v1alpha1.ClassSpec{ Kind: "MachineClass", Name: "class-0", }, expect: expect{ - machineClass: &machinev1.MachineClass{ + machineClass: &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, @@ -423,19 +333,19 @@ var _ = Describe("machine", func() { ObjectMeta: *newObjectMeta(objMeta, 0), }, }, - aws: []*machinev1.MachineClass{ - &machinev1.MachineClass{ + aws: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, }, }, - action: &machinev1.ClassSpec{ + action: &v1alpha1.ClassSpec{ Kind: "MachineClass", Name: "class-0", }, expect: expect{ - machineClass: &machinev1.MachineClass{ + machineClass: &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, @@ -447,20 +357,19 @@ var _ = Describe("machine", func() { Describe("#triggerCreationFlow", func() { type setup struct { - machineClaasses []*machinev1.MachineClass - machines []*machinev1.Machine + machineClaasses []*v1alpha1.MachineClass + machines []*v1alpha1.Machine secrets []*corev1.Secret fakeResourceActions *customfake.ResourceActions } type action struct { - machine string - fakeProviderID string - fakeNodeName string - fakeError error + machine string + fakeCMIClient *cmiclient.FakeCMIClient } type expect struct { - machine *machinev1.Machine + machine *v1alpha1.Machine err error + retry bool } type data struct { setup setup @@ -498,78 +407,287 @@ var _ = Describe("machine", func() { machine, err := controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) Expect(err).ToNot(HaveOccurred()) - _, err = controller.triggerCreationFlow( + retry, err := controller.triggerCreationFlow( machine, cmiclient.NewFakeCMIClient( - action.fakeProviderID, - action.fakeNodeName, - action.fakeError, + action.fakeCMIClient, ), ) - /* - if data.expect.err { - Expect(err).To(HaveOccurred()) - actual, err := controller.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{}) - Expect(err).To(BeNil()) - Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) - Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) - return - } + if data.expect.err != nil || err != nil { + Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(data.expect.err)) + } - Expect(err).To(BeNil()) - actual, err := controller.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{}) - Expect(err).To(BeNil()) - Expect(actual.Spec).To(Equal(data.expect.machine.Spec)) - Expect(actual.Status.Node).To(Equal(data.expect.machine.Status.Node)) - //TODO Conditions - */ + actual, err := controller.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{}) + Expect(err).To(BeNil()) + Expect(actual.Spec.ProviderID).To(Equal(data.expect.machine.Spec.ProviderID)) + Expect(actual.Status.Node).To(Equal(data.expect.machine.Status.Node)) + Expect(actual.Finalizers).To(Equal(data.expect.machine.Finalizers)) + Expect(retry).To(Equal(data.expect.retry)) }, - Entry("AWSSimple", &data{ + Entry("Machine creation in process. Machine finalizers are UPDATED", &data{ setup: setup{ secrets: []*corev1.Secret{ { ObjectMeta: *newObjectMeta(objMeta, 0), }, }, - machineClaasses: []*machinev1.MachineClass{ - &machinev1.MachineClass{ + machineClaasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, }, - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: machinev1.MachineSpec{ - Class: machinev1.ClassSpec{ + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ Kind: "MachineClass", Name: "machine-0", }, }, - }, nil, nil, nil, nil), + }, nil, nil, nil, nil, false), }, action: action{ - machine: "machine-0", - fakeProviderID: "fakeID-0", - fakeNodeName: "fakeNode-0", - fakeError: nil, + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: false, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machineClass", + }, + }, + }, nil, nil, nil, nil, true), + err: fmt.Errorf("Machine creation in process. Machine finalizers are UPDATED"), + retry: true, + }, + }), + Entry("Machine creation succeeds with object UPDATE", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClaasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: machinev1.MachineSpec{ - Class: machinev1.ClassSpec{ + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ Kind: "MachineClass", Name: "machine-0", }, + }, + }, nil, nil, nil, nil, true), + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: false, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machineClass", + }, ProviderID: "fakeID", }, - }, &machinev1.MachineStatus{ - Node: "fakeNode", - //TODO conditions - }, nil, nil, nil), - err: nil, + }, nil, nil, nil, nil, true), + err: fmt.Errorf("Machine creation in process. Machine UPDATE successful"), + retry: true, + }, + }), + Entry("Machine creation succeeds with status UPDATE", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClaasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + nil, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + err: fmt.Errorf("Machine creation in process. Machine/Status UPDATE successful"), + retry: true, + }, + }), + Entry("Machine creation has already succeeded, so no update", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClaasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachinePending, + + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Creating machine on cloud provider", + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachinePending, + + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Creating machine on cloud provider", + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + err: nil, + retry: false, }, }), @@ -581,16 +699,16 @@ var _ = Describe("machine", func() { ObjectMeta: *newObjectMeta(objMeta, 0), }, }, - aws: []*machinev1.MachineClass{ + aws: []*v1alpha1.MachineClass{ { ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, }, - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: machinev1.MachineSpec{ - Class: machinev1.ClassSpec{ + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ Kind: "AWSMachineClass", Name: "machine-0", }, @@ -609,16 +727,16 @@ var _ = Describe("machine", func() { fakeError: nil, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: machinev1.MachineSpec{ - Class: machinev1.ClassSpec{ + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ Kind: "AWSMachineClass", Name: "machine-0", }, ProviderID: "fakeID", }, - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "fakeNode", //TODO conditions }, nil, nil, nil), @@ -629,25 +747,25 @@ var _ = Describe("machine", func() { ) }) - Describe("#machineDelete", func() { + Describe("#triggerDeletionFlow", func() { type setup struct { secrets []*corev1.Secret - aws []*machinev1.MachineClass - machines []*machinev1.Machine + machineClasses []*v1alpha1.MachineClass + machines []*v1alpha1.Machine + nodes []*corev1.Node fakeResourceActions *customfake.ResourceActions } type action struct { machine string - fakeProviderID string - fakeNodeName string - fakeError error forceDeleteLabelPresent bool - fakeMachineStatus *machinev1.MachineStatus + fakeMachineStatus *v1alpha1.MachineStatus + fakeCMIClient *cmiclient.FakeCMIClient } type expect struct { - machine *machinev1.Machine - errOccurred bool - machineDeleted bool + machine *v1alpha1.Machine + err error + nodeDeleted bool + retry bool } type data struct { setup setup @@ -664,7 +782,7 @@ var _ = Describe("machine", func() { defer close(stop) machineObjects := []runtime.Object{} - for _, o := range data.setup.aws { + for _, o := range data.setup.machineClasses { machineObjects = append(machineObjects, o) } for _, o := range data.setup.machines { @@ -675,6 +793,9 @@ var _ = Describe("machine", func() { for _, o := range data.setup.secrets { coreObjects = append(coreObjects, o) } + for _, o := range data.setup.nodes { + coreObjects = append(coreObjects, o) + } controller, trackers := createController(stop, objMeta.Namespace, machineObjects, nil, coreObjects) defer trackers.Stop() @@ -685,242 +806,1122 @@ var _ = Describe("machine", func() { Expect(err).ToNot(HaveOccurred()) fakeCMIClient := cmiclient.NewFakeCMIClient( - action.fakeProviderID, - action.fakeNodeName, - action.fakeError, + action.fakeCMIClient, ) - // Create a machine that is to be deleted later - _, err = controller.triggerCreationFlow(machine, fakeCMIClient) - Expect(err).ToNot(HaveOccurred()) - - _, err = controller.targetCoreClient.Core().Nodes().Create(&v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: action.fakeNodeName, - }, - }) - Expect(err).ToNot(HaveOccurred()) - - // Add finalizers - controller.addMachineFinalizers(machine) - - // Fetch the latest machine version - machine, err = controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) - Expect(err).ToNot(HaveOccurred()) - - if data.action.forceDeleteLabelPresent { - // Add labels for force deletion - clone := machine.DeepCopy() - clone.Labels["force-deletion"] = "True" - machine, err = controller.controlMachineClient.Machines(objMeta.Namespace).Update(clone) - Expect(err).ToNot(HaveOccurred()) - } - - if data.action.fakeMachineStatus != nil { - clone := machine.DeepCopy() - clone.Status = *data.action.fakeMachineStatus - machine, err = controller.controlMachineClient.Machines(objMeta.Namespace).UpdateStatus(clone) - Expect(err).ToNot(HaveOccurred()) - } - if data.setup.fakeResourceActions != nil { trackers.TargetCore.SetFakeResourceActions(data.setup.fakeResourceActions, math.MaxInt32) } // Deletion of machine is triggered - _, err = controller.triggerDeletionFlow(machine, fakeCMIClient) - if data.expect.errOccurred { - Expect(err).To(HaveOccurred()) - } else { - Expect(err).ToNot(HaveOccurred()) + retry, err := controller.triggerDeletionFlow(machine, fakeCMIClient) + if err != nil || data.expect.err != nil { + Expect(err).To(Equal(data.expect.err)) } + Expect(retry).To(Equal(data.expect.retry)) - node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(machine.Status.Node, metav1.GetOptions{}) - machine, machineErr := controller.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{}) - - if data.expect.machineDeleted { - Expect(machineErr).To(HaveOccurred()) + machine, err = controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(machine.Spec).To(Equal(data.expect.machine.Spec)) + Expect(machine.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) + Expect(machine.Status.LastOperation.State).To(Equal(data.expect.machine.Status.LastOperation.State)) + Expect(machine.Status.LastOperation.Type).To(Equal(data.expect.machine.Status.LastOperation.Type)) + Expect(machine.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) + Expect(machine.Finalizers).To(Equal(data.expect.machine.Finalizers)) + + if data.expect.nodeDeleted { + _, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(machine.Status.Node, metav1.GetOptions{}) Expect(nodeErr).To(HaveOccurred()) - } else { - Expect(machineErr).ToNot(HaveOccurred()) - Expect(machine).ToNot(BeNil()) - - Expect(nodeErr).ToNot(HaveOccurred()) - Expect(node).ToNot(BeNil()) } + }, - Entry("Simple machine deletion", &data{ + Entry("Do not process machine deletion for object without finalizer", &data{ setup: setup{ secrets: []*corev1.Secret{ { ObjectMeta: *newObjectMeta(objMeta, 0), }, }, - aws: []*machinev1.MachineClass{ - &machinev1.MachineClass{ + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, }, - machines: newMachines(1, &machinev1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: machinev1.MachineSpec{ - Class: machinev1.ClassSpec{ - Kind: "MachineClass", - Name: "machine-0", + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + LastUpdateTime: metav1.Now(), }, + LastOperation: v1alpha1.LastOperation{ + Description: "Machine machine-0 successfully joined the cluster", + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", }, - }, nil, nil, nil, nil), + map[string]string{ + "node": "fakeID-0", + }, + false, + ), }, action: action{ - machine: "machine-0", - fakeProviderID: "fakeID-0", - fakeNodeName: "fakeNode-0", - fakeError: nil, + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, }, expect: expect{ - errOccurred: false, - machineDeleted: true, + err: fmt.Errorf("Machine \"machine-0\" is missing finalizers. Deletion cannot proceed"), + retry: DoNotRetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Machine machine-0 successfully joined the cluster", + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + false, + ), }, }), - Entry("Machine deletion when drain fails", &data{ + Entry("Change machine phase to termination successfully", &data{ setup: setup{ secrets: []*corev1.Secret{ { ObjectMeta: *newObjectMeta(objMeta, 0), }, }, - aws: []*machinev1.MachineClass{ - &machinev1.MachineClass{ + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, }, - machines: newMachines(1, &machinev1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: machinev1.MachineSpec{ - Class: machinev1.ClassSpec{ - Kind: "MachineClass", - Name: "machine-0", + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", }, }, - }, nil, nil, nil, nil), - fakeResourceActions: &customfake.ResourceActions{ - Node: customfake.Actions{ - Update: "Failed to update nodes", + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Machine machine-0 successfully joined the cluster", + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.Now(), + }, }, - }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), }, action: action{ - machine: "machine-0", - fakeProviderID: "fakeID-0", - fakeNodeName: "fakeNode-0", - fakeError: nil, + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, }, expect: expect{ - errOccurred: true, - machineDeleted: false, + err: fmt.Errorf("Machine deletion in process. Phase set to termination"), + retry: RetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: getVMStatus, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), }, }), - Entry("Machine force deletion label is present", &data{ + Entry("Checking existance of VM at provider successfully", &data{ setup: setup{ secrets: []*corev1.Secret{ { ObjectMeta: *newObjectMeta(objMeta, 0), }, }, - aws: []*machinev1.MachineClass{ - &machinev1.MachineClass{ + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, }, - machines: newMachines(1, &machinev1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: machinev1.MachineSpec{ - Class: machinev1.ClassSpec{ - Kind: "MachineClass", - Name: "machine-0", - }, - }, - }, nil, nil, nil, nil), - }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: getVMStatus, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, action: action{ - machine: "machine-0", - fakeProviderID: "fakeID-0", - fakeNodeName: "fakeNode-0", - fakeError: nil, - fakeMachineStatus: &machinev1.MachineStatus{ - Node: "fakeNode-0", - LastOperation: machinev1.LastOperation{ - Description: "Drain failed - for random reason", - State: v1alpha1.MachineStateFailed, - Type: v1alpha1.MachineOperationDelete, - LastUpdateTime: metav1.Now(), - }, - CurrentStatus: machinev1.CurrentStatus{ - Phase: v1alpha1.MachineTerminating, - TimeoutActive: false, - LastUpdateTime: metav1.NewTime(time.Now().Add(-2 * time.Minute)), + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + err: fmt.Errorf("Machine deletion in process. VM with matching ID found"), + retry: RetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: initiateDrain, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + }), + Entry("Drain machine successfully", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: initiateDrain, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", }, + true, + ), + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, }, }, expect: expect{ - errOccurred: false, - machineDeleted: true, + err: fmt.Errorf("Machine deletion in process. Drain successful. %s", initiateVMDeletion), + retry: RetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Drain successful. %s", initiateVMDeletion), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), }, }), - Entry("Machine deletion when last drain failed and current drain call also fails (APIServer call fails)", &data{ + Entry("Drain machine failure, but since force deletion label is present deletion continues", &data{ setup: setup{ secrets: []*corev1.Secret{ { ObjectMeta: *newObjectMeta(objMeta, 0), }, }, - aws: []*machinev1.MachineClass{ - &machinev1.MachineClass{ + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ ObjectMeta: *newObjectMeta(objMeta, 0), SecretRef: newSecretReference(objMeta, 0), }, }, - machines: newMachines(1, &machinev1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - Spec: machinev1.MachineSpec{ - Class: machinev1.ClassSpec{ - Kind: "MachineClass", - Name: "machine-0", + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: initiateDrain, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + "force-deletion": "True", + }, + true, + ), + nodes: []*corev1.Node{ + &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fakeID-0", + }, + }, + }, + fakeResourceActions: &customfake.ResourceActions{ + Node: customfake.Actions{ + Update: "Failed to update node", + }, + }, + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + err: fmt.Errorf("Failed to update node"), + retry: RetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Drain failed due to - Failed to update node. However, since it's a force deletion shall continue deletion of VM. %s", initiateVMDeletion), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + }), + Entry("Drain machine failure after drain timeout, hence deletion continues", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.NewTime(time.Now().Add(-2 * time.Hour)), + }, + LastOperation: v1alpha1.LastOperation{ + Description: initiateDrain, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.NewTime(time.Now().Add(-2 * time.Hour)), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + nodes: []*corev1.Node{ + &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fakeID-0", + }, + }, + }, + fakeResourceActions: &customfake.ResourceActions{ + Node: customfake.Actions{ + Update: "Failed to update node", + }, + }, + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + err: fmt.Errorf("Failed to update node"), + retry: RetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Drain failed due to - Failed to update node. However, since it's a force deletion shall continue deletion of VM. %s", initiateVMDeletion), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + }), + Entry("Drain machine failure", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: initiateDrain, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), }, }, - }, nil, nil, nil, nil), + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + nodes: []*corev1.Node{ + &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fakeID-0", + }, + }, + }, fakeResourceActions: &customfake.ResourceActions{ Node: customfake.Actions{ - Update: "Failed to update nodes", + Update: "Failed to update node", + }, + }, + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + err: fmt.Errorf("Failed to update node"), + retry: RetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Drain failed due to - Failed to update node. Will retry in next sync. %s", initiateDrain), + State: v1alpha1.MachineStateFailed, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + }), + Entry("Delete VM successfully", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Drain successful. %s", initiateVMDeletion), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + err: fmt.Errorf("Machine deletion in process. VM deletion was successful. " + initiateNodeDeletion), + retry: RetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("VM deletion was successful. %s", initiateNodeDeletion), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + }), + Entry("Delete node object successfully", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("VM deletion was successful. %s", initiateNodeDeletion), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + nodes: []*corev1.Node{ + &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fakeID-0", + }, + }, + }, + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + err: fmt.Errorf("Machine deletion in process. Deletion of node object was succesful"), + retry: RetryOp, + nodeDeleted: true, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Deletion of Node Object %q is successful. %s", "fakeID-0", initiateFinalizerRemoval), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + }), + Entry("Delete machine finalizer successfully", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), }, }, + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Deletion of Node Object %q is successful. %s", "fakeID-0", initiateFinalizerRemoval), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), }, action: action{ - machine: "machine-0", - fakeProviderID: "fakeID-0", - fakeNodeName: "fakeNode-0", - fakeError: nil, - fakeMachineStatus: &machinev1.MachineStatus{ - Node: "fakeNode-0", - LastOperation: machinev1.LastOperation{ - Description: "Drain failed - for random reason", - State: v1alpha1.MachineStateFailed, - Type: v1alpha1.MachineOperationDelete, - LastUpdateTime: metav1.Now(), - }, - CurrentStatus: machinev1.CurrentStatus{ - Phase: v1alpha1.MachineTerminating, - TimeoutActive: false, - LastUpdateTime: metav1.NewTime(time.Now().Add(-2 * time.Minute)), + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, + }, + }, + expect: expect{ + retry: DoNotRetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Deletion of Node Object %q is successful. %s", "fakeID-0", initiateFinalizerRemoval), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + false, + ), + }, + }), + Entry("Unable to decode deletion flow state for machine", &data{ + setup: setup{ + secrets: []*corev1.Secret{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + }, + }, + machineClasses: []*v1alpha1.MachineClass{ + &v1alpha1.MachineClass{ + ObjectMeta: *newObjectMeta(objMeta, 0), + SecretRef: newSecretReference(objMeta, 0), + }, + }, + machines: newMachines( + 1, + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Some random last op description", + State: v1alpha1.MachineStateFailed, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), + }, + action: action{ + machine: "machine-0", + fakeCMIClient: &cmiclient.FakeCMIClient{ + VMExists: true, + ProviderID: "fakeID-0", + NodeName: "fakeNode-0", + Err: nil, }, }, expect: expect{ - errOccurred: false, - machineDeleted: true, + err: fmt.Errorf("Unable to decode deletion flow state for machine machine-0"), + retry: DoNotRetryOp, + machine: newMachine( + &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + Spec: v1alpha1.MachineSpec{ + Class: v1alpha1.ClassSpec{ + Kind: "MachineClass", + Name: "machine-0", + }, + ProviderID: "fakeID", + }, + }, + &v1alpha1.MachineStatus{ + Node: "fakeNode", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Some random last op description", + State: v1alpha1.MachineStateFailed, + Type: v1alpha1.MachineOperationDelete, + LastUpdateTime: metav1.Now(), + }, + }, + nil, + map[string]string{ + MachinePriority: "3", + }, + map[string]string{ + "node": "fakeID-0", + }, + true, + ), }, }), ) @@ -929,13 +1930,13 @@ var _ = Describe("machine", func() { /* Describe("#checkMachineTimeout", func() { type setup struct { - machines []*machinev1.Machine + machines []*v1alpha1.Machine } type action struct { machine string } type expect struct { - machine *machinev1.Machine + machine *v1alpha1.Machine err bool } type data struct { @@ -979,25 +1980,25 @@ var _ = Describe("machine", func() { actual, err := controller.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{}) Expect(err).To(BeNil()) Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) - Expect(actual.Status.CurrentStatus.TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.TimeoutActive)) + Expect(actual.Status.CurrentStatus.//TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.//TimeoutActive)) Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) Expect(actual.Status.LastOperation.State).To(Equal(data.expect.machine.Status.LastOperation.State)) Expect(actual.Status.LastOperation.Type).To(Equal(data.expect.machine.Status.LastOperation.Type)) }, Entry("Machine is still running", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineRunning, - TimeoutActive: false, + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + //TimeoutActive: false, LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), - State: machinev1.MachineStateSuccessful, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), }, }, nil, nil, nil), @@ -1006,35 +2007,33 @@ var _ = Describe("machine", func() { machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineRunning, - TimeoutActive: false, + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), - State: machinev1.MachineStateSuccessful, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, }, }, nil, nil, nil), }, }), Entry("Machine creation has still not timed out", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineUnknown, - TimeoutActive: true, + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), - State: machinev1.MachineStateProcessing, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), }, }, nil, nil, nil), @@ -1043,35 +2042,33 @@ var _ = Describe("machine", func() { machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineUnknown, - TimeoutActive: true, + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineUnknown, }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), - State: machinev1.MachineStateProcessing, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, }, }, nil, nil, nil), }, }), Entry("Machine creation has timed out", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachinePending, - TimeoutActive: true, + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: "Creating machine on cloud provider", - State: machinev1.MachineStateProcessing, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), }, }, nil, nil, nil), @@ -1080,39 +2077,39 @@ var _ = Describe("machine", func() { machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineFailed, - TimeoutActive: false, + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineFailed, + }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf( "Machine %s failed to join the cluster in %s minutes.", machineName, creationTimeOut, ), - State: machinev1.MachineStateFailed, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateFailed, + Type: v1alpha1.MachineOperationCreate, }, }, nil, nil, nil), }, }), Entry("Machine health has timed out", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineUnknown, - TimeoutActive: true, + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineUnknown, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), - State: machinev1.MachineStateProcessing, - Type: machinev1.MachineOperationHealthCheck, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationHealthCheck, LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), }, }, nil, nil, nil), @@ -1121,22 +2118,22 @@ var _ = Describe("machine", func() { machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineFailed, - TimeoutActive: false, + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineFailed, + }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf( "Machine %s is not healthy since %s minutes. Changing status to failed. Node Conditions: %+v", machineName, healthTimeOut, []corev1.NodeCondition{}, ), - State: machinev1.MachineStateFailed, - Type: machinev1.MachineOperationHealthCheck, + State: v1alpha1.MachineStateFailed, + Type: v1alpha1.MachineOperationHealthCheck, }, }, nil, nil, nil), }, @@ -1146,14 +2143,14 @@ var _ = Describe("machine", func() { Describe("#updateMachineState", func() { type setup struct { - machines []*machinev1.Machine + machines []*v1alpha1.Machine nodes []*corev1.Node } type action struct { machine string } type expect struct { - machine *machinev1.Machine + machine *v1alpha1.Machine err bool } type data struct { @@ -1201,7 +2198,7 @@ var _ = Describe("machine", func() { Expect(actual.Name).To(Equal(data.expect.machine.Name)) Expect(actual.Status.Node).To(Equal(data.expect.machine.Status.Node)) Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) - Expect(actual.Status.CurrentStatus.TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.TimeoutActive)) + Expect(actual.Status.CurrentStatus.//TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.//TimeoutActive)) Expect(actual.Status.LastOperation.State).To(Equal(data.expect.machine.Status.LastOperation.State)) Expect(actual.Status.LastOperation.Type).To(Equal(data.expect.machine.Status.LastOperation.Type)) Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) @@ -1221,24 +2218,24 @@ var _ = Describe("machine", func() { }, Entry("Machine does not have a node backing", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{}, nil, nil, nil), + }, &v1alpha1.MachineStatus{}, nil, nil, nil), }, action: action{ machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{}, nil, nil, nil), + }, &v1alpha1.MachineStatus{}, nil, nil, nil), }, }), Entry("Node object backing machine not found and machine conditions are empty", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "dummy-node", }, nil, nil, nil), }, @@ -1246,28 +2243,28 @@ var _ = Describe("machine", func() { machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "dummy-node", }, nil, nil, nil), }, }), Entry("Machine is running but node object is lost", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "dummy-node", - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineRunning, - TimeoutActive: false, + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + //TimeoutActive: false, LastUpdateTime: metav1.Now(), }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), - State: machinev1.MachineStateSuccessful, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, LastUpdateTime: metav1.Now(), }, Conditions: []corev1.NodeCondition{ @@ -1284,22 +2281,22 @@ var _ = Describe("machine", func() { machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "dummy-node", - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineUnknown, - TimeoutActive: true, + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineUnknown, + LastUpdateTime: metav1.Now(), }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: fmt.Sprintf( "Node object went missing. Machine %s is unhealthy - changing MachineState to Unknown", machineName, ), - State: machinev1.MachineStateProcessing, - Type: machinev1.MachineOperationHealthCheck, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationHealthCheck, LastUpdateTime: metav1.Now(), }, Conditions: []corev1.NodeCondition{ @@ -1315,19 +2312,19 @@ var _ = Describe("machine", func() { }), Entry("Machine and node both are present and kubelet ready status is updated", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "machine", - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachinePending, - TimeoutActive: true, + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachinePending, + LastUpdateTime: metav1.Now(), }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: "Creating machine on cloud provider", - State: machinev1.MachineStateProcessing, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, LastUpdateTime: metav1.Now(), }, Conditions: []corev1.NodeCondition{ @@ -1359,19 +2356,19 @@ var _ = Describe("machine", func() { machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "machine", - CurrentStatus: machinev1.CurrentStatus{ - Phase: machinev1.MachineRunning, - TimeoutActive: false, + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + //TimeoutActive: false, LastUpdateTime: metav1.Now(), }, - LastOperation: machinev1.LastOperation{ + LastOperation: v1alpha1.LastOperation{ Description: "Machine machine-0 successfully joined the cluster", - State: machinev1.MachineStateSuccessful, - Type: machinev1.MachineOperationCreate, + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, LastUpdateTime: metav1.Now(), }, Conditions: []corev1.NodeCondition{ @@ -1387,9 +2384,9 @@ var _ = Describe("machine", func() { }), Entry("Machine object does not have node-label and node exists", &data{ setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "node", }, nil, nil, nil), nodes: []*corev1.Node{ @@ -1404,11 +2401,11 @@ var _ = Describe("machine", func() { machine: machineName, }, expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Name: "machine-0", }, - }, &machinev1.MachineStatus{ + }, &v1alpha1.MachineStatus{ Node: "node", }, nil, nil, map[string]string{ diff --git a/pkg/controller/machine_util.go b/pkg/controller/machine_util.go index 014af2e54..5a61b8fd8 100644 --- a/pkg/controller/machine_util.go +++ b/pkg/controller/machine_util.go @@ -53,12 +53,11 @@ const ( RetryOp = true // DoNotRetryOp tells the controller to not retry for now. Resync after re-sync period DoNotRetryOp = false - getVMStatus = "Get VM status from provider. " - initiateDrain = "Initiate node drain. " - initiateVMDeletion = "Initiate VM deletion. " - initiateNodeDeletion = "Initiate node object deletion. " - initiateFinalizerRemoval = "Initiate machine object finalizer removal. " - permanentError = "Permanent error occurred in VM deletion." + getVMStatus = "Set machine status to termination. Now, getting VM Status" + initiateDrain = "Initiate node drain" + initiateVMDeletion = "Initiate VM deletion" + initiateNodeDeletion = "Initiate node object deletion" + initiateFinalizerRemoval = "Initiate machine object finalizer removal" ) var ( @@ -615,7 +614,7 @@ func (c *controller) reconcileMachineHealth(machine *v1alpha1.Machine) (bool, er } else { glog.V(2).Infof("Machine State has been updated for %q", machine.Name) // Return error for continuing in next iteration - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine creation is successful. Machine State has been UPDATED") } return RetryOp, err @@ -642,7 +641,7 @@ func (c *controller) addMachineFinalizers(machine *v1alpha1.Machine) (bool, erro } else { // Return error even when machine object is updated glog.V(2).Infof("Added finalizer to machine %q", machine.Name) - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine creation in process. Machine finalizers are UPDATED") } return RetryOp, err @@ -725,7 +724,7 @@ func (c *controller) setMachineTerminationStatus(machine *v1alpha1.Machine, driv } else { glog.V(2).Infof("Machine %q status updated to terminating ", machine.Name) // Return error even when machine object is updated to ensure reconcilation is restarted - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine deletion in process. Phase set to termination") } return RetryOp, err } @@ -736,6 +735,7 @@ func (c *controller) getVMStatus(machine *v1alpha1.Machine, driver cmiclient.CMI retry bool description string state v1alpha1.MachineState + phase v1alpha1.MachinePhase ) _, _, _, err := driver.GetMachineStatus() @@ -743,15 +743,20 @@ func (c *controller) getVMStatus(machine *v1alpha1.Machine, driver cmiclient.CMI // VM Found description = initiateDrain state = v1alpha1.MachineStateProcessing + retry = RetryOp + phase = v1alpha1.MachineTerminating // Return error even when machine object is updated to ensure reconcilation is restarted - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine deletion in process. VM with matching ID found") } else { if grpcErr, ok := status.FromError(err); !ok { // Error occurred with decoding gRPC error status, aborting without retry. - description = permanentError + "Error occurred with decoding gRPC error status while getting VM status, aborting without retry." + description = "Error occurred with decoding gRPC error status while getting VM status, aborting without retry. " + getVMStatus state = v1alpha1.MachineStateFailed + phase = v1alpha1.MachineFailed retry = DoNotRetryOp + + err = fmt.Errorf("Machine deletion has failed. " + description) } else { // Decoding gRPC error code switch grpcErr.Code() { @@ -761,23 +766,27 @@ func (c *controller) getVMStatus(machine *v1alpha1.Machine, driver cmiclient.CMI // In this case, try to drain and delete description = initiateDrain state = v1alpha1.MachineStateProcessing + phase = v1alpha1.MachineTerminating retry = RetryOp case codes.NotFound: // VM was not found at provder - description = initiateNodeDeletion + description = "VM was not found at provider. " + initiateNodeDeletion state = v1alpha1.MachineStateProcessing + phase = v1alpha1.MachineTerminating retry = RetryOp case codes.Unknown, codes.DeadlineExceeded, codes.Aborted, codes.Unavailable: - description = getVMStatus + "Error occurred with decoding gRPC error status while getting VM status, aborting with retry." + description = "Error occurred with decoding gRPC error status while getting VM status, aborting with retry. " + getVMStatus state = v1alpha1.MachineStateFailed + phase = v1alpha1.MachineTerminating retry = RetryOp default: // Error occurred with decoding gRPC error status, abort with retry. - description = getVMStatus + " Error occurred with decoding gRPC error status while getting VM status, aborting without retry. gRPC code: " + grpcErr.Message() + description = "Error occurred with decoding gRPC error status while getting VM status, aborting without retry. gRPC code: " + grpcErr.Message() + " " + getVMStatus state = v1alpha1.MachineStateFailed + phase = v1alpha1.MachineTerminating retry = DoNotRetryOp } } @@ -792,8 +801,7 @@ func (c *controller) getVMStatus(machine *v1alpha1.Machine, driver cmiclient.CMI LastUpdateTime: metav1.Now(), } clone.Status.CurrentStatus = v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineTerminating, - //TimeoutActive: false, + Phase: phase, LastUpdateTime: metav1.Now(), } @@ -814,11 +822,11 @@ func (c *controller) drainNode(machine *v1alpha1.Machine, driver cmiclient.CMICl timeOutOccurred = false description = "" state v1alpha1.MachineState + phase v1alpha1.MachinePhase maxEvictRetries = c.safetyOptions.MaxEvictRetries pvDetachTimeOut = c.safetyOptions.PvDetachTimeout.Duration timeOutDuration = c.safetyOptions.MachineDrainTimeout.Duration forceDeleteLabelPresent = machine.Labels["force-deletion"] == "True" - lastDrainFailed = machine.Status.LastOperation.Description[0:12] == "Drain failed" nodeName = machine.Labels["node"] ) @@ -826,7 +834,7 @@ func (c *controller) drainNode(machine *v1alpha1.Machine, driver cmiclient.CMICl timeOut := metav1.Now().Add(-timeOutDuration).Sub(machine.Status.CurrentStatus.LastUpdateTime.Time) timeOutOccurred = timeOut > 0 - if forceDeleteLabelPresent || timeOutOccurred || lastDrainFailed { + if forceDeleteLabelPresent || timeOutOccurred { // To perform forceful machine drain/delete 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 @@ -837,11 +845,10 @@ func (c *controller) drainNode(machine *v1alpha1.Machine, driver cmiclient.CMICl maxEvictRetries = 1 glog.V(2).Infof( - "Force deletion has been triggerred for machine %q due to Label:%t, timeout:%t, lastDrainFailed:%t", + "Force deletion has been triggerred for machine %q due to Label:%t, timeout:%t", machine.Name, forceDeleteLabelPresent, timeOutOccurred, - lastDrainFailed, ) } @@ -870,22 +877,25 @@ func (c *controller) drainNode(machine *v1alpha1.Machine, driver cmiclient.CMICl // Drain successful glog.V(2).Infof("Drain successful for machine %q. \nBuf:%v \nErrBuf:%v", machine.Name, buf, errBuf) - description = fmt.Sprintf("%s. Drain successful", initiateVMDeletion) + description = fmt.Sprintf("Drain successful. %s", initiateVMDeletion) state = v1alpha1.MachineStateProcessing + phase = v1alpha1.MachineTerminating // Return error even when machine object is updated - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine deletion in process. " + description) } else if err != nil && forceDeleteMachine { // Drain failed on force deletion glog.Warningf("Drain failed for machine %q. However, since it's a force deletion shall continue deletion of VM. \nBuf:%v \nErrBuf:%v \nErr-Message:%v", machine.Name, buf, errBuf, err) - description = fmt.Sprintf("%s. Drain failed due to - %s. However, since it's a force deletion shall continue deletion of VM. ", initiateVMDeletion, err.Error()) + description = fmt.Sprintf("Drain failed due to - %s. However, since it's a force deletion shall continue deletion of VM. %s", err.Error(), initiateVMDeletion) state = v1alpha1.MachineStateProcessing + phase = v1alpha1.MachineTerminating } else { glog.Warningf("Drain failed for machine %q. \nBuf:%v \nErrBuf:%v \nErr-Message:%v", machine.Name, buf, errBuf, err) - description = fmt.Sprintf("%s. Drain failed due to - %s. Will retry in next sync.", initiateDrain, err.Error()) + description = fmt.Sprintf("Drain failed due to - %s. Will retry in next sync. %s", err.Error(), initiateDrain) state = v1alpha1.MachineStateFailed + phase = v1alpha1.MachineTerminating } clone := machine.DeepCopy() @@ -896,8 +906,7 @@ func (c *controller) drainNode(machine *v1alpha1.Machine, driver cmiclient.CMICl LastUpdateTime: metav1.Now(), } clone.Status.CurrentStatus = v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineTerminating, - //TimeoutActive: false, + Phase: phase, LastUpdateTime: metav1.Now(), } @@ -916,6 +925,7 @@ func (c *controller) deleteVM(machine *v1alpha1.Machine, driver cmiclient.CMICli retryRequired bool description string state v1alpha1.MachineState + phase v1alpha1.MachinePhase ) lastKnownState, err := driver.DeleteMachine() @@ -927,29 +937,34 @@ func (c *controller) deleteVM(machine *v1alpha1.Machine, driver cmiclient.CMICli switch grpcErr.Code() { case codes.Unknown, codes.DeadlineExceeded, codes.Aborted, codes.Unavailable: retryRequired = RetryOp - description = fmt.Sprintf("%s. VM deletion failed due to - %s. However, will re-try in the next resync.", initiateVMDeletion, err.Error()) + description = fmt.Sprintf("VM deletion failed due to - %s. However, will re-try in the next resync. %s", err.Error(), initiateVMDeletion) state = v1alpha1.MachineStateFailed + phase = v1alpha1.MachineTerminating case codes.NotFound: retryRequired = RetryOp - description = fmt.Sprintf("%s. VM not found. Continuing deletion", initiateNodeDeletion) + description = fmt.Sprintf("VM not found. Continuing deletion flow. %s", initiateNodeDeletion) state = v1alpha1.MachineStateProcessing + phase = v1alpha1.MachineTerminating default: retryRequired = DoNotRetryOp - description = fmt.Sprintf("%s. VM deletion failed due to - %s. However, aborting operation", permanentError, err.Error()) + description = fmt.Sprintf("VM deletion failed due to - %s. Aborting operation. %s", err.Error(), initiateVMDeletion) state = v1alpha1.MachineStateFailed + phase = v1alpha1.MachineTerminating } } else { retryRequired = DoNotRetryOp - description = fmt.Sprintf("%s. Error occurred while decoding gRPC error for machine %q: %s", permanentError, machine.Name, err) + description = fmt.Sprintf("Error occurred while decoding gRPC error: %s. %s", err.Error(), initiateVMDeletion) state = v1alpha1.MachineStateFailed + phase = v1alpha1.MachineFailed } } else { retryRequired = RetryOp - description = fmt.Sprintf("%s. VM deletion was successful for %q: %s", initiateNodeDeletion, machine.Name, err) + description = fmt.Sprintf("VM deletion was successful. %s", initiateNodeDeletion) state = v1alpha1.MachineStateProcessing + phase = v1alpha1.MachineTerminating - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine deletion in process. " + description) } clone := machine.DeepCopy() @@ -960,8 +975,7 @@ func (c *controller) deleteVM(machine *v1alpha1.Machine, driver cmiclient.CMICli LastUpdateTime: metav1.Now(), } clone.Status.CurrentStatus = v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineTerminating, - //TimeoutActive: false, + Phase: phase, LastUpdateTime: metav1.Now(), } clone.Status.LastKnownState = lastKnownState @@ -987,25 +1001,25 @@ func (c *controller) deleteNodeObject(machine *v1alpha1.Machine) (bool, error) { if nodeName != "" { // Delete node object - err := c.targetCoreClient.CoreV1().Nodes().Delete(nodeName, &metav1.DeleteOptions{}) + err = c.targetCoreClient.CoreV1().Nodes().Delete(nodeName, &metav1.DeleteOptions{}) if err != nil && !apierrors.IsNotFound(err) { // If its an error, and anyother error than object not found - description = fmt.Sprintf("%s. Deletion of Node Object %q failed due to error: %s", initiateNodeDeletion, machine.Name, err) + description = fmt.Sprintf("Deletion of Node Object %q failed due to error: %s. %s", nodeName, err, initiateNodeDeletion) state = v1alpha1.MachineStateFailed } else if err == nil { - description = fmt.Sprintf("%s. Deletion of Node Object %q is successful", initiateFinalizerRemoval, machine.Name) + description = fmt.Sprintf("Deletion of Node Object %q is successful. %s", nodeName, initiateFinalizerRemoval) state = v1alpha1.MachineStateProcessing - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine deletion in process. Deletion of node object was succesful") } else { - description = fmt.Sprintf("%s. No node object found for %q, continuing deletion", initiateFinalizerRemoval, machine.Name) + description = fmt.Sprintf("No node object found for %q, continuing deletion flow. %s", nodeName, initiateFinalizerRemoval) state = v1alpha1.MachineStateProcessing } } else { - description = fmt.Sprintf("%s. No node object found for %q, continuing deletion", initiateFinalizerRemoval, machine.Name) + description = fmt.Sprintf("No node object found for machine, continuing deletion flow. %s", initiateFinalizerRemoval) state = v1alpha1.MachineStateProcessing - err = fmt.Errorf("Machine reconcilation in process") + err = fmt.Errorf("Machine deletion in process. No node object found") } clone := machine.DeepCopy() @@ -1016,8 +1030,7 @@ func (c *controller) deleteNodeObject(machine *v1alpha1.Machine) (bool, error) { LastUpdateTime: metav1.Now(), } clone.Status.CurrentStatus = v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineTerminating, - //TimeoutActive: false, + Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), } diff --git a/pkg/controller/machine_util_test.go b/pkg/controller/machine_util_test.go index 8c5b64632..09d97a66e 100644 --- a/pkg/controller/machine_util_test.go +++ b/pkg/controller/machine_util_test.go @@ -130,7 +130,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ Node: "test-node", }, - nil, nil, nil), + nil, nil, nil, true), }, action: action{ node: &corev1.Node{ @@ -227,7 +227,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ Node: "test-node", }, - nil, nil, nil), + nil, nil, nil, true), }, action: action{ node: &corev1.Node{ @@ -308,7 +308,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ Node: "test-node", }, - nil, nil, nil), + nil, nil, nil, true), }, action: action{ node: &corev1.Node{}, @@ -362,7 +362,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ Node: "test-node", }, - nil, nil, nil), + nil, nil, nil, true), }, action: action{ node: &corev1.Node{ @@ -505,7 +505,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -559,7 +559,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -614,7 +614,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -670,7 +670,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -724,7 +724,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -779,7 +779,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -833,7 +833,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -938,7 +938,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -990,7 +990,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1043,7 +1043,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1093,7 +1093,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1145,7 +1145,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1198,7 +1198,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1251,7 +1251,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1366,7 +1366,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1436,7 +1436,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1511,7 +1511,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1591,7 +1591,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1666,7 +1666,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1746,7 +1746,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{ @@ -1820,7 +1820,7 @@ var _ = Describe("machine_util", func() { }, }, }, - nil, nil, nil, nil), + nil, nil, nil, nil, true), }, expect: expect{ node: &corev1.Node{