Skip to content

Commit 98e5fc2

Browse files
andyzhangxandyzhangx
authored
Merge pull request #1199 from andyzhangx/support-volume-mount-group
feat: support volume mount group and add FSGroupChangePolicy param for NFS mount
2 parents 7a939de + 3e4a784 commit 98e5fc2

13 files changed

+296
-10
lines changed

‎docs/driver-parameters.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ nodeStageSecretRef.name | secret name that stores(check below examples):<br>`azu
9797
nodeStageSecretRef.namespace | secret namespace | k8s namespace | Yes |
9898
--- | **Following parameters are only for NFS protocol** | --- | --- |
9999
volumeAttributes.mountPermissions | mounted folder permissions | `0777` | No |
100+
fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | `OnRootMismatch`(by default), `Always`, `None` | No | `OnRootMismatch`
100101
--- | **Following parameters are only for feature: blobfuse [Managed Identity and Service Principal Name auth](https://github.com/Azure/azure-storage-fuse/tree/blobfuse-1.4.5#environment-variables)** | --- | --- |
101102
volumeAttributes.AzureStorageAuthType | Authentication Type | `Key`, `SAS`, `MSI`, `SPN` | No | `Key`
102103
volumeAttributes.AzureStorageIdentityClientID | Identity Client ID | | No |

‎pkg/blob/blob.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ const (
114114
accessTierField = "accesstier"
115115
networkEndpointTypeField = "networkendpointtype"
116116
mountPermissionsField = "mountpermissions"
117+
fsGroupChangePolicyField = "fsgroupchangepolicy"
117118
useDataPlaneAPIField = "usedataplaneapi"
118119

119120
// See https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata#container-names
@@ -145,11 +146,14 @@ const (
145146
VolumeID = "volumeid"
146147

147148
defaultStorageEndPointSuffix = "core.windows.net"
149+
150+
FSGroupChangeNone = "None"
148151
)
149152

150153
var (
151-
supportedProtocolList = []string{Fuse, Fuse2, NFS}
152-
retriableErrors = []string{accountNotProvisioned, tooManyRequests, statusCodeNotFound, containerBeingDeletedDataplaneAPIError, containerBeingDeletedManagementAPIError, clientThrottled}
154+
supportedProtocolList = []string{Fuse, Fuse2, NFS}
155+
retriableErrors = []string{accountNotProvisioned, tooManyRequests, statusCodeNotFound, containerBeingDeletedDataplaneAPIError, containerBeingDeletedManagementAPIError, clientThrottled}
156+
supportedFSGroupChangePolicyList = []string{FSGroupChangeNone, string(v1.FSGroupChangeAlways), string(v1.FSGroupChangeOnRootMismatch)}
153157
)
154158

155159
// DriverOptions defines driver parameters specified in driver deployment
@@ -168,6 +172,8 @@ type DriverOptions struct {
168172
EnableAznfsMount bool
169173
VolStatsCacheExpireInMinutes int
170174
SasTokenExpirationMinutes int
175+
EnableVolumeMountGroup bool
176+
FSGroupChangePolicy string
171177
}
172178

173179
func (option *DriverOptions) AddFlags() {
@@ -185,6 +191,8 @@ func (option *DriverOptions) AddFlags() {
185191
flag.BoolVar(&option.EnableAznfsMount, "enable-aznfs-mount", false, "replace nfs mount with aznfs mount")
186192
flag.IntVar(&option.VolStatsCacheExpireInMinutes, "vol-stats-cache-expire-in-minutes", 10, "The cache expire time in minutes for volume stats cache")
187193
flag.IntVar(&option.SasTokenExpirationMinutes, "sas-token-expiration-minutes", 1440, "sas token expiration minutes during volume cloning")
194+
flag.BoolVar(&option.EnableVolumeMountGroup, "enable-volume-mount-group", true, "indicates whether enabling VOLUME_MOUNT_GROUP")
195+
flag.StringVar(&option.FSGroupChangePolicy, "fsgroup-change-policy", "", "indicates how the volume's ownership will be changed by the driver, OnRootMismatch is the default value")
188196
}
189197

190198
// Driver implements all interfaces of CSI drivers
@@ -204,6 +212,8 @@ type Driver struct {
204212
blobfuseProxyConnTimout int
205213
mountPermissions uint64
206214
enableAznfsMount bool
215+
enableVolumeMountGroup bool
216+
fsGroupChangePolicy string
207217
mounter *mount.SafeFormatAndMount
208218
volLockMap *util.LockMap
209219
// A map storing all volumes with ongoing operations so that additional operations
@@ -230,7 +240,6 @@ type Driver struct {
230240
// NewDriver Creates a NewCSIDriver object. Assumes vendor version is equal to driver version &
231241
// does not support optional driver plugin info manifest field. Refer to CSI spec for more details.
232242
func NewDriver(options *DriverOptions, kubeClient kubernetes.Interface, cloud *provider.Cloud) *Driver {
233-
var err error
234243
d := Driver{
235244
volLockMap: util.NewLockMap(),
236245
subnetLockMap: util.NewLockMap(),
@@ -241,10 +250,12 @@ func NewDriver(options *DriverOptions, kubeClient kubernetes.Interface, cloud *p
241250
blobfuseProxyConnTimout: options.BlobfuseProxyConnTimout,
242251
enableBlobMockMount: options.EnableBlobMockMount,
243252
enableGetVolumeStats: options.EnableGetVolumeStats,
253+
enableVolumeMountGroup: options.EnableVolumeMountGroup,
244254
appendMountErrorHelpLink: options.AppendMountErrorHelpLink,
245255
mountPermissions: options.MountPermissions,
246256
enableAznfsMount: options.EnableAznfsMount,
247257
sasTokenExpirationMinutes: options.SasTokenExpirationMinutes,
258+
fsGroupChangePolicy: options.FSGroupChangePolicy,
248259
azcopy: &util.Azcopy{},
249260
KubeClient: kubeClient,
250261
cloud: cloud,
@@ -253,6 +264,7 @@ func NewDriver(options *DriverOptions, kubeClient kubernetes.Interface, cloud *p
253264
d.Version = driverVersion
254265
d.NodeID = options.NodeID
255266

267+
var err error
256268
getter := func(key string) (interface{}, error) { return nil, nil }
257269
if d.accountSearchCache, err = azcache.NewTimedCache(time.Minute, getter, false); err != nil {
258270
klog.Fatalf("%v", err)
@@ -302,6 +314,9 @@ func NewDriver(options *DriverOptions, kubeClient kubernetes.Interface, cloud *p
302314
if d.enableGetVolumeStats {
303315
nodeCap = append(nodeCap, csi.NodeServiceCapability_RPC_GET_VOLUME_STATS)
304316
}
317+
if d.enableVolumeMountGroup {
318+
nodeCap = append(nodeCap, csi.NodeServiceCapability_RPC_VOLUME_MOUNT_GROUP)
319+
}
305320
d.AddNodeServiceCapabilities(nodeCap)
306321

307322
return &d
@@ -1022,3 +1037,15 @@ func replaceWithMap(str string, m map[string]string) string {
10221037
}
10231038
return str
10241039
}
1040+
1041+
func isSupportedFSGroupChangePolicy(policy string) bool {
1042+
if policy == "" {
1043+
return true
1044+
}
1045+
for _, v := range supportedFSGroupChangePolicyList {
1046+
if policy == v {
1047+
return true
1048+
}
1049+
}
1050+
return false
1051+
}

‎pkg/blob/blob_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1784,3 +1784,42 @@ func TestDriverOptions_AddFlags(t *testing.T) {
17841784
}
17851785
})
17861786
}
1787+
1788+
func TestIsSupportedFSGroupChangePolicy(t *testing.T) {
1789+
tests := []struct {
1790+
policy string
1791+
expectedResult bool
1792+
}{
1793+
{
1794+
policy: "",
1795+
expectedResult: true,
1796+
},
1797+
{
1798+
policy: "None",
1799+
expectedResult: true,
1800+
},
1801+
{
1802+
policy: "Always",
1803+
expectedResult: true,
1804+
},
1805+
{
1806+
policy: "OnRootMismatch",
1807+
expectedResult: true,
1808+
},
1809+
{
1810+
policy: "onRootMismatch",
1811+
expectedResult: false,
1812+
},
1813+
{
1814+
policy: "invalid",
1815+
expectedResult: false,
1816+
},
1817+
}
1818+
1819+
for _, test := range tests {
1820+
result := isSupportedFSGroupChangePolicy(test.policy)
1821+
if result != test.expectedResult {
1822+
t.Errorf("isSupportedFSGroupChangePolicy(%s) returned with %v, not equal to %v", test.policy, result, test.expectedResult)
1823+
}
1824+
}
1825+
}

‎pkg/blob/controllerserver.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
9999
}
100100
var storageAccountType, subsID, resourceGroup, location, account, containerName, containerNamePrefix, protocol, customTags, secretName, secretNamespace, pvcNamespace string
101101
var isHnsEnabled, requireInfraEncryption, enableBlobVersioning, createPrivateEndpoint, enableNfsV3 *bool
102-
var vnetResourceGroup, vnetName, subnetName, accessTier, networkEndpointType, storageEndpointSuffix string
102+
var vnetResourceGroup, vnetName, subnetName, accessTier, networkEndpointType, storageEndpointSuffix, fsGroupChangePolicy string
103103
var matchTags, useDataPlaneAPI, getLatestAccountKey bool
104104
var softDeleteBlobs, softDeleteContainers int32
105105
var vnetResourceIDs []string
@@ -212,6 +212,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
212212
}
213213
case useDataPlaneAPIField:
214214
useDataPlaneAPI = strings.EqualFold(v, trueValue)
215+
case fsGroupChangePolicyField:
216+
fsGroupChangePolicy = v
215217
default:
216218
return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid parameter %q in storage class", k))
217219
}
@@ -223,6 +225,10 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
223225
}
224226
}
225227

228+
if !isSupportedFSGroupChangePolicy(fsGroupChangePolicy) {
229+
return nil, status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(%s) is not supported, supported fsGroupChangePolicy list: %v", fsGroupChangePolicy, supportedFSGroupChangePolicyList)
230+
}
231+
226232
if matchTags && account != "" {
227233
return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("matchTags must set as false when storageAccount(%s) is provided", account))
228234
}

‎pkg/blob/controllerserver_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,29 @@ func TestCreateVolume(t *testing.T) {
238238
}
239239
},
240240
},
241+
{
242+
name: "Invalid fsGroupChangePolicy",
243+
testFunc: func(t *testing.T) {
244+
d := NewFakeDriver()
245+
d.cloud = &azure.Cloud{}
246+
mp := map[string]string{
247+
fsGroupChangePolicyField: "test_fsGroupChangePolicy",
248+
}
249+
req := &csi.CreateVolumeRequest{
250+
Name: "unit-test",
251+
VolumeCapabilities: stdVolumeCapabilities,
252+
Parameters: mp,
253+
}
254+
d.Cap = []*csi.ControllerServiceCapability{
255+
controllerServiceCapability,
256+
}
257+
_, err := d.CreateVolume(context.Background(), req)
258+
expectedErr := status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(test_fsGroupChangePolicy) is not supported, supported fsGroupChangePolicy list: [None Always OnRootMismatch]")
259+
if !reflect.DeepEqual(err, expectedErr) {
260+
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
261+
}
262+
},
263+
},
241264
{
242265
name: "invalid getLatestAccountKey value",
243266
testFunc: func(t *testing.T) {

‎pkg/blob/nodeserver.go

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
237237
defer d.volumeLocks.Release(lockKey)
238238

239239
mountFlags := req.GetVolumeCapability().GetMount().GetMountFlags()
240+
volumeMountGroup := req.GetVolumeCapability().GetMount().GetVolumeMountGroup()
240241
attrib := req.GetVolumeContext()
241242
secrets := req.GetSecrets()
242243

@@ -251,6 +252,8 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
251252

252253
containerNameReplaceMap := map[string]string{}
253254

255+
fsGroupChangePolicy := d.fsGroupChangePolicy
256+
254257
mountPermissions := d.mountPermissions
255258
performChmodOp := (mountPermissions > 0)
256259
for k, v := range attrib {
@@ -286,9 +289,15 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
286289
mountPermissions = perm
287290
}
288291
}
292+
case fsGroupChangePolicyField:
293+
fsGroupChangePolicy = v
289294
}
290295
}
291296

297+
if !isSupportedFSGroupChangePolicy(fsGroupChangePolicy) {
298+
return nil, status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(%s) is not supported, supported fsGroupChangePolicy list: %v", fsGroupChangePolicy, supportedFSGroupChangePolicyList)
299+
}
300+
292301
mnt, err := d.ensureMountPoint(targetPath, fs.FileMode(mountPermissions))
293302
if err != nil {
294303
return nil, status.Errorf(codes.Internal, "Could not mount target %q: %v", targetPath, err)
@@ -348,6 +357,13 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
348357
klog.V(2).Infof("skip chmod on targetPath(%s) since mountPermissions is set as 0", targetPath)
349358
}
350359

360+
if volumeMountGroup != "" && fsGroupChangePolicy != FSGroupChangeNone {
361+
klog.V(2).Infof("set gid of volume(%s) as %s using fsGroupChangePolicy(%s)", volumeID, volumeMountGroup, fsGroupChangePolicy)
362+
if err := volumehelper.SetVolumeOwnership(targetPath, volumeMountGroup, fsGroupChangePolicy); err != nil {
363+
return nil, status.Error(codes.Internal, fmt.Sprintf("SetVolumeOwnership with volume(%s) on %s failed with %v", volumeID, targetPath, err))
364+
}
365+
}
366+
351367
isOperationSucceeded = true
352368
klog.V(2).Infof("volume(%s) mount %s on %s succeeded", volumeID, source, targetPath)
353369
return &csi.NodeStageVolumeResponse{}, nil
@@ -361,6 +377,12 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
361377
if isHnsEnabled {
362378
mountOptions = util.JoinMountOptions(mountOptions, []string{"--use-adls=true"})
363379
}
380+
381+
if !checkGidPresentInMountFlags(mountFlags) && volumeMountGroup != "" {
382+
klog.V(2).Infof("append volumeMountGroup %s", volumeMountGroup)
383+
mountOptions = append(mountOptions, fmt.Sprintf("-o gid=%s", volumeMountGroup))
384+
}
385+
364386
tmpPath := fmt.Sprintf("%s/%s", "/mnt", volumeID)
365387
if d.appendTimeStampInCacheDir {
366388
tmpPath += fmt.Sprintf("#%d", time.Now().Unix())
@@ -372,8 +394,8 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
372394
args = args + " " + opt
373395
}
374396

375-
klog.V(2).Infof("target %v\nprotocol %v\n\nvolumeId %v\ncontext %v\nmountflags %v\nmountOptions %v\nargs %v\nserverAddress %v",
376-
targetPath, protocol, volumeID, attrib, mountFlags, mountOptions, args, serverAddress)
397+
klog.V(2).Infof("target %v\nprotocol %v\n\nvolumeId %v\ncontext %v\nmountflags %v mountOptions %v volumeMountGroup %s\nargs %v\nserverAddress %v",
398+
targetPath, protocol, volumeID, attrib, mountFlags, mountOptions, volumeMountGroup, args, serverAddress)
377399

378400
authEnv = append(authEnv, "AZURE_STORAGE_ACCOUNT="+accountName, "AZURE_STORAGE_BLOB_ENDPOINT="+serverAddress)
379401
if d.enableBlobMockMount {
@@ -653,3 +675,12 @@ func waitForMount(path string, intervel, timeout time.Duration) error {
653675
}
654676
}
655677
}
678+
679+
func checkGidPresentInMountFlags(mountFlags []string) bool {
680+
for _, mountFlag := range mountFlags {
681+
if strings.Contains(mountFlag, "gid=") {
682+
return true
683+
}
684+
}
685+
return false
686+
}

‎pkg/blob/nodeserver_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,25 @@ func TestNodeStageVolume(t *testing.T) {
463463
}
464464
},
465465
},
466+
{
467+
name: "[Error] Invalid fsGroupChangePolicy",
468+
testFunc: func(t *testing.T) {
469+
req := &csi.NodeStageVolumeRequest{
470+
VolumeId: "unit-test",
471+
StagingTargetPath: "unit-test",
472+
VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap},
473+
VolumeContext: map[string]string{
474+
fsGroupChangePolicyField: "test_fsGroupChangePolicy",
475+
},
476+
}
477+
d := NewFakeDriver()
478+
_, err := d.NodeStageVolume(context.TODO(), req)
479+
expectedErr := status.Error(codes.InvalidArgument, "fsGroupChangePolicy(test_fsGroupChangePolicy) is not supported, supported fsGroupChangePolicy list: [None Always OnRootMismatch]")
480+
if !reflect.DeepEqual(err, expectedErr) {
481+
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
482+
}
483+
},
484+
},
466485
{
467486
name: "[Error] Could not mount to target",
468487
testFunc: func(t *testing.T) {
@@ -812,3 +831,34 @@ func Test_waitForMount(t *testing.T) {
812831
})
813832
}
814833
}
834+
835+
func TestCheckGidPresentInMountFlags(t *testing.T) {
836+
tests := []struct {
837+
desc string
838+
MountFlags []string
839+
result bool
840+
}{
841+
{
842+
desc: "[Success] Gid present in mount flags",
843+
MountFlags: []string{"gid=3000"},
844+
result: true,
845+
},
846+
{
847+
desc: "[Success] Gid present in mount flags",
848+
MountFlags: []string{"-o gid=3000"},
849+
result: true,
850+
},
851+
{
852+
desc: "[Success] Gid not present in mount flags",
853+
MountFlags: []string{},
854+
result: false,
855+
},
856+
}
857+
858+
for _, test := range tests {
859+
gIDPresent := checkGidPresentInMountFlags(test.MountFlags)
860+
if gIDPresent != test.result {
861+
t.Errorf("[%s]: Expected result : %t, Actual result: %t", test.desc, test.result, gIDPresent)
862+
}
863+
}
864+
}

0 commit comments

Comments
 (0)