Skip to content

Commit e667792

Browse files
nikhilbargenikhilbarge
authored
Patch VM from guest cluster instead of update and remove conversions (#3732)
1 parent 8beb4c6 commit e667792

File tree

18 files changed

+910
-1130
lines changed

18 files changed

+910
-1130
lines changed

‎pkg/common/utils/utils.go‎

Lines changed: 31 additions & 410 deletions
Large diffs are not rendered by default.

‎pkg/common/utils/utils_test.go‎

Lines changed: 784 additions & 493 deletions
Large diffs are not rendered by default.

‎pkg/csi/service/wcp/controller_helper.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"strings"
2525

2626
"github.com/container-storage-interface/spec/lib/go/csi"
27-
vmoperatorv1alpha5 "github.com/vmware-tanzu/vm-operator/api/v1alpha5"
27+
vmoperatortypes "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
2828
"github.com/vmware/govmomi/object"
2929
"github.com/vmware/govmomi/property"
3030
"github.com/vmware/govmomi/vim25/mo"
@@ -165,12 +165,12 @@ func validateWCPControllerExpandVolumeRequest(ctx context.Context, req *csi.Cont
165165
return logger.LogNewErrorCodef(log, codes.Internal,
166166
"failed to get config with error: %+v", err)
167167
}
168-
vmOperatorClient, err := k8s.NewClientForGroup(ctx, cfg, vmoperatorv1alpha5.GroupName)
168+
vmOperatorClient, err := k8s.NewClientForGroup(ctx, cfg, vmoperatortypes.GroupName)
169169
if err != nil {
170170
return logger.LogNewErrorCodef(log, codes.Internal,
171-
"failed to get client for group %s with error: %+v", vmoperatorv1alpha5.GroupName, err)
171+
"failed to get client for group %s with error: %+v", vmoperatortypes.GroupName, err)
172172
}
173-
vmList, err := utils.ListVirtualMachines(ctx, vmOperatorClient, "")
173+
vmList, err := utils.ListVirtualMachinesAllNamespaces(ctx, vmOperatorClient)
174174
if err != nil {
175175
return logger.LogNewErrorCodef(log, codes.Internal,
176176
"failed to list virtualmachines with error: %+v", err)

‎pkg/csi/service/wcpguest/controller.go‎

Lines changed: 25 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,7 @@ import (
3232
"github.com/fsnotify/fsnotify"
3333
snapshotterClientSet "github.com/kubernetes-csi/external-snapshotter/client/v8/clientset/versioned"
3434
"github.com/prometheus/client_golang/prometheus/promhttp"
35-
vmoperatorv1alpha1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1"
36-
vmoperatorv1alpha2 "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
37-
vmoperatorv1alpha3 "github.com/vmware-tanzu/vm-operator/api/v1alpha3"
38-
vmoperatorv1alpha4 "github.com/vmware-tanzu/vm-operator/api/v1alpha4"
39-
vmoperatorv1alpha5 "github.com/vmware-tanzu/vm-operator/api/v1alpha5"
35+
vmoperatortypes "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
4036
cnstypes "github.com/vmware/govmomi/cns/types"
4137
"google.golang.org/grpc/codes"
4238
"google.golang.org/grpc/status"
@@ -130,7 +126,7 @@ func (c *controller) Init(config *commonconfig.Config, version string) error {
130126
return err
131127
}
132128

133-
c.vmOperatorClient, err = k8s.NewClientForGroup(ctx, c.restClientConfig, vmoperatorv1alpha5.GroupName)
129+
c.vmOperatorClient, err = k8s.NewClientForGroup(ctx, c.restClientConfig, vmoperatortypes.GroupName)
134130
if err != nil {
135131
log.Errorf("failed to create vmOperatorClient. Error: %+v", err)
136132
return err
@@ -268,7 +264,7 @@ func (c *controller) ReloadConfiguration() error {
268264
return err
269265
}
270266
log.Infof("successfully re-created supervisorClient using updated configuration")
271-
c.vmOperatorClient, err = k8s.NewClientForGroup(ctx, c.restClientConfig, vmoperatorv1alpha5.GroupName)
267+
c.vmOperatorClient, err = k8s.NewClientForGroup(ctx, c.restClientConfig, vmoperatortypes.GroupName)
272268
if err != nil {
273269
log.Errorf("failed to create vmOperatorClient. Error: %+v", err)
274270
return err
@@ -756,7 +752,7 @@ func controllerPublishForBlockVolume(ctx context.Context, req *csi.ControllerPub
756752
var diskUUID string
757753
var err error
758754

759-
virtualMachine := &vmoperatorv1alpha5.VirtualMachine{}
755+
virtualMachine := &vmoperatortypes.VirtualMachine{}
760756
vmKey := types.NamespacedName{
761757
Namespace: c.supervisorNamespace,
762758
Name: req.NodeId,
@@ -765,7 +761,7 @@ func controllerPublishForBlockVolume(ctx context.Context, req *csi.ControllerPub
765761
timeoutSeconds := int64(getAttacherTimeoutInMin(ctx) * 60)
766762
timeout := time.Now().Add(time.Duration(timeoutSeconds) * time.Second)
767763
for {
768-
virtualMachine, _, err = utils.GetVirtualMachineAllApiVersions(
764+
virtualMachine, _, err = utils.GetVirtualMachine(
769765
ctx, vmKey, c.vmOperatorClient)
770766
if err != nil {
771767
msg := fmt.Sprintf("failed to get VirtualMachines for the node: %q. Error: %+v", req.NodeId, err)
@@ -784,13 +780,13 @@ func controllerPublishForBlockVolume(ctx context.Context, req *csi.ControllerPub
784780
break
785781
}
786782
// Create a patch for the VM prior to modifying it with the new volumes.
787-
old_virtualMachine := virtualMachine.DeepCopy()
783+
oldVirtualMachine := virtualMachine.DeepCopy()
788784
// Volume is not present in the virtualMachine.Spec.Volumes, so adding
789785
// volume in the spec and patching virtualMachine instance.
790-
vmvolumes := vmoperatorv1alpha5.VirtualMachineVolume{
786+
vmvolumes := vmoperatortypes.VirtualMachineVolume{
791787
Name: req.VolumeId,
792-
VirtualMachineVolumeSource: vmoperatorv1alpha5.VirtualMachineVolumeSource{
793-
PersistentVolumeClaim: &vmoperatorv1alpha5.PersistentVolumeClaimVolumeSource{
788+
VirtualMachineVolumeSource: vmoperatortypes.VirtualMachineVolumeSource{
789+
PersistentVolumeClaim: &vmoperatortypes.PersistentVolumeClaimVolumeSource{
794790
PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{
795791
ClaimName: req.VolumeId,
796792
},
@@ -799,7 +795,7 @@ func controllerPublishForBlockVolume(ctx context.Context, req *csi.ControllerPub
799795
}
800796
virtualMachine.Spec.Volumes = append(virtualMachine.Spec.Volumes, vmvolumes)
801797
// Issue a patch with the modified VM against the patch created above.
802-
if err := utils.PatchVirtualMachine(ctx, c.vmOperatorClient, virtualMachine, old_virtualMachine); err == nil {
798+
if err := utils.PatchVirtualMachine(ctx, c.vmOperatorClient, virtualMachine, oldVirtualMachine); err == nil {
803799
break
804800
} else {
805801
log.Errorf("failed to update virtualmachine. Err: %v", err)
@@ -809,7 +805,7 @@ func controllerPublishForBlockVolume(ctx context.Context, req *csi.ControllerPub
809805
log.Error(msg)
810806
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
811807
}
812-
virtualMachine = &vmoperatorv1alpha5.VirtualMachine{}
808+
virtualMachine = &vmoperatortypes.VirtualMachine{}
813809
}
814810

815811
for _, volume := range virtualMachine.Status.Volumes {
@@ -839,54 +835,11 @@ func controllerPublishForBlockVolume(ctx context.Context, req *csi.ControllerPub
839835
// blocking wait for update event
840836
log.Debugf("waiting for update on virtualmachine: %q", virtualMachine.Name)
841837
event := <-watchVirtualMachine.ResultChan()
842-
vm := &vmoperatorv1alpha5.VirtualMachine{}
843-
vm5, ok := event.Object.(*vmoperatorv1alpha5.VirtualMachine)
838+
vm, ok := event.Object.(*vmoperatortypes.VirtualMachine)
844839
if !ok {
845-
vm4, ok := event.Object.(*vmoperatorv1alpha4.VirtualMachine)
846-
if !ok {
847-
vm3, ok := event.Object.(*vmoperatorv1alpha3.VirtualMachine)
848-
if !ok {
849-
vm2, ok := event.Object.(*vmoperatorv1alpha2.VirtualMachine)
850-
if !ok {
851-
vm1, ok := event.Object.(*vmoperatorv1alpha1.VirtualMachine)
852-
if !ok {
853-
msg := fmt.Sprintf("Watch on virtualmachine %q timed out", virtualMachine.Name)
854-
log.Error(msg)
855-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
856-
} else {
857-
log.Infof("converting v1alpha1 VirtualMachine to v1alpha5 VirtualMachine, name %s", vm1.Name)
858-
err = vmoperatorv1alpha1.Convert_v1alpha1_VirtualMachine_To_v1alpha5_VirtualMachine(
859-
vm1, vm, nil)
860-
if err != nil {
861-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, err.Error())
862-
}
863-
}
864-
} else {
865-
log.Infof("converting v1alpha2 VirtualMachine to v1alpha5 VirtualMachine, name %s", vm2.Name)
866-
err = vmoperatorv1alpha2.Convert_v1alpha2_VirtualMachine_To_v1alpha5_VirtualMachine(
867-
vm2, vm, nil)
868-
if err != nil {
869-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, err.Error())
870-
}
871-
}
872-
} else {
873-
log.Infof("converting v1alpha3 VirtualMachine to v1alpha5 VirtualMachine, name %s", vm3.Name)
874-
err = vmoperatorv1alpha3.Convert_v1alpha3_VirtualMachine_To_v1alpha5_VirtualMachine(
875-
vm3, vm, nil)
876-
if err != nil {
877-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, err.Error())
878-
}
879-
}
880-
} else {
881-
log.Infof("converting v1alpha4 VirtualMachine to v1alpha5 VirtualMachine, name %s", vm4.Name)
882-
err = vmoperatorv1alpha4.Convert_v1alpha4_VirtualMachine_To_v1alpha5_VirtualMachine(
883-
vm4, vm, nil)
884-
if err != nil {
885-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, err.Error())
886-
}
887-
}
888-
} else {
889-
vm = vm5
840+
msg := fmt.Sprintf("Watch on virtualmachine %q timed out", virtualMachine.Name)
841+
log.Error(msg)
842+
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
890843
}
891844
if vm.Name != virtualMachine.Name {
892845
log.Debugf("Observed vm name: %q, expecting vm name: %q, volumeID: %q",
@@ -1147,7 +1100,7 @@ func (c *controller) ControllerUnpublishVolume(ctx context.Context, req *csi.Con
11471100
func controllerUnpublishForBlockVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest, c *controller) (
11481101
*csi.ControllerUnpublishVolumeResponse, string, error) {
11491102
log := logger.GetLogger(ctx)
1150-
virtualMachine := &vmoperatorv1alpha5.VirtualMachine{}
1103+
virtualMachine := &vmoperatortypes.VirtualMachine{}
11511104
vmKey := types.NamespacedName{
11521105
Namespace: c.supervisorNamespace,
11531106
Name: req.NodeId,
@@ -1156,7 +1109,7 @@ func controllerUnpublishForBlockVolume(ctx context.Context, req *csi.ControllerU
11561109
timeoutSeconds := int64(getAttacherTimeoutInMin(ctx) * 60)
11571110
timeout := time.Now().Add(time.Duration(timeoutSeconds) * time.Second)
11581111
for {
1159-
virtualMachine, _, err = utils.GetVirtualMachineAllApiVersions(
1112+
virtualMachine, _, err = utils.GetVirtualMachine(
11601113
ctx, vmKey, c.vmOperatorClient)
11611114
if err != nil {
11621115
if errors.IsNotFound(err) {
@@ -1169,13 +1122,13 @@ func controllerUnpublishForBlockVolume(ctx context.Context, req *csi.ControllerU
11691122
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
11701123
}
11711124
log.Debugf("Found VirtualMachine for node: %q.", req.NodeId)
1172-
1125+
oldVirtualMachine := virtualMachine.DeepCopy()
11731126
for index, volume := range virtualMachine.Spec.Volumes {
11741127
if volume.Name == req.VolumeId {
11751128
log.Debugf("Removing volume %q from VirtualMachine %q", volume.Name, virtualMachine.Name)
11761129
virtualMachine.Spec.Volumes = append(virtualMachine.Spec.Volumes[:index],
11771130
virtualMachine.Spec.Volumes[index+1:]...)
1178-
err = utils.UpdateVirtualMachine(ctx, c.vmOperatorClient, virtualMachine)
1131+
err = utils.PatchVirtualMachine(ctx, c.vmOperatorClient, virtualMachine, oldVirtualMachine)
11791132
break
11801133
}
11811134
}
@@ -1189,7 +1142,7 @@ func controllerUnpublishForBlockVolume(ctx context.Context, req *csi.ControllerU
11891142
log.Error(msg)
11901143
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
11911144
}
1192-
virtualMachine = &vmoperatorv1alpha5.VirtualMachine{}
1145+
virtualMachine = &vmoperatortypes.VirtualMachine{}
11931146
}
11941147
isVolumePresentInVMStatus := false
11951148
for _, volume := range virtualMachine.Status.Volumes {
@@ -1225,52 +1178,12 @@ func controllerUnpublishForBlockVolume(ctx context.Context, req *csi.ControllerU
12251178
log.Debugf("Waiting for update on VirtualMachine: %q", virtualMachine.Name)
12261179
// Block on update events
12271180
event := <-watchVirtualMachine.ResultChan()
1228-
vm := &vmoperatorv1alpha5.VirtualMachine{}
1229-
vm5, ok := event.Object.(*vmoperatorv1alpha5.VirtualMachine)
1181+
vm, ok := event.Object.(*vmoperatortypes.VirtualMachine)
12301182
if !ok {
1231-
vm4, ok := event.Object.(*vmoperatorv1alpha4.VirtualMachine)
1232-
if !ok {
1233-
vm3, ok := event.Object.(*vmoperatorv1alpha3.VirtualMachine)
1234-
if !ok {
1235-
vm2, ok := event.Object.(*vmoperatorv1alpha2.VirtualMachine)
1236-
if !ok {
1237-
vm1, ok := event.Object.(*vmoperatorv1alpha1.VirtualMachine)
1238-
if !ok {
1239-
msg := fmt.Sprintf("Watch on virtualmachine %q timed out", virtualMachine.Name)
1240-
log.Error(msg)
1241-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
1242-
} else {
1243-
err = vmoperatorv1alpha1.Convert_v1alpha1_VirtualMachine_To_v1alpha5_VirtualMachine(
1244-
vm1, vm, nil)
1245-
if err != nil {
1246-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, err.Error())
1247-
}
1248-
}
1249-
} else {
1250-
err = vmoperatorv1alpha2.Convert_v1alpha2_VirtualMachine_To_v1alpha5_VirtualMachine(
1251-
vm2, vm, nil)
1252-
if err != nil {
1253-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, err.Error())
1254-
}
1255-
}
1256-
} else {
1257-
err = vmoperatorv1alpha3.Convert_v1alpha3_VirtualMachine_To_v1alpha5_VirtualMachine(
1258-
vm3, vm, nil)
1259-
if err != nil {
1260-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, err.Error())
1261-
}
1262-
}
1263-
} else {
1264-
err = vmoperatorv1alpha4.Convert_v1alpha4_VirtualMachine_To_v1alpha5_VirtualMachine(
1265-
vm4, vm, nil)
1266-
if err != nil {
1267-
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, err.Error())
1268-
}
1269-
}
1270-
} else {
1271-
vm = vm5
1183+
msg := fmt.Sprintf("Watch on virtualmachine %q timed out", virtualMachine.Name)
1184+
log.Error(msg)
1185+
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
12721186
}
1273-
12741187
if vm.Name != virtualMachine.Name {
12751188
log.Debugf("Observed vm name: %q, expecting vm name: %q, volumeID: %q",
12761189
vm.Name, virtualMachine.Name, req.VolumeId)
@@ -1430,7 +1343,7 @@ func (c *controller) ControllerExpandVolume(ctx context.Context, req *csi.Contro
14301343
volSizeBytes := int64(req.GetCapacityRange().GetRequiredBytes())
14311344

14321345
if !commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.OnlineVolumeExtend) {
1433-
vmList, err := utils.GetVirtualMachineListAllApiVersions(ctx, c.supervisorNamespace, c.vmOperatorClient)
1346+
vmList, err := utils.ListVirtualMachines(ctx, c.vmOperatorClient, c.supervisorNamespace)
14341347
if err != nil {
14351348
msg := fmt.Sprintf("failed to list virtualmachines with error: %+v", err)
14361349
log.Error(msg)

‎pkg/csi/service/wcpguest/controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"testing"
2525
"time"
2626

27-
vmoperatortypes "github.com/vmware-tanzu/vm-operator/api/v1alpha5"
27+
vmoperatortypes "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
2828
v1 "k8s.io/api/core/v1"
2929
"k8s.io/apimachinery/pkg/api/errors"
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

0 commit comments

Comments
 (0)