Skip to content

Commit

Permalink
cleanup: simplify rbdGetDeviceList()
Browse files Browse the repository at this point in the history
The `rbdGetDeviceList()` function uses two very similar types for
converting krbd and NBD device information from JSON. There is no need
to use this distinction, and callers of `rbdGetDeviceList()` should not
need to care about it either.

By introducing a `deviceInfo` interface with Get-functions, the
`rbdGetDeviceList()` function becomes a little simpler, with a clearly
defined API for the returned list.

Signed-off-by: Niels de Vos <[email protected]>
  • Loading branch information
nixpanic authored and mergify[bot] committed Jan 11, 2024
1 parent 2dab8f2 commit 3bf5c0e
Showing 1 changed file with 43 additions and 43 deletions.
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 {
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

0 comments on commit 3bf5c0e

Please sign in to comment.