feat: channel-based image notifications + erofs default#105
Conversation
… erofs Replace the 500ms polling loop in waitForImageReady() with a channel-based pub/sub notification system on the image manager, reducing build-to-SSE lag. Switch the default image format from ext4 to erofs (LZ4-compressed read-only filesystem) for faster, smaller rootfs images. The VM init mounts erofs first with an ext4 fallback for backward compatibility with legacy images. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sjmiller609
left a comment
sjmiller609
left a comment
There was a problem hiding this comment.
nice. and existing tests seem to cover risks well
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // Notify subscribers of terminal status | ||
| if status == StatusReady || status == StatusFailed { | ||
| m.notifyReady(ref.DigestHex(), status, err) | ||
| } |
There was a problem hiding this comment.
Dual notification paths risk duplicate subscriber notifications
Low Severity
notifyReady for StatusReady is called directly in buildImage at line 302 and also conditionally in updateStatusByDigest at line 337–339. Currently no double notification occurs because updateStatusByDigest is never called with StatusReady, making the StatusReady guard at line 337 dead code. However, having two separate notification paths for the same event is fragile — if the success flow is ever refactored to use updateStatusByDigest, subscribers would receive duplicate events on a buffered channel of size 1, causing the second send to be silently dropped.
Additional Locations (1)
… erofs Replace the 500ms polling loop in waitForImageReady() with a channel-based pub/sub notification system on the image manager, reducing build-to-SSE lag. Switch the default image format from ext4 to erofs (LZ4-compressed read-only filesystem) for faster, smaller rootfs images. The VM init mounts erofs first with an ext4 fallback for backward compatibility with legacy images. Log which filesystem type (erofs or ext4) was actually mounted so operators can verify erofs is being used and diagnose fallback scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Make DefaultImageFormat platform-aware: - Linux: erofs (compressed, smaller images) - Darwin: ext4 (VZ kernel doesn't have erofs support) This fixes the Darwin CI failure where VMs couldn't mount erofs rootfs. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| if event.Status == StatusReady { | ||
| return nil | ||
| } | ||
| return fmt.Errorf("image conversion failed") |
There was a problem hiding this comment.
StatusEvent.Err is populated but never consumed
Medium Severity
The StatusEvent.Err field is populated with the actual conversion error in notifyReady but never read by WaitForReady, which only checks event.Status and returns a generic "image conversion failed" string. The original error (e.g., "convert to erofs: mkfs.erofs failed: …") is silently discarded, making production debugging harder. The Err field is effectively dead code.
Additional Locations (1)
2 participants
Summary
waitForImageReady()in the builds manager previously polled every 500ms (up to 120 attempts). Now it delegates toimageManager.WaitForReady(), which uses a channel-based pub/sub system for instant notification when image conversion completes. Subscribes BEFORE re-checking status to avoid TOCTOU races. Includes a 30s retry loop for the initialGetImagecall to handle the race whereWaitForReadyis called before the registry's asyncImportLocalImagehas executed.DefaultImageFormatfrom ext4 to erofs (LZ4-compressed read-only filesystem) for smaller, faster rootfs images. UpdatesImageDigestPathfromrootfs.ext4torootfs.erofs. VM init now tries erofs first with ext4 fallback for backward compatibility with legacy images.Files changed
lib/images/manager.goStatusEvent,WaitForReady(), subscribe/unsubscribe/notify methods,readySubscribersmaplib/builds/manager.goWaitForReady()lib/builds/manager_test.goWaitForReadyto mock, addsync.RWMutexfor thread safetylib/builds/race_test.golib/images/disk.goDefaultImageFormattoFormatErofslib/paths/paths.gorootfs.ext4torootfs.erofslib/images/storage.golib/system/init/mount.goTest plan
go test -race ./lib/builds/...passes (including race detector)go build ./...compiles successfullygo vetpasses on all modified packagesstatus: "ready"rootfs.erofsfiles created with correct EROFS filesystem + LZ4 compression (filecommand confirms)🤖 Generated with Claude Code
Note
Medium Risk
Touches image lifecycle synchronization and changes on-disk rootfs format/default mounting behavior; failures would surface as builds that hang/fail to become ready or instances that can’t boot/mount images.
Overview
Builds no longer “go ready” while image conversion is still in flight:
builds.manager.waitForImageReadydrops polling and delegates to a newimages.Manager.WaitForReady, which uses a digest-keyed pub/sub to notify waiters onready/failedterminal states (with a short existence-poll to handle asyncImportLocalImage).Image export defaults change from ext4 to compressed
erofson Linux, while keeping ext4 on Darwin; paths now selectrootfs.erofsvsrootfs.ext4by OS, and init mounting trieserofsfirst with ext4 fallback. Tests/mocks were updated for the new interface and to add thread-safe status mutation in concurrency/race tests.Written by Cursor Bugbot for commit d02edc1. This will update automatically on new commits. Configure here.