Bug 1983612: Make InstanceCreate idempotent

Prior to this change, if machine-controller gets reprovisioned in the
middle of running InstanceCreate, stale volumes might be left on the
cluster.

With this change, before creating a new root volume for an instance,
InstanceCreate deletes any volumes with the same name.

Author: pierreprinetti

pkg/cloud/openstack/clients/machineservice.go

@@ -707,8 +707,28 @@ func (is *InstanceService) InstanceCreate(clusterName string, name string, clust
 		}
 
 		if bootfromvolume.SourceType(config.RootVolume.SourceType) == bootfromvolume.SourceImage {
+			// Name the volume after the instance
+			volumeName := name
+
 			// if source type is "image" then we have to create a volume from the image first
-			klog.Infof("Creating a bootable volume from image %v.", config.RootVolume.SourceUUID)
+			klog.Infof("Creating bootable volume with name %q from image %v.", volumeName, config.RootVolume.SourceUUID)
+
+			// Deleting any volumes with the same name, as they may
+			// be leftovers from a previous failed try.
+			{
+				volumeIDs, err := volumeIDsFromName(is.volumeClient, volumeName)
+				if err != nil {
+					klog.Infof("unable to list volumes with name %q: %v.", volumeName, err)
+				}
+
+				for _, volumeID := range volumeIDs {
+					if err := volumes.Delete(is.volumeClient, volumeID, nil).ExtractErr(); err != nil {
+						klog.Infof("unable to delete volume with ID %q: %v.", volumeID, err)
+					} else {
+						klog.Infof("deleted volume with name %q and ID %q", volumeName, volumeID)
+					}
+				}
+			}
 
 			imageID, err := imageutils.IDFromName(is.imagesClient, config.RootVolume.SourceUUID)
 			if err != nil {
@@ -717,11 +737,10 @@ func (is *InstanceService) InstanceCreate(clusterName string, name string, clust
 
 			// Create a volume first
 			volumeCreateOpts := volumes.CreateOpts{
-				Size:       config.RootVolume.Size,
-				VolumeType: config.RootVolume.VolumeType,
-				ImageID:    imageID,
-				// The same name as the instance
-				Name:             name,
+				Size:             config.RootVolume.Size,
+				VolumeType:       config.RootVolume.VolumeType,
+				ImageID:          imageID,
+				Name:             volumeName,
 				AvailabilityZone: config.RootVolume.Zone,
 			}
 

pkg/cloud/openstack/clients/utils.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/gophercloud/openstack"
+	"github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes"
 	"github.com/gophercloud/utils/openstack/clientconfig"
 	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
 
@@ -106,3 +107,30 @@ func GetProviderClient(cloud clientconfig.Cloud, cert []byte) (*gophercloud.Prov
 
 	return provider, nil
 }
+
+// volumeIDsFromName returns zero or more volume IDs corresponding to a name.
+// The returned error is only non-nil in case of failure.
+func volumeIDsFromName(client *gophercloud.ServiceClient, name string) ([]string, error) {
+	listOpts := volumes.ListOpts{
+		Name: name,
+	}
+
+	pages, err := volumes.List(client, listOpts).AllPages()
+	if err != nil {
+		return nil, err
+	}
+
+	all, err := volumes.ExtractVolumes(pages)
+	if err != nil {
+		return nil, err
+	}
+
+	IDs := make([]string, 0, len(all))
+	for _, s := range all {
+		if s.Name == name {
+			IDs = append(IDs, s.ID)
+		}
+	}
+
+	return IDs, nil
+}