Move instancestorage helpers to vmopv1util

Doesn't need to be in old provider anymore, and makes more sense
to be under pkg/util/vmopv1 in our current hierarchy.

controllers/virtualmachine/volume/volume_controller.go

@@ -38,10 +38,10 @@ import (
 	"github.com/vmware-tanzu/vm-operator/pkg/patch"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants"
-	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/instancestorage"
 	"github.com/vmware-tanzu/vm-operator/pkg/record"
 	"github.com/vmware-tanzu/vm-operator/pkg/util"
 	"github.com/vmware-tanzu/vm-operator/pkg/util/annotations"
+	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
 )
 
 const (
@@ -271,7 +271,7 @@ func (r *Reconciler) reconcileResult(ctx *pkgctx.VolumeContext) ctrl.Result {
 	if pkgcfg.FromContext(ctx).Features.InstanceStorage {
 		// Requeue the request if all instance storage PVCs are not bound.
 		_, pvcsBound := ctx.VM.Annotations[constants.InstanceStoragePVCsBoundAnnotationKey]
-		if !pvcsBound && instancestorage.IsPresent(ctx.VM) {
+		if !pvcsBound && vmopv1util.IsInstanceStoragePresent(ctx.VM) {
 			return ctrl.Result{RequeueAfter: wait.Jitter(
 				pkgcfg.FromContext(ctx).InstanceStorage.SeedRequeueDuration,
 				pkgcfg.FromContext(ctx).InstanceStorage.JitterMaxFactor,
@@ -344,7 +344,7 @@ func (r *Reconciler) reconcileInstanceStoragePVCs(ctx *pkgctx.VolumeContext) (bo
 
 	// If the VM Spec doesn't have any instance storage volumes, there is nothing for us to do.
 	// We do not support removing - or changing really - this type of volume.
-	isVolumes := instancestorage.FilterVolumes(ctx.VM)
+	isVolumes := vmopv1util.FilterInstanceStorageVolumes(ctx.VM)
 	if len(isVolumes) == 0 {
 		return true, nil
 	}
@@ -538,7 +538,7 @@ func (r *Reconciler) createInstanceStoragePVC(
 	// We merely consider creating non-existing PVCs in reconcileInstanceStoragePVCs flow.
 	// We specifically don't need of CreateOrUpdate / CreateOrPatch.
 	if err := r.Create(ctx, pvc); err != nil {
-		if instancestorage.IsInsufficientQuota(err) {
+		if vmopv1util.IsInsufficientQuota(err) {
 			r.recorder.EmitEvent(ctx.VM, "Create", err, true)
 		}
 		return err

controllers/virtualmachine/volume/volume_controller_intg_test.go

@@ -25,8 +25,8 @@ import (
 	"github.com/vmware-tanzu/vm-operator/pkg/constants/testlabels"
 	"github.com/vmware-tanzu/vm-operator/pkg/patch"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants"
-	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/instancestorage"
 	"github.com/vmware-tanzu/vm-operator/pkg/util"
+	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
 	"github.com/vmware-tanzu/vm-operator/test/builder"
 )
 
@@ -154,7 +154,7 @@ func intgTestsReconcile() {
 		vm := getVirtualMachine(objKey)
 		Expect(vm).ToNot(BeNil())
 
-		volumes := instancestorage.FilterVolumes(vm)
+		volumes := vmopv1util.FilterInstanceStorageVolumes(vm)
 		Expect(volumes).ToNot(BeEmpty())
 
 		Eventually(func() bool {
@@ -185,7 +185,7 @@ func intgTestsReconcile() {
 		vm := getVirtualMachine(objKey)
 		Expect(vm).ToNot(BeNil())
 
-		volumes := instancestorage.FilterVolumes(vm)
+		volumes := vmopv1util.FilterInstanceStorageVolumes(vm)
 		Expect(volumes).ToNot(BeEmpty())
 
 		Eventually(func() bool {
@@ -209,7 +209,7 @@ func intgTestsReconcile() {
 		vm := getVirtualMachine(objKey)
 		Expect(vm).ToNot(BeNil())
 
-		volumes := instancestorage.FilterVolumes(vm)
+		volumes := vmopv1util.FilterInstanceStorageVolumes(vm)
 		Expect(volumes).ToNot(BeEmpty())
 
 		for _, vol := range volumes {
@@ -323,7 +323,7 @@ func intgTestsReconcile() {
 			Specify("PVCs are created with the expected annotation", func() {
 				Expect(ctx.Client.Get(ctx, vmKey, vm)).To(Succeed())
 
-				volumes := instancestorage.FilterVolumes(vm)
+				volumes := vmopv1util.FilterInstanceStorageVolumes(vm)
 				Expect(volumes).ToNot(BeEmpty())
 
 				Eventually(func(g Gomega) {

controllers/virtualmachine/volume/volume_controller_unit_test.go

@@ -31,9 +31,9 @@ import (
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
 	providerfake "github.com/vmware-tanzu/vm-operator/pkg/providers/fake"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants"
-	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/instancestorage"
 	"github.com/vmware-tanzu/vm-operator/pkg/util"
 	"github.com/vmware-tanzu/vm-operator/pkg/util/ptr"
+	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
 	"github.com/vmware-tanzu/vm-operator/test/builder"
 )
 
@@ -1230,7 +1230,7 @@ func getInstanceStoragePVCs(ctx *pkgctx.VolumeContext, testCtx *builder.UnitTest
 	var errs []error
 	pvcList := make([]corev1.PersistentVolumeClaim, 0)
 
-	volumes := instancestorage.FilterVolumes(ctx.VM)
+	volumes := vmopv1util.FilterInstanceStorageVolumes(ctx.VM)
 	for _, vol := range volumes {
 		objKey := client.ObjectKey{
 			Namespace: ctx.VM.Namespace,

pkg/providers/vsphere/placement/zone_placement.go

@@ -20,9 +20,9 @@ import (
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants"
-	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/instancestorage"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/vcenter"
 	"github.com/vmware-tanzu/vm-operator/pkg/topology"
+	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
 )
 
 type Constraints struct {
@@ -57,7 +57,7 @@ func doesVMNeedPlacement(vmCtx pkgctx.VirtualMachineContext) (res Result, needZo
 	}
 
 	if pkgcfg.FromContext(vmCtx).Features.InstanceStorage {
-		if instancestorage.IsPresent(vmCtx.VM) {
+		if vmopv1util.IsInstanceStoragePresent(vmCtx.VM) {
 			res.InstanceStoragePlacement = true
 
 			if hostMoID := vmCtx.VM.Annotations[constants.InstanceStorageSelectedNodeMOIDAnnotationKey]; hostMoID != "" {

pkg/providers/vsphere/virtualmachine/configspec.go

@@ -10,7 +10,6 @@ import (
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants"
-	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/instancestorage"
 	"github.com/vmware-tanzu/vm-operator/pkg/util"
 	"github.com/vmware-tanzu/vm-operator/pkg/util/ptr"
 	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
@@ -206,7 +205,7 @@ func CreateConfigSpecForPlacement(
 	})
 
 	if pkgcfg.FromContext(vmCtx).Features.InstanceStorage {
-		isVolumes := instancestorage.FilterVolumes(vmCtx.VM)
+		isVolumes := vmopv1util.FilterInstanceStorageVolumes(vmCtx.VM)
 
 		for idx, dev := range CreateInstanceStorageDiskDevices(isVolumes) {
 			configSpec.DeviceChange = append(configSpec.DeviceChange, &vimtypes.VirtualDeviceConfigSpec{

pkg/providers/vsphere/vmprovider_vm_test.go

@@ -38,12 +38,12 @@ import (
 	"github.com/vmware-tanzu/vm-operator/pkg/providers"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants"
-	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/instancestorage"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/virtualmachine"
 	"github.com/vmware-tanzu/vm-operator/pkg/topology"
 	pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util"
 	kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube"
 	"github.com/vmware-tanzu/vm-operator/pkg/util/ptr"
+	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
 	"github.com/vmware-tanzu/vm-operator/pkg/vmconfig"
 	"github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto"
 	"github.com/vmware-tanzu/vm-operator/test/builder"
@@ -2049,7 +2049,7 @@ func vmTests() {
 					isStorage vmopv1.InstanceStorage) {
 
 					ExpectWithOffset(1, isStorage.Volumes).ToNot(BeEmpty())
-					isVolumes := instancestorage.FilterVolumes(vm)
+					isVolumes := vmopv1util.FilterInstanceStorageVolumes(vm)
 					ExpectWithOffset(1, isVolumes).To(HaveLen(len(isStorage.Volumes)))
 
 					for _, isVol := range isStorage.Volumes {
@@ -2094,7 +2094,7 @@ func vmTests() {
 					Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineConditionCreated)).To(BeFalse())
 
 					By("Instance storage volumes should be added to VM", func() {
-						Expect(instancestorage.IsPresent(vm)).To(BeTrue())
+						Expect(vmopv1util.IsInstanceStoragePresent(vm)).To(BeTrue())
 						expectInstanceStorageVolumes(vm, vmClass.Spec.Hardware.InstanceStorage)
 					})
 

pkg/providers/vsphere/vmprovider_vm_utils.go

@@ -21,7 +21,6 @@ import (
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants"
-	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/instancestorage"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/sysprep"
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/vmlifecycle"
 	"github.com/vmware-tanzu/vm-operator/pkg/util"
@@ -492,7 +491,7 @@ func AddInstanceStorageVolumes(
 	vmCtx pkgctx.VirtualMachineContext,
 	is vmopv1.InstanceStorage) bool {
 
-	if instancestorage.IsPresent(vmCtx.VM) {
+	if vmopv1util.IsInstanceStoragePresent(vmCtx.VM) {
 		// Instance storage disks are copied from the class to the VM only once, regardless
 		// if the class changes.
 		return true

pkg/providers/vsphere/vmprovider_vm_utils_test.go

@@ -21,9 +21,9 @@ import (
 	"github.com/vmware-tanzu/vm-operator/pkg/conditions"
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
-	vsphere "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere"
-	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/instancestorage"
+	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere"
 	"github.com/vmware-tanzu/vm-operator/pkg/util"
+	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
 	"github.com/vmware-tanzu/vm-operator/test/builder"
 )
 
@@ -911,7 +911,7 @@ func vmUtilTests() {
 			isStorage vmopv1.InstanceStorage) {
 
 			ExpectWithOffset(1, isStorage.Volumes).ToNot(BeEmpty())
-			isVolumes := instancestorage.FilterVolumes(vm)
+			isVolumes := vmopv1util.FilterInstanceStorageVolumes(vm)
 			ExpectWithOffset(1, isVolumes).To(HaveLen(len(isStorage.Volumes)))
 
 			for _, isVol := range isStorage.Volumes {
@@ -939,7 +939,7 @@ func vmUtilTests() {
 			It("VM Class does not contain instance storage volumes", func() {
 				is := vsphere.AddInstanceStorageVolumes(vmCtx, vmClass.Spec.Hardware.InstanceStorage)
 				Expect(is).To(BeFalse())
-				Expect(instancestorage.FilterVolumes(vmCtx.VM)).To(BeEmpty())
+				Expect(vmopv1util.FilterInstanceStorageVolumes(vmCtx.VM)).To(BeEmpty())
 			})
 
 			When("Instance Volume is added in VM Class", func() {
@@ -957,13 +957,13 @@ func vmUtilTests() {
 					is := vsphere.AddInstanceStorageVolumes(vmCtx, vmClass.Spec.Hardware.InstanceStorage)
 					Expect(is).To(BeTrue())
 
-					isVolumesBefore := instancestorage.FilterVolumes(vmCtx.VM)
+					isVolumesBefore := vmopv1util.FilterInstanceStorageVolumes(vmCtx.VM)
 					expectInstanceStorageVolumes(vmCtx.VM, vmClass.Spec.Hardware.InstanceStorage)
 
 					// Instance Storage is already configured, should not patch again
 					is = vsphere.AddInstanceStorageVolumes(vmCtx, vmClass.Spec.Hardware.InstanceStorage)
 					Expect(is).To(BeTrue())
-					isVolumesAfter := instancestorage.FilterVolumes(vmCtx.VM)
+					isVolumesAfter := vmopv1util.FilterInstanceStorageVolumes(vmCtx.VM)
 					Expect(isVolumesAfter).To(HaveLen(len(isVolumesBefore)))
 					Expect(isVolumesAfter).To(Equal(isVolumesBefore))
 				})

pkg/providers/vsphere/instancestorage/instance_storage.go → pkg/util/vmopv1/instancestorage.go

@@ -1,7 +1,7 @@
 // Copyright (c) 2021 VMware, Inc. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-package instancestorage
+package vmopv1
 
 import (
 	"strings"
@@ -11,8 +11,8 @@ import (
 	vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3"
 )
 
-// IsPresent checks if VM Spec has instance volumes added to its Volumes list.
-func IsPresent(vm *vmopv1.VirtualMachine) bool {
+// IsInstanceStoragePresent checks if VM Spec has instance volumes added to its Volumes list.
+func IsInstanceStoragePresent(vm *vmopv1.VirtualMachine) bool {
 	for _, vol := range vm.Spec.Volumes {
 		if pvc := vol.PersistentVolumeClaim; pvc != nil && pvc.InstanceVolumeClaim != nil {
 			return true
@@ -21,8 +21,8 @@ func IsPresent(vm *vmopv1.VirtualMachine) bool {
 	return false
 }
 
-// FilterVolumes returns instance storage volumes present in VM spec.
-func FilterVolumes(vm *vmopv1.VirtualMachine) []vmopv1.VirtualMachineVolume {
+// FilterInstanceStorageVolumes returns instance storage volumes present in VM spec.
+func FilterInstanceStorageVolumes(vm *vmopv1.VirtualMachine) []vmopv1.VirtualMachineVolume {
 	var volumes []vmopv1.VirtualMachineVolume
 	for _, vol := range vm.Spec.Volumes {
 		if pvc := vol.PersistentVolumeClaim; pvc != nil && pvc.InstanceVolumeClaim != nil {
@@ -34,8 +34,10 @@ func FilterVolumes(vm *vmopv1.VirtualMachine) []vmopv1.VirtualMachineVolume {
 }
 
 func IsInsufficientQuota(err error) bool {
-	if apierrors.IsForbidden(err) && (strings.Contains(err.Error(), "insufficient quota") || strings.Contains(err.Error(), "exceeded quota")) {
-		return true
+	if apierrors.IsForbidden(err) {
+		e := err.Error()
+		return strings.Contains(e, "insufficient quota") || strings.Contains(e, "exceeded quota")
 	}
+
 	return false
 }