Skip to content

Commit

Permalink
Merge pull request #425 from hime/main
Browse files Browse the repository at this point in the history
Remove max_ratio kernel parameter option.
  • Loading branch information
hime authored Jan 8, 2025
2 parents 25312c0 + d6c8c53 commit 809b168
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 41 deletions.
23 changes: 3 additions & 20 deletions pkg/csi_mounter/csi_mounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,10 @@ import (
const (
socketName = "socket"
readAheadKBMountFlagRegexPattern = "^read_ahead_kb=(.+)$"
maxRatioMountFlagRegexPattern = "^max_ratio=(.+)$"
readAheadKBMountFlag = "read_ahead_kb"
maxRatioMountFlag = "max_ratio"
)

var (
readAheadKBMountFlagRegex = regexp.MustCompile(readAheadKBMountFlagRegexPattern)
maxRatioMountFlagRegex = regexp.MustCompile(maxRatioMountFlagRegexPattern)
)
var readAheadKBMountFlagRegex = regexp.MustCompile(readAheadKBMountFlagRegexPattern)

// Mounter provides the Cloud Storage FUSE CSI implementation of mount.Interface
// for the linux platform.
Expand Down Expand Up @@ -119,7 +114,7 @@ func (m *Mounter) Mount(source string, target string, fstype string, options []s
// It will succeed once dfuse finishes the mount process, or it will fail if dfuse fails
// or the mount point is cleaned up due to mounting failures.
if err := updateSysfsConfig(target, sysfsBDI); err != nil {
klog.Errorf("%v failed to update read_ahead_kb or max_ratio: %v", logPrefix, err)
klog.Errorf("%v failed to update kernel parameters: %v", logPrefix, err)
}
}()
}
Expand Down Expand Up @@ -153,7 +148,7 @@ func (m *Mounter) Mount(source string, target string, fstype string, options []s
return nil
}

// updateSysfsConfig modifies the kernel page cache settings based on the read_ahead_kb or max_ratio provided in the mountOption,
// updateSysfsConfig modifies the kernel page cache settings based on the read_ahead_kb provided in the mountOption,
// and verifies that the values are successfully updated after the operation completes.
func updateSysfsConfig(targetMountPath string, sysfsBDI map[string]int64) error {
// Command will hang until mount completes.
Expand Down Expand Up @@ -340,18 +335,6 @@ func prepareMountOptions(options []string) ([]string, []string, map[string]int64
sysfsBDI[readAheadKBMountFlag] = readAheadKBInt
optionSet.Delete(o)
}

if maxRatio := maxRatioMountFlagRegex.FindStringSubmatch(o); len(maxRatio) == 2 {
maxRatioInt, err := strconv.ParseInt(maxRatio[1], 10, 0)
if err != nil {
return nil, nil, nil, fmt.Errorf("invalid max_ratio mount flag %q: %w", o, err)
}
if maxRatioInt < 0 || maxRatioInt > 100 {
return nil, nil, nil, fmt.Errorf("invalid value for max_ratio mount flag: %q", o)
}
sysfsBDI[maxRatioMountFlag] = maxRatioInt
optionSet.Delete(o)
}
}

return csiMountOptions, optionSet.List(), sysfsBDI, nil
Expand Down
19 changes: 2 additions & 17 deletions pkg/csi_mounter/csi_mounter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ func TestPrepareMountArgs(t *testing.T) {
},
{
name: "should return valid options correctly with CSI and sidecar mount options with read ahead configs",
inputMountOptions: []string{"ro", "implicit-dirs", "max-conns-per-host=10", "o=noexec", "o=noatime", "o=invalid", "read_ahead_kb=4096", "max_ratio=100"},
inputMountOptions: []string{"ro", "implicit-dirs", "max-conns-per-host=10", "o=noexec", "o=noatime", "o=invalid", "read_ahead_kb=4096"},
expecteCsiMountOptions: append(defaultCsiMountOptions, "ro", "noexec", "noatime"),
expecteSidecarMountOptions: []string{"implicit-dirs", "max-conns-per-host=10"},
expectedSysfsBDI: map[string]int64{"read_ahead_kb": 4096, "max_ratio": 100},
expectedSysfsBDI: map[string]int64{"read_ahead_kb": 4096},
},
{
name: "invalid read ahead - not int",
Expand All @@ -90,21 +90,6 @@ func TestPrepareMountArgs(t *testing.T) {
inputMountOptions: append(defaultCsiMountOptions, "read_ahead_kb=-1"),
expectErr: true,
},
{
name: "invalid max ratio - not int",
inputMountOptions: append(defaultCsiMountOptions, "max_ratio=abc"),
expectErr: true,
},
{
name: "invalid max ratio - negative",
inputMountOptions: append(defaultCsiMountOptions, "max_ratio=-1"),
expectErr: true,
},
{
name: "invalid max ratio - greater than 100",
inputMountOptions: append(defaultCsiMountOptions, "max_ratio=101"),
expectErr: true,
},
}

for _, tc := range testCases {
Expand Down
1 change: 0 additions & 1 deletion test/e2e/specs/testdriver.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,6 @@ func (n *GCSFuseCSITestDriver) CreateVolume(ctx context.Context, config *storage
v.metadataPrefetch = true
case EnableCustomReadAhead:
mountOptions += ",read_ahead_kb=" + ReadAheadCustomReadAheadKb
mountOptions += ",max_ratio=" + ReadAheadCustomMaxRatio
case EnableMetadataPrefetchAndInvalidMountOptionsVolumePrefix:
mountOptions += ",file-system:kernel-list-cache-ttl-secs:-1,invalid-option"
v.metadataPrefetch = true
Expand Down
3 changes: 0 additions & 3 deletions test/e2e/testsuites/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,10 @@ func (t *gcsFuseCSIMountTestSuite) DefineTests(driver storageframework.TestDrive
ginkgo.By("Checking that the first pod command exits with no error")
bdi := tPod1.VerifyExecInPodSucceedWithOutput(f, specs.TesterContainerName, fmt.Sprintf(`mountpoint -d "%s"`, mountPath))
readAheadPath := fmt.Sprintf("/sys/class/bdi/%s/read_ahead_kb", bdi)
maxRatioPath := fmt.Sprintf("/sys/class/bdi/%s/max_ratio", bdi)

currentReadAhead := tPod1.VerifyExecInPodSucceedWithOutput(f, specs.TesterContainerName, "cat "+readAheadPath)
currentMaxRatio := tPod1.VerifyExecInPodSucceedWithOutput(f, specs.TesterContainerName, "cat "+maxRatioPath)

gomega.Expect(currentReadAhead).To(gomega.Equal(specs.ReadAheadCustomReadAheadKb))
gomega.Expect(currentMaxRatio).To(gomega.Equal(specs.ReadAheadCustomMaxRatio))

ginkgo.By("Deleting the first pod")
tPod1.Cleanup(ctx)
Expand Down

0 comments on commit 809b168

Please sign in to comment.