Skip to content

Commit

Permalink
Where possible write fstab entries using the device UUID
Browse files Browse the repository at this point in the history
  • Loading branch information
@wallyworld
wallyworld committed Jul 17, 2020
1 parent 9265643 commit 796e151
Show file tree
Hide file tree
Showing 11 changed files with 290 additions and 62 deletions.
25 changes: 22 additions & 3 deletions apiserver/common/storagecommon/blockdevices.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,33 @@ func MatchingBlockDevice(
}

if attachmentInfo.DeviceLink != "" {
// We'll prefer to use a mounted partition if available.
// This will be the case for filesystem storage; it will be part1.
var devWithUUID, parentDev *state.BlockDeviceInfo
devMatch:
for _, dev := range blockDevices {
for _, link := range dev.DeviceLinks {
if attachmentInfo.DeviceLink == link {
logger.Tracef("device link match on %v", attachmentInfo.DeviceLink)
return &dev, true
if attachmentInfo.DeviceLink == link || attachmentInfo.DeviceLink+"-part1" == link {
devCopy := dev
if dev.UUID != "" {
devWithUUID = &devCopy
} else {
parentDev = &devCopy
}
}
if devWithUUID != nil {
break devMatch
}
}
}
if devWithUUID != nil {
logger.Tracef("device link with UUID match on %v", attachmentInfo.DeviceLink)
return devWithUUID, true
}
if parentDev != nil {
logger.Tracef("device link without UUID match on %v", attachmentInfo.DeviceLink)
return parentDev, true
}
logger.Tracef("no match for block device dev link: %v", attachmentInfo.DeviceLink)
}

Expand Down
34 changes: 30 additions & 4 deletions apiserver/common/storagecommon/blockdevices_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,37 @@ func (s *BlockDeviceSuite) TestBlockDevicesGCE(c *gc.C) {
})
}

func (s *BlockDeviceSuite) TestBlockDevicesGCEPreferUUID(c *gc.C) {
blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(gceTestBlockDevices, gceTestVolumeInfo, gceTestAttachmentInfoForUUID, gceTestPlanBlockInfo)
c.Assert(ok, jc.IsTrue)
c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
DeviceName: "sda1",
DeviceLinks: []string{
"/dev/disk/by-id/google-persistent-disk-0-part1",
"/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0-part1",
"/dev/disk/by-label/cloudimg-rootfs",
"/dev/disk/by-partuuid/8c3230b8-1ecf-45d9-a6c8-41f4bc51a849",
"/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0-part1",
"/dev/disk/by-uuid/27514291-b7f6-4b83-bc8a-07c7d7467218",
},
Label: "cloudimg-rootfs",
UUID: "27514291-b7f6-4b83-bc8a-07c7d7467218",
HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0",
BusAddress: "scsi@0:0.1.0",
Size: 0x2790,
FilesystemType: "ext4",
InUse: true,
MountPoint: "/",
SerialId: "0Google_PersistentDisk_persistent-disk-0",
})
}

var (
gceTestBlockDevices = []state.BlockDeviceInfo{{DeviceName: "loop0", Size: 0x59, FilesystemType: "squashfs", InUse: true, MountPoint: "/snap/core/7713"}, {DeviceName: "loop1", Size: 0x42, FilesystemType: "squashfs", InUse: true, MountPoint: "/snap/google-cloud-sdk/102"}, {DeviceName: "sda", DeviceLinks: []string{"/dev/disk/by-id/google-persistent-disk-0", "/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0"}, HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0", BusAddress: "scsi@0:0.1.0", Size: 0x2800, InUse: true, SerialId: "0Google_PersistentDisk_persistent-disk-0"}, {DeviceName: "sda1", DeviceLinks: []string{"/dev/disk/by-id/google-persistent-disk-0-part1", "/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0-part1", "/dev/disk/by-label/cloudimg-rootfs", "/dev/disk/by-partuuid/8c3230b8-1ecf-45d9-a6c8-41f4bc51a849", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0-part1", "/dev/disk/by-uuid/27514291-b7f6-4b83-bc8a-07c7d7467218"}, Label: "cloudimg-rootfs", UUID: "27514291-b7f6-4b83-bc8a-07c7d7467218", HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0", BusAddress: "scsi@0:0.1.0", Size: 0x2790, FilesystemType: "ext4", InUse: true, MountPoint: "/", SerialId: "0Google_PersistentDisk_persistent-disk-0"}, {DeviceName: "sda14", DeviceLinks: []string{"/dev/disk/by-id/google-persistent-disk-0-part14", "/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0-part14", "/dev/disk/by-partuuid/d82926ca-95f8-46fe-ab94-61bb6cc2a879", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0-part14"}, HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0", BusAddress: "scsi@0:0.1.0", Size: 0x4, SerialId: "0Google_PersistentDisk_persistent-disk-0"}, {DeviceName: "sda15", DeviceLinks: []string{"/dev/disk/by-id/google-persistent-disk-0-part15", "/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0-part15", "/dev/disk/by-label/UEFI", "/dev/disk/by-partuuid/264a576d-0211-45fa-9bdd-e674c08517f4", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0-part15", "/dev/disk/by-uuid/9889-C357"}, Label: "UEFI", UUID: "9889-C357", HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0", BusAddress: "scsi@0:0.1.0", Size: 0x6a, FilesystemType: "vfat", InUse: true, MountPoint: "/boot/efi", SerialId: "0Google_PersistentDisk_persistent-disk-0"}, {DeviceName: "sdb", DeviceLinks: []string{"/dev/disk/by-id/google-us-east1-d-9082123458182365433", "/dev/disk/by-id/scsi-0Google_PersistentDisk_us-east1-d-9082123458182365433", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:2:0"}, HardwareId: "scsi-0Google_PersistentDisk_us-east1-d-9082123458182365433", BusAddress: "scsi@0:0.2.0", Size: 0x2800, SerialId: "0Google_PersistentDisk_us-east1-d-9082123458182365433"}, {DeviceName: "sdc", DeviceLinks: []string{"/dev/disk/by-id/google-us-east1-d-2880464023067017457", "/dev/disk/by-id/scsi-0Google_PersistentDisk_us-east1-d-2880464023067017457", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:3:0"}, HardwareId: "scsi-0Google_PersistentDisk_us-east1-d-2880464023067017457", BusAddress: "scsi@0:0.3.0", Size: 0x2800, SerialId: "0Google_PersistentDisk_us-east1-d-2880464023067017457"}, {DeviceName: "sdd", DeviceLinks: []string{"/dev/disk/by-id/google-us-east1-d-5005808815463186635", "/dev/disk/by-id/scsi-0Google_PersistentDisk_us-east1-d-5005808815463186635", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:4:0"}, HardwareId: "scsi-0Google_PersistentDisk_us-east1-d-5005808815463186635", BusAddress: "scsi@0:0.4.0", Size: 0x2800, SerialId: "0Google_PersistentDisk_us-east1-d-5005808815463186635"}}
gceTestPlanBlockInfo = state.BlockDeviceInfo{}
gceTestVolumeInfo = state.VolumeInfo{Size: 0x2800, Pool: "gce", VolumeId: "us-east1-d--515cb1ad-5d23-4d53-8cc1-b79c75a03908", Persistent: true}
gceTestAttachmentInfo = state.VolumeAttachmentInfo{DeviceLink: "/dev/disk/by-id/google-us-east1-d-5005808815463186635", ReadOnly: false, PlanInfo: (*state.VolumeAttachmentPlanInfo)(nil)}
gceTestBlockDevices = []state.BlockDeviceInfo{{DeviceName: "loop0", Size: 0x59, FilesystemType: "squashfs", InUse: true, MountPoint: "/snap/core/7713"}, {DeviceName: "loop1", Size: 0x42, FilesystemType: "squashfs", InUse: true, MountPoint: "/snap/google-cloud-sdk/102"}, {DeviceName: "sda", DeviceLinks: []string{"/dev/disk/by-id/google-persistent-disk-0", "/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0"}, HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0", BusAddress: "scsi@0:0.1.0", Size: 0x2800, InUse: true, SerialId: "0Google_PersistentDisk_persistent-disk-0"}, {DeviceName: "sda1", DeviceLinks: []string{"/dev/disk/by-id/google-persistent-disk-0-part1", "/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0-part1", "/dev/disk/by-label/cloudimg-rootfs", "/dev/disk/by-partuuid/8c3230b8-1ecf-45d9-a6c8-41f4bc51a849", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0-part1", "/dev/disk/by-uuid/27514291-b7f6-4b83-bc8a-07c7d7467218"}, Label: "cloudimg-rootfs", UUID: "27514291-b7f6-4b83-bc8a-07c7d7467218", HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0", BusAddress: "scsi@0:0.1.0", Size: 0x2790, FilesystemType: "ext4", InUse: true, MountPoint: "/", SerialId: "0Google_PersistentDisk_persistent-disk-0"}, {DeviceName: "sda14", DeviceLinks: []string{"/dev/disk/by-id/google-persistent-disk-0-part14", "/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0-part14", "/dev/disk/by-partuuid/d82926ca-95f8-46fe-ab94-61bb6cc2a879", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0-part14"}, HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0", BusAddress: "scsi@0:0.1.0", Size: 0x4, SerialId: "0Google_PersistentDisk_persistent-disk-0"}, {DeviceName: "sda15", DeviceLinks: []string{"/dev/disk/by-id/google-persistent-disk-0-part15", "/dev/disk/by-id/scsi-0Google_PersistentDisk_persistent-disk-0-part15", "/dev/disk/by-label/UEFI", "/dev/disk/by-partuuid/264a576d-0211-45fa-9bdd-e674c08517f4", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:1:0-part15", "/dev/disk/by-uuid/9889-C357"}, Label: "UEFI", UUID: "9889-C357", HardwareId: "scsi-0Google_PersistentDisk_persistent-disk-0", BusAddress: "scsi@0:0.1.0", Size: 0x6a, FilesystemType: "vfat", InUse: true, MountPoint: "/boot/efi", SerialId: "0Google_PersistentDisk_persistent-disk-0"}, {DeviceName: "sdb", DeviceLinks: []string{"/dev/disk/by-id/google-us-east1-d-9082123458182365433", "/dev/disk/by-id/scsi-0Google_PersistentDisk_us-east1-d-9082123458182365433", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:2:0"}, HardwareId: "scsi-0Google_PersistentDisk_us-east1-d-9082123458182365433", BusAddress: "scsi@0:0.2.0", Size: 0x2800, SerialId: "0Google_PersistentDisk_us-east1-d-9082123458182365433"}, {DeviceName: "sdc", DeviceLinks: []string{"/dev/disk/by-id/google-us-east1-d-2880464023067017457", "/dev/disk/by-id/scsi-0Google_PersistentDisk_us-east1-d-2880464023067017457", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:3:0"}, HardwareId: "scsi-0Google_PersistentDisk_us-east1-d-2880464023067017457", BusAddress: "scsi@0:0.3.0", Size: 0x2800, SerialId: "0Google_PersistentDisk_us-east1-d-2880464023067017457"}, {DeviceName: "sdd", DeviceLinks: []string{"/dev/disk/by-id/google-us-east1-d-5005808815463186635", "/dev/disk/by-id/scsi-0Google_PersistentDisk_us-east1-d-5005808815463186635", "/dev/disk/by-path/pci-0000:00:03.0-scsi-0:0:4:0"}, HardwareId: "scsi-0Google_PersistentDisk_us-east1-d-5005808815463186635", BusAddress: "scsi@0:0.4.0", Size: 0x2800, SerialId: "0Google_PersistentDisk_us-east1-d-5005808815463186635"}}
gceTestPlanBlockInfo = state.BlockDeviceInfo{}
gceTestVolumeInfo = state.VolumeInfo{Size: 0x2800, Pool: "gce", VolumeId: "us-east1-d--515cb1ad-5d23-4d53-8cc1-b79c75a03908", Persistent: true}
gceTestAttachmentInfo = state.VolumeAttachmentInfo{DeviceLink: "/dev/disk/by-id/google-us-east1-d-5005808815463186635", ReadOnly: false, PlanInfo: (*state.VolumeAttachmentPlanInfo)(nil)}
gceTestAttachmentInfoForUUID = state.VolumeAttachmentInfo{DeviceLink: "/dev/disk/by-id/google-persistent-disk-0", ReadOnly: false, PlanInfo: (*state.VolumeAttachmentPlanInfo)(nil)}
)

func (s *BlockDeviceSuite) TestBlockDevicesOpenStack(c *gc.C) {
Expand Down
1 change: 1 addition & 0 deletions apiserver/common/storagecommon/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ func volumeAttachmentDevicePath(
return storage.BlockDevicePath(storage.BlockDevice{
HardwareId: volumeInfo.HardwareId,
WWN: volumeInfo.WWN,
UUID: blockDevice.UUID,
DeviceName: deviceName,
DeviceLinks: deviceLinks,
})
Expand Down
4 changes: 4 additions & 0 deletions storage/path.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

const (
diskByID = "/dev/disk/by-id"
diskByUUID = "/dev/disk/by-uuid"
diskByWWN = "/dev/disk/by-id/wwn-"
diskByDeviceName = "/dev"
)
Expand All @@ -26,6 +27,9 @@ func BlockDevicePath(device BlockDevice) (string, error) {
if device.HardwareId != "" {
return path.Join(diskByID, device.HardwareId), nil
}
if device.UUID != "" {
return path.Join(diskByUUID, device.UUID), nil
}
if len(device.DeviceLinks) > 0 {
// return the first device link in the list
return device.DeviceLinks[0], nil
Expand Down
7 changes: 7 additions & 0 deletions storage/path_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ func (s *BlockDevicePathSuite) TestBlockDevicePathWWN(c *gc.C) {
}, "/dev/disk/by-id/wwn-rr!")
}

func (s *BlockDevicePathSuite) TestBlockDevicePathUUID(c *gc.C) {
testBlockDevicePath(c, storage.BlockDevice{
UUID: "deadbeaf",
DeviceName: "name",
}, "/dev/disk/by-uuid/deadbeaf")
}

func (s *BlockDevicePathSuite) TestBlockDevicePathDeviceName(c *gc.C) {
testBlockDevicePath(c, storage.BlockDevice{
DeviceName: "name",
Expand Down
120 changes: 93 additions & 27 deletions storage/provider/managedfs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ package provider

import (
"bufio"
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
"strings"
"unicode"

"github.com/juju/collections/set"
"github.com/juju/errors"
"github.com/juju/names/v4"

Expand Down Expand Up @@ -154,7 +156,7 @@ func (s *managedFilesystemSource) attachFilesystem(arg storage.FilesystemAttachm
if isDiskDevice(devicePath) {
devicePath = partitionDevicePath(devicePath)
}
if err := mountFilesystem(s.run, s.dirFuncs, devicePath, arg.Path, arg.ReadOnly); err != nil {
if err := mountFilesystem(s.run, s.dirFuncs, devicePath, blockDevice.UUID, arg.Path, arg.ReadOnly); err != nil {
return nil, errors.Trace(err)
}
return &storage.FilesystemAttachment{
Expand Down Expand Up @@ -207,7 +209,7 @@ func createFilesystem(run runCommandFunc, devicePath string) error {
return nil
}

func mountFilesystem(run runCommandFunc, dirFuncs dirFuncs, devicePath, mountPoint string, readOnly bool) error {
func mountFilesystem(run runCommandFunc, dirFuncs dirFuncs, devicePath, UUID, mountPoint string, readOnly bool) error {
logger.Debugf("attempting to mount filesystem on %q at %q", devicePath, mountPoint)
if err := dirFuncs.mkDirAll(mountPoint, 0755); err != nil {
return errors.Annotate(err, "creating mount point")
Expand All @@ -218,18 +220,17 @@ func mountFilesystem(run runCommandFunc, dirFuncs dirFuncs, devicePath, mountPoi
}
if mounted {
logger.Debugf("filesystem on %q already mounted at %q", mountSource, mountPoint)
return nil
}
var args []string
if readOnly {
args = append(args, "-o", "ro")
}
args = append(args, devicePath, mountPoint)
if _, err := run("mount", args...); err != nil {
return errors.Annotate(err, "mount failed")
} else {
var args []string
if readOnly {
args = append(args, "-o", "ro")
}
args = append(args, devicePath, mountPoint)
if _, err := run("mount", args...); err != nil {
return errors.Annotate(err, "mount failed")
}
logger.Debugf("mounted filesystem on %q at %q", devicePath, mountPoint)
}
logger.Infof("mounted filesystem on %q at %q", devicePath, mountPoint)

// Look for the mtab entry resulting from the mount and copy it to fstab.
// This ensures the mount is available available after a reboot.
etcDir := dirFuncs.etcDir()
Expand All @@ -240,7 +241,7 @@ func mountFilesystem(run runCommandFunc, dirFuncs dirFuncs, devicePath, mountPoi
if mtabEntry == "" {
return nil
}
return addFstabEntry(etcDir, devicePath, mountPoint, mtabEntry)
return ensureFstabEntry(etcDir, devicePath, UUID, mountPoint, mtabEntry)
}

// extractMtabEntry returns any /etc/mtab entry for the specified
Expand Down Expand Up @@ -270,33 +271,98 @@ func extractMtabEntry(etcDir string, devicePath, mountPoint string) (string, err
return "", nil
}

// addFstabEntry creates an entry in /etc/fstab for the specified
// ensureFstabEntry creates an entry in /etc/fstab for the specified
// device path and mount point so long as there's no existing entry already.
func addFstabEntry(etcDir string, devicePath, mountPoint, entry string) error {
f, err := os.OpenFile(filepath.Join(etcDir, "fstab"), os.O_CREATE|os.O_APPEND|os.O_RDWR, 0644)
if err != nil {
func ensureFstabEntry(etcDir, devicePath, UUID, mountPoint, entry string) error {
f, err := os.Open(filepath.Join(etcDir, "fstab"))
if err != nil && !os.IsNotExist(err) {
return errors.Annotate(err, "opening /etc/fstab")
}
defer f.Close()
if err == nil {
defer f.Close()
}

// Ensure there's no entry there already
newFsTab, err := ioutil.TempFile(etcDir, "juju-fstab-")
if err != nil {
return errors.Trace(err)
}
defer func() {
newFsTab.Close()
os.Remove(newFsTab.Name())
}()
if err := os.Chmod(newFsTab.Name(), 0644); err != nil {
return errors.Trace(err)
}

// Add nofail if not there already
resultFields := strings.Fields(entry)
options := set.NewStrings()
if len(resultFields) >= 4 {
options = set.NewStrings(strings.Split(resultFields[3], ",")...)
}
if !options.Contains("nofail") {
options.Add("nofail")
opts := strings.Join(options.SortedValues(), ",")
if len(resultFields) >= 4 {
resultFields[3] = opts
} else {
resultFields = append(resultFields, opts)
}
}

uuidField := "UUID=" + UUID
addNewEntry := true
// Scan all the fstab lines, searching for one
// which describes the entry we want to create.
scanner := bufio.NewScanner(f)
for scanner.Scan() {
for f != nil && scanner.Scan() {
line := scanner.Text()
fields := strings.Fields(line)
if len(fields) >= 2 && fields[0] == devicePath && fields[1] == mountPoint {
return nil
if len(fields) < 2 || fields[1] != mountPoint {
goto writeLine
}
// Is the line the UUID based mount entry we want.
if fields[0] == uuidField {
addNewEntry = false
goto writeLine
}
// Is the line for some other entry.
if fields[0] != devicePath {
goto writeLine
}
// We have a match, if UUID is not yet known, retain the line.
if UUID == "" {
addNewEntry = false
goto writeLine
}
continue
writeLine:
_, err := newFsTab.WriteString(line + "\n")
if err != nil {
return errors.Trace(err)
}
}
if err := scanner.Err(); err != nil {
return errors.Trace(err)
}

// The entry will be written at the end of the fstab file.
if _, err = f.WriteString("\n" + entry + "\n"); err != nil {
return errors.Annotate(err, "writing /etc/fstab")
if addNewEntry {
if UUID != "" {
if len(resultFields) >= 2 { // just being defensive, check should never fail.
_, err := newFsTab.WriteString(fmt.Sprintf("# %s was on %s during installation\n", resultFields[1], resultFields[0]))
if err != nil {
return errors.Trace(err)
}
}
resultFields[0] = uuidField
}
_, err := newFsTab.WriteString(strings.Join(resultFields, " ") + "\n")
if err != nil {
return errors.Trace(err)
}

}
return nil
return os.Rename(newFsTab.Name(), filepath.Join(etcDir, "fstab"))
}

func maybeUnmount(run runCommandFunc, dirFuncs dirFuncs, mountPoint string) error {
Expand Down
Loading

0 comments on commit 796e151

Please sign in to comment.