openstack: avoid NPE when there's only an image attached

It's possible to not create a storage map when only an image is attached to a VM, this will lead to an NPE later on Signed-off-by: Benny Zlotnik <[email protected]> openstack: add storagemap Signed-off-by: Benny Zlotnik <[email protected]> openstack: exclude glance storagemap from validation Signed-off-by: Benny Zlotnik <[email protected]> openstack: add validation for glance source storage map Signed-off-by: Benny Zlotnik <[email protected]> address comments Signed-off-by: Benny Zlotnik <[email protected]> openstack: do not return an error in the validation Signed-off-by: Benny Zlotnik <[email protected]> address comments Signed-off-by: Benny Zlotnik <[email protected]>
kubev2v · Mar 28, 2024 · 4e7b135 · 4e7b135
1 parent aabe33a
commit 4e7b135
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 9 deletions.
diff --git a/pkg/apis/forklift/v1beta1/doc.go b/pkg/apis/forklift/v1beta1/doc.go
@@ -21,3 +21,6 @@ limitations under the License.
 // +k8s:defaulter-gen=TypeMeta
 // +groupName=forklift.konveyor.io
 package v1beta1
+
+// GlanceSource represents an image of which the source is Glance, to be used in storage mapping
+const GlanceSource = "glance"
diff --git a/pkg/controller/plan/adapter/openstack/builder.go b/pkg/controller/plan/adapter/openstack/builder.go
@@ -1010,12 +1010,32 @@ func (r *Builder) ensureVolumePopulatorPVC(workload *model.Workload, image *mode
 			originalVolumeDiskId = imageProperty.(string)
 		}
 
-		storageClassName := r.Context.Map.Storage.Spec.Map[0].Destination.StorageClass
-		if volumeType := r.getVolumeType(workload, originalVolumeDiskId); volumeType != "" {
-			storageClassName, err = r.getStorageClassName(workload, volumeType)
-			if err != nil {
-				err = liberr.Wrap(err)
-				return
+		mapList := r.Context.Map.Storage.Spec.Map
+
+		// Check if there's a storage map available
+		if len(mapList) == 0 {
+			err = liberr.New("no storage map found in the migration plan")
+			return
+		}
+
+		var storageClassName string
+
+		// VM is image based, look for a glance key in the mapping
+		if workload.ImageID != "" {
+			// At this point the StorageMap has been validated, and the VM has to be fully mapped
+			for _, storageMap := range mapList {
+				if storageMap.Source.Name == api.GlanceSource {
+					storageClassName = storageMap.Destination.StorageClass
+				}
+			}
+		} else {
+			// VM has a volume, look for the volume type in the mapping
+			if volumeType := r.getVolumeType(workload, originalVolumeDiskId); volumeType != "" {
+				storageClassName, err = r.getStorageClassName(workload, volumeType)
+				if err != nil {
+					err = liberr.Wrap(err)
+					return
+				}
 			}
 		}
 		if pvc, err = r.persistentVolumeClaimWithSourceRef(*image, storageClassName, populatorName, annotations, workload.ID); err != nil {

diff --git a/pkg/controller/plan/adapter/openstack/builder_test.go b/pkg/controller/plan/adapter/openstack/builder_test.go
@@ -1,11 +1,12 @@
 package openstack
 
 import (
+	v1beta1 "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("vSphere builder", func() {
+var _ = Describe("OpenStack builder", func() {
 	DescribeTable("should", func(os, version, distro, matchPreferenceName string) {
 		Expect(getPreferenceOs(os, version, distro)).Should(Equal(matchPreferenceName))
 	},
@@ -16,3 +17,9 @@ var _ = Describe("vSphere builder", func() {
 		Entry("ubuntu 22", Ubuntu, "22.04.3", Ubuntu, "ubuntu"),
 	)
 })
+
+var _ = Describe("OpenStack Glance const test", func() {
+	It("GlanceSource should be glance, changing it may break the UI", func() {
+		Expect(v1beta1.GlanceSource).Should(Equal("glance"))
+	})
+})
diff --git a/pkg/controller/plan/adapter/openstack/validator.go b/pkg/controller/plan/adapter/openstack/validator.go
@@ -35,6 +35,12 @@ func (r *Validator) StorageMapped(vmRef ref.Ref) (ok bool, err error) {
 			return
 		}
 	}
+
+	// If vm is image based, we need to see glance in the storage map
+	if vm.ImageID != "" && !r.plan.Referenced.Map.Storage.Status.Refs.Find(ref.Ref{Name: api.GlanceSource}) {
+		return
+	}
+
 	ok = true
 	return
 }

diff --git a/pkg/controller/provider/web/openstack/client.go b/pkg/controller/provider/web/openstack/client.go
@@ -396,6 +396,10 @@ func (r *Finder) Network(ref *base.Ref) (object interface{}, err error) {
 //	NotFoundErr
 //	RefNotUniqueErr
 func (r *Finder) Storage(ref *base.Ref) (object interface{}, err error) {
+	if ref.Name == api.GlanceSource {
+		return
+	}
+
 	volumeType := &VolumeType{}
 	err = r.ByRef(volumeType, *ref)
 	if err == nil {

diff --git a/tests/suit/BUILD.bazel b/tests/suit/BUILD.bazel
@@ -67,6 +67,7 @@ go_test(
     ],
     deps = [
         "//pkg/apis/forklift/v1beta1",
+        "//pkg/apis/forklift/v1beta1/ref",
         "//pkg/lib/client/openstack",
         "//pkg/lib/logging",
         "//tests/suit/framework",

diff --git a/tests/suit/openstack_test.go b/tests/suit/openstack_test.go
@@ -4,6 +4,7 @@ import (
 	"time"
 
 	forkliftv1 "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1"
+	"github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1/ref"
 	"github.com/konveyor/forklift-controller/tests/suit/framework"
 	"github.com/konveyor/forklift-controller/tests/suit/utils"
 	. "github.com/onsi/ginkgo"
@@ -62,6 +63,13 @@ var _ = Describe("[level:component]Migration tests for OpenStack provider", func
 
 		//TODO: Add storage-class  pass here
 		storageMapDef := utils.NewStorageMap(namespace, *provider, test_storage_map_name, []string{vmData.GetVolumeId()}, openstackStorageClass)
+		storageMapDef.Spec.Map = append(storageMapDef.Spec.Map,
+			forkliftv1.StoragePair{
+				Source: ref.Ref{Name: forkliftv1.GlanceSource},
+				Destination: forkliftv1.DestinationStorage{
+					StorageClass: openstackStorageClass,
+				},
+			})
 
 		err = utils.CreateStorageMapFromDefinition(f.CrClient, storageMapDef)
 		Expect(err).ToNot(HaveOccurred())

diff --git a/tests/suit/utils/storagemap.go b/tests/suit/utils/storagemap.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"time"
 
-	api "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1"
 	forkliftv1 "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1"
 	"github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1/provider"
 	"github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1/ref"
@@ -38,7 +37,7 @@ func NewStorageMap(namespace string, providerIdentifier forkliftv1.Provider, sto
 		}
 
 		switch providerIdentifier.Type() {
-		case api.Ova:
+		case forkliftv1.Ova:
 			pair.Source = ref.Ref{Name: sd}
 		default:
 			pair.Source = ref.Ref{ID: sd}
-Original file line number
+Diff line change
@@ Expand Up @@
     			return
     		}
     	}
+    	// If vm is image based, we need to see glance in the storage map
+    	if vm.ImageID != "" && !r.plan.Referenced.Map.Storage.Status.Refs.Find(ref.Ref{Name: api.GlanceSource}) {
+    		return
+    	}
     	ok = true
     	return
     }
@@ Expand Down @@