Evict remote objects from every cache layer (#128)
Evicting a remote object only deleted its Postgres row. The S3 index and the Redis TTL/ETag keys survived, so stale mirror metadata such as EPEL repodata kept being served. - evict the artifact row, S3 index and Redis keys via `Engine.Evict` - evict a directory with `<dir>/*`; other wildcards return 400, unknown remotes 404 - wait on the per-path fetch lock for single-path evicts; return 503 on timeout or a Redis error - wildcard evicts take no lock; a fetch already in flight may re-cache its path - return S3 list errors from `DeletePrefix` instead of deleting nothing Reviewed-on: #128 Co-authored-by: unkin-agent <unkin-agent@unkin.net> Co-committed-by: unkin-agent <unkin-agent@unkin.net>
This commit was merged in pull request #128.
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
package v2
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strconv"
|
||||
@@ -8,8 +10,14 @@ import (
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/database"
|
||||
"git.unkin.net/unkin/artifactapi/internal/proxy"
|
||||
)
|
||||
|
||||
// Evictor drops a remote path from every cache layer.
|
||||
type Evictor interface {
|
||||
Evict(ctx context.Context, remoteName, path string) error
|
||||
}
|
||||
|
||||
type ObjectsHandler struct {
|
||||
db *database.DB
|
||||
}
|
||||
@@ -18,10 +26,13 @@ func NewObjectsHandler(db *database.DB) *ObjectsHandler {
|
||||
return &ObjectsHandler{db: db}
|
||||
}
|
||||
|
||||
func (h *ObjectsHandler) Routes() chi.Router {
|
||||
// Routes lists and evicts objects for remote repos; evictor serves the DELETE.
|
||||
func (h *ObjectsHandler) Routes(evictor Evictor) chi.Router {
|
||||
r := chi.NewRouter()
|
||||
r.Get("/", h.list)
|
||||
r.Delete("/*", h.evict)
|
||||
r.Delete("/*", func(w http.ResponseWriter, r *http.Request) {
|
||||
evict(w, r, evictor)
|
||||
})
|
||||
return r
|
||||
}
|
||||
|
||||
@@ -82,11 +93,16 @@ func (h *ObjectsHandler) evictLocal(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
func (h *ObjectsHandler) evict(w http.ResponseWriter, r *http.Request) {
|
||||
func evict(w http.ResponseWriter, r *http.Request, evictor Evictor) {
|
||||
remoteName := chi.URLParam(r, "name")
|
||||
path := chi.URLParam(r, "*")
|
||||
|
||||
if err := h.db.DeleteArtifact(r.Context(), remoteName, path); err != nil {
|
||||
if err := evictor.Evict(r.Context(), remoteName, path); err != nil {
|
||||
var proxyErr *proxy.ProxyError
|
||||
if errors.As(err, &proxyErr) {
|
||||
http.Error(w, proxyErr.Message, proxyErr.Status)
|
||||
return
|
||||
}
|
||||
http.Error(w, fmt.Sprintf("evict failed: %v", err), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
package v2
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/proxy"
|
||||
)
|
||||
|
||||
type fakeEvictor struct {
|
||||
remote, path string
|
||||
err error
|
||||
}
|
||||
|
||||
func (f *fakeEvictor) Evict(_ context.Context, remote, path string) error {
|
||||
f.remote, f.path = remote, path
|
||||
return f.err
|
||||
}
|
||||
|
||||
func deleteObject(ev Evictor, path string) int {
|
||||
router := chi.NewRouter()
|
||||
router.Route("/remotes/{name}/objects", func(r chi.Router) {
|
||||
r.Delete("/*", NewObjectsHandler(nil).Routes(ev).ServeHTTP)
|
||||
})
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, httptest.NewRequest("DELETE", "/remotes/epel/objects/"+path, nil))
|
||||
return w.Code
|
||||
}
|
||||
|
||||
func TestRemoteEvictDelegatesToEvictor(t *testing.T) {
|
||||
for _, path := range []string{"8/Everything/x86_64/repodata/repomd.xml", "8/Everything/x86_64/repodata/*"} {
|
||||
ev := &fakeEvictor{}
|
||||
if code := deleteObject(ev, path); code != 204 || ev.remote != "epel" || ev.path != path {
|
||||
t.Errorf("DELETE %s: code=%d evicted=%q/%q", path, code, ev.remote, ev.path)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemoteEvictMapsErrorStatus(t *testing.T) {
|
||||
for name, tc := range map[string]struct {
|
||||
err error
|
||||
want int
|
||||
}{
|
||||
"bad wildcard": {&proxy.ProxyError{Status: http.StatusBadRequest, Message: "wildcard evict must be <dir>/*"}, http.StatusBadRequest},
|
||||
"unknown remote": {&proxy.ProxyError{Status: http.StatusNotFound, Message: "remote not found"}, http.StatusNotFound},
|
||||
"lock busy": {fmt.Errorf("wrapped: %w", &proxy.ProxyError{Status: http.StatusServiceUnavailable, Message: "retry"}), http.StatusServiceUnavailable},
|
||||
"other error": {errors.New("db down"), http.StatusInternalServerError},
|
||||
} {
|
||||
if code := deleteObject(&fakeEvictor{err: tc.err}, "x"); code != tc.want {
|
||||
t.Errorf("%s: code = %d, want %d", name, code, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user