diff --git a/cli_tools/common/mount/inspector.go b/cli_tools/common/mount/inspector.go new file mode 100644 index 000000000..6a32a38fa --- /dev/null +++ b/cli_tools/common/mount/inspector.go @@ -0,0 +1,121 @@ +// Copyright 2021 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package mount + +import ( + "fmt" + "strings" + + "github.com/GoogleCloudPlatform/compute-image-tools/cli_tools/common/utils/shell" +) + +// To rebuild mocks, run `go generate ./...` +//go:generate go run github.com/golang/mock/mockgen -package mount -source $GOFILE -destination mock_mount_inspector.go + +// Inspector inspects a mount directory to return: +// - The underlying block device. +// - The block device's type. +// - If the block device is virtual, the number of block devices +// that comprise it. +type Inspector interface { + Inspect(dir string) (InspectionResults, error) +} + +// InspectionResults contains information about the mountpoint of a directory. +type InspectionResults struct { + BlockDevicePath string + BlockDeviceIsVirtual bool + UnderlyingBlockDevices []string +} + +// NewMountInspector returns a new inspector that uses command-line utilities. +func NewMountInspector() Inspector { + return &defaultMountInspector{shell.NewShellExecutor()} +} + +type defaultMountInspector struct { + shellExecutor shell.Executor +} + +// Inspect returns the mount information for dir. +func (mi *defaultMountInspector) Inspect(dir string) (mountInfo InspectionResults, err error) { + if mountInfo.BlockDevicePath, err = mi.getDeviceForMount(dir); err != nil { + return InspectionResults{}, fmt.Errorf("unable to find mount information for `%s`: %w", dir, err) + } + if mountInfo.BlockDeviceIsVirtual, err = mi.isDeviceVirtual(mountInfo.BlockDevicePath); err != nil { + return InspectionResults{}, fmt.Errorf("unable to find the type of device `%s`: %w", mountInfo.BlockDevicePath, err) + } + if mountInfo.UnderlyingBlockDevices, err = mi.getPhysicalDisks(mountInfo.BlockDevicePath); err != nil { + return InspectionResults{}, fmt.Errorf("unable to find the physical disks for the block device `%s`: %w", + mountInfo.BlockDevicePath, err) + } + return mountInfo, nil +} + +// getDeviceForMount returns the path of the block device that is mounted for dir. +func (mi *defaultMountInspector) getDeviceForMount(dir string) (string, error) { + stdout, err := mi.shellExecutor.Exec("getDeviceForMount", "--noheadings", "--output=SOURCE", dir) + if err != nil { + return "", err + } + return strings.TrimSpace(stdout), nil +} + +// isDeviceVirtual returns whether the block device is LVM +func (mi *defaultMountInspector) isDeviceVirtual(device string) (bool, error) { + stdout, err := mi.shellExecutor.Exec("lsblk", "--noheadings", "--output=TYPE", device) + return strings.TrimSpace(strings.ToLower(stdout)) == "lvm", err +} + +// getPhysicalDisks returns the list of physical disks that are used for the blockDevice. +// For example: +// 1. blockDevice is an MBR-style partition: +// blockDevice: /dev/sdb1 +// getPhysicalDisks: [/dev/sdb] +// 2. blockDevice is an LVM logical volume that is spread across three disks: +// blockDevice: /dev/mapper/vg-lv +// getPhysicalDisks: [/dev/sda, /dev/sdb, /dev/sdc] +func (mi *defaultMountInspector) getPhysicalDisks(blockDevice string) (disksForDevice []string, err error) { + disks, err := mi.getAllPhysicalDisks() + if err != nil { + return nil, err + } + + for _, disk := range disks { + blkDevices, err := mi.blockDevicesOnDisk(disk) + if err != nil { + return nil, err + } + for _, blkDevice := range blkDevices { + if blkDevice == blockDevice { + disksForDevice = append(disksForDevice, disk) + break + } + } + } + return disksForDevice, nil +} + +// getAllPhysicalDisks returns the paths of all physical disks on the system. +func (mi *defaultMountInspector) getAllPhysicalDisks() (allDisks []string, err error) { + return mi.shellExecutor.ExecLines( + "lsblk", "--noheadings", "--paths", "--list", "--nodeps", "--output=NAME") +} + +// blockDevicesOnDisk returns the paths of all block devices contained on a disk. +func (mi *defaultMountInspector) blockDevicesOnDisk(disk string) ([]string, error) { + return mi.shellExecutor.ExecLines( + "lsblk", "--noheadings", "--paths", "--list", "--output=NAME", disk) +} diff --git a/cli_tools/common/mount/inspector_test.go b/cli_tools/common/mount/inspector_test.go new file mode 100644 index 000000000..53aabfeda --- /dev/null +++ b/cli_tools/common/mount/inspector_test.go @@ -0,0 +1,143 @@ +// Copyright 2021 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package mount + +import ( + "errors" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + + "github.com/GoogleCloudPlatform/compute-image-tools/cli_tools/mocks" +) + +func TestMountInspector_Inspect_HappyCase_NotVirtual(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockShell := mocks.NewMockShellExecutor(mockCtrl) + setupRootMount(mockShell, "/dev/sdb1", "part") + setupPhysicalDisks(mockShell, map[string][]string{ + "/dev/sda": {"/dev/sda", "/dev/sda1"}, + "/dev/sdb": {"/dev/sdb", "/dev/sdb1"}, + }) + + mountInspector := &defaultMountInspector{mockShell} + mountInfo, err := mountInspector.Inspect("/") + assert.NoError(t, err) + assert.Equal(t, InspectionResults{ + BlockDevicePath: "/dev/sdb1", + BlockDeviceIsVirtual: false, + UnderlyingBlockDevices: []string{"/dev/sdb"}, + }, mountInfo) +} + +func TestMountInspector_Inspect_HappyCase_Virtual(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockShell := mocks.NewMockShellExecutor(mockCtrl) + setupRootMount(mockShell, "/dev/mapper/vg-device", "lvm") + setupPhysicalDisks(mockShell, map[string][]string{ + "/dev/sda": {"/dev/sda", "/dev/sda1", "/dev/mapper/vg-device"}, + "/dev/sdb": {"/dev/sdb", "/dev/sdb1", "/dev/mapper/vg-device"}, + }) + + mountInspector := &defaultMountInspector{mockShell} + mountInfo, err := mountInspector.Inspect("/") + assert.NoError(t, err) + assert.Equal(t, InspectionResults{ + BlockDevicePath: "/dev/mapper/vg-device", + BlockDeviceIsVirtual: true, + UnderlyingBlockDevices: []string{"/dev/sda", "/dev/sdb"}, + }, mountInfo) +} + +func TestMountInspector_Inspect_PropagatesErrorFromFindMnt(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockShell := mocks.NewMockShellExecutor(mockCtrl) + mockShell.EXPECT().Exec("getDeviceForMount", "--noheadings", + "--output=SOURCE", "/").Return("", errors.New("[getDeviceForMount] not executable")) + + mountInspector := &defaultMountInspector{mockShell} + _, err := mountInspector.Inspect("/") + assert.Equal(t, err.Error(), "unable to find mount information for `/`: [getDeviceForMount] not executable") +} + +func TestMountInspector_Inspect_PropagatesErrorFromGettingDeviceType(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockShell := mocks.NewMockShellExecutor(mockCtrl) + mockShell.EXPECT().Exec("getDeviceForMount", "--noheadings", + "--output=SOURCE", "/").Return("/dev/mapper/vg-device", nil) + mockShell.EXPECT().Exec("lsblk", "--noheadings", + "--output=TYPE", "/dev/mapper/vg-device").Return("", errors.New("[lsblk] not executable")) + + mountInspector := &defaultMountInspector{mockShell} + _, err := mountInspector.Inspect("/") + assert.Equal(t, err.Error(), "unable to find the type of device `/dev/mapper/vg-device`: [lsblk] not executable") +} + +func TestMountInspector_Inspect_PropagatesErrorFromGettingAllDisks(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockShell := mocks.NewMockShellExecutor(mockCtrl) + setupRootMount(mockShell, "/dev/sdb1", "part") + mockShell.EXPECT().ExecLines("lsblk", "--noheadings", "--paths", "--list", "--nodeps", "--output=NAME").Return( + nil, errors.New("[lsblk] not executable")) + + mountInspector := &defaultMountInspector{mockShell} + _, err := mountInspector.Inspect("/") + assert.Equal(t, err.Error(), "unable to find the physical disks for the block device `/dev/sdb1`: [lsblk] not executable") +} + +func TestMountInspector_Inspect_PropagatesErrorFromGettingDevicesOnDisk(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockShell := mocks.NewMockShellExecutor(mockCtrl) + setupRootMount(mockShell, "/dev/sdb1", "part") + mockShell.EXPECT().ExecLines("lsblk", "--noheadings", "--paths", "--list", "--nodeps", "--output=NAME").Return( + []string{"/dev/sda", "/dev/sdb"}, nil) + mockShell.EXPECT().ExecLines("lsblk", "--noheadings", "--paths", "--list", "--output=NAME", "/dev/sda").Return( + nil, errors.New("[lsblk] not executable")) + + mountInspector := &defaultMountInspector{mockShell} + _, err := mountInspector.Inspect("/") + assert.Equal(t, err.Error(), "unable to find the physical disks for the block device `/dev/sdb1`: [lsblk] not executable") +} + +func setupPhysicalDisks(mockShell *mocks.MockShellExecutor, deviceMap map[string][]string) { + var disks []string + for disk := range deviceMap { + disks = append(disks, disk) + } + mockShell.EXPECT().ExecLines("lsblk", "--noheadings", "--paths", "--list", "--nodeps", "--output=NAME").Return( + disks, nil) + for disk, devices := range deviceMap { + mockShell.EXPECT().ExecLines("lsblk", "--noheadings", "--paths", "--list", "--output=NAME", disk).Return( + devices, nil) + } +} + +func setupRootMount(mockShell *mocks.MockShellExecutor, mointPoint string, mointPointType string) { + mockShell.EXPECT().Exec("getDeviceForMount", "--noheadings", "--output=SOURCE", "/").Return(mointPoint, nil) + mockShell.EXPECT().Exec("lsblk", "--noheadings", "--output=TYPE", mointPoint).Return(mointPointType, nil) +} diff --git a/cli_tools/common/mount/mock_mount_inspector.go b/cli_tools/common/mount/mock_mount_inspector.go new file mode 100644 index 000000000..3c83416f0 --- /dev/null +++ b/cli_tools/common/mount/mock_mount_inspector.go @@ -0,0 +1,63 @@ +// Copyright 2021 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Code generated by MockGen. DO NOT EDIT. +// Source: inspector.go + +// Package mount is a generated GoMock package. +package mount + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockInspector is a mock of Inspector interface. +type MockInspector struct { + ctrl *gomock.Controller + recorder *MockInspectorMockRecorder +} + +// MockInspectorMockRecorder is the mock recorder for MockInspector. +type MockInspectorMockRecorder struct { + mock *MockInspector +} + +// NewMockInspector creates a new mock instance. +func NewMockInspector(ctrl *gomock.Controller) *MockInspector { + mock := &MockInspector{ctrl: ctrl} + mock.recorder = &MockInspectorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockInspector) EXPECT() *MockInspectorMockRecorder { + return m.recorder +} + +// Inspect mocks base method. +func (m *MockInspector) Inspect(dir string) (InspectionResults, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Inspect", dir) + ret0, _ := ret[0].(InspectionResults) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Inspect indicates an expected call of Inspect. +func (mr *MockInspectorMockRecorder) Inspect(dir interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Inspect", reflect.TypeOf((*MockInspector)(nil).Inspect), dir) +} diff --git a/cli_tools/common/utils/shell/executor.go b/cli_tools/common/utils/shell/executor.go new file mode 100644 index 000000000..956d833e6 --- /dev/null +++ b/cli_tools/common/utils/shell/executor.go @@ -0,0 +1,61 @@ +// Copyright 2021 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package shell + +import ( + "bufio" + "bytes" + "os/exec" +) + +// To rebuild mocks, run `go generate ./...` +//go:generate go run github.com/golang/mock/mockgen -package mocks -source $GOFILE -mock_names=Executor=MockShellExecutor -destination ../../../mocks/mock_shell_exececutor.go + +// Executor is a shim over cmd.Output() that allows for testing. +type Executor interface { + // Exec executes program with args, and returns stdout if the return code is zero. + // If nonzero, stderr is included in error. + Exec(program string, args ...string) (string, error) + // ExecLines is similar to Exec, except it splits the output on newlines. All empty + // lines are discarded. + ExecLines(program string, args ...string) ([]string, error) +} + +// NewShellExecutor creates a shell.Executor that is implemented by exec.Command. +func NewShellExecutor() Executor { + return &defaultShellExecutor{} +} + +type defaultShellExecutor struct { +} + +func (d *defaultShellExecutor) Exec(program string, args ...string) (string, error) { + cmd := exec.Command(program, args...) + stdout, err := cmd.Output() + return string(stdout), err +} + +func (d *defaultShellExecutor) ExecLines(program string, args ...string) (allLines []string, err error) { + cmd := exec.Command(program, args...) + stdout, err := cmd.Output() + scanner := bufio.NewScanner(bytes.NewReader(stdout)) + for scanner.Scan() { + line := scanner.Text() + if line != "" { + allLines = append(allLines, line) + } + } + return allLines, err +} diff --git a/cli_tools/import_precheck/main.go b/cli_tools/import_precheck/main.go index a9c9e8915..f676f4122 100644 --- a/cli_tools/import_precheck/main.go +++ b/cli_tools/import_precheck/main.go @@ -35,7 +35,7 @@ var ( func getChecks(osInfo *osinfo.OSInfo) []precheck.Check { return []precheck.Check{ &precheck.OSVersionCheck{OSInfo: osInfo}, - &precheck.DisksCheck{}, + precheck.NewDisksCheck(), &precheck.SSHCheck{}, &precheck.PowershellCheck{}, &precheck.SHA2DriverSigningCheck{OSInfo: osInfo}, diff --git a/cli_tools/import_precheck/precheck/check_disks_linux.go b/cli_tools/import_precheck/precheck/check_disks_linux.go index 6e5532748..57ad01b6d 100644 --- a/cli_tools/import_precheck/precheck/check_disks_linux.go +++ b/cli_tools/import_precheck/precheck/check_disks_linux.go @@ -16,13 +16,11 @@ package precheck import ( "bytes" - "encoding/json" - "errors" "fmt" "os" - "os/exec" - "path/filepath" - "sort" + "strings" + + "github.com/GoogleCloudPlatform/compute-image-tools/cli_tools/common/mount" ) // DisksCheck performs disk configuration checking: @@ -35,52 +33,12 @@ import ( // - warning for any mount points from partitions from other devices type DisksCheck struct { getMBROverride func(devName string) ([]byte, error) - lsblkOverride func() ([]byte, error) + inspector mount.Inspector } -func (c *DisksCheck) getMBR(devName string) ([]byte, error) { - devPath := filepath.Join("/dev", devName) - f, err := os.Open(devPath) - if err != nil { - return nil, err - } - data := make([]byte, mbrSize) - _, err = f.Read(data) - if err != nil { - return nil, fmt.Errorf("error reading %s: %v", devPath, err) - } - return data, nil -} - -func (c *DisksCheck) readMounts() (*mountPoints, error) { - var lsblkOut []byte - var err error - if c.lsblkOverride != nil { - lsblkOut, err = c.lsblkOverride() - } else { - cmd := exec.Command("lsblk", "--json", "--output", "name,mountpoint,type") - lsblkOut, err = cmd.Output() - if err != nil { - var exitErr *exec.ExitError - if errors.As(err, &exitErr) { - err = fmt.Errorf("lsblk: %v, stderr: %s", err, exitErr.Stderr) - } else { - err = fmt.Errorf("lsblk: %v", err) - } - } - } - - if err != nil { - return nil, err - } - parsed := &LSBLKOutput{} - err = json.Unmarshal(lsblkOut, parsed) - if err != nil { - return nil, err - } - mounts := &mountPoints{} - mounts.addAll(parsed.Blockdevices, []string{}) - return mounts, nil +// NewDisksCheck instantiates a DisksCheck instance. +func NewDisksCheck() Check { + return &DisksCheck{inspector: mount.NewMountInspector()} } // GetName returns the name of the precheck step; this is shown to the user. @@ -88,168 +46,63 @@ func (c *DisksCheck) GetName() string { return "Disks Check" } -// mountPoints supports listing the mounts on a system, and determining which block device(s) -// the mount is physically contained. This is helpful since technologies such as LVM allow -// logical mounts to span physical devices. -// -// The data structure is modeled as a one-to-many inverse index of mountDir and its associated -// device hierarchy. For example, given the following device tree: -// -// /dev/sda -// ∟ sda1 -// ∟ logical-volume -> / -// /dev/sdb -// ∟ sdb2 -// ∟ logical-volume -> / -// -// mounts would contain two entries: -// -// {/, [sda, sda1, logical-volume]} -// {/, [sdb, sdb2, logical-volume]} -type mountPoints struct { - mounts []mountPoint -} - -// mountPoint describes the device hierarchy for a mounted directory. -// The hierarchy starts at the physical device, and contains an entry for each -// partition or logical volume. -type mountPoint struct { - dir string - hierarchy []string -} - -// getPhysicalDevice returns the physical device of this mountPoint. -func (m *mountPoint) getPhysicalDevice() string { - return m.hierarchy[0] -} - -// listPhysicalDevicesForMount returns the the block device(s) that contain the mountDir. -// An empty return value means the mountDir was not found. -func (m *mountPoints) listPhysicalDevicesForMount(mountDir string) (devices []string) { - set := map[string]struct{}{} - for _, mp := range m.mounts { - if mp.dir == mountDir { - set[mp.getPhysicalDevice()] = struct{}{} - } - } - for k := range set { - devices = append(devices, k) - } - // sorted for stability in testing. - sort.Strings(devices) - return devices -} - -// listMountedDirectories returns all mounted directories. -func (m *mountPoints) listMountedDirectories() (mountedDirectories []string) { - set := map[string]struct{}{} - for _, mount := range m.mounts { - set[mount.dir] = struct{}{} - } - for k := range set { - mountedDirectories = append(mountedDirectories, k) - } - // sorted for stability in testing. - sort.Strings(mountedDirectories) - return mountedDirectories -} - -// addAll populates the mountPoints data structure with the response from lsblk. -func (m *mountPoints) addAll(elements []DiskElement, basePath []string) { - for _, element := range elements { - path := make([]string, len(basePath)) - copy(path, basePath) - path = append(path, element.Name) - if element.Mountpoint != "" { - m.mounts = append(m.mounts, mountPoint{element.Mountpoint, path}) - } - if len(element.Children) > 0 { - m.addAll(element.Children, path) - } - } -} - // Run executes the precheck step. func (c *DisksCheck) Run() (r *Report, err error) { r = &Report{name: c.GetName()} - allMounts, err := c.readMounts() + mountInfo, err := c.inspector.Inspect("/") if err != nil { - r.Warn(fmt.Sprintf("Failed to execute lsblk: %s", err)) + r.result = Unknown + r.Warn("Failed to inspect the boot disk. Prior to importing, verify that the boot disk " + + "contains the root filesystem, and that the root filesystem isn't virtualized over " + + "multiple disks (using LVM, for example).") return r, nil } - rootDevices := allMounts.listPhysicalDevicesForMount("/") - switch len(rootDevices) { - case 0: - r.Fatal("root filesystem partition not found on any block devices.") - return r, nil - case 1: - r.Info(fmt.Sprintf("root filesystem found on device: %s", rootDevices[0])) - default: + r.Info(fmt.Sprintf("root filesystem mounted on %s", mountInfo.BlockDevicePath)) + + if len(mountInfo.UnderlyingBlockDevices) > 1 { format := "root filesystem spans multiple block devices (%s). Typically this occurs when an LVM logical " + "volume spans multiple block devices. Image import only supports single block device." - r.Fatal(fmt.Sprintf(format, rootDevices)) + r.Fatal(fmt.Sprintf(format, strings.Join(mountInfo.UnderlyingBlockDevices, ", "))) return r, nil } - rootDevice := rootDevices[0] - for _, mountDir := range allMounts.listMountedDirectories() { - if mountDir == "/" { - continue - } - devices := allMounts.listPhysicalDevicesForMount(mountDir) - switch len(devices) { - case 0: - // This implies a bug in mountPoints.addAll. - panic(fmt.Sprintf("Invalid parse of mount %s", mountDir)) - case 1: - // devices[0] is the only physical device that contains this mountDir. - if devices[0] != rootDevice { - format := "mount %s is not on the root device %s and will be OMITTED from image import." - r.Warn(fmt.Sprintf(format, mountDir, rootDevice)) - } - default: - format := "mount %s is on multiple physical devices (%s) and will be OMITTED from image import." - r.Warn(fmt.Sprintf(format, mountDir, devices)) - } - } - + bootDisk := mountInfo.UnderlyingBlockDevices[0] + r.Info(fmt.Sprintf("boot disk detected as %s", bootDisk)) // MBR checking. var mbrData []byte if c.getMBROverride != nil { - mbrData, err = c.getMBROverride(rootDevice) + mbrData, err = c.getMBROverride(bootDisk) } else { - mbrData, err = c.getMBR(rootDevice) + mbrData, err = c.getMBR(bootDisk) } if err != nil { return nil, err } if mbrData[510] != 0x55 || mbrData[511] != 0xAA { - r.Fatal("root filesystem device is NOT MBR") + r.Fatal("boot disk does not have an MBR partition table") } else { - r.Info("root filesystem device is MBR.") + r.Info("boot disk has an MBR partition table") } if !bytes.Contains(mbrData, []byte("GRUB")) { - r.Fatal("GRUB not detected on MBR") + r.Fatal("GRUB not detected in MBR") } else { - r.Info("GRUB found in root filesystem device MBR") + r.Info("GRUB found in MBR") } return r, nil } -// LSBLKOutput is a struct representing the output from running -// `lsblk --json`. -type LSBLKOutput struct { - Blockdevices []DiskElement `json:"blockdevices"` -} - -// DiskElement is a struct representing the nested fields within -// the output of `lsblk --json`. See the testdata directory for -// examples of nesting. -type DiskElement struct { - Name string `json:"name"` - Mountpoint string `json:"mountpoint"` - Children []DiskElement `json:"children"` +func (c *DisksCheck) getMBR(devPath string) ([]byte, error) { + f, err := os.Open(devPath) + if err != nil { + return nil, err + } + data := make([]byte, mbrSize) + _, err = f.Read(data) + if err != nil { + return nil, fmt.Errorf("error reading %s: %v", devPath, err) + } + return data, nil } diff --git a/cli_tools/import_precheck/precheck/check_disks_linux_test.go b/cli_tools/import_precheck/precheck/check_disks_linux_test.go index 2c93caf93..69d621a83 100644 --- a/cli_tools/import_precheck/precheck/check_disks_linux_test.go +++ b/cli_tools/import_precheck/precheck/check_disks_linux_test.go @@ -15,127 +15,151 @@ package precheck import ( - "io/ioutil" + "errors" "testing" + "github.com/golang/mock/gomock" + + "github.com/GoogleCloudPlatform/compute-image-tools/cli_tools/common/mount" + "github.com/stretchr/testify/assert" ) -func Test_readMounts(t *testing.T) { - for _, tc := range []struct { - lsblkOutputFile string - expectedPhysicalDevices map[string][]string - expectedListMounts []string - }{ +type disksCheckTest struct { + name string + mountInfo mount.InspectionResults + inspectError error + byteTrailer []byte + expectAllLogs []string + expectedStatus Result +} + +func TestDisksCheck_Inspector(t *testing.T) { + for _, tc := range []disksCheckTest{ { - lsblkOutputFile: "lsblk-one-partition.json", - expectedListMounts: []string{"/"}, - expectedPhysicalDevices: map[string][]string{ - "/": []string{"sda"}, + name: "pass if boot device is non virtual", + mountInfo: mount.InspectionResults{ + BlockDevicePath: "/dev/sda1", + BlockDeviceIsVirtual: false, + UnderlyingBlockDevices: []string{"/dev/sda"}, }, + expectAllLogs: []string{ + "INFO: root filesystem mounted on /dev/sda1", + }, + expectedStatus: Passed, }, { - lsblkOutputFile: "lsblk-multi-disk-lvm.json", - expectedListMounts: []string{"/", "/boot", "/mnt/tmp"}, - expectedPhysicalDevices: map[string][]string{ - "/": []string{"sda", "sdb"}, - "/boot": []string{"sda"}, - "/mnt/tmp": []string{"sdc"}, + name: "pass if boot device is virtual with one underlying device", + mountInfo: mount.InspectionResults{ + BlockDevicePath: "/dev/mapper/vg-lv", + BlockDeviceIsVirtual: true, + UnderlyingBlockDevices: []string{"/dev/sda"}, + }, + expectAllLogs: []string{ + "INFO: root filesystem mounted on /dev/mapper/vg-lv", }, + expectedStatus: Passed, }, { - lsblkOutputFile: "lsblk-efi.json", - expectedListMounts: []string{"/", "/boot/efi"}, - expectedPhysicalDevices: map[string][]string{ - "/": []string{"sda"}, - "/boot/efi": []string{"sda"}, + name: "fail if boot device is virtual with multiple underlying devices", + mountInfo: mount.InspectionResults{ + BlockDevicePath: "/dev/mapper/vg-lv", + BlockDeviceIsVirtual: true, + UnderlyingBlockDevices: []string{"/dev/sda", "/dev/sdb"}, + }, + expectAllLogs: []string{ + "FATAL: root filesystem spans multiple block devices (/dev/sda, /dev/sdb). Typically this occurs when an LVM " + + "logical volume spans multiple block devices. Image import only supports single block device.", }, + expectedStatus: Failed, }, { - lsblkOutputFile: "lsblk-single-disk-lvm.json", - expectedListMounts: []string{"/", "/boot", "/boot/efi", "[SWAP]"}, - expectedPhysicalDevices: map[string][]string{ - "/": []string{"sda"}, - "/boot": []string{"sda"}, - "/boot/efi": []string{"sda"}, - "[SWAP]": []string{"sda"}, + name: "fail if inspect fails", + inspectError: errors.New("failed to find root device"), + expectAllLogs: []string{ + "WARN: Failed to inspect the boot disk. Prior to importing, verify that the boot disk " + + "contains the root filesystem, and that the root filesystem isn't virtualized " + + "over multiple disks (using LVM, for example).", }, + expectedStatus: Unknown, }, } { - t.Run(tc.lsblkOutputFile, func(t *testing.T) { - actual, err := (&DisksCheck{ - lsblkOverride: func() ([]byte, error) { - return ioutil.ReadFile("testdata/" + tc.lsblkOutputFile) - }, - }).readMounts() - if err != nil { - t.Fatal(err) - } - assert.Equal(t, tc.expectedListMounts, actual.listMountedDirectories()) - for mountDir, expectedPhysicalDevices := range tc.expectedPhysicalDevices { - assert.Equal(t, expectedPhysicalDevices, actual.listPhysicalDevicesForMount(mountDir)) - } + t.Run(tc.name, func(t *testing.T) { + tc.byteTrailer = []byte{'G', 'R', 'U', 'B', 0x55, 0xAA} + runDisksCheck(t, tc) }) } } -func Test_run(t *testing.T) { - for _, tc := range []struct { - lsblkOutputFile string - expectAllLogs []string - expectToPass bool - }{ +func TestDisksCheck_MBR(t *testing.T) { + for _, tc := range []disksCheckTest{ { - lsblkOutputFile: "lsblk-one-partition.json", + name: "happy case", expectAllLogs: []string{ - "INFO: root filesystem found on device: sda", + "INFO: boot disk has an MBR partition table", + "INFO: GRUB found in MBR", }, - expectToPass: true, - }, - { - lsblkOutputFile: "lsblk-multi-disk-lvm.json", + expectedStatus: Passed, + byteTrailer: []byte{'G', 'R', 'U', 'B', 0x55, 0xAA}, + }, { + name: "fail if GRUB not in first 512 bytes", expectAllLogs: []string{ - "FATAL: root filesystem spans multiple block devices ([sda sdb]). Typically this occurs when an LVM " + - "logical volume spans multiple block devices. Image import only supports single block device.", + "INFO: boot disk has an MBR partition table", + "FATAL: GRUB not detected in MBR", }, - expectToPass: false, + expectedStatus: Failed, + byteTrailer: []byte{0x55, 0xAA}, }, { - lsblkOutputFile: "lsblk-efi.json", + name: "fail if 0x55 not at 510", expectAllLogs: []string{ - "INFO: root filesystem found on device: sda", + "FATAL: boot disk does not have an MBR partition table", + "FATAL: GRUB not detected in MBR", }, - expectToPass: true, + expectedStatus: Failed, + byteTrailer: []byte{0xAA}, }, { - lsblkOutputFile: "lsblk-single-disk-lvm.json", + name: "fail if 0xAA not at 511", expectAllLogs: []string{ - "INFO: root filesystem found on device: sda", + "FATAL: boot disk does not have an MBR partition table", + "FATAL: GRUB not detected in MBR", }, - expectToPass: true, + expectedStatus: Failed, + byteTrailer: []byte{0x55}, }, } { - t.Run(tc.lsblkOutputFile, func(t *testing.T) { - report, err := (&DisksCheck{ - lsblkOverride: func() ([]byte, error) { - return ioutil.ReadFile("testdata/" + tc.lsblkOutputFile) - }, - getMBROverride: func(devName string) ([]byte, error) { - bytes := make([]byte, 512) - copy(bytes, "GRUB") - bytes[510] = 0x55 - bytes[511] = 0xAA - return bytes, nil - }, - }).Run() - if err != nil { - t.Fatal(err) - } - - for _, expectedLog := range tc.expectAllLogs { - assert.Contains(t, report.logs, expectedLog) + t.Run(tc.name, func(t *testing.T) { + tc.mountInfo = mount.InspectionResults{ + BlockDevicePath: "/dev/mapper/vg-lv", + BlockDeviceIsVirtual: true, + UnderlyingBlockDevices: []string{"/dev/sda"}, } + runDisksCheck(t, tc) + }) + } +} - if tc.expectToPass { - assert.False(t, report.failed, "Expected check to pass") - } else { - assert.True(t, report.failed, "Expected check to fail") +func runDisksCheck(t *testing.T, tc disksCheckTest) { + t.Helper() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockMountInspector := mount.NewMockInspector(ctrl) + mockMountInspector.EXPECT().Inspect("/").Return( + tc.mountInfo, tc.inspectError) + report, err := (&DisksCheck{ + getMBROverride: func(devName string) ([]byte, error) { + assert.Len(t, tc.mountInfo.UnderlyingBlockDevices, 1, "Only check grub when there's a single block device") + assert.Equal(t, tc.mountInfo.UnderlyingBlockDevices[0], devName) + bytes := make([]byte, 512) + for i := range tc.byteTrailer { + bytes[len(bytes)-len(tc.byteTrailer)+i] = tc.byteTrailer[i] } - }) + return bytes, nil + }, + inspector: mockMountInspector, + }).Run() + if err != nil { + t.Fatal(err) + } + for _, expectedLog := range tc.expectAllLogs { + assert.Contains(t, report.logs, expectedLog) } + + assert.Equal(t, tc.expectedStatus.String(), report.result.String()) } diff --git a/cli_tools/import_precheck/precheck/check_disks_windows.go b/cli_tools/import_precheck/precheck/check_disks_windows.go index 8564adc7b..d1b94c588 100644 --- a/cli_tools/import_precheck/precheck/check_disks_windows.go +++ b/cli_tools/import_precheck/precheck/check_disks_windows.go @@ -23,8 +23,14 @@ import ( "golang.org/x/sys/windows" ) +// DisksCheck performs disk configuration checking. type DisksCheck struct{} +// NewDisksCheck instantiates a DisksCheck instance. +func NewDisksCheck() Check { + return &DisksCheck{} +} + // GetName returns the name of the precheck step; this is shown to the user. func (c *DisksCheck) GetName() string { return "Disks Check" diff --git a/cli_tools/import_precheck/precheck/check_osversion.go b/cli_tools/import_precheck/precheck/check_osversion.go index c55e95704..5a402db2a 100644 --- a/cli_tools/import_precheck/precheck/check_osversion.go +++ b/cli_tools/import_precheck/precheck/check_osversion.go @@ -47,7 +47,7 @@ func (c *OSVersionCheck) Run() (*Report, error) { if osID == "" { r.Info("Unable to determine whether your system is supported for import. " + "For supported versions, see " + docsURL) - r.skipped = true + r.result = Skipped return r, nil } // Check whether the osID is supported for import. diff --git a/cli_tools/import_precheck/precheck/check_osversion_test.go b/cli_tools/import_precheck/precheck/check_osversion_test.go index 0509897f8..0cd97ebcd 100644 --- a/cli_tools/import_precheck/precheck/check_osversion_test.go +++ b/cli_tools/import_precheck/precheck/check_osversion_test.go @@ -122,7 +122,7 @@ func Test_osVersionCheck_skipWhenOSDetectionFails(t *testing.T) { assert.Contains(t, r.String(), tt.expectedLog) assert.Contains(t, r.String(), "Unable to determine whether your system is supported for import. "+ "For supported versions, see https://cloud.google.com/sdk/gcloud/reference/compute/images/import") - assert.True(t, r.skipped) + assert.Equal(t, Skipped, r.result) t.Logf("\n%s", r) }) } diff --git a/cli_tools/import_precheck/precheck/check_powershell.go b/cli_tools/import_precheck/precheck/check_powershell.go index d45e9416c..3d94ff58c 100644 --- a/cli_tools/import_precheck/precheck/check_powershell.go +++ b/cli_tools/import_precheck/precheck/check_powershell.go @@ -36,7 +36,7 @@ func (c *PowershellCheck) GetName() string { func (c *PowershellCheck) Run() (*Report, error) { r := &Report{name: c.GetName()} if runtime.GOOS != "windows" { - r.skipped = true + r.result = Skipped r.Info("Not applicable on non-Windows systems.") return r, nil } diff --git a/cli_tools/import_precheck/precheck/check_sha2.go b/cli_tools/import_precheck/precheck/check_sha2.go index 4fd74269e..8c067822d 100644 --- a/cli_tools/import_precheck/precheck/check_sha2.go +++ b/cli_tools/import_precheck/precheck/check_sha2.go @@ -41,7 +41,7 @@ func (s *SHA2DriverSigningCheck) GetName() string { func (s *SHA2DriverSigningCheck) Run() (*Report, error) { r := &Report{name: s.GetName()} if runtime.GOOS != "windows" || !strings.Contains(s.OSInfo.Version, "6.1") { - r.skipped = true + r.result = Skipped r.Info("Only applicable on Windows 2008 systems.") return r, nil } diff --git a/cli_tools/import_precheck/precheck/check_ssh.go b/cli_tools/import_precheck/precheck/check_ssh.go index 2d9285f43..a2e628b10 100644 --- a/cli_tools/import_precheck/precheck/check_ssh.go +++ b/cli_tools/import_precheck/precheck/check_ssh.go @@ -39,7 +39,7 @@ func (c *SSHCheck) GetName() string { func (c *SSHCheck) Run() (*Report, error) { r := &Report{name: c.GetName()} if runtime.GOOS == "windows" { - r.skipped = true + r.result = Skipped r.Info("Not applicable on Windows systems.") return r, nil } diff --git a/cli_tools/import_precheck/precheck/report.go b/cli_tools/import_precheck/precheck/report.go index a0dc1dcbc..a7df0d01e 100644 --- a/cli_tools/import_precheck/precheck/report.go +++ b/cli_tools/import_precheck/precheck/report.go @@ -15,25 +15,59 @@ package precheck import ( + "fmt" "strings" ) +// Result indicates the result of the check. +type Result int + +const ( + // Passed means the check passed. + Passed Result = iota + + // Failed means the check ran and found an error such that + // the machine is not importable. + Failed + + // Skipped means the check didn't run. + Skipped + + // Unknown means the test couldn't run, and that the user + // should run a manual verification. + Unknown +) + +func (r Result) String() string { + switch r { + case Passed: + return "PASSED" + case Failed: + return "FAILED" + case Skipped: + return "SKIPPED" + case Unknown: + return "UNKNOWN" + default: + panic(fmt.Sprintf("Unmapped status: %d", r)) + } +} + // Report contains the results of running one or more precheck steps. type Report struct { - name string - skipped bool - failed bool - logs []string + name string + result Result + logs []string } // Failed returns whether one or more precheck steps failed. func (r *Report) Failed() bool { - return r.failed + return r.result == Failed } // Fatal messages indicate that the system is not importable. func (r *Report) Fatal(s string) { - r.failed = true + r.result = Failed r.logs = append(r.logs, "FATAL: "+s) } @@ -42,21 +76,13 @@ func (r *Report) Info(s string) { r.logs = append(r.logs, "INFO: "+s) } -// Warn messages indicate that a precheck step couldn't run, -// and that the user should manually verify the check on their own. +// Warn messages indicate that the user should perform a manual check. func (r *Report) Warn(s string) { r.logs = append(r.logs, "WARN: "+s) } func (r *Report) String() string { - status := "PASSED" - if r.skipped { - status = "SKIPPED" - } else if r.failed { - status = "FAILED" - } - - title := strings.Join([]string{r.name, status}, " -- ") + title := strings.Join([]string{r.name, r.result.String()}, " -- ") border := strings.Repeat("#", len(title)+4) lines := []string{border, "# " + title + " #", border} diff --git a/cli_tools/import_precheck/precheck/testdata/lsblk-efi.json b/cli_tools/import_precheck/precheck/testdata/lsblk-efi.json deleted file mode 100644 index 9f2e95b0e..000000000 --- a/cli_tools/import_precheck/precheck/testdata/lsblk-efi.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "blockdevices": [ - {"name":"sda", "mountpoint":null, "type":"disk", - "children": [ - {"name":"sda1", "mountpoint":"/boot/efi", "type":"part"}, - {"name":"sda2", "mountpoint":"/", "type":"part"} - ] - } - ] -} diff --git a/cli_tools/import_precheck/precheck/testdata/lsblk-multi-disk-lvm.json b/cli_tools/import_precheck/precheck/testdata/lsblk-multi-disk-lvm.json deleted file mode 100644 index bf0e5249f..000000000 --- a/cli_tools/import_precheck/precheck/testdata/lsblk-multi-disk-lvm.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "blockdevices": [ - {"name":"fd0", "mountpoint":null, "type":"disk"}, - {"name":"sda", "mountpoint":null, "type":"disk", - "children": [ - {"name":"sda1", "mountpoint":"/boot", "type":"part"}, - {"name":"sda2", "mountpoint":null, "type":"part", - "children": [ - {"name":"vg1-lv1", "mountpoint":"/", "type":"lvm"} - ] - } - ] - }, - {"name":"sdb", "mountpoint":null, "type":"disk", - "children": [ - {"name":"sdb1", "mountpoint":null, "type":"part", - "children": [ - {"name":"vg1-lv1", "mountpoint":"/", "type":"lvm"} - ] - } - ] - }, - {"name":"sdc", "mountpoint":null, "type":"disk", - "children": [ - {"name":"sdc1", "mountpoint":"/mnt/tmp", "type":"part"} - ] - } - ] -} diff --git a/cli_tools/import_precheck/precheck/testdata/lsblk-one-partition.json b/cli_tools/import_precheck/precheck/testdata/lsblk-one-partition.json deleted file mode 100644 index 13bd3da26..000000000 --- a/cli_tools/import_precheck/precheck/testdata/lsblk-one-partition.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "blockdevices": [ - {"name": "sda", "mountpoint": null, "type": "disk", - "children": [ - {"name": "sda1", "mountpoint": "/", "type": "part"} - ] - } - ] -} diff --git a/cli_tools/import_precheck/precheck/testdata/lsblk-single-disk-lvm.json b/cli_tools/import_precheck/precheck/testdata/lsblk-single-disk-lvm.json deleted file mode 100644 index 2654fcbb4..000000000 --- a/cli_tools/import_precheck/precheck/testdata/lsblk-single-disk-lvm.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "blockdevices": [ - {"name":"sda", "mountpoint":null, "type":"disk", - "children": [ - {"name":"sda1", "mountpoint":"/boot/efi", "type":"part"}, - {"name":"sda2", "mountpoint":"/boot", "type":"part"}, - {"name":"sda3", "mountpoint":null, "type":"part", - "children": [ - {"name":"root", "mountpoint":"/", "type":"lvm"}, - {"name":"swap", "mountpoint":"[SWAP]", "type":"lvm"} - ] - } - ] - } - ] -} diff --git a/cli_tools/mocks/mock_shell_exececutor.go b/cli_tools/mocks/mock_shell_exececutor.go new file mode 100644 index 000000000..1e4da4c2f --- /dev/null +++ b/cli_tools/mocks/mock_shell_exececutor.go @@ -0,0 +1,88 @@ +// Copyright 2021 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Code generated by MockGen. DO NOT EDIT. +// Source: executor.go + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockShellExecutor is a mock of Executor interface. +type MockShellExecutor struct { + ctrl *gomock.Controller + recorder *MockShellExecutorMockRecorder +} + +// MockShellExecutorMockRecorder is the mock recorder for MockShellExecutor. +type MockShellExecutorMockRecorder struct { + mock *MockShellExecutor +} + +// NewMockShellExecutor creates a new mock instance. +func NewMockShellExecutor(ctrl *gomock.Controller) *MockShellExecutor { + mock := &MockShellExecutor{ctrl: ctrl} + mock.recorder = &MockShellExecutorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockShellExecutor) EXPECT() *MockShellExecutorMockRecorder { + return m.recorder +} + +// Exec mocks base method. +func (m *MockShellExecutor) Exec(program string, args ...string) (string, error) { + m.ctrl.T.Helper() + varargs := []interface{}{program} + for _, a := range args { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Exec", varargs...) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Exec indicates an expected call of Exec. +func (mr *MockShellExecutorMockRecorder) Exec(program interface{}, args ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{program}, args...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Exec", reflect.TypeOf((*MockShellExecutor)(nil).Exec), varargs...) +} + +// ExecLines mocks base method. +func (m *MockShellExecutor) ExecLines(program string, args ...string) ([]string, error) { + m.ctrl.T.Helper() + varargs := []interface{}{program} + for _, a := range args { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "ExecLines", varargs...) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ExecLines indicates an expected call of ExecLines. +func (mr *MockShellExecutorMockRecorder) ExecLines(program interface{}, args ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{program}, args...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExecLines", reflect.TypeOf((*MockShellExecutor)(nil).ExecLines), varargs...) +}