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

cleanup: simplify rbdGetDeviceList() #4364

Merged
merged 1 commit into from
Jan 11, 2024
Merged
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
86 changes: 43 additions & 43 deletions internal/rbd/rbd_attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"encoding/json"
"fmt"
"os"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -91,25 +90,43 @@ var (
}
)

type deviceInfo interface {
GetName() string
GetPool() string
GetRadosNamespace() string
GetDevice() string
}

// rbdDeviceInfo strongly typed JSON spec for rbd device list output (of type krbd).
type rbdDeviceInfo struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

We no longer have to save ID ?
It's not used anywhere ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, it is not used anywhere for attaching/mounting.

ID string `json:"id"`
// Name will only be set for krbd devices (NBD uses Image)
Name string `json:"name"`
// Image will only be set for NBD devices (krbd uses Name)
Image string `json:"image"`

Pool string `json:"pool"`
RadosNamespace string `json:"namespace"`
Name string `json:"name"`
Device string `json:"device"`
}

// nbdDeviceInfo strongly typed JSON spec for rbd-nbd device list output (of type nbd)
// NOTE: There is a bug in rbd output that returns id as number for nbd, and string for krbd, thus
// requiring 2 different JSON structures to unmarshal the output.
// NOTE: image key is "name" in krbd output and "image" in nbd output, which is another difference.
type nbdDeviceInfo struct {
ID int64 `json:"id"`
Pool string `json:"pool"`
RadosNamespace string `json:"namespace"`
Name string `json:"image"`
Device string `json:"device"`
func (rdi *rbdDeviceInfo) GetName() string {
if rdi.Name != "" {
return rdi.Name
}

return rdi.Image
}

func (rdi *rbdDeviceInfo) GetPool() string {
return rdi.Pool
}

func (rdi *rbdDeviceInfo) GetRadosNamespace() string {
return rdi.RadosNamespace
}

func (rdi *rbdDeviceInfo) GetDevice() string {
return rdi.Device
}

type detachRBDImageArgs struct {
Expand All @@ -123,48 +140,31 @@ type detachRBDImageArgs struct {
logStrategy string
}

// rbdGetDeviceList queries rbd about mapped devices and returns a list of rbdDeviceInfo
// getDeviceList queries rbd about mapped devices and returns a list of deviceInfo
// It will selectively list devices mapped using krbd or nbd as specified by accessType.
func rbdGetDeviceList(ctx context.Context, accessType string) ([]rbdDeviceInfo, error) {
func getDeviceList(ctx context.Context, accessType string) ([]deviceInfo, error) {
// rbd device list --format json --device-type [krbd|nbd]
var (
rbdDeviceList []rbdDeviceInfo
nbdDeviceList []nbdDeviceInfo
)

stdout, _, err := util.ExecCommand(ctx, rbd, "device", "list", "--format="+"json", "--device-type", accessType)
if err != nil {
return nil, fmt.Errorf("error getting device list from rbd for devices of type (%s): %w", accessType, err)
}

if accessType == accessTypeKRbd {
err = json.Unmarshal([]byte(stdout), &rbdDeviceList)
} else {
err = json.Unmarshal([]byte(stdout), &nbdDeviceList)
}
var devices []*rbdDeviceInfo
err = json.Unmarshal([]byte(stdout), &devices)
if err != nil {
return nil, fmt.Errorf(
"error to parse JSON output of device list for devices of type (%s): %w",
accessType,
err)
}

// convert output to a rbdDeviceInfo list for consumers
if accessType == accessTypeNbd {
for _, device := range nbdDeviceList {
rbdDeviceList = append(
rbdDeviceList,
rbdDeviceInfo{
ID: strconv.FormatInt(device.ID, 10),
Pool: device.Pool,
RadosNamespace: device.RadosNamespace,
Name: device.Name,
Device: device.Device,
})
}
// convert []rbdDeviceInfo to []deviceInfo
deviceList := make([]deviceInfo, len(devices))
for i := range devices {
deviceList[i] = devices[i]
}

return rbdDeviceList, nil
return deviceList, nil
}

// findDeviceMappingImage finds a devicePath, if available, based on image spec (pool/{namespace/}image) on the node.
Expand All @@ -179,16 +179,16 @@ func findDeviceMappingImage(ctx context.Context, pool, namespace, image string,
imageSpec = fmt.Sprintf("%s/%s/%s", pool, namespace, image)
}

rbdDeviceList, err := rbdGetDeviceList(ctx, accessType)
deviceList, err := getDeviceList(ctx, accessType)
if err != nil {
log.WarningLog(ctx, "failed to determine if image (%s) is mapped to a device (%v)", imageSpec, err)

return "", false
}

for _, device := range rbdDeviceList {
if device.Name == image && device.Pool == pool && device.RadosNamespace == namespace {
return device.Device, true
for _, device := range deviceList {
if device.GetName() == image && device.GetPool() == pool && device.GetRadosNamespace() == namespace {
return device.GetDevice(), true
}
}

Expand Down