From 0cc77b8b38517ac35c491e2822dcb6c3f4b3cf10 Mon Sep 17 00:00:00 2001
From: Josh Sandlin <joshs@replicated.com>
Date: Thu, 30 Jul 2026 17:49:42 -0400
Subject: [PATCH] dont throw away errors, more gracefully exit, and dont wait
 30 seconds for a delete response

---
 sidecar/internal/app/app.go           | 22 ++++++++++++++++++++--
 sidecar/internal/registry/registry.go | 11 ++++++++---
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/sidecar/internal/app/app.go b/sidecar/internal/app/app.go
index 9c68570..6ddb790 100644
--- a/sidecar/internal/app/app.go
+++ b/sidecar/internal/app/app.go
@@ -56,8 +56,18 @@ func Run(ctx context.Context, cfg config.Config) error {
 		ReadHeaderTimeout: 10 * time.Second,
 	}
 
+	// The sweep loop gets its own cancellable context so it can be stopped on
+	// the way out even when shutdown was triggered by a server error rather
+	// than by ctx.
+	reaperCtx, stopReaper := context.WithCancel(ctx)
+	defer stopReaper()
+
 	rp := reaper.New(cfg.SweepInterval, st, registry.New(cfg.ZotURL))
-	go rp.Run(ctx)
+	reaperDone := make(chan struct{})
+	go func() {
+		defer close(reaperDone)
+		rp.Run(reaperCtx)
+	}()
 
 	serveErr := make(chan error, 1)
 	go func() {
@@ -76,5 +86,13 @@ func Run(ctx context.Context, cfg config.Config) error {
 	log.Printf("shutting down")
 	shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
-	return httpSrv.Shutdown(shutdownCtx)
+	shutdownErr := httpSrv.Shutdown(shutdownCtx)
+
+	// Wait for an in-flight sweep to unwind before returning, so it cannot
+	// still be talking to Redis when the deferred Close runs. Cancelling
+	// reaperCtx aborts the delete request in flight, so this does not block for
+	// long.
+	stopReaper()
+	<-reaperDone
+	return shutdownErr
 }
diff --git a/sidecar/internal/registry/registry.go b/sidecar/internal/registry/registry.go
index ce18f94..8bc4f0f 100644
--- a/sidecar/internal/registry/registry.go
+++ b/sidecar/internal/registry/registry.go
@@ -21,7 +21,11 @@ func New(baseURL string) *Client {
 	return &Client{
 		baseURL: baseURL,
 		http: &http.Client{
-			Timeout: 30 * time.Second,
+			// A manifest delete is a metadata operation against a zot running
+			// alongside this process, so it should be fast. Failing quickly
+			// keeps one wedged tag from stalling the rest of the sweep; the row
+			// stays in the store and is retried on the next tick.
+			Timeout: 5 * time.Second,
 		},
 	}
 }
@@ -41,11 +45,12 @@ func (c *Client) DeleteManifest(ctx context.Context, repo, tag string) error {
 		return err
 	}
 	defer func() { _ = resp.Body.Close() }()
-	_, _ = io.Copy(io.Discard, resp.Body)
+	body, _ := io.ReadAll(resp.Body)
 	switch resp.StatusCode {
 	case http.StatusOK, http.StatusAccepted, http.StatusNoContent, http.StatusNotFound:
 		return nil
 	default:
-		return fmt.Errorf("DELETE %s -> %d", endpoint, resp.StatusCode)
+		// zot describes the failure in the body; %q keeps it on one log line.
+		return fmt.Errorf("DELETE %s -> %d: %q", endpoint, resp.StatusCode, body)
 	}
 }