From 845bcdb1842580743268fb1acd1b2c5a54361752 Mon Sep 17 00:00:00 2001
From: Shengwen YU <yshengwen@vmware.com>
Date: Thu, 18 May 2023 09:35:33 +0800
Subject: [PATCH] fix: add retry on the caller of v2DeleteManifest instead
 within v2DeleteManifest (#18662)

Signed-off-by: Shengwen Yu <yshengwen@vmware.com>
Co-authored-by: Wang Yan <wangyan@vmware.com>
---
 .../job/impl/gc/garbage_collection.go         | 13 +++++++++++-
 src/jobservice/job/impl/gc/util.go            | 21 ++-----------------
 2 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/src/jobservice/job/impl/gc/garbage_collection.go b/src/jobservice/job/impl/gc/garbage_collection.go
index 20fb07d1b09..aba2990f0d7 100644
--- a/src/jobservice/job/impl/gc/garbage_collection.go
+++ b/src/jobservice/job/impl/gc/garbage_collection.go
@@ -282,7 +282,18 @@ func (gc *GarbageCollector) sweep(ctx job.Context) error {
 				// Harbor cannot know the existing tags in the backend from its database, so let the v2 DELETE manifest to remove all of them.
 				gc.logger.Infof("[%d/%d] delete the manifest with registry v2 API: %s, %s, %s",
 					idx, total, art.RepositoryName, blob.ContentType, blob.Digest)
-				if err := v2DeleteManifest(gc.logger, art.RepositoryName, blob.Digest); err != nil {
+				if err := retry.Retry(func() error {
+					return ignoreNotFound(func() error {
+						err := v2DeleteManifest(art.RepositoryName, blob.Digest)
+						// if the system is in read-only mode, return an Abort error to skip retrying
+						if err == readonly.Err {
+							return retry.Abort(err)
+						}
+						return err
+					})
+				}, retry.Callback(func(err error, sleep time.Duration) {
+					gc.logger.Infof("[%d/%d] failed to exec v2DeleteManifest, error: %v, will retry again after: %s", idx, total, err, sleep)
+				})); err != nil {
 					gc.logger.Errorf("[%d/%d] failed to delete manifest with v2 API, %s, %s, %v", idx, total, art.RepositoryName, blob.Digest, err)
 					if err := ignoreNotFound(func() error {
 						return gc.markDeleteFailed(ctx, blob)
diff --git a/src/jobservice/job/impl/gc/util.go b/src/jobservice/job/impl/gc/util.go
index c9e3f717a47..6f92d42742e 100644
--- a/src/jobservice/job/impl/gc/util.go
+++ b/src/jobservice/job/impl/gc/util.go
@@ -16,15 +16,11 @@ package gc
 
 import (
 	"fmt"
-	"time"
 
 	"github.com/gomodule/redigo/redis"
 
-	"github.com/goharbor/harbor/src/jobservice/logger"
 	"github.com/goharbor/harbor/src/lib/errors"
-	"github.com/goharbor/harbor/src/lib/retry"
 	"github.com/goharbor/harbor/src/pkg/registry"
-	"github.com/goharbor/harbor/src/pkg/registry/interceptor/readonly"
 )
 
 // delKeys ...
@@ -60,7 +56,7 @@ func delKeys(con redis.Conn, pattern string) error {
 }
 
 // v2DeleteManifest calls the registry API to remove manifest
-func v2DeleteManifest(logger logger.Interface, repository, digest string) error {
+func v2DeleteManifest(repository, digest string) error {
 	exist, _, err := registry.Cli.ManifestExist(repository, digest)
 	if err != nil {
 		return err
@@ -70,20 +66,7 @@ func v2DeleteManifest(logger logger.Interface, repository, digest string) error
 	if !exist {
 		return nil
 	}
-	return retry.Retry(func() error {
-		err := registry.Cli.DeleteManifest(repository, digest)
-		// if the system is in read-only mode, return an Abort error to skip retrying
-		if err == readonly.Err {
-			return retry.Abort(err)
-		}
-		// If delete returned an err because the manifest is unknown, consider success
-		if errors.IsNotFoundErr(err) {
-			return nil
-		}
-		return err
-	}, retry.Callback(func(err error, sleep time.Duration) {
-		logger.Infof("failed to exec v2DeleteManifest, error: %v, will retry again after: %s", err, sleep)
-	}))
+	return registry.Cli.DeleteManifest(repository, digest)
 }
 
 // ignoreNotFound ignores the NotFoundErr error