Skip to content

Commit cd909a4

Browse files
divyenpateldivyenpatel
committed
fix cnsunregister API to not use CNS metadata
1 parent c16a940 commit cd909a4

File tree

5 files changed

+41
-75
lines changed

5 files changed

+41
-75
lines changed

‎pkg/common/unittestcommon/utils.go

+5
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,11 @@ func (c *FakeK8SOrchestrator) GetPVNameFromCSIVolumeID(volumeID string) (string,
349349
return "", false
350350
}
351351

352+
// GetPVCNameFromCSIVolumeID retrieves the pvc name from volumeID.
353+
func (c *FakeK8SOrchestrator) GetPVCNameFromCSIVolumeID(volumeID string) (string, bool) {
354+
return "", false
355+
}
356+
352357
// InitializeCSINodes creates CSINode instances for each K8s node with the appropriate topology keys.
353358
func (c *FakeK8SOrchestrator) InitializeCSINodes(ctx context.Context) error {
354359
return nil

‎pkg/csi/service/common/commonco/coagnostic.go

+2
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ type COCommonInterface interface {
9393
// GetPVNameFromCSIVolumeID retrieves the pv name from the volumeID.
9494
// This method will not return pv name in case of in-tree migrated volumes
9595
GetPVNameFromCSIVolumeID(volumeID string) (string, bool)
96+
// GetPVCNameFromCSIVolumeID returns PV claim name for the volume ID
97+
GetPVCNameFromCSIVolumeID(volumeID string) (string, bool)
9698
// InitializeCSINodes creates CSINode instances for each K8s node with the appropriate topology keys.
9799
InitializeCSINodes(ctx context.Context) error
98100
// StartZonesInformer starts a dynamic informer which listens on Zones CR in

‎pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,11 @@ func (m *volumeIDToPvcMap) remove(volumeHandle string) {
107107
}
108108

109109
// Returns the namespaced pvc name corresponding to volumeHandle.
110-
func (m *volumeIDToPvcMap) get(volumeHandle string) string {
110+
func (m *volumeIDToPvcMap) get(volumeHandle string) (string, bool) {
111111
m.RLock()
112112
defer m.RUnlock()
113-
return m.items[volumeHandle]
113+
pvcname, found := m.items[volumeHandle]
114+
return pvcname, found
114115
}
115116

116117
// Map of the volumeName which refers to the PVName, to the list of node names in the cluster.
@@ -1792,3 +1793,8 @@ func (c *K8sOrchestrator) CreateConfigMap(ctx context.Context, name string, name
17921793
func (c *K8sOrchestrator) GetPVNameFromCSIVolumeID(volumeID string) (string, bool) {
17931794
return c.volumeIDToNameMap.get(volumeID)
17941795
}
1796+
1797+
// GetPVCNameFromCSIVolumeID retrieves the pvc name from volumeID using volumeIDToPvcMap.
1798+
func (c *K8sOrchestrator) GetPVCNameFromCSIVolumeID(volumeID string) (string, bool) {
1799+
return c.volumeIDToPvcMap.get(volumeID)
1800+
}

‎pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator_helper.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import (
3939
func (c *K8sOrchestrator) getPVCAnnotations(ctx context.Context, volumeID string) (map[string]string, error) {
4040
log := logger.GetLogger(ctx)
4141
log.Debugf("Getting annotations on pvc corresponding to volume: %s", volumeID)
42-
if pvc := c.volumeIDToPvcMap.get(volumeID); pvc != "" {
42+
if pvc, _ := c.volumeIDToPvcMap.get(volumeID); pvc != "" {
4343
parts := strings.Split(pvc, "/")
4444
pvcNamespace := parts[0]
4545
pvcName := parts[1]
@@ -67,7 +67,7 @@ func (c *K8sOrchestrator) getPVCAnnotations(ctx context.Context, volumeID string
6767
func (c *K8sOrchestrator) updatePVCAnnotations(ctx context.Context,
6868
volumeID string, annotations map[string]string) error {
6969
log := logger.GetLogger(ctx)
70-
if pvc := c.volumeIDToPvcMap.get(volumeID); pvc != "" {
70+
if pvc, _ := c.volumeIDToPvcMap.get(volumeID); pvc != "" {
7171
parts := strings.Split(pvc, "/")
7272
pvcNamespace := parts[0]
7373
pvcName := parts[1]

‎pkg/syncer/cnsoperator/controller/cnsunregistervolume/cnsunregistervolume_controller.go

+24-71
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package cnsunregistervolume
1919
import (
2020
"context"
2121
"fmt"
22+
"strings"
2223
"sync"
2324
"time"
2425

@@ -55,7 +56,6 @@ import (
5556

5657
const (
5758
defaultMaxWorkerThreadsForUnregisterVolume = 40
58-
metadata = "VOLUME_METADATA"
5959
)
6060

6161
var (
@@ -211,57 +211,23 @@ func (r *ReconcileCnsUnregisterVolume) Reconcile(ctx context.Context,
211211
// 5. Invoke CNS DeleteVolume API with deleteDisk set to false.
212212
// 6. Set the CnsUnregisterVolumeStatus.Unregistered to true.
213213

214-
// TODO - Add validations whether the volume is not in use in a TKC or by a VM service VM.
215-
216-
queryFilter := cnstypes.CnsQueryFilter{
217-
VolumeIds: []cnstypes.CnsVolumeId{{Id: instance.Spec.VolumeID}},
218-
}
219-
querySelection := cnstypes.CnsQuerySelection{
220-
Names: []string{
221-
metadata,
222-
},
223-
}
224-
225-
queryResult, err := r.volumeManager.QueryVolumeAsync(ctx, queryFilter, &querySelection)
226-
if err != nil {
227-
msg := fmt.Sprintf("Unable to query volume %q . Error: %+v", instance.Spec.VolumeID, err)
228-
log.Error(msg)
229-
setInstanceError(ctx, r, instance, msg)
230-
return reconcile.Result{RequeueAfter: timeout}, nil
231-
}
232-
if len(queryResult.Volumes) == 0 {
233-
msg := fmt.Sprintf("Volume: %q not found while querying CNS. It may have already been unregistered.",
234-
instance.Spec.VolumeID)
235-
err = setInstanceSuccess(ctx, r, instance, msg)
236-
if err != nil {
237-
msg := fmt.Sprintf("Failed to update CnsUnregistered instance with error: %+v", err)
238-
log.Error(msg)
239-
setInstanceError(ctx, r, instance, msg)
240-
return reconcile.Result{RequeueAfter: timeout}, nil
241-
}
242-
backOffDurationMapMutex.Lock()
243-
delete(backOffDuration, instance.Name)
244-
backOffDurationMapMutex.Unlock()
245-
log.Info(msg)
246-
return reconcile.Result{}, nil
247-
}
248-
249-
cnsVol := queryResult.Volumes[0]
250-
251214
var pvName, pvcName, pvcNamespace string
252-
for _, entity := range cnsVol.Metadata.EntityMetadata {
253-
if k8sEntityMetadata, ok := entity.(*cnstypes.CnsKubernetesEntityMetadata); ok {
254-
entityType := k8sEntityMetadata.EntityType
255-
256-
if entityType == string(cnstypes.CnsKubernetesEntityTypePV) {
257-
pvName = entity.(*cnstypes.CnsKubernetesEntityMetadata).EntityName
258-
}
259-
260-
if entityType == string(cnstypes.CnsKubernetesEntityTypePVC) {
261-
pvcName = entity.(*cnstypes.CnsKubernetesEntityMetadata).EntityName
262-
pvcNamespace = entity.(*cnstypes.CnsKubernetesEntityMetadata).Namespace
263-
}
215+
pvName, pvfound := commonco.ContainerOrchestratorUtility.GetPVNameFromCSIVolumeID(instance.Spec.VolumeID)
216+
if pvfound {
217+
log.Infof("found PV: %q for the volumd Id: %q", pvName, instance.Spec.VolumeID)
218+
pvcNamewithNamespace, pvcfound := commonco.ContainerOrchestratorUtility.
219+
GetPVCNameFromCSIVolumeID(instance.Spec.VolumeID)
220+
if pvcfound {
221+
parts := strings.Split(pvcNamewithNamespace, "/")
222+
pvcNamespace = parts[0]
223+
pvcName = parts[1]
224+
log.Infof("found PVC: %q in the namespace:%q for the volumd Id: %q", pvcName, pvcNamespace,
225+
instance.Spec.VolumeID)
226+
} else {
227+
log.Infof("cound not find PVC for the volume Id: %q", instance.Spec.VolumeID)
264228
}
229+
} else {
230+
log.Infof("cound not find PV for the volume Id: %q", instance.Spec.VolumeID)
265231
}
266232

267233
k8sclient, err := k8s.NewClient(ctx)
@@ -271,8 +237,9 @@ func (r *ReconcileCnsUnregisterVolume) Reconcile(ctx context.Context,
271237
setInstanceError(ctx, r, instance, "Failed to init K8S client for volume unregistration")
272238
return reconcile.Result{RequeueAfter: timeout}, nil
273239
}
274-
275-
err = validateVolumeNotInUse(ctx, cnsVol, pvcName, pvcNamespace, k8sclient)
240+
// TODO - Add validations whether the volume is not in use in a TKC
241+
// validateVolumeNotInUse does not check if detached PVC/PV in TKC, having reference to supervisor PVC/PV
242+
err = validateVolumeNotInUse(ctx, instance.Spec.VolumeID, pvcName, pvcNamespace, k8sclient)
276243
if err != nil {
277244
log.Error(err)
278245
setInstanceError(ctx, r, instance, err.Error())
@@ -301,9 +268,6 @@ func (r *ReconcileCnsUnregisterVolume) Reconcile(ctx context.Context,
301268
}
302269
log.Infof("Updated ReclaimPolicy on PV %q to %q", pvName, v1.PersistentVolumeReclaimRetain)
303270
}
304-
} else {
305-
log.Infof("CNS metadata for volume %s has missing pvName."+
306-
"PV may have already been deleted. Continuing with other operations..", instance.Spec.VolumeID)
307271
}
308272

309273
// Delete PVC.
@@ -322,9 +286,6 @@ func (r *ReconcileCnsUnregisterVolume) Reconcile(ctx context.Context,
322286
} else {
323287
log.Infof("Deleted PVC %q in namespace %q", pvcName, pvcNamespace)
324288
}
325-
} else {
326-
log.Infof("CNS metadata for volume %s has missing pvcName or namespace."+
327-
"PVC may have already been deleted. Continuing with other operations..", instance.Spec.VolumeID)
328289
}
329290

330291
if pvName != "" {
@@ -377,7 +338,7 @@ func (r *ReconcileCnsUnregisterVolume) Reconcile(ctx context.Context,
377338

378339
// validateVolumeNotInUse validates whether the volume to be unregistered is not in use by
379340
// either PodVM, TKG cluster or Volume service VM.
380-
func validateVolumeNotInUse(ctx context.Context, cnsVol cnstypes.CnsVolume, pvcName string,
341+
func validateVolumeNotInUse(ctx context.Context, volumdId string, pvcName string,
381342
pvcNamespace string, k8sClient clientset.Interface) error {
382343

383344
log := logger.GetLogger(ctx)
@@ -394,10 +355,10 @@ func validateVolumeNotInUse(ctx context.Context, cnsVol cnstypes.CnsVolume, pvcN
394355
for _, podVol := range pod.Spec.Volumes {
395356
if podVol.PersistentVolumeClaim != nil &&
396357
podVol.PersistentVolumeClaim.ClaimName == pvcName {
397-
log.Debugf("Volume %s is in use by pod %s in namespace %s", cnsVol.VolumeId.Id,
358+
log.Debugf("Volume %s is in use by pod %s in namespace %s", volumdId,
398359
pod.Name, pvcNamespace)
399360
return fmt.Errorf("cannot unregister the volume %s as it's in use by pod %s in namespace %s",
400-
cnsVol.VolumeId.Id, pod.Name, pvcNamespace)
361+
volumdId, pod.Name, pvcNamespace)
401362
}
402363
}
403364
}
@@ -406,14 +367,6 @@ func validateVolumeNotInUse(ctx context.Context, cnsVol cnstypes.CnsVolume, pvcN
406367
// For volumes created from TKGs Cluster, CNS metadata will have two entries for containerClusterArray.
407368
// One for clusterFlavor: "WORKLOAD" & clusterDistribution "SupervisorCluster",
408369
// another for clusterFlavor: "GUEST_CLUSTER" & clusterDistribution: "TKGService".
409-
for _, containerCluster := range cnsVol.Metadata.ContainerClusterArray {
410-
if containerCluster.ClusterFlavor == "GUEST_CLUSTER" {
411-
log.Debugf("Volume %s is in use by guest cluster with CNS clusterId %s", cnsVol.VolumeId.Id,
412-
containerCluster.ClusterId)
413-
return fmt.Errorf("cannot unregister the volume %s as it's in use by guest cluster with CNS clusterId %s",
414-
cnsVol.VolumeId.Id, containerCluster.ClusterId)
415-
}
416-
}
417370

418371
// Check if the Supervisor volume is not used by a volume service VM.
419372
// If the volume is specified in the VirtualMachine's spec, then it intends
@@ -444,10 +397,10 @@ func validateVolumeNotInUse(ctx context.Context, cnsVol cnstypes.CnsVolume, pvcN
444397
for _, vmVol := range vmInstance.Spec.Volumes {
445398
if vmVol.PersistentVolumeClaim != nil &&
446399
vmVol.PersistentVolumeClaim.ClaimName == pvcName {
447-
log.Debugf("Volume %s is in use by VirtualMachine %s in namespace %s", cnsVol.VolumeId.Id,
400+
log.Debugf("Volume %s is in use by VirtualMachine %s in namespace %s", volumdId,
448401
vmInstance.Name, pvcNamespace)
449402
return fmt.Errorf("cannot unregister the volume %s as it's in use by VirtualMachine %s in namespace %s",
450-
cnsVol.VolumeId.Id, vmInstance.Name, pvcNamespace)
403+
volumdId, vmInstance.Name, pvcNamespace)
451404
}
452405
}
453406
}

0 commit comments

Comments
 (0)