Fix calls to liberr.Wrap with even number of args

As the signature of the function is: Wrap(err error, kvpair ...interface{}) It expects to get an error and key-value pairs. However, in some places we also pass a message after the error, which breaks this expectation. In this PR those messages are dropped to remain with an odd number of arguments. Signed-off-by: Arik Hadas <[email protected]>
kubev2v · Feb 18, 2024 · 4042c66 · 4042c66
1 parent 0e79b91
commit 4042c66
Show file tree

Hide file tree

Showing 12 changed files with 54 additions and 271 deletions.
diff --git a/pkg/controller/plan/adapter/openstack/builder.go b/pkg/controller/plan/adapter/openstack/builder.go
@@ -246,8 +246,6 @@ const (
 	Multus = "multus"
 )
 
-const VM_LOOKUP_FAILED = "VM lookup failed."
-
 // Default properties
 var DefaultProperties = map[string]string{
 	CpuPolicy:       CpuPolicyShared,
@@ -269,11 +267,7 @@ func (r *Builder) VirtualMachine(vmRef ref.Ref, vmSpec *cnv.VirtualMachineSpec,
 	vm := &model.Workload{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			VM_LOOKUP_FAILED,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -300,11 +294,7 @@ func (r *Builder) VirtualMachine(vmRef ref.Ref, vmSpec *cnv.VirtualMachineSpec,
 	r.mapDisks(vm, persistentVolumeClaims, vmSpec)
 	err = r.mapNetworks(vm, vmSpec)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			"network mapping failed",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -730,11 +720,7 @@ func (r *Builder) Tasks(vmRef ref.Ref) (tasks []*plan.Task, err error) {
 	workload := &model.Workload{}
 	err = r.Source.Inventory.Find(workload, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			VM_LOOKUP_FAILED,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 	}
 
 	taskMap := map[string]int64{}
@@ -783,11 +769,7 @@ func (r *Builder) DataVolumes(vmRef ref.Ref, secret *core.Secret, configMap *cor
 func (r *Builder) PreferenceName(vmRef ref.Ref, configMap *core.ConfigMap) (name string, err error) {
 	vm := &model.Workload{}
 	if err = r.Source.Inventory.Find(vm, vmRef); err != nil {
-		err = liberr.Wrap(
-			err,
-			VM_LOOKUP_FAILED,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	os, version, distro := r.getOs(vm)
@@ -877,11 +859,7 @@ func getTemplateOs(os, version, distro string) string {
 func (r *Builder) TemplateLabels(vmRef ref.Ref) (labels map[string]string, err error) {
 	vm := &model.Workload{}
 	if err = r.Source.Inventory.Find(vm, vmRef); err != nil {
-		err = liberr.Wrap(
-			err,
-			VM_LOOKUP_FAILED,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -1155,7 +1133,7 @@ func (r *Builder) getVolumeAndAccessMode(storageClassName string) ([]core.Persis
 	storageProfile := &cdi.StorageProfile{}
 	err := r.Client.Get(context.TODO(), types.NamespacedName{Name: storageClassName}, storageProfile)
 	if err != nil {
-		return nil, nil, liberr.Wrap(err, "cannot get storage profile", "storageClassName", storageClassName)
+		return nil, nil, liberr.Wrap(err, "storageClassName", storageClassName)
 	}
 
 	if len(storageProfile.Status.ClaimPropertySets) > 0 &&

diff --git a/pkg/controller/plan/adapter/openstack/client.go b/pkg/controller/plan/adapter/openstack/client.go
@@ -185,11 +185,7 @@ func (r *Client) DetachDisks(vmRef ref.Ref) (err error) {
 func (r *Client) PreTransferActions(vmRef ref.Ref) (ready bool, err error) {
 	vm, err := r.getVM(vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			"VM lookup failed.",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	ready, err = r.ensureVmSnapshot(vm)
@@ -470,10 +466,7 @@ func (r *Client) createImageFromVolume(vm *libclient.VM, volumeID string) (image
 		if strings.HasPrefix(key, "os_glance") {
 			err = r.UnsetImageMetadata(volumeID, key)
 			if err != nil {
-				err = liberr.Wrap(
-					err,
-					"failed to remove reserved glance metadata from volume.",
-					"vm", vm.Name, "volumeID", volumeID, "key", key)
+				err = liberr.Wrap(err, "vm", vm.Name, "volumeID", volumeID, "key", key)
 				return
 			}
 		}

diff --git a/pkg/controller/plan/adapter/openstack/validator.go b/pkg/controller/plan/adapter/openstack/validator.go
@@ -8,8 +8,6 @@ import (
 	liberr "github.com/konveyor/forklift-controller/pkg/lib/error"
 )
 
-const VM_NOT_FOUND_IN_INVENTORY = "VM not found in inventory."
-
 // Validator
 type Validator struct {
 	plan      *api.Plan
@@ -29,11 +27,7 @@ func (r *Validator) StorageMapped(vmRef ref.Ref) (ok bool, err error) {
 	vm := &model.Workload{}
 	err = r.inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			VM_NOT_FOUND_IN_INVENTORY,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	for _, volType := range vm.VolumeTypes {
@@ -53,11 +47,7 @@ func (r *Validator) NetworksMapped(vmRef ref.Ref) (ok bool, err error) {
 	vm := &model.Workload{}
 	err = r.inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			VM_NOT_FOUND_IN_INVENTORY,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	for _, network := range vm.Networks {
@@ -89,11 +79,7 @@ func (r *Validator) PodNetwork(vmRef ref.Ref) (ok bool, err error) {
 	vm := &model.Workload{}
 	err = r.inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			VM_NOT_FOUND_IN_INVENTORY,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 

diff --git a/pkg/controller/plan/adapter/ova/builder.go b/pkg/controller/plan/adapter/ova/builder.go
@@ -59,11 +59,6 @@ const (
 	Unknown = "unknown"
 )
 
-// Error messages
-const (
-	ErrVMLookupFailed = "VM lookup failed."
-)
-
 // Regex which matches the snapshot identifier suffix of a
 // OVA disk backing file.
 var backingFilePattern = regexp.MustCompile("-\\d\\d\\d\\d\\d\\d.vmdk")
@@ -121,11 +116,7 @@ func (r *Builder) PodEnvironment(vmRef ref.Ref, sourceSecret *core.Secret) (env
 	vm := &model.VM{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			ErrVMLookupFailed,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -157,11 +148,7 @@ func (r *Builder) DataVolumes(vmRef ref.Ref, secret *core.Secret, _ *core.Config
 	vm := &model.VM{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			ErrVMLookupFailed,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -220,11 +207,7 @@ func (r *Builder) VirtualMachine(vmRef ref.Ref, object *cnv.VirtualMachineSpec,
 	vm := &model.VM{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			ErrVMLookupFailed,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -415,11 +398,7 @@ func (r *Builder) Tasks(vmRef ref.Ref) (list []*plan.Task, err error) {
 	vm := &model.VM{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			ErrVMLookupFailed,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	for _, disk := range vm.Disks {
@@ -450,11 +429,7 @@ func (r *Builder) TemplateLabels(vmRef ref.Ref) (labels map[string]string, err e
 	vm := &model.VM{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			ErrVMLookupFailed,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 

diff --git a/pkg/controller/plan/adapter/ova/validator.go b/pkg/controller/plan/adapter/ova/validator.go
@@ -14,11 +14,6 @@ type Validator struct {
 	inventory web.Client
 }
 
-// Error messages
-const (
-	ErrVMNotFound = "VM not found in inventory."
-)
-
 // Load.
 func (r *Validator) Load() (err error) {
 	r.inventory, err = web.NewClient(r.plan.Referenced.Provider.Source)
@@ -39,11 +34,7 @@ func (r *Validator) NetworksMapped(vmRef ref.Ref) (ok bool, err error) {
 	vm := &model.VM{}
 	err = r.inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			ErrVMNotFound,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -64,11 +55,7 @@ func (r *Validator) PodNetwork(vmRef ref.Ref) (ok bool, err error) {
 	vm := &model.Workload{}
 	err = r.inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			ErrVMNotFound,
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 

diff --git a/pkg/controller/plan/adapter/ovirt/builder.go b/pkg/controller/plan/adapter/ovirt/builder.go
@@ -187,11 +187,7 @@ func (r *Builder) DataVolumes(vmRef ref.Ref, secret *core.Secret, configMap *cor
 	vm := &model.Workload{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			"VM lookup failed.",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	url := r.Source.Provider.Spec.URL
@@ -259,11 +255,7 @@ func (r *Builder) VirtualMachine(vmRef ref.Ref, object *cnv.VirtualMachineSpec,
 	vm := &model.Workload{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			"VM lookup failed.",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -531,11 +523,7 @@ func (r *Builder) Tasks(vmRef ref.Ref) (list []*plan.Task, err error) {
 	vm := &model.Workload{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			"VM lookup failed.",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 	}
 	for _, da := range vm.DiskAttachments {
 		// We don't add a task for LUNs because we don't copy their content but rather assume we can connect to
@@ -562,19 +550,12 @@ func (r *Builder) Tasks(vmRef ref.Ref) (list []*plan.Task, err error) {
 func (r *Builder) PreferenceName(vmRef ref.Ref, configMap *core.ConfigMap) (name string, err error) {
 	vm := &model.Workload{}
 	if err = r.Source.Inventory.Find(vm, vmRef); err != nil {
-		err = liberr.Wrap(
-			err,
-			"VM lookup failed.",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	name, ok := configMap.Data[vm.OSType]
 	if !ok {
-		err = liberr.Wrap(err,
-			"nothing fits the input OS",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 	}
 	return
 }
@@ -583,11 +564,7 @@ func (r *Builder) TemplateLabels(vmRef ref.Ref) (labels map[string]string, err e
 	vm := &model.Workload{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			"VM lookup failed.",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 
@@ -625,11 +602,7 @@ func (r *Builder) LunPersistentVolumes(vmRef ref.Ref) (pvs []core.PersistentVolu
 	vm := &model.Workload{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			"VM lookup failed.",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	for _, da := range vm.DiskAttachments {
@@ -692,11 +665,7 @@ func (r *Builder) LunPersistentVolumeClaims(vmRef ref.Ref) (pvcs []core.Persiste
 	vm := &model.Workload{}
 	err = r.Source.Inventory.Find(vm, vmRef)
 	if err != nil {
-		err = liberr.Wrap(
-			err,
-			"VM lookup failed.",
-			"vm",
-			vmRef.String())
+		err = liberr.Wrap(err, "vm", vmRef.String())
 		return
 	}
 	for _, da := range vm.DiskAttachments {
@@ -834,7 +803,7 @@ func (r *Builder) getDefaultVolumeAndAccessMode(storageClassName string) ([]core
 	storageProfile := &cdi.StorageProfile{}
 	err := r.Client.Get(context.TODO(), types.NamespacedName{Name: storageClassName}, storageProfile)
 	if err != nil {
-		return nil, nil, liberr.Wrap(err, "cannot get StorageProfile")
+		return nil, nil, liberr.Wrap(err)
 	}
 
 	if len(storageProfile.Status.ClaimPropertySets) > 0 &&