Change plugin interfaces to use progress monitoring

This commit is contained in:
Hemant Kumar 2025-02-24 12:48:38 -05:00
parent 32752fe395
commit b51cbb1d17
11 changed files with 25 additions and 47 deletions

View File

@ -95,7 +95,8 @@ func (f *flexVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs)
if !f.readOnly {
if f.plugin.capabilities.FSGroup {
// fullPluginName helps to distinguish different driver from flex volume plugin
volume.SetVolumeOwnership(f, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(f.plugin, f.spec))
ownershipChanger := volume.NewVolumeOwnership(f, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(f.plugin, f.spec))
ownershipChanger.ChangePermissions()
}
}

View File

@ -229,8 +229,8 @@ func (b *gitRepoVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArg
return fmt.Errorf("failed to exec 'git reset --hard': %s: %v", output, err)
}
volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
ownershipChanger := volume.NewVolumeOwnership(b, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
ownershipChanger.ChangePermissions()
volumeutil.SetReady(b.getMetaDir())
return nil
}

View File

@ -19,7 +19,6 @@ package iscsi
import (
"os"
v1 "k8s.io/api/core/v1"
"k8s.io/klog/v2"
"k8s.io/mount-utils"
@ -42,7 +41,9 @@ type diskManager interface {
// utility to mount a disk based filesystem
// globalPDPath: global mount path like, /var/lib/kubelet/plugins/kubernetes.io/iscsi/{ifaceName}/{portal-some_iqn-lun-lun_id}
// volPath: pod volume dir path like, /var/lib/kubelet/pods/{podUID}/volumes/kubernetes.io~iscsi/{volumeName}
func diskSetUp(manager diskManager, b iscsiDiskMounter, volPath string, mounter mount.Interface, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy) error {
func diskSetUp(manager diskManager, b iscsiDiskMounter, volPath string, mounter mount.Interface, mounterArgs volume.MounterArgs) error {
fsGroup := mounterArgs.FsGroup
fsGroupChangePolicy := mounterArgs.FSGroupChangePolicy
notMnt, err := mounter.IsLikelyNotMountPoint(volPath)
if err != nil && !os.IsNotExist(err) {
klog.Errorf("cannot validate mountpoint: %s", volPath)
@ -96,7 +97,9 @@ func diskSetUp(manager diskManager, b iscsiDiskMounter, volPath string, mounter
}
if !b.readOnly {
volume.SetVolumeOwnership(&b, volPath, fsGroup, fsGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
// This code requires larger refactor to monitor progress of ownership change
ownershipChanger := volume.NewVolumeOwnership(&b, volPath, fsGroup, fsGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
ownershipChanger.ChangePermissions()
}
return nil

View File

@ -377,7 +377,7 @@ func (b *iscsiDiskMounter) SetUp(mounterArgs volume.MounterArgs) error {
func (b *iscsiDiskMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error {
// diskSetUp checks mountpoints and prevent repeated calls
err := diskSetUp(b.manager, *b, dir, b.mounter, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy)
err := diskSetUp(b.manager, *b, dir, b.mounter, mounterArgs)
if err != nil {
klog.Errorf("iscsi: failed to setup")
}

View File

@ -610,7 +610,9 @@ func (m *localVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs)
if !m.readOnly {
// Volume owner will be written only once on the first volume mount
if len(refs) == 0 {
return volume.SetVolumeOwnership(m, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(m.plugin, nil))
ownershipChanger := volume.NewVolumeOwnership(m, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(m.plugin, nil))
ownershipChanger.AddProgressNotifier(m.pod, mounterArgs.Recorder)
return ownershipChanger.ChangePermissions()
}
}
return nil

View File

@ -331,7 +331,9 @@ func (b *portworxVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterAr
return err
}
if !b.readOnly {
volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
// Since portworxVolume is in process of being removed from in-tree, we avoid larger refactor to add progress tracking for ownership operation
ownershipChanger := volume.NewVolumeOwnership(b, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
ownershipChanger.ChangePermissions()
}
klog.Infof("Portworx Volume %s setup at %s", b.volumeID, dir)
return nil

View File

@ -229,7 +229,8 @@ func (s *projectedVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterA
setPerms := func(_ string) error {
// This may be the first time writing and new files get created outside the timestamp subdirectory:
// change the permissions on the whole volume and not only in the timestamp directory.
return volume.SetVolumeOwnership(s, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(s.plugin, nil))
ownershipChanger := volume.NewVolumeOwnership(s, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(s.plugin, nil))
return ownershipChanger.ChangePermissions()
}
err = writer.Write(data, setPerms)
if err != nil {

View File

@ -242,7 +242,8 @@ func (b *secretVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs
setPerms := func(_ string) error {
// This may be the first time writing and new files get created outside the timestamp subdirectory:
// change the permissions on the whole volume and not only in the timestamp directory.
return volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
ownershipChanger := volume.NewVolumeOwnership(b, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
return ownershipChanger.ChangePermissions()
}
err = writer.Write(payload, setPerms)
if err != nil {

View File

@ -25,7 +25,6 @@ import (
"google.golang.org/grpc/status"
"k8s.io/component-base/metrics"
"k8s.io/component-base/metrics/legacyregistry"
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/volume"
"k8s.io/kubernetes/pkg/volume/util/types"
)
@ -103,7 +102,6 @@ func OperationCompleteHook(plugin, operationName string) func(types.CompleteFunc
if c.Migrated != nil {
migrated = *c.Migrated
}
klog.Infof("foobar Operation %s took %f", operationName, timeTaken)
StorageOperationMetric.WithLabelValues(plugin, operationName, status, strconv.FormatBool(migrated)).Observe(timeTaken)
}
return opComplete

View File

@ -141,38 +141,6 @@ func (vo *VolumeOwnership) logWarning() {
vo.recorder.Event(vo.pod, v1.EventTypeWarning, events.VolumePermissionChangeInProgress, msg)
}
// SetVolumeOwnership modifies the given volume to be owned by
// fsGroup, and sets SetGid so that newly created files are owned by
// fsGroup. If fsGroup is nil nothing is done.
func SetVolumeOwnership(mounter Mounter, dir string, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy, completeFunc func(types.CompleteFuncParam)) error {
if fsGroup == nil {
return nil
}
timer := time.AfterFunc(30*time.Second, func() {
klog.Warningf("Setting volume ownership for %s and fsGroup set. If the volume has a lot of files then setting volume ownership could be slow, see https://github.com/kubernetes/kubernetes/issues/69699", dir)
})
defer timer.Stop()
if skipPermissionChange(mounter, dir, fsGroup, fsGroupChangePolicy) {
klog.V(3).InfoS("Skipping permission and ownership change for volume", "path", dir)
return nil
}
err := walkDeep(dir, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
return changeFilePermission(path, fsGroup, mounter.GetAttributes().ReadOnly, info)
})
if completeFunc != nil {
completeFunc(types.CompleteFuncParam{
Err: &err,
})
}
return err
}
func changeFilePermission(filename string, fsGroup *int64, readonly bool, info os.FileInfo) error {
err := os.Lchown(filename, -1, int(*fsGroup))
if err != nil {

View File

@ -303,7 +303,8 @@ func TestSetVolumeOwnershipMode(t *testing.T) {
}
mounter := &localFakeMounter{path: "FAKE_DIR_DOESNT_EXIST"} // SetVolumeOwnership() must rely on tmpDir
err = SetVolumeOwnership(mounter, tmpDir, &expectedGid, test.fsGroupChangePolicy, nil)
ownershipChanger := NewVolumeOwnership(mounter, tmpDir, &expectedGid, test.fsGroupChangePolicy, nil)
err = ownershipChanger.ChangePermissions()
if err != nil {
t.Errorf("for %s error changing ownership with: %v", test.description, err)
}
@ -439,7 +440,8 @@ func TestSetVolumeOwnershipOwner(t *testing.T) {
mounter := &localFakeMounter{path: tmpDir}
always := v1.FSGroupChangeAlways
err = SetVolumeOwnership(mounter, tmpDir, test.fsGroup, &always, nil)
ownershipChanger := NewVolumeOwnership(mounter, tmpDir, test.fsGroup, &always, nil)
err = ownershipChanger.ChangePermissions()
if err != nil {
t.Errorf("for %s error changing ownership with: %v", test.description, err)
}