Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: sort broken mountpoint and put subpaths at the end. #4423

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions pkg/csi/recover/recover.go
Original file line number Diff line number Diff line change
@@ -19,15 +19,11 @@ package recover
import (
"context"
"os"
"sort"
"strconv"
"strings"
"time"

"github.com/fluid-cloudnative/fluid/pkg/common"
"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/dataset/volume"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubelet"
"github.com/fluid-cloudnative/fluid/pkg/utils/mountinfo"
"github.com/golang/glog"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
@@ -38,6 +34,12 @@ import (
"k8s.io/utils/mount"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager"

"github.com/fluid-cloudnative/fluid/pkg/common"
"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/dataset/volume"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubelet"
"github.com/fluid-cloudnative/fluid/pkg/utils/mountinfo"
)

const (
@@ -161,6 +163,8 @@ func (r *FuseRecover) recover() {
return
}

sort.Sort(brokenMounts)

for _, point := range brokenMounts {
r.doRecover(point)
}
45 changes: 38 additions & 7 deletions pkg/utils/mountinfo/mountpoint.go
Original file line number Diff line number Diff line change
@@ -21,8 +21,13 @@
"path"
"strings"

"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/golang/glog"

"github.com/fluid-cloudnative/fluid/pkg/utils"
)

const (
subPahVolumeKeyWord = "volume-subpaths"
)

type MountPoint struct {
@@ -34,7 +39,7 @@
NamespacedDatasetName string // <namespace>-<dataset>
}

func GetBrokenMountPoints() ([]MountPoint, error) {
func GetBrokenMountPoints() (MountPoints, error) {

Check warning on line 42 in pkg/utils/mountinfo/mountpoint.go

Codecov / codecov/patch

pkg/utils/mountinfo/mountpoint.go#L42

Added line #L42 was not covered by tests
// get mountinfo from proc
mountByPath, err := loadMountInfo()
if err != nil {
@@ -64,14 +69,15 @@

for k, v := range mountByPath {
if strings.Contains(k, mountRoot) {
vCopy := v
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think vCopy := v does not create a deep copy of the Mount object; instead, it creates a new pointer to the same Mount object. If the original Mount object is modified, vCopy will reflect these changes. If this behavior is not intended, implementing a method for deep copying might be necessary.

fields := strings.Split(k, "/")
if len(fields) < 6 {
continue
}
// fluid global mount path is: /{rootPath}/{runtimeType}/{namespace}/{datasetName}/{runtimeTypeFuse}
namespace, datasetName := fields[3], fields[4]
namespacedName := fmt.Sprintf("%s-%s", namespace, datasetName)
globalMountByName[namespacedName] = v
globalMountByName[namespacedName] = vCopy
}
}
return
@@ -81,23 +87,26 @@
bindMountByName = make(map[string][]*Mount)
for k, m := range mountByPath {
var datasetNamespacedName string
// Shallow copy mCopy intends to address loop scoping issue in golang version <= 1.21
// ref: https://go.dev/blog/loopvar-preview
mCopy := m
if strings.Contains(k, "kubernetes.io~csi") && strings.Contains(k, "mount") {
// fluid bind mount target path is: /{kubeletRootDir}(default: /var/lib/kubelet)/pods/{podUID}/volumes/kubernetes.io~csi/{namespace}-{datasetName}/mount
fields := strings.Split(k, "/")
if len(fields) < 3 {
continue
}
datasetNamespacedName = fields[len(fields)-2]
bindMountByName[datasetNamespacedName] = append(bindMountByName[datasetNamespacedName], m)
bindMountByName[datasetNamespacedName] = append(bindMountByName[datasetNamespacedName], mCopy)
}
if strings.Contains(k, "volume-subpaths") {
if strings.Contains(k, subPahVolumeKeyWord) {
// pod using subPath, bind mount path is: /{kubeletRootDir}(default: /var/lib/kubelet)/pods/{podUID}/volume-subpaths/{namespace}-{datasetName}/{containerName}/{volumeIndex}
fields := strings.Split(k, "/")
if len(fields) < 4 {
continue
}
datasetNamespacedName = fields[len(fields)-3]
bindMountByName[datasetNamespacedName] = append(bindMountByName[datasetNamespacedName], m)
bindMountByName[datasetNamespacedName] = append(bindMountByName[datasetNamespacedName], mCopy)
}
}
return
@@ -111,7 +120,13 @@
glog.V(6).Infof("ignoring mountpoint %s because of not finding its global mount point", name)
continue
}
for _, bindMount := range bindMounts {
for _, bm := range bindMounts {
bindMount := bm
if strings.HasSuffix(bindMount.Subtree, "//deleted") {
glog.V(5).Infof("bindMount subtree has been deleted, trim /deleted suffix, bindmount: %v", bindMount)
bindMount.Subtree = strings.TrimSuffix(bindMount.Subtree, "//deleted")

Check warning on line 127 in pkg/utils/mountinfo/mountpoint.go

Codecov / codecov/patch

pkg/utils/mountinfo/mountpoint.go#L126-L127

Added lines #L126 - L127 were not covered by tests
}

// In case of not sharing same peer group in mount info, meaning it a broken mount point
if len(utils.IntersectIntegerSets(bindMount.PeerGroups, globalMount.PeerGroups)) == 0 {
brokenMounts = append(brokenMounts, MountPoint{
@@ -127,3 +142,19 @@
}
return
}

type MountPoints []MountPoint

func (mp MountPoints) Len() int { return len(mp) }
func (mp MountPoints) Less(i, j int) bool {
if strings.Contains(mp[i].MountPath, subPahVolumeKeyWord) && !strings.Contains(mp[j].MountPath, subPahVolumeKeyWord) {
return false

Check warning on line 151 in pkg/utils/mountinfo/mountpoint.go

Codecov / codecov/patch

pkg/utils/mountinfo/mountpoint.go#L148-L151

Added lines #L148 - L151 were not covered by tests
}
if !strings.Contains(mp[i].MountPath, subPahVolumeKeyWord) && strings.Contains(mp[j].MountPath, subPahVolumeKeyWord) {
return true

Check warning on line 154 in pkg/utils/mountinfo/mountpoint.go

Codecov / codecov/patch

pkg/utils/mountinfo/mountpoint.go#L153-L154

Added lines #L153 - L154 were not covered by tests
}
return mp[i].MountPath < mp[j].MountPath

Check warning on line 156 in pkg/utils/mountinfo/mountpoint.go

Codecov / codecov/patch

pkg/utils/mountinfo/mountpoint.go#L156

Added line #L156 was not covered by tests
}
func (mp MountPoints) Swap(i, j int) {
mp[i], mp[j] = mp[j], mp[i]

Check warning on line 159 in pkg/utils/mountinfo/mountpoint.go

Codecov / codecov/patch

pkg/utils/mountinfo/mountpoint.go#L158-L159

Added lines #L158 - L159 were not covered by tests
}
Loading