Skip to content

Commit f4b1ec2

Browse files
skogtaskogta
authored
Update CnsRegisterApi to include volumeMode for shared disks (#3286)
* Update CnsRegisterColume API for multiwriter * Change default volumeMode to filesystem
1 parent cba0a02 commit f4b1ec2

File tree

5 files changed

+200
-23
lines changed

5 files changed

+200
-23
lines changed

‎pkg/apis/cnsoperator/cnsregistervolume/v1alpha1/cnsregistervolume_types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ type CnsRegisterVolumeSpec struct {
5454
// This is for a 34a9c05d-5f03-e254-e692-02004479cb91/vm2_1.vmdk
5555
// file under datacenter "Datacenter-1" and datastore "vsanDatastore".
5656
DiskURLPath string `json:"diskURLPath,omitempty"`
57+
58+
// VolumeMode can either be Block (for raw block volume) or
59+
// Filesystem. Default values is Filesystem.
60+
VolumeMode v1.PersistentVolumeMode `json:"volumeMode,omitempty"`
5761
}
5862

5963
// CnsRegisterVolumeStatus defines the observed state of CnsRegisterVolume

‎pkg/apis/cnsoperator/config/cnsregistervolume_crd.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ spec:
6767
together.
6868
type: string
6969
pattern: '^[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}$'
70+
volumeMode:
71+
description: VolumeMode can either be Block (for raw block volume) or
72+
Filesystem. Default value is Filesystem.
73+
type: string
7074
required:
7175
- pvcName
7276
type: object

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

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2020 The Kubernetes Authors.
2+
Copyright 2020-2025 The Kubernetes Authors.
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
@@ -84,6 +84,7 @@ var (
8484
workloadDomainIsolationEnabled bool
8585
isTKGSHAEnabled bool
8686
isMultipleClustersPerVsphereZoneEnabled bool
87+
isSharedDiskEnabled bool
8788
)
8889

8990
// Add creates a new CnsRegisterVolume Controller and adds it to the Manager,
@@ -98,6 +99,8 @@ func Add(mgr manager.Manager, clusterFlavor cnstypes.CnsClusterFlavor,
9899
}
99100
workloadDomainIsolationEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx,
100101
common.WorkloadDomainIsolation)
102+
isSharedDiskEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx,
103+
common.SharedDiskFss)
101104
isTKGSHAEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.TKGsHA)
102105
isMultipleClustersPerVsphereZoneEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx,
103106
common.MultipleClustersPerVsphereZone)
@@ -261,7 +264,9 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
261264
// Currently file volume registration is not supported.
262265
ok := isBlockVolumeRegisterRequest(ctx, instance)
263266
if !ok {
264-
msg := fmt.Sprintf("AccessMode: %s is not supported", instance.Spec.AccessMode)
267+
// File volumes are not supported, so error out.
268+
msg := fmt.Sprintf("AccessMode: %s is not supported with volumemode %s",
269+
instance.Spec.AccessMode, instance.Spec.VolumeMode)
265270
log.Error(msg)
266271
setInstanceError(ctx, r, instance, msg)
267272
return reconcile.Result{RequeueAfter: timeout}, nil
@@ -280,10 +285,9 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
280285
pvNodeAffinity *v1.VolumeNodeAffinity
281286
)
282287
// Create Volume for the input CnsRegisterVolume instance.
283-
createSpec := constructCreateSpecForInstance(r, instance, vc.Config.Host, isTKGSHAEnabled)
288+
createSpec := constructCreateSpecForInstance(ctx, r, instance, vc.Config.Host, isTKGSHAEnabled)
284289
log.Infof("Creating CNS volume: %+v for CnsRegisterVolume request with name: %q on namespace: %q",
285290
instance, instance.Name, instance.Namespace)
286-
log.Debugf("CNS Volume create spec is: %+v", createSpec)
287291
volInfo, _, err := r.volumeManager.CreateVolume(ctx, createSpec, nil)
288292
if err != nil {
289293
msg := fmt.Sprintf("failed to create CNS volume. Error: %v", err)
@@ -596,7 +600,7 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
596600
Name: instance.Spec.PvcName,
597601
}
598602
pvSpec := getPersistentVolumeSpec(pvName, volumeID, capacityInMb,
599-
accessMode, storageClassName, claimRef)
603+
accessMode, instance.Spec.VolumeMode, storageClassName, claimRef)
600604
pvSpec.Spec.NodeAffinity = pvNodeAffinity
601605
log.Debugf("PV spec is: %+v", pvSpec)
602606
pv, err = k8sclient.CoreV1().PersistentVolumes().Create(ctx, pvSpec, metav1.CreateOptions{})
@@ -658,7 +662,7 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
658662
// Create PVC mapping to above created PV.
659663
log.Infof("Creating PVC: %s", instance.Spec.PvcName)
660664
pvcSpec, err := getPersistentVolumeClaimSpec(ctx, instance.Spec.PvcName, instance.Namespace, capacityInMb,
661-
storageClassName, accessMode, pvName, datastoreAccessibleTopology, instance)
665+
storageClassName, accessMode, *pv.Spec.VolumeMode, pvName, datastoreAccessibleTopology, instance)
662666
if err != nil {
663667
msg := fmt.Sprintf("Failed to create spec for PVC: %q. Error: %v", instance.Spec.PvcName, err)
664668
log.Errorf(msg)
@@ -945,14 +949,28 @@ func validateCnsRegisterVolumeSpec(ctx context.Context, instance *cnsregistervol
945949
msg = "VolumeID and DiskURLPath cannot be specified together"
946950
} else if instance.Spec.DiskURLPath != "" && instance.Spec.AccessMode != "" &&
947951
instance.Spec.AccessMode != v1.ReadWriteOnce {
948-
msg = fmt.Sprintf("DiskURLPath cannot be used with accessMode: %q", instance.Spec.AccessMode)
952+
if isSharedDiskEnabled {
953+
if instance.Spec.AccessMode == v1.ReadWriteMany &&
954+
instance.Spec.VolumeMode == v1.PersistentVolumeFilesystem {
955+
// File volume is not support for disk URL path.
956+
msg = fmt.Sprintf("DiskURLPath cannot be used with accessMode: %s and volumeMode: %s",
957+
instance.Spec.AccessMode, instance.Spec.VolumeMode)
958+
}
959+
} else {
960+
msg = fmt.Sprintf("DiskURLPath cannot be used with accessMode: %q", instance.Spec.AccessMode)
961+
}
949962
}
950963
if instance.Spec.VolumeID != "" {
951-
pvName, found := commonco.ContainerOrchestratorUtility.GetPVNameFromCSIVolumeID(instance.Spec.VolumeID)
952-
if found {
953-
if pvName != staticPvNamePrefix+instance.Spec.VolumeID {
954-
msg = fmt.Sprintf("PV: %q with the volume ID: %q "+
955-
"is already present. Can not create multiple PV with same volume Id.", pvName, instance.Spec.VolumeID)
964+
// Accessmode is required if volumeID is specified by the user.
965+
if instance.Spec.AccessMode == "" {
966+
msg = "AccessMode cannot be empty when volumeID is specified"
967+
} else {
968+
pvName, found := commonco.ContainerOrchestratorUtility.GetPVNameFromCSIVolumeID(instance.Spec.VolumeID)
969+
if found {
970+
if pvName != staticPvNamePrefix+instance.Spec.VolumeID {
971+
msg = fmt.Sprintf("PV: %q with the volume ID: %q "+
972+
"is already present. Can not create multiple PV with same volume Id.", pvName, instance.Spec.VolumeID)
973+
}
956974
}
957975
}
958976
}
@@ -969,6 +987,13 @@ func isBlockVolumeRegisterRequest(ctx context.Context, instance *cnsregistervolu
969987
if instance.Spec.AccessMode == v1.ReadWriteOnce {
970988
return true
971989
}
990+
if isSharedDiskEnabled {
991+
// Shared block volume
992+
if instance.Spec.AccessMode == v1.ReadWriteMany &&
993+
instance.Spec.VolumeMode == v1.PersistentVolumeBlock {
994+
return true
995+
}
996+
}
972997
} else {
973998
if instance.Spec.DiskURLPath != "" {
974999
return true

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

Lines changed: 133 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/agiledragon/gomonkey/v2"
2727
. "github.com/onsi/ginkgo/v2"
2828
. "github.com/onsi/gomega"
29+
"github.com/stretchr/testify/assert"
2930
cnstypes "github.com/vmware/govmomi/cns/types"
3031
"github.com/vmware/govmomi/object"
3132
vim25types "github.com/vmware/govmomi/vim25/types"
@@ -517,6 +518,7 @@ var _ = Describe("Reconcile Accessibility Logic", func() {
517518
})
518519

519520
patches.ApplyFunc(constructCreateSpecForInstance, func(
521+
ctx context.Context,
520522
r *ReconcileCnsRegisterVolume,
521523
instance *cnsregistervolumev1alpha1.CnsRegisterVolume,
522524
host string,
@@ -568,7 +570,7 @@ var _ = Describe("Reconcile Accessibility Logic", func() {
568570

569571
// Test getPersistentVolumeClaimSpec function directly
570572
pvcSpec, err := getPersistentVolumeClaimSpec(ctx, "test-pvc", "test-ns", 1024,
571-
"test-storage-class", corev1.ReadWriteOnce, "test-pv", nil, instance)
573+
"test-storage-class", corev1.ReadWriteOnce, corev1.PersistentVolumeFilesystem, "test-pv", nil, instance)
572574

573575
Expect(err).NotTo(HaveOccurred())
574576
Expect(pvcSpec).NotTo(BeNil())
@@ -594,7 +596,7 @@ var _ = Describe("Reconcile Accessibility Logic", func() {
594596

595597
// Test getPersistentVolumeClaimSpec function directly
596598
pvcSpec, err := getPersistentVolumeClaimSpec(ctx, "test-pvc", "test-ns", 1024,
597-
"test-storage-class", corev1.ReadWriteOnce, "test-pv", nil, instance)
599+
"test-storage-class", corev1.ReadWriteOnce, corev1.PersistentVolumeFilesystem, "test-pv", nil, instance)
598600

599601
Expect(err).NotTo(HaveOccurred())
600602
Expect(pvcSpec).NotTo(BeNil())
@@ -622,7 +624,7 @@ var _ = Describe("Reconcile Accessibility Logic", func() {
622624

623625
// Test getPersistentVolumeClaimSpec function directly
624626
pvcSpec, err := getPersistentVolumeClaimSpec(ctx, "test-pvc", "test-ns", 1024,
625-
"test-storage-class", corev1.ReadWriteOnce, "test-pv", nil, instance)
627+
"test-storage-class", corev1.ReadWriteOnce, corev1.PersistentVolumeFilesystem, "test-pv", nil, instance)
626628

627629
Expect(err).NotTo(HaveOccurred())
628630
Expect(pvcSpec).NotTo(BeNil())
@@ -1089,3 +1091,131 @@ func TestCnsRegisterVolumeController(t *testing.T) {
10891091
RegisterFailHandler(Fail)
10901092
RunSpecs(t, "CnsRegisterVolumeController Suite")
10911093
}
1094+
1095+
func TestValidateCnsRegisterVolumeSpecWithDiskUrlPath(t *testing.T) {
1096+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1097+
ObjectMeta: metav1.ObjectMeta{
1098+
Name: "register-vol",
1099+
Namespace: "test-ns",
1100+
},
1101+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1102+
PvcName: "pvc-1",
1103+
DiskURLPath: "som-url",
1104+
AccessMode: corev1.ReadWriteMany,
1105+
VolumeMode: corev1.PersistentVolumeFilesystem,
1106+
},
1107+
}
1108+
1109+
isSharedDiskEnabled = true
1110+
err := validateCnsRegisterVolumeSpec(context.TODO(), instance)
1111+
assert.Error(t, err)
1112+
assert.Equal(t, "DiskURLPath cannot be used with accessMode: ReadWriteMany and volumeMode: Filesystem", err.Error())
1113+
}
1114+
1115+
func TestValidateCnsRegisterVolumeSpecWithVolumeIdAndNoAccessMode(t *testing.T) {
1116+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1117+
ObjectMeta: metav1.ObjectMeta{
1118+
Name: "register-vol",
1119+
Namespace: "test-ns",
1120+
},
1121+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1122+
PvcName: "pvc-1",
1123+
VolumeID: "123456",
1124+
VolumeMode: corev1.PersistentVolumeFilesystem,
1125+
},
1126+
}
1127+
1128+
isSharedDiskEnabled = true
1129+
err := validateCnsRegisterVolumeSpec(context.TODO(), instance)
1130+
assert.Error(t, err)
1131+
assert.Equal(t, "AccessMode cannot be empty when volumeID is specified", err.Error())
1132+
}
1133+
1134+
func TestValidateCnsRegisterVolumeSpecWithVolumeIdAndAccessMode(t *testing.T) {
1135+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1136+
ObjectMeta: metav1.ObjectMeta{
1137+
Name: "register-vol",
1138+
Namespace: "test-ns",
1139+
},
1140+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1141+
PvcName: "pvc-1",
1142+
VolumeID: "123456",
1143+
AccessMode: corev1.ReadWriteMany,
1144+
VolumeMode: corev1.PersistentVolumeFilesystem,
1145+
},
1146+
}
1147+
1148+
isSharedDiskEnabled = true
1149+
commonco.ContainerOrchestratorUtility = &mockCOCommon{}
1150+
err := validateCnsRegisterVolumeSpec(context.TODO(), instance)
1151+
assert.NoError(t, err)
1152+
}
1153+
1154+
func TestIsBlockVolumeRegisterRequestWithSharedBlockVolume(t *testing.T) {
1155+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1156+
ObjectMeta: metav1.ObjectMeta{
1157+
Name: "register-vol",
1158+
Namespace: "test-ns",
1159+
},
1160+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1161+
PvcName: "pvc-1",
1162+
VolumeID: "123456",
1163+
VolumeMode: corev1.PersistentVolumeBlock,
1164+
AccessMode: corev1.ReadWriteMany,
1165+
},
1166+
}
1167+
1168+
isSharedDiskEnabled = true
1169+
isBlockVolume := isBlockVolumeRegisterRequest(t.Context(), instance)
1170+
assert.Equal(t, true, isBlockVolume)
1171+
}
1172+
1173+
func TestIsBlockVolumeRegisterRequestWithFileVolume(t *testing.T) {
1174+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1175+
ObjectMeta: metav1.ObjectMeta{
1176+
Name: "register-vol",
1177+
Namespace: "test-ns",
1178+
},
1179+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1180+
PvcName: "pvc-1",
1181+
VolumeID: "123456",
1182+
AccessMode: corev1.ReadWriteMany,
1183+
VolumeMode: corev1.PersistentVolumeFilesystem,
1184+
},
1185+
}
1186+
1187+
isSharedDiskEnabled = true
1188+
isBlockVolume := isBlockVolumeRegisterRequest(t.Context(), instance)
1189+
assert.Equal(t, false, isBlockVolume)
1190+
}
1191+
1192+
func TestGetPersistentVolumeSpecWhenVolumeModeIsEmpty(t *testing.T) {
1193+
var (
1194+
volumeName = "vol-1"
1195+
volumeID = "123456"
1196+
capacity = 256
1197+
accessMode = corev1.ReadWriteMany
1198+
scName = "testsc"
1199+
)
1200+
1201+
isSharedDiskEnabled = true
1202+
commonco.ContainerOrchestratorUtility = &mockCOCommon{}
1203+
pv := getPersistentVolumeSpec(volumeName, volumeID, int64(capacity), accessMode, "", scName, nil)
1204+
assert.Equal(t, corev1.PersistentVolumeFilesystem, *pv.Spec.VolumeMode)
1205+
}
1206+
1207+
func TestGetPersistentVolumeSpecWithVolumeMode(t *testing.T) {
1208+
var (
1209+
volumeName = "vol-1"
1210+
volumeID = "123456"
1211+
capacity = 256
1212+
accessMode = corev1.ReadWriteMany
1213+
scName = "testsc"
1214+
volumeMode = corev1.PersistentVolumeBlock
1215+
)
1216+
1217+
isSharedDiskEnabled = true
1218+
pv := getPersistentVolumeSpec(volumeName, volumeID,
1219+
int64(capacity), accessMode, volumeMode, scName, nil)
1220+
assert.Equal(t, volumeMode, *pv.Spec.VolumeMode)
1221+
}

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

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func isDatastoreAccessibleToAZClusters(ctx context.Context, vc *vsphere.VirtualC
113113
}
114114

115115
// constructCreateSpecForInstance creates CNS CreateVolume spec.
116-
func constructCreateSpecForInstance(r *ReconcileCnsRegisterVolume,
116+
func constructCreateSpecForInstance(ctx context.Context, r *ReconcileCnsRegisterVolume,
117117
instance *cnsregistervolumev1alpha1.CnsRegisterVolume,
118118
host string, useSupervisorId bool) *cnstypes.CnsVolumeCreateSpec {
119119
var volumeName string
@@ -148,11 +148,9 @@ func constructCreateSpecForInstance(r *ReconcileCnsRegisterVolume,
148148
BackingDiskUrlPath: instance.Spec.DiskURLPath,
149149
}
150150
}
151-
if instance.Spec.AccessMode == v1.ReadWriteOnce || instance.Spec.AccessMode == "" {
152-
createSpec.VolumeType = common.BlockVolumeType
153-
} else {
154-
createSpec.VolumeType = common.FileVolumeType
155-
}
151+
152+
createSpec.VolumeType = common.BlockVolumeType
153+
156154
return createSpec
157155
}
158156

@@ -251,7 +249,8 @@ func getK8sStorageClassNameWithImmediateBindingModeForPolicy(ctx context.Context
251249

252250
// getPersistentVolumeSpec to create PV volume spec for the given input params.
253251
func getPersistentVolumeSpec(volumeName string, volumeID string, capacity int64,
254-
accessMode v1.PersistentVolumeAccessMode, scName string, claimRef *v1.ObjectReference) *v1.PersistentVolume {
252+
accessMode v1.PersistentVolumeAccessMode, volumeMode v1.PersistentVolumeMode, scName string,
253+
claimRef *v1.ObjectReference) *v1.PersistentVolume {
255254
capacityInMb := strconv.FormatInt(capacity, 10) + "Mi"
256255
pv := &v1.PersistentVolume{
257256
TypeMeta: metav1.TypeMeta{},
@@ -279,6 +278,15 @@ func getPersistentVolumeSpec(volumeName string, volumeID string, capacity int64,
279278
},
280279
Status: v1.PersistentVolumeStatus{},
281280
}
281+
282+
if isSharedDiskEnabled {
283+
if volumeMode == "" {
284+
// For both RWO and RWX volumes, default volumeMode is Filesystem.
285+
volumeMode = v1.PersistentVolumeFilesystem
286+
}
287+
pv.Spec.VolumeMode = &volumeMode
288+
}
289+
282290
annotations := make(map[string]string)
283291
annotations["pv.kubernetes.io/provisioned-by"] = cnsoperatortypes.VSphereCSIDriverName
284292
pv.Annotations = annotations
@@ -288,7 +296,8 @@ func getPersistentVolumeSpec(volumeName string, volumeID string, capacity int64,
288296
// getPersistentVolumeClaimSpec return the PersistentVolumeClaim spec with
289297
// specified storage class.
290298
func getPersistentVolumeClaimSpec(ctx context.Context, name string, namespace string, capacity int64,
291-
storageClassName string, accessMode v1.PersistentVolumeAccessMode, pvName string,
299+
storageClassName string, accessMode v1.PersistentVolumeAccessMode, volumeMode v1.PersistentVolumeMode,
300+
pvName string,
292301
datastoreAccessibleTopology []map[string]string,
293302
instance *cnsregistervolumev1alpha1.CnsRegisterVolume) (*v1.PersistentVolumeClaim, error) {
294303

@@ -344,6 +353,11 @@ func getPersistentVolumeClaimSpec(ctx context.Context, name string, namespace st
344353
VolumeName: pvName,
345354
},
346355
}
356+
357+
if isSharedDiskEnabled {
358+
claim.Spec.VolumeMode = &volumeMode
359+
}
360+
347361
return claim, nil
348362
}
349363

0 commit comments

Comments
 (0)