Skip to content

Commit

Permalink
(feat): Refactor lsblk -J to lsblk -P
Browse files Browse the repository at this point in the history
  • Loading branch information
Lasith Koswatta Gamage committed Jun 4, 2024
1 parent 2370a1e commit cf4d0ad
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 175 deletions.
7 changes: 7 additions & 0 deletions internal/config/validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import (
"github.com/reecetech/ebs-bootstrap/internal/service"
)

const (
LvmWikiDocumentationUrl = "https://github.com/reecetech/ebs-bootstrap/wiki/LVM"
)

type Validator interface {
Validate(c *Config) error
}
Expand Down Expand Up @@ -48,6 +52,9 @@ func (fsv *FileSystemValidator) Validate(c *Config) error {
if fs == model.Unformatted {
return fmt.Errorf("🔴 %s: Must provide a supported file system", name)
}
if fs == model.Lvm {
return fmt.Errorf("🔴 %s: Refer to %s on how to manage LVM file systems", name, LvmWikiDocumentationUrl)
}
}
return nil
}
Expand Down
11 changes: 11 additions & 0 deletions internal/config/validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,17 @@ func TestFileSystemValidator(t *testing.T) {
},
ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Must provide a supported file system"),
},
{
Name: "LVM File System",
Config: &Config{
Devices: map[string]Device{
"/dev/xvdf": {
Fs: model.Lvm,
},
},
},
ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Refer to %s on how to manage LVM file systems", LvmWikiDocumentationUrl),
},
}
for _, subtest := range subtests {
fsv := NewFileSystemValidator()
Expand Down
56 changes: 25 additions & 31 deletions internal/service/device.go
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package service

import (
"encoding/json"
"fmt"
"regexp"
"strconv"
"strings"

"github.com/reecetech/ebs-bootstrap/internal/model"
"github.com/reecetech/ebs-bootstrap/internal/utils"
)

var deviceNameRegex = regexp.MustCompile(`^NAME="(.*)"`)
var devicePropertiesRegex = regexp.MustCompile(`^LABEL="(.*)" FSTYPE="(.*)" MOUNTPOINT="(.*)"`)

type DeviceService interface {
GetSize(name string) (uint64, error) // bytes
GetBlockDevices() ([]string, error)
Expand All @@ -21,15 +25,6 @@ type LinuxDeviceService struct {
runnerFactory utils.RunnerFactory
}

type LsblkBlockDeviceResponse struct {
BlockDevices []struct {
Name *string `json:"name"`
Label *string `json:"label"`
FsType *string `json:"fstype"`
MountPoint *string `json:"mountpoint"`
} `json:"blockdevices"`
}

func NewLinuxDeviceService(rc utils.RunnerFactory) *LinuxDeviceService {
return &LinuxDeviceService{
runnerFactory: rc,
Expand All @@ -51,46 +46,45 @@ func (du *LinuxDeviceService) GetSize(name string) (uint64, error) {

func (du *LinuxDeviceService) GetBlockDevices() ([]string, error) {
r := du.runnerFactory.Select(utils.Lsblk)
output, err := r.Command("--nodeps", "-o", "NAME", "-J")
output, err := r.Command("--nodeps", "-o", "NAME", "-P")
if err != nil {
return nil, err
}
lbd := &LsblkBlockDeviceResponse{}
err = json.Unmarshal([]byte(output), lbd)
if err != nil {
return nil, fmt.Errorf("🔴 Failed to decode lsblk response: %v", err)
}
d := make([]string, len(lbd.BlockDevices))
for i := range d {
d[i] = "/dev/" + utils.Safe(lbd.BlockDevices[i].Name)

lines := strings.Split(strings.TrimSuffix(output, "\n"), "\n")
d := make([]string, len(lines))

for i, line := range lines {
matches := deviceNameRegex.FindStringSubmatch(line)
if len(matches) != 2 {
return nil, fmt.Errorf("🔴 Failed to decode lsblk response")
}
d[i] = "/dev/" + matches[1]
}
return d, nil
}

func (du *LinuxDeviceService) GetBlockDevice(name string) (*model.BlockDevice, error) {
r := du.runnerFactory.Select(utils.Lsblk)
output, err := r.Command("--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", name)
output, err := r.Command("--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", name)
if err != nil {
return nil, err
}
lbd := &LsblkBlockDeviceResponse{}
err = json.Unmarshal([]byte(output), lbd)
if err != nil {
return nil, fmt.Errorf("🔴 Failed to decode lsblk response: %v", err)
}
if len(lbd.BlockDevices) != 1 {
return nil, fmt.Errorf("🔴 %s: An unexpected number of block devices were returned: Expected=1 Actual=%d", name, len(lbd.BlockDevices))

matches := devicePropertiesRegex.FindStringSubmatch(output)
if len(matches) != 4 {
return nil, fmt.Errorf("🔴 Failed to decode lsblk response")
}
fst := utils.Safe(lbd.BlockDevices[0].FsType)
fs, err := model.ParseFileSystem(fst)

fs, err := model.ParseFileSystem(matches[2])
if err != nil {
return nil, fmt.Errorf("🔴 %s: %s", name, err)
}
return &model.BlockDevice{
Name: name,
Label: utils.Safe(lbd.BlockDevices[0].Label),
Label: matches[1],
FileSystem: fs,
MountPoint: utils.Safe(lbd.BlockDevices[0].MountPoint),
MountPoint: matches[3],
}, nil
}

Expand Down
102 changes: 26 additions & 76 deletions internal/service/device_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,34 +72,32 @@ func TestGetBlockDevices(t *testing.T) {
ExpectedError error
}{
{
Name: "lsblk=success + json=valid",
Name: "lsblk=success",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "NAME", "-J"},
RunnerOutput: `{"blockdevices": [
{"name":"nvme1n1"},
{"name":"nvme0n1"}
]}`,
RunnerArgs: []string{"--nodeps", "-o", "NAME", "-P"},
RunnerOutput: `NAME="nvme1n1"
NAME="nvme0n1"`,
RunnerError: nil,
ExpectedOutput: []string{"/dev/nvme1n1", "/dev/nvme0n1"},
ExpectedError: nil,
},
{
Name: "lsblk=success + json=invalid",
Name: "lsblk=success + malformed response",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "NAME", "-J"},
RunnerOutput: `{"invalid_json"}`,
RunnerArgs: []string{"--nodeps", "-o", "NAME", "-P"},
RunnerOutput: `malformed`,
RunnerError: nil,
ExpectedOutput: nil,
ExpectedError: fmt.Errorf("🔴 Failed to decode lsblk response: *"),
ExpectedError: fmt.Errorf("🔴 Failed to decode lsblk response"),
},
{
Name: "lsblk=failure",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "NAME", "-J"},
RunnerArgs: []string{"--nodeps", "-o", "NAME", "-P"},
RunnerOutput: "",
RunnerError: fmt.Errorf("🔴 lsblk: invalid option -- 'J'"),
RunnerError: fmt.Errorf("🔴 lsblk: invalid option -- 'P'"),
ExpectedOutput: nil,
ExpectedError: fmt.Errorf("🔴 lsblk: invalid option -- 'J'"),
ExpectedError: fmt.Errorf("🔴 lsblk: invalid option -- 'P'"),
},
}
for _, subtest := range subtests {
Expand Down Expand Up @@ -128,11 +126,9 @@ func TestGetBlockDevice(t *testing.T) {
Name: "lsblk=success",
Device: "/dev/nvme1n1",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/nvme1n1"},
RunnerOutput: `{"blockdevices": [
{"name":"nvme1n1", "label":"external-vol", "fstype":"xfs", "mountpoint":"/mnt/app"}
]}`,
RunnerError: nil,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/nvme1n1"},
RunnerOutput: `LABEL="external-vol" FSTYPE="xfs" MOUNTPOINT="/mnt/app"`,
RunnerError: nil,
ExpectedOutput: &model.BlockDevice{
Name: "/dev/nvme1n1",
Label: "external-vol",
Expand All @@ -145,11 +141,9 @@ func TestGetBlockDevice(t *testing.T) {
Name: "lsblk=success + device=unformatted",
Device: "/dev/nvme1n1",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/nvme1n1"},
RunnerOutput: `{"blockdevices": [
{"name":"nvme1n1", "label":null, "fstype":null, "mountpoint":null}
]}`,
RunnerError: nil,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/nvme1n1"},
RunnerOutput: `LABEL="" FSTYPE="" MOUNTPOINT=""`,
RunnerError: nil,
ExpectedOutput: &model.BlockDevice{
Name: "/dev/nvme1n1",
Label: "",
Expand All @@ -158,75 +152,31 @@ func TestGetBlockDevice(t *testing.T) {
},
ExpectedError: nil,
},
/* Reference [https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/device_naming.html]
---
For example, if /dev/sdb is renamed /dev/xvdf, then /dev/sdc is renamed /dev/xvdg.
Amazon Linux creates a symbolic link for the name you specified to the renamed device.
Other operating systems could behave differently.
--
With this behaviour established on Amazon Linux, it is possible for lsblk to return a
device name that might not reflect the one that was provided to lsblk. To simplify the
output of ebs-bootstrap, lets force the BlockDevice to house the name that was provided
to the lsblk call
*/
{
Name: "lsblk=success + symlink=true",
Device: "/dev/sdb",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdb"},
RunnerOutput: `{"blockdevices": [
{"name":"xvdf", "label":"external-vol", "fstype":"xfs", "mountpoint":"/mnt/app"}
]}`,
RunnerError: nil,
ExpectedOutput: &model.BlockDevice{
Name: "/dev/sdb",
Label: "external-vol",
FileSystem: model.Xfs,
MountPoint: "/mnt/app",
},
ExpectedError: nil,
},
{
Name: "lsblk=success + json=invalid",
Name: "lsblk=success + malformed response",
Device: "/dev/sdb",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdb"},
RunnerOutput: `{"invalid_json"}`,
RunnerError: nil,
ExpectedOutput: nil,
ExpectedError: fmt.Errorf("🔴 Failed to decode lsblk response: *"),
},
{
Name: "lsblk=success + filesystem=unsupported",
Device: "/dev/sdb",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdb"},
RunnerOutput: `{"blockdevices": [
{"name":"sdb", "label":null, "fstype":"jfs", "mountpoint":"/mnt/app"}
]}`,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/sdb"},
RunnerOutput: "malformed",
RunnerError: nil,
ExpectedOutput: nil,
ExpectedError: fmt.Errorf("🔴 /dev/sdb: File system 'jfs' is not supported"),
ExpectedError: fmt.Errorf("🔴 Failed to decode lsblk response"),
},
/* I haven't encountered a scenario where lsblk successfully returns an empty array
Typically, if it cannot find a specific block device, it would produce an error.
Nevertheless, lets test this scenario...
*/
{
Name: "lsblk=success + len(devices)==0",
Name: "lsblk=success + filesystem=unsupported",
Device: "/dev/sdb",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdb"},
RunnerOutput: `{"blockdevices": []}`,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/sdb"},
RunnerOutput: `LABEL="" FSTYPE="jfs" MOUNTPOINT="/mnt/app"`,
RunnerError: nil,
ExpectedOutput: nil,
ExpectedError: fmt.Errorf("🔴 /dev/sdb: An unexpected number of block devices were returned: Expected=1 Actual=0"),
ExpectedError: fmt.Errorf("🔴 /dev/sdb: File system 'jfs' is not supported"),
},
{
Name: "lsblk=failure",
Device: "/dev/sdc",
RunnerBinary: utils.Lsblk,
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdc"},
RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/sdc"},
RunnerOutput: "",
RunnerError: fmt.Errorf("🔴 lsblk: /dev/sdc: not a block device"),
ExpectedOutput: nil,
Expand Down
2 changes: 1 addition & 1 deletion internal/service/filesystem.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (fsf *LinuxFileSystemServiceFactory) Select(fs model.FileSystem) (FileSyste
return NewXfsService(fsf.RunnerFactory), nil
case model.Lvm:
//lint:ignore ST1005 This Error Message Is Supposed to Be Prepended With A Device Name
return nil, fmt.Errorf("Refer to https://github.com/reecetech/ebs-bootstrap/wiki/LVM on how to manage LVM file systems")
return nil, fmt.Errorf("A Physical Volume cannot be queried/modified")
case model.Unformatted:
//lint:ignore ST1005 This Error Message Is Supposed to Be Prepended With A Device Name
return nil, fmt.Errorf("An unformatted file system can not be queried/modified")
Expand Down
7 changes: 7 additions & 0 deletions internal/service/filesystem_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ func TestFileSystemFactory(t *testing.T) {
ExpectedOutput: nil,
ExpectedError: fmt.Errorf("An unformatted file system can not be queried/modified"),
},
{
Name: "LVM",
FileSystem: model.Lvm,
CmpOption: cmp.AllowUnexported(),
ExpectedOutput: nil,
ExpectedError: fmt.Errorf("A Physical Volume cannot be queried/modified"),
},
}
for _, subtest := range subtests {
t.Run(subtest.Name, func(t *testing.T) {
Expand Down
9 changes: 5 additions & 4 deletions internal/service/nvme.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ type nvmeIdentifyController struct {
Vs nvmeIdentifyControllerAmznVS
}

var instanceStoreRegex = regexp.MustCompile(`^(ephemeral[0-9]):(sd[a-z]|none)`)

type NVMeIoctlResult struct {
Name string
IdCtrl nvmeIdentifyController
Expand Down Expand Up @@ -162,12 +164,11 @@ func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nir *NVMeIoctlResult) (stri
if ns.isInstanceStoreVolume(nir) {
// Vendor Specfic (vs)
vs := strings.TrimRightFunc(string(nir.IdCtrl.Vs.Bdev[:]), ns.trimBlockDevice)
// Regex Block device Mapping
rebdm := regexp.MustCompile(`^(ephemeral[0-9]):(sd[a-z]|none)`)

// Match Block Device Mapping
mbdm := rebdm.FindStringSubmatch(vs)
mbdm := instanceStoreRegex.FindStringSubmatch(vs)
if len(mbdm) != 3 {
return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern. Pattern=%s, Actual=%s", nir.Name, rebdm.String(), vs)
return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern. Pattern=%s, Actual=%s", nir.Name, instanceStoreRegex.String(), vs)
}
// If the block device mapping is "none", then lets default to assigning the
// the block device mapping to the match result from ephemeral[0-9]
Expand Down
Loading

0 comments on commit cf4d0ad

Please sign in to comment.