Skip to content

Commit

Permalink
Add identity matcher and testcases
Browse files Browse the repository at this point in the history
  • Loading branch information
@Praveenrajmani
Praveenrajmani committed Mar 8, 2022
1 parent 45ea70e commit 5e5c54c
Show file tree
Hide file tree
Showing 7 changed files with 496 additions and 121 deletions.
80 changes: 0 additions & 80 deletions pkg/sys/sys_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,83 +99,3 @@ func TestGetBlockFile(t1 *testing.T) {
}

}

func TestGetRootBlockFile(t1 *testing.T) {

testCases := []struct {
name string
devName string
rootFile string
}{
{
name: "test1",
devName: "/dev/xvdb",
rootFile: "/dev/xvdb",
},
{
name: "test2",
devName: "/dev/xvdb1",
rootFile: "/dev/xvdb1",
},
{
name: "test3",
devName: "/var/lib/direct-csi/devices/xvdb",
rootFile: "/dev/xvdb",
},
{
name: "test4",
devName: "/var/lib/direct-csi/devices/xvdb-part-3",
rootFile: "/dev/xvdb3",
},
{
name: "test5",
devName: "/var/lib/direct-csi/devices/xvdb-part-15",
rootFile: "/dev/xvdb15",
},
{
name: "test6",
devName: "/var/lib/direct-csi/devices/nvmen1p-part-4",
rootFile: "/dev/nvmen1p4",
},
{
name: "test7",
devName: "/var/lib/direct-csi/devices/nvmen12p-part-11",
rootFile: "/dev/nvmen12p11",
},
{
name: "test8",
devName: "/var/lib/direct-csi/devices/loop0",
rootFile: "/dev/loop0",
},
{
name: "test9",
devName: "/var/lib/direct-csi/devices/loop-part-5",
rootFile: "/dev/loop5",
},
{
name: "test10",
devName: "/var/lib/direct-csi/devices/loop-part-12",
rootFile: "/dev/loop12",
},
{
name: "test11",
devName: "loop12",
rootFile: "/dev/loop12",
},
{
name: "test12",
devName: "loop0",
rootFile: "/dev/loop0",
},
}

for _, tt := range testCases {
t1.Run(tt.name, func(t1 *testing.T) {
rootFile := getRootBlockFile(tt.devName)
if rootFile != tt.rootFile {
t1.Errorf("Test case name %s: Expected root file = (%s) got: %s", tt.name, tt.rootFile, rootFile)
}
})
}

}
15 changes: 0 additions & 15 deletions pkg/sys/sys_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,6 @@ func makeBlockDeviceName(devName string) string {
return strings.Join([]string{dName, partNumStr}, DirectCSIPartitionInfix)
}

func getRootBlockFile(devName string) string {
if strings.Contains(devName, DirectCSIDevRoot) {
return getRootBlockFile(filepath.Base(devName))
}
if strings.HasPrefix(devName, HostDevRoot) {
return devName
}
return filepath.Join(HostDevRoot, makeRootDeviceName(devName))
}

func makeRootDeviceName(devName string) string {
cleanPrefix := strings.Replace(devName, DirectCSIPartitionInfix, "", 1)
return strings.ReplaceAll(cleanPrefix, DirectCSIPartitionInfix, HostPartitionInfix)
}

func splitDevAndPartNum(s string) (string, int) {
possibleNum := strings.Builder{}
toRet := strings.Builder{}
Expand Down
30 changes: 23 additions & 7 deletions pkg/uevent/listener.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,19 +202,36 @@ func (l *listener) handle(ctx context.Context, dEvent *deviceEvent) error {
if err != nil {
return err
}
drive, matchResult := runMatchers(drives, device, stageOneMatchers, stageTwoMatchers)

switch dEvent.action {
case Add, Change, Sync:
return l.processUpdate(ctx, drives, device)
case Add:
return l.processAdd(ctx, matchResult, device, drive)
case Change, Sync:
return l.processUpdate(ctx, matchResult, device, drive)
case Remove:
return l.processRemove(ctx, drives, device)
return l.processRemove(ctx, matchResult, device, drive)
default:
return fmt.Errorf("invalid device action: %s", dEvent.action)
}
}

func (l *listener) processUpdate(ctx context.Context, drives []*directcsi.DirectCSIDrive, device *sys.Device) error {
drive, matchResult := runMatcher(drives, device)
func (l *listener) processAdd(ctx context.Context, matchResult matchResult, device *sys.Device, drive *directcsi.DirectCSIDrive) error {
switch matchResult {
case noMatch:
return l.handler.Add(ctx, device)
case changed, noChange:
klog.V(3).Infof("ignoring ADD action for the device %s as the corresponding drive match %s is found", device.DevPath(), drive.Name)
return nil
case tooManyMatches:
klog.V(3).Infof("ignoring ADD action as too many matches are found for the device %s", device.DevPath())
return nil
default:
return fmt.Errorf("invalid match result: %v", matchResult)
}
}

func (l *listener) processUpdate(ctx context.Context, matchResult matchResult, device *sys.Device, drive *directcsi.DirectCSIDrive) error {
switch matchResult {
case noMatch:
return l.handler.Add(ctx, device)
Expand All @@ -229,8 +246,7 @@ func (l *listener) processUpdate(ctx context.Context, drives []*directcsi.Direct
}
}

func (l *listener) processRemove(ctx context.Context, drives []*directcsi.DirectCSIDrive, device *sys.Device) error {
drive, matchResult := runMatcher(drives, device)
func (l *listener) processRemove(ctx context.Context, matchResult matchResult, device *sys.Device, drive *directcsi.DirectCSIDrive) error {
switch matchResult {
case noMatch:
klog.V(3).InfoS(
Expand Down
74 changes: 55 additions & 19 deletions pkg/uevent/match.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const (
)

// prioritiy based matchers
var matchers = []matchFn{
var stageOneMatchers = []matchFn{
// HW matchers
partitionNumberMatcher,
ueventSerialNumberMatcher,
Expand All @@ -50,7 +50,7 @@ var matchers = []matchFn{
mdUUIDMatcher,
filesystemMatcher,
ueventFSUUIDMatcher,
// legacy matchers
// v1beta2 matchers
fsUUIDMatcher,
serialNumberMatcher,
// size matchers
Expand All @@ -60,6 +60,12 @@ var matchers = []matchFn{
allocatedCapacityMatcher,
}

// stageTwoMatchers are the conslusive matchers for more than one matching drives
var stageTwoMatchers = []matchFn{
majMinMatcher,
pathMatcher,
}

// ------------------------
// - Run the list of matchers on the list of drives
// - Match function SHOULD return matched (100% match) and considered (if the field is empty) results
Expand All @@ -69,14 +75,22 @@ var matchers = []matchFn{
// Else, pass the considered list to the next matching function in the priority list
// (NOTE: the returning results can be more than one incase of duplicate/identical drives)
// ------------------------
func runMatcher(drives []*directcsi.DirectCSIDrive, device *sys.Device) (*directcsi.DirectCSIDrive, matchResult) {
func runMatchers(drives []*directcsi.DirectCSIDrive,
device *sys.Device,
stageOneMatchers, stageTwoMatchers []matchFn) (*directcsi.DirectCSIDrive, matchResult) {
var matchedDrives, consideredDrives []*directcsi.DirectCSIDrive
var matched, updated bool
for _, matchFn := range matchers {
var err error

for _, matchFn := range stageOneMatchers {
if len(drives) == 0 {
break
}
matchedDrives, consideredDrives = match(drives, device, matchFn)
matchedDrives, consideredDrives, err = match(drives, device, matchFn)
if err != nil {
klog.V(3).Infof("error while matching drive %s: %v", device.DevPath(), err)
continue
}
switch {
case len(matchedDrives) > 0:
if len(matchedDrives) == 1 {
Expand All @@ -91,6 +105,17 @@ func runMatcher(drives []*directcsi.DirectCSIDrive, device *sys.Device) (*direct
}
}

if len(drives) > 1 {
for _, matchFn := range stageTwoMatchers {
// run identity matcher for more than one matched drives
drives, _, err = match(drives, device, matchFn)
if err != nil {
klog.V(3).Infof("error while matching drive %s: %v", device.DevPath(), err)
continue
}
}
}

switch len(drives) {
case 0:
return nil, noMatch
Expand All @@ -100,40 +125,51 @@ func runMatcher(drives []*directcsi.DirectCSIDrive, device *sys.Device) (*direct
}
return drives[0], noChange
default:
// handle too many matches
//
// case 1: It is possible to have an empty/partial drive (&directCSIDrive.Status{Path: /dev/sdb0, "", "", "", ...})
// to match with a correct match

// case 2: A terminating drive and an actual drive can be matched with the single device
//
// case 3: A duplicate drive (due to any bug)
//
// ToDo: make these drives invalid / decide based on drive status / calculate ranks and decide
return nil, tooManyMatches
}
}

func majMinMatcher(drive *directcsi.DirectCSIDrive, device *sys.Device) (match bool, consider bool, err error) {
if drive.Status.MajorNumber != uint32(device.Major) {
return false, false, nil
}
if drive.Status.MinorNumber != uint32(device.Minor) {
return false, false, nil
}
return true, true, nil
}

func pathMatcher(drive *directcsi.DirectCSIDrive, device *sys.Device) (match bool, consider bool, err error) {
if getRootBlockPath(drive.Status.Path) != device.DevPath() {
return false, false, nil
}
return true, true, nil
}

// ------------------------
// - Run the list of drives over the provided match function
// - If matched, append the drive to the matched list
// - If considered, append the the drive to the considered list
// ------------------------
func match(drives []*directcsi.DirectCSIDrive, device *sys.Device, matchFn matchFn) ([]*directcsi.DirectCSIDrive, []*directcsi.DirectCSIDrive) {
func match(drives []*directcsi.DirectCSIDrive,
device *sys.Device,
matchFn matchFn) ([]*directcsi.DirectCSIDrive, []*directcsi.DirectCSIDrive, error) {
var matchedDrives, consideredDrives []*directcsi.DirectCSIDrive
for _, drive := range drives {
if drive.Status.DriveStatus == directcsi.DriveStatusTerminating {
continue
}
match, consider, err := matchFn(drive, device)
if err != nil {
klog.V(3).Infof("error while matching drive %s: %v", device.DevPath(), err)
continue
return nil, nil, err
}
if match {
matchedDrives = append(matchedDrives, drive)
} else if consider {
consideredDrives = append(consideredDrives, drive)
}
}
return matchedDrives, consideredDrives
return matchedDrives, consideredDrives, nil
}

func fsUUIDMatcher(drive *directcsi.DirectCSIDrive, device *sys.Device) (match bool, consider bool, err error) {
Expand Down
Loading

0 comments on commit 5e5c54c

Please sign in to comment.