Adding support for deletion when a VM is detected as expunging (#70)

* Adding support for deletion when a VM is detected as expunging or expunged

* Cleaning up misses and refactoring per comments

Author: maxdrib

pkg/cloud/client.go

@@ -58,8 +58,12 @@ func NewClient(ccPath string) (Client, error) {
 		return nil, errors.Wrapf(err, "error encountered while parsing [Global] section from config at path: %s", ccPath)
 	}
 
+	// The client returned from NewAsyncClient works in a synchronous way. On the other hand,
+	// a client returned from NewClient works in an asynchronous way. Dive into the constructor definition
+	// comments for more details
 	c.cs = cloudstack.NewAsyncClient(cfg.APIURL, cfg.APIKey, cfg.SecretKey, cfg.VerifySSL)
 	c.csAsync = cloudstack.NewClient(cfg.APIURL, cfg.APIKey, cfg.SecretKey, cfg.VerifySSL)
+	
 	_, err := c.cs.APIDiscovery.ListApis(c.cs.APIDiscovery.NewListApisParams())
 	if err != nil && strings.Contains(strings.ToLower(err.Error()), "i/o timeout") {
 		return c, errors.Wrap(err, "timeout while checking CloudStack API Client connectivity")
@@ -68,6 +72,6 @@ func NewClient(ccPath string) (Client, error) {
 }
 
 func NewClientFromCSAPIClient(cs *cloudstack.CloudStackClient) Client {
-	c := &client{cs: cs}
+	c := &client{cs: cs, csAsync: cs}
 	return c
 }

pkg/cloud/instance.go

@@ -18,7 +18,6 @@ package cloud
 
 import (
 	"fmt"
-
 	"strings"
 
 	capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -239,5 +238,18 @@ func (c *client) DestroyVMInstance(csMachine *infrav1.CloudStackMachine) error {
 	} else if err != nil {
 		return err
 	}
+
+	if err := c.ResolveVMInstanceDetails(csMachine); err == nil && (csMachine.Status.InstanceState == "Expunging" ||
+		csMachine.Status.InstanceState == "Expunged") {
+		// VM is stopped and getting expunged.  So the desired state is getting satisfied.  Let's move on.
+		return nil
+	} else if err != nil {
+		if strings.Contains(strings.ToLower(err.Error()), "no match found") {
+			// VM doesn't exist.  So the desired state is in effect.  Our work is done here.
+			return nil
+		}
+		return err
+	}
+
 	return errors.New("VM deletion in progress")
 }

pkg/cloud/instance_test.go

@@ -325,4 +325,69 @@ var _ = Describe("Instance", func() {
 			})
 		})
 	})
+
+	Context("when destroying a VM instance", func() {
+		expungeDestroyParams := &cloudstack.DestroyVirtualMachineParams{}
+		expungeDestroyParams.SetExpunge(true)
+
+		It("calls destroy and finds VM doesn't exist, then returns nil", func() {
+			vms.EXPECT().NewDestroyVirtualMachineParams(*dummies.CSMachine1.Spec.InstanceID).
+				Return(expungeDestroyParams)
+			vms.EXPECT().DestroyVirtualMachine(expungeDestroyParams).Return(nil, fmt.Errorf("unable to find uuid for id"))
+			Ω(client.DestroyVMInstance(dummies.CSMachine1)).
+				Should(Succeed())
+		})
+
+		It("calls destroy and returns unexpected error", func() {
+			vms.EXPECT().NewDestroyVirtualMachineParams(*dummies.CSMachine1.Spec.InstanceID).
+				Return(expungeDestroyParams)
+			vms.EXPECT().DestroyVirtualMachine(expungeDestroyParams).Return(nil, fmt.Errorf("new error"))
+			Ω(client.DestroyVMInstance(dummies.CSMachine1)).Should(MatchError("new error"))
+		})
+
+		It("calls destroy without error but cannot resolve VM after", func() {
+			vms.EXPECT().NewDestroyVirtualMachineParams(*dummies.CSMachine1.Spec.InstanceID).
+				Return(expungeDestroyParams)
+			vms.EXPECT().DestroyVirtualMachine(expungeDestroyParams).Return(nil, nil)
+			vms.EXPECT().GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID).Return(nil, -1, notFoundError)
+			vms.EXPECT().GetVirtualMachinesMetricByName(dummies.CSMachine1.Name).Return(nil, -1, notFoundError)
+			Ω(client.DestroyVMInstance(dummies.CSMachine1)).
+				Should(Succeed())
+		})
+
+		It("calls destroy without error and identifies it as expunging", func() {
+			vms.EXPECT().NewDestroyVirtualMachineParams(*dummies.CSMachine1.Spec.InstanceID).
+				Return(expungeDestroyParams)
+			vms.EXPECT().DestroyVirtualMachine(expungeDestroyParams).Return(nil, nil)
+			vms.EXPECT().GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID).
+				Return(&cloudstack.VirtualMachinesMetric{
+					State: "Expunging",
+				}, 1, nil)
+			Ω(client.DestroyVMInstance(dummies.CSMachine1)).
+				Should(Succeed())
+		})
+
+		It("calls destroy without error and identifies it as expunged", func() {
+			vms.EXPECT().NewDestroyVirtualMachineParams(*dummies.CSMachine1.Spec.InstanceID).
+				Return(expungeDestroyParams)
+			vms.EXPECT().DestroyVirtualMachine(expungeDestroyParams).Return(nil, nil)
+			vms.EXPECT().GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID).
+				Return(&cloudstack.VirtualMachinesMetric{
+					State: "Expunged",
+				}, 1, nil)
+			Ω(client.DestroyVMInstance(dummies.CSMachine1)).
+				Should(Succeed())
+		})
+
+		It("calls destroy without error and identifies it as stopping", func() {
+			vms.EXPECT().NewDestroyVirtualMachineParams(*dummies.CSMachine1.Spec.InstanceID).
+				Return(expungeDestroyParams)
+			vms.EXPECT().DestroyVirtualMachine(expungeDestroyParams).Return(nil, nil)
+			vms.EXPECT().GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID).
+				Return(&cloudstack.VirtualMachinesMetric{
+					State: "Stopping",
+				}, 1, nil)
+			Ω(client.DestroyVMInstance(dummies.CSMachine1)).Should(MatchError("VM deletion in progress"))
+		})
+	})
 })