Skip to content

Commit

Permalink
feat: add fsGroupChangePolicy None parameter
Browse files Browse the repository at this point in the history
  • Loading branch information
andyzhangx committed May 20, 2022
1 parent ec673f9 commit 91bd20a
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 8 deletions.
1 change: 0 additions & 1 deletion deploy/example/storageclass-azurefile-nfs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ provisioner: file.csi.azure.com
parameters:
protocol: nfs
skuName: Premium_LRS # available values: Premium_LRS, Premium_ZRS
fsGroupChangePolicy: "" # optional, available values: "", "OnRootMismatch", "Always"
reclaimPolicy: Delete
volumeBindingMode: Immediate
allowVolumeExpansion: true
Expand Down
6 changes: 3 additions & 3 deletions docs/driver-parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ mountPermissions | mounted folder permissions. The default is `0777`, if set as
vnetResourceGroup | specify vnet resource group where virtual network is | existing resource group name | No | if empty, driver will use the `vnetResourceGroup` value in azure cloud config file
vnetName | virtual network name | existing virtual network name | No | if empty, driver will use the `vnetName` value in azure cloud config file
subnetName | subnet name | existing subnet name of the agent node | No | if empty, driver will use the `subnetName` value in azure cloud config file
fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | "",`OnRootMismatch`, `Always` | No | ""(no change by default)
fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | `OnRootMismatch`(by default), `Always`, `None` | No | `OnRootMismatch`
--- | **Following parameters are only for [VHD disk feature](../deploy/example/disk)** | --- | --- |
fsType | File System Type | `ext4`, `ext3`, `ext2`, `xfs` | Yes | `ext4`
diskName | existing VHD disk file name | `pvc-062196a6-6436-11ea-ab51-9efb888c0afb.vhd` | No |
fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | "",`OnRootMismatch`, `Always` | No | ""(no change by default)
fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | `OnRootMismatch`(by default), `Always`, `None` | No | `OnRootMismatch`

- account tags format created by dynamic provisioning
```
Expand Down Expand Up @@ -76,7 +76,7 @@ volumeAttributes.secretNamespace | secret namespace | `default`,`kube-system`, e
nodeStageSecretRef.name | secret name that stores storage account name and key | existing secret name | Yes |
nodeStageSecretRef.namespace | secret namespace | k8s namespace | Yes |
--- | **Following parameters are only for NFS protocol** | --- | --- |
fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | "",`OnRootMismatch`, `Always` | No | ""(no change by default)
fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | `OnRootMismatch`(by default), `Always`, `None` | No | `OnRootMismatch`
volumeAttributes.mountPermissions | mounted folder permissions. The default is `0777` | | No |
- Note
- only mounting Azure File using SMB protocol requires account key, and if secret is not provided in PV config, it would try to get `azure-storage-account-{accountname}-secret` in the pod namespace, if not found, it would try using kubelet identity to get account key directly using Azure API.
Expand Down
7 changes: 6 additions & 1 deletion pkg/azurefile/azurefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,15 @@ const (
SourceResourceID = "source_resource_id"
SnapshotName = "snapshot_name"
SnapshotID = "snapshot_id"

FSGroupChangeNone = "None"
)

var (
supportedFsTypeList = []string{cifs, smb, nfs, ext4, ext3, ext2, xfs}
supportedProtocolList = []string{smb, nfs}
supportedDiskFsTypeList = []string{ext4, ext3, ext2, xfs}
supportedFSGroupChangePolicyList = []string{"Always", "OnRootMismatch"}
supportedFSGroupChangePolicyList = []string{FSGroupChangeNone, string(v1.FSGroupChangeAlways), string(v1.FSGroupChangeOnRootMismatch)}

retriableErrors = []string{accountNotProvisioned, tooManyRequests, shareBeingDeleted, clientThrottled}
)
Expand All @@ -184,6 +186,7 @@ type DriverOptions struct {
AllowInlineVolumeKeyAccessWithIdentity bool
EnableGetVolumeStats bool
MountPermissions uint64
FSGroupChangePolicy string
}

// Driver implements all interfaces of CSI drivers
Expand All @@ -194,6 +197,7 @@ type Driver struct {
cloudConfigSecretNamespace string
customUserAgent string
userAgentSuffix string
fsGroupChangePolicy string
allowEmptyCloudConfig bool
allowInlineVolumeKeyAccessWithIdentity bool
enableGetVolumeStats bool
Expand Down Expand Up @@ -236,6 +240,7 @@ func NewDriver(options *DriverOptions) *Driver {
driver.allowInlineVolumeKeyAccessWithIdentity = options.AllowInlineVolumeKeyAccessWithIdentity
driver.enableGetVolumeStats = options.EnableGetVolumeStats
driver.mountPermissions = options.MountPermissions
driver.fsGroupChangePolicy = options.FSGroupChangePolicy
driver.volLockMap = newLockMap()
driver.subnetLockMap = newLockMap()
driver.volumeLocks = newVolumeLocks()
Expand Down
6 changes: 4 additions & 2 deletions pkg/azurefile/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,12 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
}
// don't respect fsType from req.GetVolumeCapability().GetMount().GetFsType()
// since it's ext4 by default on Linux
var fsType, server, protocol, ephemeralVolMountOptions, storageEndpointSuffix, folderName, fsGroupChangePolicy string
var fsType, server, protocol, ephemeralVolMountOptions, storageEndpointSuffix, folderName string
var ephemeralVol bool
mountPermissions := d.mountPermissions
performChmodOp := (mountPermissions > 0)
fsGroupChangePolicy := d.fsGroupChangePolicy

for k, v := range context {
switch strings.ToLower(k) {
case fsTypeField:
Expand Down Expand Up @@ -345,7 +347,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
}

if protocol == nfs || isDiskMount {
if volumeMountGroup != "" && fsGroupChangePolicy != "" {
if volumeMountGroup != "" && fsGroupChangePolicy != FSGroupChangeNone {
klog.V(2).Infof("set gid of volume(%s) as %s using fsGroupChangePolicy(%s)", volumeID, volumeMountGroup, fsGroupChangePolicy)
if err := SetVolumeOwnership(cifsMountPath, volumeMountGroup, fsGroupChangePolicy); err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("SetVolumeOwnership with volume(%s) on %s failed with %v", volumeID, cifsMountPath, err))
Expand Down
4 changes: 4 additions & 0 deletions pkg/azurefile/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,10 @@ func TestIsSupportedFSGroupChangePolicy(t *testing.T) {
policy: "",
expectedResult: true,
},
{
policy: "None",
expectedResult: true,
},
{
policy: "Always",
expectedResult: true,
Expand Down
2 changes: 2 additions & 0 deletions pkg/azurefileplugin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ var (
enableGetVolumeStats = flag.Bool("enable-get-volume-stats", true, "allow GET_VOLUME_STATS on agent node")
mountPermissions = flag.Uint64("mount-permissions", 0777, "mounted folder permissions")
allowInlineVolumeKeyAccessWithIdentity = flag.Bool("allow-inline-volume-key-access-with-identity", false, "allow accessing storage account key using cluster identity for inline volume")
fsGroupChangePolicy = flag.String("fsgroup-change-policy", "", "indicates how the volume's ownership will be changed by the driver, OnRootMismatch is the default value")
)

func main() {
Expand Down Expand Up @@ -85,6 +86,7 @@ func handle() {
EnableGetVolumeStats: *enableGetVolumeStats,
MountPermissions: *mountPermissions,
AllowInlineVolumeKeyAccessWithIdentity: *allowInlineVolumeKeyAccessWithIdentity,
FSGroupChangePolicy: *fsGroupChangePolicy,
}
driver := azurefile.NewDriver(&driverOptions)
if driver == nil {
Expand Down
1 change: 0 additions & 1 deletion test/external-e2e/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ fi
if [ ! -z ${EXTERNAL_E2E_TEST_NFS} ]; then
echo "begin to run NFS protocol tests ...."
cp deploy/example/storageclass-azurefile-nfs.yaml /tmp/csi/storageclass.yaml
sed -i 's/fsGroupChangePolicy: ""/fsGroupChangePolicy: "Always"/g' /tmp/csi/storageclass.yaml
ginkgo -p --progress --v -focus="External.Storage.*$DRIVER.csi.azure.com" \
-skip='\[Disruptive\]|should provision storage with any volume data source|should mount multiple PV pointing to the same storage on the same node' kubernetes/test/bin/e2e.test -- \
-storage.testdriver=$PROJECT_ROOT/test/external-e2e/testdriver-nfs.yaml \
Expand Down

0 comments on commit 91bd20a

Please sign in to comment.