Skip to content

Commit

Permalink
Move instancestorage to vmopv1util
Browse files Browse the repository at this point in the history
Doesn't need to be in old provider anymore
  • Loading branch information
Bryan Venteicher committed Nov 13, 2024
1 parent c948168 commit f7f959a
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 36 deletions.
8 changes: 4 additions & 4 deletions controllers/virtualmachine/volume/volume_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions controllers/virtualmachine/volume/volume_controller_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions pkg/providers/vsphere/placement/zone_placement.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 != "" {
Expand Down
3 changes: 1 addition & 2 deletions pkg/providers/vsphere/virtualmachine/configspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down
6 changes: 3 additions & 3 deletions pkg/providers/vsphere/vmprovider_vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
})

Expand Down
3 changes: 1 addition & 2 deletions pkg/providers/vsphere/vmprovider_vm_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions pkg/providers/vsphere/vmprovider_vm_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand All @@ -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))
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2021 VMware, Inc. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package instancestorage
package vmopv1

import (
"strings"
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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
}
Loading

0 comments on commit f7f959a

Please sign in to comment.