Feat: Introduce Transparent LVM Support for azure-chroot Builder#583
Feat: Introduce Transparent LVM Support for azure-chroot Builder#583tanmay-hc merged 14 commits intohashicorp:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds transparent LVM handling to the azure-chroot builder so LVM-based source disks can be mounted automatically, plus a new hook to run host-side commands just before unmount/teardown.
Changes:
- Introduces a new
StepSetupLVMstep that detects/activates VGs and selects a root LV, with cleanup to deactivate VGs. - Adds
pre_unmount_commandssupport via a new build step executed after provisioning and before unmount. - Extends builder config/docs (
lvm_root_device,pre_unmount_commands) and replaces SDK early-cleanup with an LVM-aware version.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/builders/chroot.mdx | Documents new LVM behavior, options, and examples. |
| docs-partials/builder/azure/chroot/Config-not-required.mdx | Adds config reference entries for lvm_root_device and pre_unmount_commands. |
| builder/azure/chroot/step_setup_lvm.go | Implements LVM discovery, activation, root LV selection, and VG deactivation cleanup. |
| builder/azure/chroot/step_setup_lvm_other.go | Provides no-op step stub for unsupported OS build tags. |
| builder/azure/chroot/step_setup_lvm_test.go | Verifies step presence/order and config passthrough for new steps. |
| builder/azure/chroot/step_pre_unmount_commands.go | Runs host-side commands before unmount/LVM deactivation. |
| builder/azure/chroot/step_mount_device.go | Adjusts mount device computation when LVM is active. |
| builder/azure/chroot/step_early_cleanup.go | Adds custom early cleanup ordering to include LVM deactivation. |
| builder/azure/chroot/builder.go | Adds config fields, validation, and wires new steps into the chain. |
| builder/azure/chroot/builder.hcl2spec.go | Regenerates HCL2 spec to include new fields. |
| .web-docs/components/builder/chroot/README.md | Regenerated web docs reflecting new LVM support and options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `vgscan`, `vgchange`, `lvs`) to be installed on the host VM. The `partprobe` | ||
| and `udevadm` utilities are also used for device discovery. |
There was a problem hiding this comment.
The docs list lvm2 utilities required for LVM support but omits a few commands used by the implementation (e.g., pvs, lvchange, vgmknodes; plus blkid/dmsetup are used for swap detection/diagnostics). Either update the note to include these dependencies or clarify which ones are strictly required vs. best-effort diagnostics.
| `vgscan`, `vgchange`, `lvs`) to be installed on the host VM. The `partprobe` | |
| and `udevadm` utilities are also used for device discovery. | |
| `pvs`, `vgscan`, `vgchange`, `lvchange`, `lvs`, `vgmknodes`) to be installed on | |
| the host VM. The `partprobe` and `udevadm` utilities are also used for device | |
| discovery, and `blkid`/`dmsetup` are used for best-effort swap detection and | |
| diagnostics (the builder can run without them, but with reduced introspection). |
| func Test_buildsteps_LVM(t *testing.T) { | ||
| info := &client.ComputeInfo{ | ||
| Location: "northpole", | ||
| Name: "unittestVM", | ||
| ResourceGroupName: "unittestResourceGroup", |
There was a problem hiding this comment.
These tests validate step ordering/config passthrough, but they don’t cover the new LV-selection heuristics in step_setup_lvm.go (attr filtering, swap exclusion via blkid, name matching, /dev/mapper parsing, etc.). Consider adding unit tests for the pure helper functions and/or refactoring command output parsing so findRootLV can be tested deterministically.
|
I've accepted the changes raised by CoPilot, from its additions in this thread; tests around LVM have been added, with cases expanded using AI - for broader logic coverage. |
|
|
||
| func (s *StepSetupLVM) CleanupFunc(state multistep.StateBag) error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Can you explain what's this "other" file?
There was a problem hiding this comment.
step_setup_lvm_other.go acts as a non-functional stub file, ensuring that it can be compiled on multiple platforms. Without this stub file, attempting to compile this on Windows or Mac will result in a compilation error - as it'll be expecting a "StepSetupLVM" (which wouldn't be provided without this).
| return multistep.ActionContinue | ||
| } | ||
|
|
||
| func (s *StepPreUnmountCommands) Cleanup(state multistep.StateBag) {} |
There was a problem hiding this comment.
Consider adding some tests for this step
|
@MrCaedes Please rebase the branch. Changes looks good to me. |
- Introduced LVMRootDevice configuration option to specify the root device path for LVM. - Added PreUnmountCommands to execute commands before unmounting the chroot. - Implemented StepSetupLVM to detect and activate LVM volume groups on attached disks. - Created StepPreUnmountCommands to run user-defined commands prior to unmounting. - Developed custom StepEarlyCleanup to handle LVM deactivation during cleanup. - Updated documentation to include LVM support details and usage examples. - Added tests to verify LVM integration and step order in the build process.
…osssubscription-acg
Removing redundant message (i.e., originally outputted unconditionally)
Removes hardcoded values in favour of a const, in line with other filles; moves other timouts. UdevSettle passed as CLI (hence string)
Respect context cancellation; remove blind loops. Retain `attempt` as gate for retry loop and for logging purposes.
Switch to "slices.Contains" for improved readability; negligible performance improvement.
Description
Adds simplified built-in LVM support to the
azure-chrootbuilder.Multiple Azure Marketplace images (i.e., RHEL 8/9/10, Oracle Linux, etc) ship with LVM-based disk layouts. Previously, the
azure-chrootbuilder only supported mounting raw partitions - making these images unusable without manual workarounds which are squirreled away in Enterprise Wikis.This PR adds transparent LVM detection and activation so that LVM-based source disks "just work" - testing based on RHEL 8 / 9 / 10, and follows on from the changes made as part of #582. Fist bump to VRay27 for their continued support in testing 🙌
What changed:
step_setup_lvm.go: New build step; leveragespartprobeandudevadm settlefor device discovery, alongsidepvscanandvgchange.step_pre_unmount_commands.go: Facilitate users in running commands on the host after provisioning but before unmount (e.g.,fstrim,sync, etc - traditionally done using a shell script step)step_early_cleanup.go: handles teardown for LVM; built-in cleanup isn't LVM-aware.builder.go: addition oflvm_root_deviceto override auto-detection with explicit device paths (w/ validation),pre_unmount_commandsfor on-host commands to run before unmount.Updated required documentation; regenerated
hcl2specviago generate- and webdocs usingmake.Rollback Plan
If a change needs to be reverted, we will roll out an update to the code within 7 days.
Changes to Security Controls
No changes to security controls; LVM step(s) execute standard Linux storage utilities on the host VM. No new changes introduced beyond this.