feat(mount-disks): mount auto-provisioned disks by UUID - #65
Conversation
Linux device names (/dev/sd*) aren't stable — adding a controller, re-seating a drive, or a kernel upgrade can reorder them on reboot, at which point fstab entries keyed by device path mount the wrong disk (or none). Switch the fstab line to a UUID= entry so the mount follows the filesystem, not the bus position. - Probe the UUID with blkid after mkfs, retrying a few times so udev has a chance to settle. Abort the deploy on failure rather than writing a broken fstab entry. - Script now writes "UUID=<uuid> <mnt> ..." and mounts by mount point so /etc/fstab is the single source of truth. - Pre-existing fstab entries are untouched; the change only affects new disks provisioned by --mount-disks. - Also quote the SUDO_PASS interpolation in the script invocation so passwords containing ' are handled correctly. Credit: cherry-picked from the UUID-mount portion of #4.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the disk deployment logic to use filesystem UUIDs instead of device paths for mounting and /etc/fstab entries, improving reliability across reboots. A new probeDiskUUID function with a retry loop is added to handle potential delays in UUID availability after formatting. Feedback suggests ensuring blkid runs with root privileges to accurately retrieve the UUID and making the /etc/fstab update idempotent to prevent duplicate entries during re-runs.
| const attempts = 5 | ||
| var lastErr error | ||
| for i := 0; i < attempts; i++ { | ||
| out, err := op.Output(fmt.Sprintf("blkid -s UUID -o value %s", shellSingleQuote(path))) |
There was a problem hiding this comment.
The blkid command often requires root privileges to access raw block devices and retrieve the most up-to-date UUID (bypassing or updating the cache), especially immediately after a mkfs operation. Consider wrapping the command with sudo if a password is provided, similar to the logic in the m.sudo helper.
cmd := fmt.Sprintf("blkid -s UUID -o value %s", shellSingleQuote(path))
if m.sudoPass != "" {
cmd = fmt.Sprintf("echo %s | sudo -S %s", shellSingleQuote(m.sudoPass), cmd)
}
out, err := op.Output(cmd)| info "mount ${DEVICE_PATH} ${MOUNT_POINT}" | ||
| $SUDO mount ${DEVICE_PATH} ${MOUNT_POINT} | ||
| info "add UUID=${DEVICE_UUID} (${DEVICE_PATH}) ${MOUNT_POINT} to fstab" | ||
| echo "UUID=${DEVICE_UUID} ${MOUNT_POINT} ext4 noatime 0 2" | $SUDO tee -a /etc/fstab |
There was a problem hiding this comment.
To ensure the script is idempotent and avoid appending duplicate entries to /etc/fstab if the deployment is re-run (e.g., after a partial failure), consider checking if the UUID already exists in the file before appending.
| echo "UUID=${DEVICE_UUID} ${MOUNT_POINT} ext4 noatime 0 2" | $SUDO tee -a /etc/fstab | |
| if ! grep -q "UUID=${DEVICE_UUID}" /etc/fstab; then | |
| echo "UUID=${DEVICE_UUID} ${MOUNT_POINT} ext4 noatime 0 2" | $SUDO tee -a /etc/fstab | |
| fi |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- blkid -p probes the superblock directly instead of reading the cache, but it needs root on block devices. Wrap with sudo -S when sudoPass is set, mirroring m.sudo(). - prepare_disk.sh now guards both the fstab append and the mount so a re-run after partial failure (disk formatted, fstab written, but mount errored) doesn't duplicate the fstab line or fail with "already mounted".
1 participant
Summary
Linux device names (`/dev/sd*`) aren't stable — adding a controller, re-seating a drive, or a kernel upgrade can reorder them on reboot. When fstab entries are keyed by device path, the volume server can come up pointing at the wrong disk (or fail to mount at all).
This PR switches new `--mount-disks` fstab entries to mount by filesystem UUID:
Backward compatibility: pre-existing fstab entries are untouched. Hosts with a prior `/dev/sdX /data1 …` entry keep working as before; the change only affects newly provisioned disks.
Credit: cherry-picked (and reworked) from the UUID portion of #4 (#4). The original PR's blind `time.Sleep(2s)` is replaced with `udevadm settle` + a retrying `blkid` probe.
Test plan