Fix GetVolumeInfo to use UUDI, fix create Server checks

plugins/module_utils/openstack/openstack.go

@@ -488,6 +488,21 @@ func GetServerVolumeAttachments(client *gophercloud.ProviderClient, serverID str
 	return attachments, nil
 }
 
+// GetServer retrieves a server (instance) by its UUID.
+func GetServer(provider *gophercloud.ProviderClient, serverID string) (*servers.Server, error) {
+	computeClient, err := openstack.NewComputeV2(provider, gophercloud.EndpointOpts{
+		Region: os.Getenv("OS_REGION_NAME"),
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create compute client: %w", err)
+	}
+	server, err := servers.Get(context.TODO(), computeClient, serverID).Extract()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get server %s: %w", serverID, err)
+	}
+	return server, nil
+}
+
 func GetServerByName(provider *gophercloud.ProviderClient, instanceName string) (*servers.Server, error) {
 	computeClient, err := openstack.NewComputeV2(provider, gophercloud.EndpointOpts{
 		Region: os.Getenv("OS_REGION_NAME"),
@@ -958,6 +973,28 @@ func GetVolumeInfo(provider *gophercloud.ProviderClient, volumeName string) (*Vo
 	return volumeInfo, nil
 }
 
+// GetVolumeInfoByID retrieves volume information by its UUID. This is useful
+// when the volume name is not known (for example the name is derived from the
+// VM name plus the disk key) but the volume UUID is available.
+func GetVolumeInfoByID(provider *gophercloud.ProviderClient, volumeID string) (*VolumeInfo, error) {
+	volume, err := GetVolume(provider, volumeID)
+	if err != nil {
+		logger.Log.Infof("Failed to get volume by ID %s: %v", volumeID, err)
+		return nil, err
+	}
+
+	volumeInfo := &VolumeInfo{
+		ID:       volume.ID,
+		Name:     volume.Name,
+		Status:   volume.Status,
+		Size:     volume.Size,
+		Bootable: volume.Bootable,
+		Metadata: volume.Metadata,
+	}
+
+	return volumeInfo, nil
+}
+
 func CinderManage(provider *gophercloud.ProviderClient, volumeName string, hostPool string) (*volumes.Volume, error) {
 	bsClient, err := openstack.NewBlockStorageV3(provider, gophercloud.EndpointOpts{
 		Region: os.Getenv("OS_REGION_NAME"),

plugins/modules/src/create_server/create_server.go

@@ -94,6 +94,26 @@ func success(changed bool, id string) {
 	os.Exit(0)
 }
 
+// activeServerForVolume returns the ID of the instance the volume is attached
+// to when that instance exists and is ACTIVE, otherwise an empty string.
+func activeServerForVolume(provider *gophercloud.ProviderClient, volume *volumes.Volume) string {
+	for _, att := range volume.Attachments {
+		if att.ServerID == "" {
+			continue
+		}
+		server, err := osm_os.GetServer(provider, att.ServerID)
+		if err != nil {
+			logger.Log.Infof("Failed to get server %s attached to volume %s: %v", att.ServerID, volume.ID, err)
+			continue
+		}
+		if server.Status == "ACTIVE" {
+			return server.ID
+		}
+		logger.Log.Infof("Server %s attached to volume %s is not ACTIVE (status: %s)", server.ID, volume.ID, server.Status)
+	}
+	return ""
+}
+
 func Run() {
 	var response ansible.Response
 	if len(os.Args) != 2 {
@@ -147,9 +167,24 @@ func Run() {
 			ansible.FailJson(response)
 		}
 
+		// A boot volume that is already "in-use" has normally been attached to an
+		// instance by a previous (successful) run. Rather than failing, treat this
+		// as idempotent success when the attached instance still exists and is
+		// ACTIVE, returning that instance ID so downstream tasks keep working.
+		if volume.Status == "in-use" {
+			if existingID := activeServerForVolume(provider, volume); existingID != "" {
+				logger.Log.Infof("Boot volume %s is already attached to active instance %s, treating as success", volume.ID, existingID)
+				success(false, existingID)
+				return
+			}
+			logger.Log.Infof("Volume is in-use but not attached to an active instance")
+			response.Msg = "Volume is in-use but not attached to an active instance"
+			ansible.FailJson(response)
+		}
+
 		if volume.Status != "available" {
 			logger.Log.Infof("Volume is not available, current status: %s", volume.Status)
-			response.Msg = "Volume is not available"
+			response.Msg = "Volume is not available, current status: " + volume.Status
 			ansible.FailJson(response)
 		}
 

plugins/modules/src/volume_info/volume_info.go

@@ -28,6 +28,7 @@ import (
 type ModuleArgs struct {
 	Cloud osm_os.DstCloud `json:"cloud"`
 	Name  string          `json:"name"`
+	ID    string          `json:"id"`
 }
 
 type VolumeInfo struct {
@@ -104,12 +105,28 @@ func Run() {
 		FailJson(response)
 	}
 
-	sharedVolumeInfo, err := osm_os.GetVolumeInfo(provider, moduleArgs.Name)
-	if err != nil {
-		response.Msg = "Failed to get volume info for: " + moduleArgs.Name + " error: " + err.Error()
+	if moduleArgs.ID == "" && moduleArgs.Name == "" {
+		response.Msg = "Either 'id' or 'name' must be provided to look up a volume"
 		FailJson(response)
 	}
 
+	// Look up by UUID when provided (the volume name is not always known at the
+	// call site), otherwise fall back to looking up by name.
+	var sharedVolumeInfo *osm_os.VolumeInfo
+	if moduleArgs.ID != "" {
+		sharedVolumeInfo, err = osm_os.GetVolumeInfoByID(provider, moduleArgs.ID)
+		if err != nil {
+			response.Msg = "Failed to get volume info for id: " + moduleArgs.ID + " error: " + err.Error()
+			FailJson(response)
+		}
+	} else {
+		sharedVolumeInfo, err = osm_os.GetVolumeInfo(provider, moduleArgs.Name)
+		if err != nil {
+			response.Msg = "Failed to get volume info for: " + moduleArgs.Name + " error: " + err.Error()
+			FailJson(response)
+		}
+	}
+
 	// Convert shared VolumeInfo to local VolumeInfo type
 	volumeInfo := &VolumeInfo{
 		ID:       sharedVolumeInfo.ID,

plugins/modules/volume_info.py

@@ -9,7 +9,8 @@
 version_added: "2.9.0"
 author: "OpenStack tenant migration tools (@os-migrate)"
 description:
-  - "Get volume information from OpenStack by name."
+  - "Get volume information from OpenStack by name or by UUID."
+  - "Either O(name) or O(id) must be provided. When O(id) is set it takes precedence."
 options:
   cloud:
     description:
@@ -35,16 +36,30 @@
   name:
     description:
       - Name of the volume to get information for.
-    required: true
+      - Mutually exclusive with O(id); one of O(name) or O(id) is required.
+    required: false
+    type: str
+  id:
+    description:
+      - UUID of the volume to get information for.
+      - Useful when the volume name is not known but its UUID is.
+      - Mutually exclusive with O(name); one of O(name) or O(id) is required.
+    required: false
     type: str
 """
 
 EXAMPLES = r"""
-- name: Get volume information
+- name: Get volume information by name
   os_migrate.vmware_migration_kit.volume_info:
     cloud: source_cloud
     name: "my-volume"
   register: volume_info
+
+- name: Get volume information by UUID
+  os_migrate.vmware_migration_kit.volume_info:
+    cloud: source_cloud
+    id: "11111111-2222-3333-4444-555555555555"
+  register: volume_info
 """
 
 RETURN = r"""

roles/import_workloads/tasks/create_os_instance.yml

@@ -22,7 +22,7 @@
 - name: Get volume information for Cinder boot
   os_migrate.vmware_migration_kit.volume_info:
     cloud: "{{ dst_cloud }}"
-    name: "{{ vm_name }}"
+    id: "{{ volume_uuid[0] }}"
   register: cinder_volume_result
   when: import_workloads_boot_from_cinder
 

tests/unit/openstack_utils_test.go

@@ -1600,3 +1600,109 @@ func TestDiscoverWrapVMsSuccess(t *testing.T) {
 		t.Fatalf("expected port-1, got %#v", vms[0].PortIDs)
 	}
 }
+
+// GetVolumeInfoByID success: look up a volume by its UUID.
+func TestGetVolumeInfoByIDSuccess(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	th.Mux.HandleFunc("/volumes/vol-uuid-1", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			t.Fatalf("Expected GET but got %s", r.Method)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{
+			"volume": {
+				"id": "vol-uuid-1",
+				"name": "rhel-9.5-1-2000",
+				"status": "available",
+				"size": 30,
+				"bootable": "true",
+				"metadata": {"osm": "true"}
+			}
+		}`))
+	})
+
+	_ = os.Setenv("OS_REGION_NAME", "RegionOne")
+	info, err := osm_os.GetVolumeInfoByID(createMockProvider(), "vol-uuid-1")
+	if err != nil {
+		t.Fatalf("GetVolumeInfoByID returned error: %v", err)
+	}
+	if info.ID != "vol-uuid-1" {
+		t.Errorf("expected ID 'vol-uuid-1', got %s", info.ID)
+	}
+	if info.Name != "rhel-9.5-1-2000" {
+		t.Errorf("expected name 'rhel-9.5-1-2000', got %s", info.Name)
+	}
+	if info.Bootable != "true" {
+		t.Errorf("expected bootable 'true', got %s", info.Bootable)
+	}
+}
+
+// GetVolumeInfoByID not found: the API returns 404.
+func TestGetVolumeInfoByIDNotFound(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	th.Mux.HandleFunc("/volumes/missing", func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	})
+
+	_ = os.Setenv("OS_REGION_NAME", "RegionOne")
+	_, err := osm_os.GetVolumeInfoByID(createMockProvider(), "missing")
+	if err == nil {
+		t.Fatal("expected error but got nil")
+	}
+}
+
+// GetVolumeInfoByID client init failure.
+func TestGetVolumeInfoByIDClientInitFailure(t *testing.T) {
+	_ = os.Setenv("OS_REGION_NAME", "RegionOne")
+	_, err := osm_os.GetVolumeInfoByID(createFailingProvider(), "vol-uuid-1")
+	if err == nil {
+		t.Fatal("expected error but got nil")
+	}
+}
+
+// GetServer success: look up an instance by its UUID.
+func TestGetServerSuccess(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	th.Mux.HandleFunc("/servers/srv-abc", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodGet {
+			t.Fatalf("Expected GET but got %s", r.Method)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{
+			"server": {
+				"id": "srv-abc",
+				"name": "rhel-9.5-1",
+				"status": "ACTIVE"
+			}
+		}`))
+	})
+
+	_ = os.Setenv("OS_REGION_NAME", "RegionOne")
+	server, err := osm_os.GetServer(createMockProvider(), "srv-abc")
+	if err != nil {
+		t.Fatalf("GetServer returned error: %v", err)
+	}
+	if server.ID != "srv-abc" {
+		t.Errorf("expected ID 'srv-abc', got %s", server.ID)
+	}
+	if server.Status != "ACTIVE" {
+		t.Errorf("expected status 'ACTIVE', got %s", server.Status)
+	}
+}
+
+// GetServer client init failure.
+func TestGetServerClientInitFailure(t *testing.T) {
+	_ = os.Setenv("OS_REGION_NAME", "RegionOne")
+	_, err := osm_os.GetServer(createFailingProvider(), "srv-abc")
+	if err == nil {
+		t.Fatal("expected error but got nil")
+	}
+}