From ade2f83685faf1b1f869fe321c4fd9c8a002d3fc Mon Sep 17 00:00:00 2001
From: Hemant Kumar <hekumar@redhat.com>
Date: Wed, 8 Jul 2020 15:25:03 -0400
Subject: [PATCH] Simplify the code

---
 pkg/volume/csi/csi_mounter.go | 83 +++++++++++------------------------
 1 file changed, 25 insertions(+), 58 deletions(-)

diff --git a/pkg/volume/csi/csi_mounter.go b/pkg/volume/csi/csi_mounter.go
index f30e02e5eeb..4195ab38759 100644
--- a/pkg/volume/csi/csi_mounter.go
+++ b/pkg/volume/csi/csi_mounter.go
@@ -28,7 +28,6 @@ import (
 	"k8s.io/klog/v2"
 
 	api "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
@@ -278,23 +277,8 @@ func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error
 		klog.V(2).Info(log("error checking for SELinux support: %s", err))
 	}
 
-	fsGroupFeatureGateEnabled := utilfeature.DefaultFeatureGate.Enabled(features.CSIVolumeFSGroupPolicy)
-	// If the feature gate isn't enabled, then adjust the CSIDriver to use the ReadWriteOnceWithFSTypeFSGroupPolicy
-	// policy. This keeps the default behavior.
-	if !fsGroupFeatureGateEnabled {
-		c.fsGroupPolicy = storage.ReadWriteOnceWithFSTypeFSGroupPolicy
-	}
-
-	// If the the FSGroupPolicy isn't NoneFSGroupPolicy, then we should attempt to modify
-	// the fsGroup. At this point the feature gate is enabled, so we should proceed,
-	// or it's disabled, at which point we should evaluate the fstype and pv.AccessMode
-	// and update the fsGroup appropriately.
-	if c.fsGroupPolicy != storage.NoneFSGroupPolicy {
-
-		// The following logic is derived from https://github.com/kubernetes/kubernetes/issues/66323
-		// if fstype is "", then skip fsgroup (could be indication of non-block filesystem)
-		// if fstype is provided and pv.AccessMode == ReadWriteOnly, then apply fsgroup
-		err = c.applyFSGroup(fsType, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy)
+	if c.supportsFSGroup(fsType, mounterArgs.FsGroup, c.fsGroupPolicy) {
+		err := volume.SetVolumeOwnership(c, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy)
 		if err != nil {
 			// At this point mount operation is successful:
 			//   1. Since volume can not be used by the pod because of invalid permissions, we must return error
@@ -302,6 +286,7 @@ func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error
 			//      cleaned up.
 			return volumetypes.NewUncertainProgressError(fmt.Sprintf("applyFSGroup failed for vol %s: %v", c.volumeID, err))
 		}
+		klog.V(4).Info(log("mounter.SetupAt fsGroup [%d] applied successfully to %s", *mounterArgs.FsGroup, c.volumeID))
 	}
 
 	klog.V(4).Infof(log("mounter.SetUp successfully requested NodePublish [%s]", dir))
@@ -386,48 +371,30 @@ func (c *csiMountMgr) TearDownAt(dir string) error {
 	return nil
 }
 
-// applyFSGroup applies the volume ownership it derives its logic
-// from https://github.com/kubernetes/kubernetes/issues/66323
-// 1) if fstype is "", then skip fsgroup (could be indication of non-block filesystem)
-// 2) if fstype is provided and pv.AccessMode == ReadWriteOnly and !c.spec.ReadOnly then apply fsgroup
-func (c *csiMountMgr) applyFSGroup(fsType string, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy) error {
-	if c.fsGroupPolicy == storage.FileFSGroupPolicy || fsGroup != nil {
-
-		// If the FSGroupPolicy is ReadWriteOnceWithFSTypeFSGroupPolicy perform additional checks
-		// to determine if we should proceed with modifying the fsGroup.
-		if c.fsGroupPolicy == storage.ReadWriteOnceWithFSTypeFSGroupPolicy {
-			if fsType == "" {
-				klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, fsType not provided"))
-				return nil
-			}
-
-			accessModes := c.spec.PersistentVolume.Spec.AccessModes
-			if c.spec.PersistentVolume.Spec.AccessModes == nil {
-				klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, access modes not provided"))
-				return nil
-			}
-			if !hasReadWriteOnce(accessModes) {
-				klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, only support ReadWriteOnce access mode"))
-				return nil
-			}
-
-			if c.readOnly {
-				klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, volume is readOnly"))
-				return nil
-			}
-		}
-
-		err := volume.SetVolumeOwnership(c, fsGroup, fsGroupChangePolicy)
-		if err != nil {
-			return err
-		}
-
-		if fsGroup != nil {
-			klog.V(4).Info(log("mounter.SetupAt fsGroup [%d] applied successfully to %s", *fsGroup, c.volumeID))
-		}
+func (c *csiMountMgr) supportsFSGroup(fsType string, fsGroup *int64, driverPolicy storage.FSGroupPolicy) bool {
+	if fsGroup == nil || driverPolicy == storage.NoneFSGroupPolicy || c.readOnly {
+		return false
 	}
 
-	return nil
+	if driverPolicy == storage.FileFSGroupPolicy {
+		return true
+	}
+
+	if fsType == "" {
+		klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, fsType not provided"))
+		return false
+	}
+
+	accessModes := c.spec.PersistentVolume.Spec.AccessModes
+	if c.spec.PersistentVolume.Spec.AccessModes == nil {
+		klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, access modes not provided"))
+		return false
+	}
+	if !hasReadWriteOnce(accessModes) {
+		klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, only support ReadWriteOnce access mode"))
+		return false
+	}
+	return true
 }
 
 // isDirMounted returns the !notMounted result from IsLikelyNotMountPoint check