Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add retry on the caller of v2DeleteManifest instead within v2DeleteManifest #18662

Merged
merged 5 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/jobservice/job/impl/gc/garbage_collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,18 @@
// 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

Check warning on line 292 in src/jobservice/job/impl/gc/garbage_collection.go

View check run for this annotation

Codecov / codecov/patch

src/jobservice/job/impl/gc/garbage_collection.go#L285-L292

Added lines #L285 - L292 were not covered by tests
})
}, 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 {

Check warning on line 296 in src/jobservice/job/impl/gc/garbage_collection.go

View check run for this annotation

Codecov / codecov/patch

src/jobservice/job/impl/gc/garbage_collection.go#L294-L296

Added lines #L294 - L296 were not covered by tests
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)
Expand Down
21 changes: 2 additions & 19 deletions src/jobservice/job/impl/gc/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@

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 ...
Expand Down Expand Up @@ -60,7 +56,7 @@
}

// v2DeleteManifest calls the registry API to remove manifest
func v2DeleteManifest(logger logger.Interface, repository, digest string) error {
func v2DeleteManifest(repository, digest string) error {

Check warning on line 59 in src/jobservice/job/impl/gc/util.go

View check run for this annotation

Codecov / codecov/patch

src/jobservice/job/impl/gc/util.go#L59

Added line #L59 was not covered by tests
exist, _, err := registry.Cli.ManifestExist(repository, digest)
if err != nil {
return err
Expand All @@ -70,20 +66,7 @@
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)

Check warning on line 69 in src/jobservice/job/impl/gc/util.go

View check run for this annotation

Codecov / codecov/patch

src/jobservice/job/impl/gc/util.go#L69

Added line #L69 was not covered by tests
}

// ignoreNotFound ignores the NotFoundErr error
Expand Down