feat(#103): snapshot retention policies and automatic cleanup#129
feat(#103): snapshot retention policies and automatic cleanup#129dennisonbertram merged 1 commit intomainfrom
Conversation
Auto-cleanup snapshots exceeding per-service count (default: 10) or age (default: 30d) limits via the GC loop. Deletes oldest snapshots first when over the count limit. Docker images are cleaned up alongside DB rows. Configurable via AH_SNAPSHOT_MAX_PER_SERVICE and AH_SNAPSHOT_MAX_AGE environment variables. Set either to 0 to disable that limit. Closes #103 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 659d0bce81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func (m *Manager) deleteSnapshot(ctx context.Context, id, tenantID, imageRef string) error { | ||
| // Best-effort image removal. | ||
| if imageRef != "" { | ||
| if err := m.docker.RemoveImage(ctx, imageRef); err != nil { | ||
| log.Printf("gc/snapshots: WARNING: failed to remove image %s: %v", imageRef, err) |
There was a problem hiding this comment.
deleteSnapshot always calls RemoveImage before deleting the snapshot row, but services created from snapshots persist that same image_ref as their deploy image (internal/api/services.go). With automatic GC now running every 5 minutes, an aged/over-limit snapshot can have its image tag removed while a service still depends on it, causing later redeploys/restarts to fail when that image ref is pulled. The cleanup path should check whether any service still references image_ref (or skip image removal and only delete metadata) before untagging.
Useful? React with 👍 / 👎.
| d, err := time.ParseDuration(v) | ||
| if err != nil { | ||
| return fallback |
There was a problem hiding this comment.
AH_SNAPSHOT_MAX_AGE is parsed with time.ParseDuration, which does not accept day-based inputs like 7d, yet parse failures silently fall back to the default. That means operators can set what looks like a valid retention value and unknowingly keep the default 30-day policy, so cleanup behavior diverges from configured intent. This should either support day suffixes explicitly or surface an error/log when parsing fails.
Useful? React with 👍 / 👎.
1 participant
Summary
Test plan
go test ./...passesCloses feat(snapshots): implement snapshot retention policies and automatic cleanup #103
🤖 Generated with Claude Code