Skip to content

Commit 61f0a34

Browse files
authored
Add SPR labels to PVC if needed (#3463)
1 parent 6340850 commit 61f0a34

File tree

4 files changed

+118
-6
lines changed

4 files changed

+118
-6
lines changed

pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
560560
// Create PVC mapping to above created PV.
561561
log.Infof("Creating PVC: %s", instance.Spec.PvcName)
562562
pvcSpec, err := getPersistentVolumeClaimSpec(ctx, instance.Spec.PvcName, instance.Namespace, capacityInMb,
563-
storageClassName, accessMode, pvName, datastoreAccessibleTopology)
563+
storageClassName, accessMode, pvName, datastoreAccessibleTopology, instance)
564564
if err != nil {
565565
msg := fmt.Sprintf("Failed to create spec for PVC: %q. Error: %v", instance.Spec.PvcName, err)
566566
log.Errorf(msg)

pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
commoncotypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco/types"
4848
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/internalapis/cnsvolumeoperationrequest"
4949
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer"
50+
cnsoperatortypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types"
5051
)
5152

5253
type mockVolumeManager struct {
@@ -450,10 +451,16 @@ var _ = Describe("Reconcile Accessibility Logic", func() {
450451
patches.ApplyFunc(common.QueryVolumeByID, func(ctx context.Context, vm cnsvolume.Manager,
451452
volumeID string, querySelection *cnstypes.CnsQuerySelection) (*cnstypes.CnsVolume, error) {
452453
return &cnstypes.CnsVolume{
453-
VolumeId: cnstypes.CnsVolumeId{Id: volumeID},
454-
DatastoreUrl: "dummy-ds-url",
455-
StoragePolicyId: "dummy-storage-policy-id",
456-
}, nil
454+
VolumeId: cnstypes.CnsVolumeId{Id: volumeID},
455+
DatastoreUrl: "dummy-ds-url",
456+
StoragePolicyId: "dummy-storage-policy-id",
457+
BackingObjectDetails: &cnstypes.CnsBlockBackingDetails{
458+
CnsBackingObjectDetails: cnstypes.CnsBackingObjectDetails{
459+
CapacityInMb: 1024,
460+
},
461+
},
462+
},
463+
nil
457464
})
458465

459466
patches.ApplyFunc(common.GetClusterComputeResourceMoIds, func(ctx context.Context) ([]string, bool, error) {
@@ -522,6 +529,88 @@ var _ = Describe("Reconcile Accessibility Logic", func() {
522529
Expect(err).NotTo(HaveOccurred())
523530
Expect(result).To(Equal(reconcile.Result{RequeueAfter: 0}))
524531
})
532+
533+
It("should add vm name and storage policy reservation labels to PVC when both present on CR", func() {
534+
// Test the getPersistentVolumeClaimSpec function directly
535+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
536+
ObjectMeta: metav1.ObjectMeta{
537+
Name: "test-volume-with-labels",
538+
Namespace: "test-ns",
539+
Labels: map[string]string{
540+
cnsoperatortypes.LabelVirtualMachineName: "test-vm-name",
541+
cnsoperatortypes.LabelStoragePolicyReservationName: "test-storage-policy-reservation-name",
542+
},
543+
},
544+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
545+
PvcName: "test-pvc-with-labels",
546+
VolumeID: "dummy-volume-id",
547+
AccessMode: "ReadWriteOnce",
548+
},
549+
}
550+
551+
// Test getPersistentVolumeClaimSpec function directly
552+
pvcSpec, err := getPersistentVolumeClaimSpec(ctx, "test-pvc", "test-ns", 1024,
553+
"test-storage-class", corev1.ReadWriteOnce, "test-pv", nil, instance)
554+
555+
Expect(err).NotTo(HaveOccurred())
556+
Expect(pvcSpec).NotTo(BeNil())
557+
Expect(pvcSpec.Labels).To(HaveKeyWithValue(cnsoperatortypes.LabelVirtualMachineName, "test-vm-name"))
558+
Expect(pvcSpec.Labels).To(HaveKeyWithValue(cnsoperatortypes.LabelStoragePolicyReservationName,
559+
"test-storage-policy-reservation-name"))
560+
})
561+
562+
It("should not add labels to PVC when not present on CR", func() {
563+
// Test the getPersistentVolumeClaimSpec function directly without labels
564+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
565+
ObjectMeta: metav1.ObjectMeta{
566+
Name: "test-volume-without-labels",
567+
Namespace: "test-ns",
568+
// No labels set
569+
},
570+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
571+
PvcName: "test-pvc-without-labels",
572+
VolumeID: "dummy-volume-id",
573+
AccessMode: "ReadWriteOnce",
574+
},
575+
}
576+
577+
// Test getPersistentVolumeClaimSpec function directly
578+
pvcSpec, err := getPersistentVolumeClaimSpec(ctx, "test-pvc", "test-ns", 1024,
579+
"test-storage-class", corev1.ReadWriteOnce, "test-pv", nil, instance)
580+
581+
Expect(err).NotTo(HaveOccurred())
582+
Expect(pvcSpec).NotTo(BeNil())
583+
Expect(pvcSpec.Labels).NotTo(HaveKey(cnsoperatortypes.LabelVirtualMachineName))
584+
Expect(pvcSpec.Labels).NotTo(HaveKey(cnsoperatortypes.LabelStoragePolicyReservationName))
585+
})
586+
587+
It("should not add vm name label when only that label is present on CR", func() {
588+
// Test the getPersistentVolumeClaimSpec function directly with only vm name label
589+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
590+
ObjectMeta: metav1.ObjectMeta{
591+
Name: "test-volume-with-vm-label",
592+
Namespace: "test-ns",
593+
Labels: map[string]string{
594+
cnsoperatortypes.LabelVirtualMachineName: "test-vm-name",
595+
// Missing storage policy reservation label
596+
},
597+
},
598+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
599+
PvcName: "test-pvc-with-vm-label",
600+
VolumeID: "dummy-volume-id",
601+
AccessMode: "ReadWriteOnce",
602+
},
603+
}
604+
605+
// Test getPersistentVolumeClaimSpec function directly
606+
pvcSpec, err := getPersistentVolumeClaimSpec(ctx, "test-pvc", "test-ns", 1024,
607+
"test-storage-class", corev1.ReadWriteOnce, "test-pv", nil, instance)
608+
609+
Expect(err).NotTo(HaveOccurred())
610+
Expect(pvcSpec).NotTo(BeNil())
611+
Expect(pvcSpec.Labels).NotTo(HaveKey(cnsoperatortypes.LabelVirtualMachineName))
612+
Expect(pvcSpec.Labels).NotTo(HaveKey(cnsoperatortypes.LabelStoragePolicyReservationName))
613+
})
525614
})
526615

527616
func TestCnsRegisterVolumeController(t *testing.T) {

pkg/syncer/cnsoperator/controller/cnsregistervolume/util.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
cnsstoragepolicyquotasv1alpha2 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/storagepolicy/v1alpha2"
3939
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/vsphere"
4040
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common"
41+
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco"
4142
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger"
4243
cnsoperatortypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types"
4344
)
@@ -279,12 +280,15 @@ func getPersistentVolumeSpec(volumeName string, volumeID string, capacity int64,
279280
// specified storage class.
280281
func getPersistentVolumeClaimSpec(ctx context.Context, name string, namespace string, capacity int64,
281282
storageClassName string, accessMode v1.PersistentVolumeAccessMode, pvName string,
282-
datastoreAccessibleTopology []map[string]string) (*v1.PersistentVolumeClaim, error) {
283+
datastoreAccessibleTopology []map[string]string,
284+
instance *cnsregistervolumev1alpha1.CnsRegisterVolume) (*v1.PersistentVolumeClaim, error) {
283285

284286
log := logger.GetLogger(ctx)
285287
capacityInMb := strconv.FormatInt(capacity, 10) + "Mi"
288+
286289
var (
287290
segmentsArray []string
291+
pvcLabels = make(map[string]string)
288292
topoAnnotation = make(map[string]string)
289293
)
290294
if datastoreAccessibleTopology != nil {
@@ -299,10 +303,24 @@ func getPersistentVolumeClaimSpec(ctx context.Context, name string, namespace st
299303
topoAnnotation[common.AnnVolumeAccessibleTopology] = "[" + strings.Join(segmentsArray, ",") + "]"
300304
}
301305

306+
// TODO: For now this FSS is added in CSI ConfigMap. Once this FSS is available in Capabilities CR, remove
307+
// it from CSI ConfigMap and fetch FSS value from Capabilities CR.
308+
if commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.StoragePolicyReservationSupport) {
309+
// Check if both labelVirtualMachineName and labelStoragePolicyReservationName are on CnsRegisterVolume CR.
310+
// If both are present, add both to PVC
311+
if vmName, vmOk := instance.Labels[cnsoperatortypes.LabelVirtualMachineName]; vmOk && vmName != "" {
312+
if spName, spOk := instance.Labels[cnsoperatortypes.LabelStoragePolicyReservationName]; spOk && spName != "" {
313+
pvcLabels[cnsoperatortypes.LabelVirtualMachineName] = vmName
314+
pvcLabels[cnsoperatortypes.LabelStoragePolicyReservationName] = spName
315+
}
316+
}
317+
}
318+
302319
claim := &v1.PersistentVolumeClaim{
303320
ObjectMeta: metav1.ObjectMeta{
304321
Name: name,
305322
Namespace: namespace,
323+
Labels: pvcLabels,
306324
Annotations: topoAnnotation,
307325
},
308326
Spec: v1.PersistentVolumeClaimSpec{

pkg/syncer/cnsoperator/types/types.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,9 @@ const (
3636

3737
// VSphereCSIDriverName is the vsphere CSI driver name
3838
VSphereCSIDriverName = "csi.vsphere.vmware.com"
39+
40+
// Label that points to a VM name. This is added to CNSRegisterVolume CR and PVC.
41+
LabelVirtualMachineName = "vm.consumer.storage.com/name"
42+
// Label that points to a StoragePolicyReservation CR name. This is added to CNSRegisterVolume CR and PVC.
43+
LabelStoragePolicyReservationName = "quota.storage.vmware.com/storagepolicyreservation-name"
3944
)

0 commit comments

Comments
 (0)