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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -161,6 +163,8 @@ func (r *FuseRecover) recover() {
return
}

sort.Sort(brokenMounts)

for _, point := range brokenMounts {
r.doRecover(point)
}
Expand Down
45 changes: 38 additions & 7 deletions pkg/utils/mountinfo/mountpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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

View check run for this annotation

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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

View check run for this annotation

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{
Expand All @@ -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

View check run for this annotation

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

View check run for this annotation

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

View check run for this annotation

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

View check run for this annotation

Codecov / codecov/patch

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

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