Skip to content

Commit 5048e6c

Browse files
skogtaskogta
committed
f
1 parent ae60328 commit 5048e6c

File tree

4 files changed

+201
-70
lines changed

4 files changed

+201
-70
lines changed

‎hack/release.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ set -o nounset
2323
set -o pipefail
2424
set -x
2525

26-
DO_WINDOWS_BUILD=${DO_WINDOWS_BUILD_ENV:-true}
26+
DO_WINDOWS_BUILD=${DO_WINDOWS_BUILD_ENV:-false}
2727

2828
# BASE_REPO is the root path of the image repository
2929
readonly BASE_IMAGE_REPO=us-central1-docker.pkg.dev/k8s-staging-images/csi-vsphere

‎pkg/internalapis/cnsoperator/cnsvolumeattachment/cnsvolumeattachment.go

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,12 @@ var (
4343
// CnsVolumeAttachment exposes an interface to support adding
4444
// and removing information about attached VMs to a PVC.
4545
type CnsVolumeAttachment interface {
46-
// Add adds the input VM UUID to the list of
46+
// Add adds the input VM instance UUID to the list of
4747
// attached VMs for the given volume.
48-
AddVmToAttachedList(ctx context.Context, volumeName, VmUUID string) error
48+
AddVmToAttachedList(ctx context.Context, volumeName, VmInstanceUUID string) error
4949
// RemoveVmFromAttachedList removes the input VM UUID from
5050
// the list of attached VMs for the given volume.
51-
RemoveVmFromAttachedList(ctx context.Context, volumeName, VmUUID string) error
51+
RemoveVmFromAttachedList(ctx context.Context, volumeName, VmInstanceUUID string) (error, bool)
5252
// CnsVolumeAttachmentExistsForPvc returns true if CnsVolumeAttachment for a PVC is found.
5353
CnsVolumeAttachmentExistsForPvc(ctx context.Context, volumeName string) (bool, error)
5454
}
@@ -98,19 +98,19 @@ func GetCnsVolumeAttachmentInstance(ctx context.Context) (CnsVolumeAttachment, e
9898
return cnsVolumeAttachmentInstance, nil
9999
}
100100

101-
// Add adds the input VM UUID to the list of
101+
// Add adds the input VM InstanceUUID to the list of
102102
// attached VMs for the given volume.
103103
// Callers need to specify cnsVolumeAttachment as a combination of
104104
// "<SV-namespace>/<SV-PVC-name>". This combination is used to uniquely
105105
// identify CnsVolumeAttachment instances.
106106
// The instance is created if it doesn't exist.
107107
// Returns an error if the operation cannot be persisted on the API server.
108108
func (f *cnsVolumeAttachment) AddVmToAttachedList(ctx context.Context,
109-
volumeName, VmUUID string) error {
109+
volumeName, VmInstanceUUID string) error {
110110
log := logger.GetLogger(ctx)
111111

112112
log.Infof("Adding VM %s to cnsVolumeAttachment %s",
113-
VmUUID, volumeName)
113+
VmInstanceUUID, volumeName)
114114
actual, _ := f.volumeLock.LoadOrStore(volumeName, &sync.Mutex{})
115115
instanceLock, ok := actual.(*sync.Mutex)
116116
if !ok {
@@ -146,7 +146,7 @@ func (f *cnsVolumeAttachment) AddVmToAttachedList(ctx context.Context,
146146
},
147147
Spec: v1alpha1.CnsVolumeAttachmentSpec{
148148
AttachedVms: []string{
149-
VmUUID,
149+
VmInstanceUUID,
150150
},
151151
},
152152
}
@@ -162,17 +162,17 @@ func (f *cnsVolumeAttachment) AddVmToAttachedList(ctx context.Context,
162162
return err
163163
}
164164

165-
// Verify if input VmUUID exists in existing AttachedVMs list.
165+
// Verify if input VmInstanceUUID exists in existing AttachedVMs list.
166166
log.Debugf("Verifying if VM %s exists in current list of attached Vms. Current list: %+v",
167-
VmUUID, instance.Spec.AttachedVms)
167+
VmInstanceUUID, instance.Spec.AttachedVms)
168168
currentAttachedVmsList := instance.Spec.AttachedVms
169169
for _, currentAttachedVM := range currentAttachedVmsList {
170-
if currentAttachedVM == VmUUID {
171-
log.Debugf("Found VM %s in list. Returning.", VmUUID)
170+
if currentAttachedVM == VmInstanceUUID {
171+
log.Debugf("Found VM %s in list. Returning.", VmInstanceUUID)
172172
return nil
173173
}
174174
}
175-
newAttachVmsList := append(currentAttachedVmsList, VmUUID)
175+
newAttachVmsList := append(currentAttachedVmsList, VmInstanceUUID)
176176
instance.Spec.AttachedVms = newAttachVmsList
177177
log.Debugf("Updating cnsVolumeAttachment instance %s with spec: %+v", volumeName, instance)
178178
err = f.client.Update(ctx, instance)
@@ -191,14 +191,15 @@ func (f *cnsVolumeAttachment) AddVmToAttachedList(ctx context.Context,
191191
// deleted from the API server.
192192
// Returns an error if the operation cannot be persisted on the API server.
193193
func (f *cnsVolumeAttachment) RemoveVmFromAttachedList(ctx context.Context,
194-
volumeName, VmUUID string) error {
194+
volumeName, VmInstanceUUID string) (error, bool) {
195195
log := logger.GetLogger(ctx)
196-
log.Infof("Removing VmUUID %s from cnsVolumeAttachment %s",
197-
VmUUID, volumeName)
196+
log.Infof("Removing VmInstanceUUID %s from cnsVolumeAttachment %s",
197+
VmInstanceUUID, volumeName)
198198
actual, _ := f.volumeLock.LoadOrStore(volumeName, &sync.Mutex{})
199199
instanceLock, ok := actual.(*sync.Mutex)
200200
if !ok {
201-
return fmt.Errorf("failed to cast lock for cnsVolumeAttachment instance: %s", volumeName)
201+
return fmt.Errorf("failed to cast lock for cnsVolumeAttachment instance: %s", volumeName),
202+
false
202203
}
203204
instanceLock.Lock()
204205
log.Infof("Acquired lock for cnsVolumeAttachment instance %s", volumeName)
@@ -211,7 +212,7 @@ func (f *cnsVolumeAttachment) RemoveVmFromAttachedList(ctx context.Context,
211212
instanceNamespace, instanceName, err := cache.SplitMetaNamespaceKey(volumeName)
212213
if err != nil {
213214
log.Errorf("failed to split key %s with error: %+v", volumeName, err)
214-
return err
215+
return err, false
215216
}
216217
instanceKey := types.NamespacedName{
217218
Namespace: instanceNamespace,
@@ -221,55 +222,57 @@ func (f *cnsVolumeAttachment) RemoveVmFromAttachedList(ctx context.Context,
221222
if err != nil {
222223
if errors.IsNotFound(err) {
223224
log.Infof("cnsVolumeAttachment instance %s does not exist on API server", volumeName)
224-
return nil
225+
return nil, true
225226
}
226227
log.Errorf("failed to get cnsVolumeAttachment instance %s with error: %+v", volumeName, err)
227-
return err
228+
return err, false
228229
}
229230

230231
log.Debugf("Verifying if VM UUID %s exists in list of already attached VMs. Current list: %+v",
231232
volumeName, instance.Spec.AttachedVms)
232233
for index, existingAttachedVM := range instance.Spec.AttachedVms {
233-
if VmUUID == existingAttachedVM {
234-
log.Infof("Removing VmUUID %s from Attached VMs list", VmUUID)
235-
instance.Spec.AttachedVms = append(
236-
instance.Spec.AttachedVms[:index],
237-
instance.Spec.AttachedVms[index+1:]...)
238-
if len(instance.Spec.AttachedVms) == 0 {
239-
log.Infof("Deleting cnsVolumeAttachment instance %s from API server", volumeName)
240-
// Remove finalizer from CnsVolumeAttachment instance
241-
err = removeFinalizer(ctx, f.client, instance)
242-
if err != nil {
243-
log.Errorf("failed to remove finalizer from cnsVolumeAttachment instance %s with error: %+v",
244-
volumeName, err)
245-
}
246-
err = f.client.Delete(ctx, instance)
247-
if err != nil {
248-
// In case of namespace deletion, we will have deletion timestamp added on the
249-
// CnsVolumeAttachment instance. So, as soon as we delete finalizer, instance might
250-
// get deleted immediately. In such cases we will get NotFound error here, return success
251-
// if instance is already deleted.
252-
if errors.IsNotFound(err) {
253-
log.Infof("cnsVolumeAttachment instance %s seems to be already deleted.", volumeName)
254-
f.volumeLock.Delete(volumeName)
255-
return nil
256-
}
257-
log.Errorf("failed to delete cnsVolumeAttachment instance %s with error: %+v", volumeName, err)
258-
return err
259-
}
260-
f.volumeLock.Delete(volumeName)
261-
return nil
234+
if VmInstanceUUID != existingAttachedVM {
235+
continue
236+
}
237+
log.Infof("Removing VmUUID %s from Attached VMs list", VmInstanceUUID)
238+
instance.Spec.AttachedVms = append(
239+
instance.Spec.AttachedVms[:index],
240+
instance.Spec.AttachedVms[index+1:]...)
241+
if len(instance.Spec.AttachedVms) == 0 {
242+
log.Infof("Deleting cnsVolumeAttachment instance %s from API server", volumeName)
243+
// Remove finalizer from CnsVolumeAttachment instance
244+
err = removeFinalizer(ctx, f.client, instance)
245+
if err != nil {
246+
log.Errorf("failed to remove finalizer from cnsVolumeAttachment instance %s with error: %+v",
247+
volumeName, err)
248+
return err, false
262249
}
263-
log.Debugf("Updating cnsVolumeAttachment instance %s with spec: %+v", volumeName, instance)
264-
err = f.client.Update(ctx, instance)
250+
err = f.client.Delete(ctx, instance)
265251
if err != nil {
266-
log.Errorf("failed to update cnsVolumeAttachment instance %s with error: %+v", volumeName, err)
252+
// In case of namespace deletion, we will have deletion timestamp added on the
253+
// CnsVolumeAttachment instance. So, as soon as we delete finalizer, instance might
254+
// get deleted immediately. In such cases we will get NotFound error here, return success
255+
// if instance is already deleted.
256+
if errors.IsNotFound(err) {
257+
log.Infof("cnsVolumeAttachment instance %s seems to be already deleted.", volumeName)
258+
f.volumeLock.Delete(volumeName)
259+
return nil, true
260+
}
261+
log.Errorf("failed to delete cnsVolumeAttachment instance %s with error: %+v", volumeName, err)
262+
return err, false
267263
}
268-
return err
264+
f.volumeLock.Delete(volumeName)
265+
return nil, true
266+
}
267+
log.Debugf("Updating cnsVolumeAttachment instance %s with spec: %+v", volumeName, instance)
268+
err = f.client.Update(ctx, instance)
269+
if err != nil {
270+
log.Errorf("failed to update cnsVolumeAttachment instance %s with error: %+v", volumeName, err)
269271
}
272+
return err, false
270273
}
271-
log.Infof("Could not find VM %s in list. Returning.", VmUUID)
272-
return nil
274+
log.Infof("Could not find VM %s in list. Returning.", VmInstanceUUID)
275+
return nil, false
273276
}
274277

275278
// CnsVolumeAttachmentExistsForPvc returns true if CnsVolumeAttachment instance

‎pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go

Lines changed: 66 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030

3131
cnsvsphere "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/vsphere"
3232
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco"
33+
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/internalapis/cnsoperator/cnsvolumeattachment"
3334
cnsoperatortypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types"
3435

3536
vmoperatorv1alpha4 "github.com/vmware-tanzu/vm-operator/api/v1alpha4"
@@ -301,6 +302,19 @@ func (r *Reconciler) Reconcile(ctx context.Context,
301302
if instance.DeletionTimestamp != nil && vm == nil {
302303
log.Infof("Instance %s is being deleted and VM object is also deleted from VC", request.NamespacedName.String())
303304
// TODO: remove PVC finalizer
305+
cnsVolumeAttachmentInstance, err := cnsvolumeattachment.GetCnsVolumeAttachmentInstance(ctx)
306+
if err != nil {
307+
return r.completeReconciliationWithError(batchAttachCtx, instance, request.NamespacedName, timeout, err)
308+
}
309+
310+
// For every PV mentioned in instance.Spec, remove finalizer from it.
311+
for _, volume := range instance.Spec.Volumes {
312+
err := removePvcFinalizer(ctx, r.client, volume.PersistentVolumeClaim.ClaimName, instance.Namespace,
313+
instance.Spec.NodeUUID, cnsVolumeAttachmentInstance)
314+
if err != nil {
315+
return r.completeReconciliationWithError(batchAttachCtx, instance, request.NamespacedName, timeout, err)
316+
}
317+
}
304318

305319
patchErr := removeFinalizerFromCRDInstance(batchAttachCtx, instance, r.client)
306320
if patchErr != nil {
@@ -373,7 +387,12 @@ func (r *Reconciler) reconcileInstanceWithDeletionTimestamp(ctx context.Context,
373387
vm *cnsvsphere.VirtualMachine) error {
374388
log := logger.GetLogger(ctx)
375389

376-
err := r.processDetach(ctx, vm, instance, volumesToDetach)
390+
cnsVolumeAttachmentInstance, err := cnsvolumeattachment.GetCnsVolumeAttachmentInstance(ctx)
391+
if err != nil {
392+
return logger.LogNewErrorf(log, "Failed to get CNSFileVolumeClient instance. Error: %+v", err)
393+
}
394+
395+
err = r.processDetach(ctx, vm, instance, volumesToDetach, cnsVolumeAttachmentInstance)
377396
if err != nil {
378397
log.Errorf("failed to detach all volumes. Err: %s", err)
379398
return err
@@ -391,16 +410,21 @@ func (r *Reconciler) reconcileInstanceWithoutDeletionTimestamp(ctx context.Conte
391410
vm *cnsvsphere.VirtualMachine) error {
392411
log := logger.GetLogger(ctx)
393412

413+
cnsVolumeAttachmentInstance, err := cnsvolumeattachment.GetCnsVolumeAttachmentInstance(ctx)
414+
if err != nil {
415+
return logger.LogNewErrorf(log, "Failed to get CNSFileVolumeClient instance. Error: %+v", err)
416+
}
417+
394418
// Call batch attach for volumes.
395-
err := r.processBatchAttach(ctx, vm, instance)
419+
err = r.processBatchAttach(ctx, vm, instance, cnsVolumeAttachmentInstance)
396420
if err != nil {
397421
log.Errorf("failed to attach all volumes. Err: %+v", err)
398422
return err
399423
}
400424

401425
// Call detach if there are some volumes which need to be detached.
402426
if len(volumesToDetach) != 0 {
403-
err := r.processDetach(ctx, vm, instance, volumesToDetach)
427+
err := r.processDetach(ctx, vm, instance, volumesToDetach, cnsVolumeAttachmentInstance)
404428
if err != nil {
405429
log.Errorf("failed to detach all volumes. Err: +v", err)
406430
return err
@@ -413,11 +437,13 @@ func (r *Reconciler) reconcileInstanceWithoutDeletionTimestamp(ctx context.Conte
413437
// processDetach detaches each of the volumes in volumesToDetach by calling CNS DetachVolume API.
414438
func (r *Reconciler) processDetach(ctx context.Context,
415439
vm *cnsvsphere.VirtualMachine,
416-
instance *v1alpha1.CnsNodeVmBatchAttachment, volumesToDetach map[string]string) error {
440+
instance *v1alpha1.CnsNodeVmBatchAttachment,
441+
volumesToDetach map[string]string,
442+
cnsVolumeAttachmentInstance cnsvolumeattachment.CnsVolumeAttachment) error {
417443
log := logger.GetLogger(ctx)
418444
log.Debugf("Calling detach volume for PVC %+v", volumesToDetach)
419445

420-
volumesThatFailedToDetach := r.detachVolumes(ctx, vm, volumesToDetach, instance)
446+
volumesThatFailedToDetach := r.detachVolumes(ctx, vm, volumesToDetach, instance, cnsVolumeAttachmentInstance)
421447

422448
var overallErr error
423449
if len(volumesThatFailedToDetach) != 0 {
@@ -434,7 +460,8 @@ func (r *Reconciler) processDetach(ctx context.Context,
434460
// detachVolumes calls Cns DetachVolume for every PVC in volumesToDetach.
435461
func (r *Reconciler) detachVolumes(ctx context.Context,
436462
vm *cnsvsphere.VirtualMachine, volumesToDetach map[string]string,
437-
instance *v1alpha1.CnsNodeVmBatchAttachment) []string {
463+
instance *v1alpha1.CnsNodeVmBatchAttachment,
464+
cnsVolumeAttachmentInstance cnsvolumeattachment.CnsVolumeAttachment) []string {
438465
log := logger.GetLogger(ctx)
439466

440467
volumesThatFailedToDetach := make([]string, 0)
@@ -449,11 +476,18 @@ func (r *Reconciler) detachVolumes(ctx context.Context,
449476
// If VM was not found, can assume that the detach is successful.
450477
if cnsvsphere.IsManagedObjectNotFound(detachErr, vm.VirtualMachine.Reference()) {
451478
log.Infof("Found a managed object not found fault for vm: %+v", vm)
452-
// TODO: remove PVC finalizer
453479

454-
// Remove entry of this volume from the instance's status.
455-
deleteVolumeFromStatus(pvc, instance)
456-
log.Infof("Successfully detached volume %s from VM %s", pvc, instance.Spec.NodeUUID)
480+
// Remove finalizer from the PVC as detach was successful.
481+
err := removePvcFinalizer(ctx, r.client, pvc, instance.Namespace, instance.Spec.NodeUUID,
482+
cnsVolumeAttachmentInstance)
483+
if err != nil {
484+
updateInstanceWithErrorForPvc(instance, pvc, err.Error())
485+
volumesThatFailedToDetach = append(volumesThatFailedToDetach, pvc)
486+
} else {
487+
// Remove entry of this volume from the instance's status.
488+
deleteVolumeFromStatus(pvc, instance)
489+
log.Infof("Successfully detached volume %s from VM %s", pvc, instance.Spec.NodeUUID)
490+
}
457491
} else {
458492
log.Errorf("failed to detach volume %s from VM %s. Fault: %s Err: %s",
459493
pvc, instance.Spec.NodeUUID, faulttype, detachErr)
@@ -462,10 +496,17 @@ func (r *Reconciler) detachVolumes(ctx context.Context,
462496
volumesThatFailedToDetach = append(volumesThatFailedToDetach, pvc)
463497
}
464498
} else {
465-
// TODO: remove PVC finalizer
466-
// Remove entry of this volume from the instance's status.
467-
deleteVolumeFromStatus(pvc, instance)
468-
log.Infof("Successfully detached volume %s from VM %s", pvc, instance.Spec.NodeUUID)
499+
// Remove finalizer from the PVC as detach was successful.
500+
err := removePvcFinalizer(ctx, r.client, pvc, instance.Namespace, instance.Spec.NodeUUID,
501+
cnsVolumeAttachmentInstance)
502+
if err != nil {
503+
updateInstanceWithErrorForPvc(instance, pvc, err.Error())
504+
volumesThatFailedToDetach = append(volumesThatFailedToDetach, pvc)
505+
} else {
506+
// Remove entry of this volume from the instance's status.
507+
deleteVolumeFromStatus(pvc, instance)
508+
log.Infof("Successfully detached volume %s from VM %s", pvc, instance.Spec.NodeUUID)
509+
}
469510
}
470511
log.Infof("Detach call ended for PVC %s in namespace %s for instance %s",
471512
pvc, instance.Namespace, instance.Name)
@@ -478,7 +519,8 @@ func (r *Reconciler) detachVolumes(ctx context.Context,
478519
// processBatchAttach first constructs the batch attach volume request for all volumes in instance spec
479520
// and then calls CNS batch attach for them.
480521
func (r *Reconciler) processBatchAttach(ctx context.Context, vm *cnsvsphere.VirtualMachine,
481-
instance *v1alpha1.CnsNodeVmBatchAttachment) error {
522+
instance *v1alpha1.CnsNodeVmBatchAttachment,
523+
cnsVolumeAttachmentInstance cnsvolumeattachment.CnsVolumeAttachment) error {
482524
log := logger.GetLogger(ctx)
483525

484526
// Construct batch attach request
@@ -509,6 +551,15 @@ func (r *Reconciler) processBatchAttach(ctx context.Context, vm *cnsvsphere.Virt
509551
return fmt.Errorf("failed to get volumeName for pvc %s", pvcName)
510552

511553
}
554+
555+
// Add finalizer on PVC as attach was successful.
556+
err = addPvcFinalizer(ctx, r.client, pvcName, instance.Namespace, instance.Spec.NodeUUID,
557+
cnsVolumeAttachmentInstance)
558+
if err != nil {
559+
log.Errorf("failed to add finalizer %s on PVC %s", cnsoperatortypes.CNSPvcFinalizer, pvcName)
560+
result.Error = err
561+
}
562+
512563
// Update instance with attach result
513564
updateInstanceWithAttachVolumeResult(instance, volumeName, pvcName, result)
514565
}

0 commit comments

Comments
 (0)