From 5a8b722bbbdb96386942de8d508b78469156767f Mon Sep 17 00:00:00 2001 From: lasith-kg Date: Mon, 12 Feb 2024 03:16:24 +0000 Subject: [PATCH 1/2] (feat): Add protection clause for non-assigned block device mapping --- internal/config/modifier.go | 2 +- internal/service/nvme.go | 15 +++++++------ internal/service/nvme_test.go | 40 +++++++++++++++++++++-------------- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/internal/config/modifier.go b/internal/config/modifier.go index 89ba6b5..38db1c6 100644 --- a/internal/config/modifier.go +++ b/internal/config/modifier.go @@ -42,13 +42,13 @@ func (andm *AwsNitroNVMeModifier) Modify(c *Config) error { if err != nil { return err } + log.Printf("🔵 Nitro NVMe detected: %s -> %s", name, bdm) cd, exists := c.Devices[bdm] // We can detect AWS NVMe Devices, but this doesn't neccesarily // mean they will be managed through configuration if !exists { continue } - log.Printf("🔵 %s: Detected Nitro-based AWS NVMe device => %s", name, bdm) // Delete the original reference to the device configuration from the // block device mapping retrieved from the NVMe IoCtl interface and // replace it with the actual device name diff --git a/internal/service/nvme.go b/internal/service/nvme.go index e0ba04c..a4be66b 100644 --- a/internal/service/nvme.go +++ b/internal/service/nvme.go @@ -163,16 +163,19 @@ func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nir *NVMeIoctlResult) (stri // 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)`) + rebdm := regexp.MustCompile(`^(ephemeral[0-9]):(sd[a-z]|none)`) // Match Block Device Mapping mbdm := rebdm.FindStringSubmatch(vs) - if len(mbdm) != 2 { - return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern . Pattern=%s, Actual=%s", nir.Name, rebdm.String(), 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) } - if mbdm[1] == "none" { - return "", fmt.Errorf("🔴 %s: Must provide a device name to the Instance Store NVMe block device mapping", nir.Name) + // 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] + if mbdm[2] == "none" { + bdm = mbdm[1] + } else { + bdm = mbdm[2] } - bdm = mbdm[1] } if len(bdm) == 0 { return "", fmt.Errorf("🔴 %s is not an AWS-managed NVME device", nir.Name) diff --git a/internal/service/nvme_test.go b/internal/service/nvme_test.go index b6ca42a..48f0e65 100644 --- a/internal/service/nvme_test.go +++ b/internal/service/nvme_test.go @@ -13,8 +13,8 @@ const ( ) const ( - Space = 0x20 - Null = 0x00 + SpaceByte = 0x20 + NullByte = 0x00 ) func TestGetBlockDeviceMapping(t *testing.T) { @@ -24,7 +24,6 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId uint16 ModelNumber string BlockDevice string - BlockDevicePadding byte ExpectedOutput string ExpectedError error }{ @@ -34,7 +33,6 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId: AMZN_NVME_VID, ModelNumber: AMZN_NVME_EBS_MN, BlockDevice: "sdb", - BlockDevicePadding: Space, ExpectedOutput: "/dev/sdb", ExpectedError: nil, }, @@ -44,7 +42,6 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId: AMZN_NVME_VID, ModelNumber: AMZN_NVME_EBS_MN, BlockDevice: "/dev/sdb", - BlockDevicePadding: Space, ExpectedOutput: "/dev/sdb", ExpectedError: nil, }, @@ -54,7 +51,6 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId: AMZN_NVME_VID, ModelNumber: AMZN_NVME_INS_MN, BlockDevice: "ephemeral0:sdb", - BlockDevicePadding: 0x53, ExpectedOutput: "/dev/sdb", ExpectedError: nil, }, @@ -64,7 +60,6 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId: AMZN_NVME_VID, ModelNumber: AMZN_NVME_INS_MN, BlockDevice: "ephemeral0:sdb\x00a", - BlockDevicePadding: Null, ExpectedOutput: "/dev/sdb", ExpectedError: nil, }, @@ -74,9 +69,8 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId: AMZN_NVME_VID, ModelNumber: AMZN_NVME_INS_MN, BlockDevice: "ephemeral0:none", - BlockDevicePadding: Null, - ExpectedOutput: "", - ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1: Must provide a device name to the Instance Store NVMe block device mapping"), + ExpectedOutput: "/dev/ephemeral0", + ExpectedError: nil, }, { Name: "Instance Store NVMe Device + Pattern Mismatch", @@ -84,9 +78,8 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId: AMZN_NVME_VID, ModelNumber: AMZN_NVME_INS_MN, BlockDevice: "ephemeral0:vdb", - BlockDevicePadding: Null, ExpectedOutput: "", - ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1: Instance-store vendor specific metadata did not match pattern . Pattern=^ephemeral[0-9]:(sd[a-z]|none), Actual=ephemeral0:vdb"), + ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1: Instance-store vendor specific metadata did not match pattern. Pattern=^(ephemeral[0-9]):(sd[a-z]|none), Actual=ephemeral0:vdb"), }, { Name: "Invalid NVMe Device (Unsupported Vendor ID)", @@ -94,7 +87,6 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId: UNSUPPORTED_NVME_VID, ModelNumber: AMZN_NVME_EBS_MN, BlockDevice: "", - BlockDevicePadding: Null, ExpectedOutput: "", ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1 is not an AWS-managed NVME device"), }, @@ -104,20 +96,21 @@ func TestGetBlockDeviceMapping(t *testing.T) { VendorId: AMZN_NVME_VID, ModelNumber: UNSUPPORTED_NVME_MN, BlockDevice: "", - BlockDevicePadding: Null, ExpectedOutput: "", ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1 is not an AWS-managed NVME device"), }, } for _, subtest := range subtests { t.Run(subtest.Name, func(t *testing.T) { + vsp, err := vendorSpecificPadding(subtest.ModelNumber) + utils.ExpectErr("vendorSpecificPadding()", t, false, err) nd := &NVMeIoctlResult{ Name: subtest.Device, IdCtrl: nvmeIdentifyController{ Vid: subtest.VendorId, - Mn: modelNumber(subtest.ModelNumber, Space), + Mn: modelNumber(subtest.ModelNumber, SpaceByte), Vs: nvmeIdentifyControllerAmznVS{ - Bdev: blockDevice(subtest.BlockDevice, subtest.BlockDevicePadding), + Bdev: blockDevice(subtest.BlockDevice, vsp), }, }, } @@ -152,3 +145,18 @@ func blockDevice(input string, padding byte) [32]byte { } return bd } + +// The padding byte used for the Vendor Specific section depends on whether the NVMe Block +// device is an EBS (Space) or Instance Store (Null) Volume +func vendorSpecificPadding(modelNumber string) (byte, error) { + switch modelNumber { + case AMZN_NVME_EBS_MN: + return SpaceByte, nil + case AMZN_NVME_INS_MN: + return NullByte, nil + case UNSUPPORTED_NVME_MN: + return NullByte, nil + default: + return NullByte, fmt.Errorf("🔴 %s: Could not determine vendor specific padding", modelNumber) + } +} From c9666f9a2fc673dc8bfe2d7bd9950439226455e6 Mon Sep 17 00:00:00 2001 From: lasith-kg Date: Mon, 12 Feb 2024 03:30:09 +0000 Subject: [PATCH 2/2] (docs): Update README with new message --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0822a56..37edfa2 100644 --- a/README.md +++ b/README.md @@ -72,8 +72,8 @@ On the first launch, `ebs-bootstrap` would refuse to perform any modifications t ``` [~] sudo ebs-bootstrap -mode=prompt -🔵 /dev/nvme1n1: Detected Nitro-based AWS NVMe device => /dev/sdb -🔵 /dev/nvme2n1: Detected Nitro-based AWS NVMe device => /dev/sdh +🔵 Nitro NVMe detected: /dev/nvme1n1 -> /dev/sdb +🔵 Nitro NVMe detected: /dev/nvme2n1 -> /dev/sdh 🟠 Formatting larger disks can take several seconds ⌛ 🟣 Would you like to format /dev/nvme1n1 to ext4? (y/n): y ⭐ Successfully formatted /dev/nvme1n1 to ext4