After my last pr - #13 we have another bug with race condition
Summary
handleDownload treats any cached file whose S3 HeadObject returns 404 as
"deleted on S3" and removes it. But a file that was just created locally (written
into the cache dir by PVE) and is still waiting for the watcher's asynchronous
S3 upload also has no S3 object yet → HeadObject 404 → it gets wrongly purged.
Because activate_volume treats a failed download as fatal, this destroys
cloud-init ISOs before they are ever uploaded, so the VM boots with no config.
Reproduced on Proxmox VE with capmox (cluster-api-provider-proxmox) creating
per-VMID cloud-init ISOs on an s3-iso storage: 4 of 6 nodes lost their
user-data-<vmid>.iso and booted with no network config.
Impact
- Cloud-init / nocloud ISOs created on an s3-backed storage are intermittently
destroyed at VM start → nodes come up with no IP → cluster provisioning fails.
- Race window = the watcher debounce (3 s) + stability + upload latency. Small
files that are attached immediately after creation almost always lose the race.
Root cause
Path of a freshly created cloud-init ISO:
- PVE writes
template/iso/user-data-<vmid>.iso directly into the proxs3 cache
dir. The fsnotify watcher debounces 3 s (+ stability + fileInUse) before
uploading to S3 (internal/api/watcher.go:40-88).
- PVE attaches the volume to start the VM →
activate_volume
(perl/S3Plugin.pm:437) → POST /v1/download.
handleDownload (internal/api/api.go:355-368):
- file is in cache →
HeadObject → ErrNotFound (upload not done yet)
- logs
download: … deleted on S3, removing stale cache
s.cache.Remove(...) — deletes the file, clears the immutable flag
- returns HTTP 404
activate_volume sees the error → die "volume '…' does not exist" →
ISO never attached.
- Watcher's next tick:
os.Stat fails (file gone) → dropped from pending →
never uploaded. File lost permanently.
Asymmetry that reveals the bug
handleList / mergeLocalFiles (internal/api/api.go:334-339) already
accounts for locally-created files not yet uploaded ("PVE may write files
directly to the cache directory; the watcher uploads them asynchronously").
handleDownload has no equivalent guard — it fail-closes and purges them.
Evidence
- proxs3 journals for the failed VMIDs show only
download: … deleted on S3, removing stale cache — no delete: line
(nothing asked proxs3 to delete) and no watcher: uploading … line
(never uploaded). proxs3 purged a file that was pending its own upload.
- Nodes whose ISO was placed on node-local storage (proxs3 uncalled) provisioned
fine; nodes whose ISO went to the s3 storage lost it.
- Per-node
pvesm list <s3-storage> was inconsistent across nodes (4 / 3 / 3
user-data ISOs) — pending-upload local files visible only where created.
Suggested fix
In handleDownload, on HeadObject ErrNotFound, do not purge/404 a
cache-present file that has no confirmed S3 provenance (i.e. locally created and
not yet uploaded). Instead serve the local copy and ensure it is enqueued for
upload. Fail-closed (purge + 404) only for files known to have come from S3 and
since deleted.
Ways to distinguish pending-upload files:
- Consult the watcher's
pending set / an explicit "not yet on S3" marker.
- Add an
uploaded/s3confirmed flag to FileMeta, set it in the watcher and
in handleDownload's store-from-S3 path; on 404, keep the file if the flag is
unset.
Regression test to add
Create a file directly in the cache dir (no S3 object), then call
/v1/download for it → must return the local path and must not delete the
file. Then confirm the watcher still uploads it.
Affected code
internal/api/api.go — handleDownload (HeadObject-404 purge branch)
internal/api/watcher.go — async upload debounce
perl/S3Plugin.pm — activate_volume (fatal on download failure)
After my last pr - #13 we have another bug with race condition
Summary
handleDownloadtreats any cached file whose S3HeadObjectreturns 404 as"deleted on S3" and removes it. But a file that was just created locally (written
into the cache dir by PVE) and is still waiting for the watcher's asynchronous
S3 upload also has no S3 object yet → HeadObject 404 → it gets wrongly purged.
Because
activate_volumetreats a failed download as fatal, this destroyscloud-init ISOs before they are ever uploaded, so the VM boots with no config.
Reproduced on Proxmox VE with capmox (cluster-api-provider-proxmox) creating
per-VMID cloud-init ISOs on an
s3-isostorage: 4 of 6 nodes lost theiruser-data-<vmid>.isoand booted with no network config.Impact
destroyed at VM start → nodes come up with no IP → cluster provisioning fails.
files that are attached immediately after creation almost always lose the race.
Root cause
Path of a freshly created cloud-init ISO:
template/iso/user-data-<vmid>.isodirectly into the proxs3 cachedir. The fsnotify watcher debounces 3 s (+ stability +
fileInUse) beforeuploading to S3 (
internal/api/watcher.go:40-88).activate_volume(
perl/S3Plugin.pm:437) →POST /v1/download.handleDownload(internal/api/api.go:355-368):HeadObject→ErrNotFound(upload not done yet)download: … deleted on S3, removing stale caches.cache.Remove(...)— deletes the file, clears the immutable flagactivate_volumesees the error →die "volume '…' does not exist"→ISO never attached.
os.Statfails (file gone) → dropped frompending→never uploaded. File lost permanently.
Asymmetry that reveals the bug
handleList/mergeLocalFiles(internal/api/api.go:334-339) alreadyaccounts for locally-created files not yet uploaded ("PVE may write files
directly to the cache directory; the watcher uploads them asynchronously").
handleDownloadhas no equivalent guard — it fail-closes and purges them.Evidence
download: … deleted on S3, removing stale cache— nodelete:line(nothing asked proxs3 to delete) and no
watcher: uploading …line(never uploaded). proxs3 purged a file that was pending its own upload.
fine; nodes whose ISO went to the s3 storage lost it.
pvesm list <s3-storage>was inconsistent across nodes (4 / 3 / 3user-data ISOs) — pending-upload local files visible only where created.
Suggested fix
In
handleDownload, onHeadObjectErrNotFound, do not purge/404 acache-present file that has no confirmed S3 provenance (i.e. locally created and
not yet uploaded). Instead serve the local copy and ensure it is enqueued for
upload. Fail-closed (purge + 404) only for files known to have come from S3 and
since deleted.
Ways to distinguish pending-upload files:
pendingset / an explicit "not yet on S3" marker.uploaded/s3confirmedflag toFileMeta, set it in the watcher andin
handleDownload's store-from-S3 path; on 404, keep the file if the flag isunset.
Regression test to add
Create a file directly in the cache dir (no S3 object), then call
/v1/downloadfor it → must return the local path and must not delete thefile. Then confirm the watcher still uploads it.
Affected code
internal/api/api.go—handleDownload(HeadObject-404 purge branch)internal/api/watcher.go— async upload debounceperl/S3Plugin.pm—activate_volume(fatal on download failure)