Warn when persistent disk filesystem is smaller than its partition - #465
Warn when persistent disk filesystem is smaller than its partition#465neddp wants to merge 1 commit into
Conversation
When a disk resize partitions successfully but the filesystem grow does not complete, subsequent deploys detect the partition already spans the disk and never revisit the filesystem — leaving it silently smaller than its partition with no indication anything is wrong. Add FilesystemNeedsGrow (ext4) to the formatter: it compares the ext4 superblock size (dumpe2fs) against the block device size (blockdev), using the same 100MB delta as SinglePartitionNeedsResize. When the partition already matches the disk, AdjustPersistentDiskPartitioning now runs this check and logs a warning if the filesystem is smaller, surfacing the condition for operator follow-up. Detection only: the filesystem is not mounted at this point (so xfs_growfs is not applicable) and the underlying cause often requires manual intervention, so no automatic grow is attempted.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
WalkthroughThe formatter contract now exposes filesystem growth checks, and the fake formatter records calls with configurable results and errors. The Linux formatter inspects ext4 metadata and block-device size, then applies a 100 MB growth threshold. Persistent disk partitioning invokes the check after formatting and logs warnings for inspection errors or incomplete growth without performing filesystem growth. Tests cover detection, parsing failures, non-ext4 filesystems, and partitioning behavior. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Pull request overview
Adds detection and operator-facing warnings for the case where a persistent disk partition already spans the disk, but the ext4 filesystem inside it remains smaller (e.g., after a prior grow attempt failed), preventing silent “stuck small filesystem” situations across subsequent deploys.
Changes:
- Add
FilesystemNeedsGrow(ext4-only) to the Linux disk formatter, comparing ext4 superblock size (dumpe2fs -h) vs block device size (blockdev --getsize64) with a 100MB tolerance. - Invoke the needs-grow check from
AdjustPersistentDiskPartitioningwhen the partition does not need resizing, logging a warning (but not attempting to grow). - Add unit/spec coverage for the formatter check and the new warn-path behavior in the Linux platform.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| platform/linux_platform.go | Calls FilesystemNeedsGrow on the non-resize path and logs warnings when mismatch is detected (or when the check fails). |
| platform/linux_platform_test.go | Adds specs ensuring the new check is invoked and does not mount/grow/fail deployments. |
| platform/disk/linux_formatter.go | Implements ext4-only needs-grow detection using blockdev + dumpe2fs. |
| platform/disk/linux_formatter_test.go | Adds unit tests for needs-grow true/false and error cases. |
| platform/disk/formatter_interface.go | Extends the Formatter interface with FilesystemNeedsGrow. |
| platform/disk/fakes/fake_formatter.go | Updates fake formatter to implement the new interface method for tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| partitionSize, err := f.blockDeviceSize(partitionPath) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| fsSize, err := f.ext4FilesystemSize(partitionPath) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| return significantlySmallerThan(fsSize, partitionSize, ConvertFromMbToBytes(deltaSize)), nil | ||
| } |
What is this change about?
When a persistent disk is resized,
AdjustPersistentDiskPartitioningresizes the partition and grows the filesystem. If the partition resize succeeds but the filesystem grow does not complete, every subsequent deploy sees the partition already spanning the disk, takes the branch that only partitions and formats, and never revisits the filesystem - leaving it silently smaller than its partition with no indication anything is wrong.This change adds a
FilesystemNeedsGrowcheck (ext4) to the formatter. When the partition already matches the disk size,AdjustPersistentDiskPartitioningruns the check and logs a warning if the filesystem is smaller than its partition, surfacing the condition for operator follow-up.Detection only - no automatic grow is attempted:
AdjustPersistentDiskPartitioningruns beforeMountPersistentDisk), soxfs_growfs, which requires a mounted filesystem, is not applicable.FilesystemNeedsGrowcompares the ext4 superblock size (dumpe2fs -h) against the block device size (blockdev --getsize64), using the same 100MB delta asSinglePartitionNeedsResizeto avoid false positives from alignment rounding.Please provide contextual information.
Found while investigating a
resize2fs: Permission deniedfailure. Three nodes had pre-existing filesystem corruption. The partition resize succeeded,GrowFilesystemfailed, and subsequent deploys silently succeeded - leaving those nodes with 98G filesystems on 1T volumes and no visible error.What tests have you run against this PR?
platformandplatform/diskunit test suites: 375 + 177 passed, 0 failedFilesystemNeedsGrowfor ext4 (smaller, already-full, command failures, missing superblock field, non-ext4); platform specs for the warn path (check invoked, no grow/mount, no deploy failure on mismatch or check error)How should this change be described in bosh-agent release notes?
The agent now logs a warning when a persistent disk's filesystem is smaller than its partition (indicating a previous grow did not complete), instead of silently leaving it unresized.
Does this PR introduce a breaking change?
No. The change only adds a read-only check and a log warning. No behaviour change to partitioning, formatting, or mounting.