Skip to content

List files from local cache when pending S3 upload#8

Merged
davekempe merged 2 commits into
sol1:mainfrom
Leonorus:terraform_upload
Mar 30, 2026
Merged

List files from local cache when pending S3 upload#8
davekempe merged 2 commits into
sol1:mainfrom
Leonorus:terraform_upload

Conversation

@Leonorus

Copy link
Copy Markdown

Issue #7: Terraform bpg/proxmox provider fails to read back files immediately after upload — failed to read file from "s3-iso:snippets/cloud-config-ubuntu.yaml".

Root cause: When PVE writes a file directly to the cache directory (e.g. via the Terraform provider), the file watcher needs 3+ seconds (debounce + stability check) before uploading to S3 and indexing metadata. The /v1/list endpoint only queries S3, so the file is invisible until the watcher completes the upload. The provider tries to read the file back immediately and fails because list_volumes() doesn't include it.

Fix: Modify handleList in internal/api/api.go to scan the local cache directory after querying S3, merging any files that exist locally but aren't yet in S3 into the response. Also improved the S3-unreachable fallback path to return locally cached files instead of an empty list.

@davekempe davekempe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall — clean approach, solid test coverage. One minor suggestion:

mergeLocalFiles should skip the .meta directory when recursing for images content. Currently it would treat .meta/ as a vmid subdirectory if it existed inside a content dir. Low risk since .meta lives at the cache root, but worth guarding:

if content == "images" {
    if entry.Name() == ".meta" {
        continue
    }
    // Recurse into VM ID subdirectories for images

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Leonorus

Copy link
Copy Markdown
Author

mergeLocalFiles now skips the .meta directory when recursing into subdirectories for images content

@davekempe davekempe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good — .meta guard added, tests pass, clean approach.

@davekempe davekempe merged commit 76a6791 into sol1:main Mar 30, 2026
2 checks passed
davekempe added a commit that referenced this pull request Apr 9, 2026
After PR #8 merged local files into /v1/list, stale cache files left
behind by failed os.Remove became visible as phantom volumes. The root
cause was handleDelete silently discarding cache.Remove errors, and
clearImmutable failing in systemd environments with limited PATH.

- Use absolute path /usr/bin/chattr with fallback to PATH lookup
- Retry removal after a second clearImmutable attempt
- Log when local cache removal fails instead of silently swallowing
- Backfill debian/changelog for v0.6.2–v0.6.4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants