Skip to content

Commit 016fcdc

Browse files
eusebiu-constantin-petu-dbkeusebiu-constantin-petu-dbk
committed
fix(zb): fixed remote repositories cleanup
fix(storage): also put deduped blobs in cache, not just origin blobs this caused an error when trying to delete deduped blobs from multiple repositories
1 parent 9ca85e0 commit 016fcdc

4 files changed

Lines changed: 122 additions & 29 deletions

File tree

‎cmd/zb/helper.go‎

Lines changed: 74 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"bytes"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"io"
89
"log"
@@ -18,21 +19,83 @@ import (
1819
ispec "github.com/opencontainers/image-spec/specs-go/v1"
1920
"gopkg.in/resty.v1"
2021

21-
"zotregistry.io/zot/errors"
22+
"zotregistry.io/zot/pkg/api"
2223
"zotregistry.io/zot/pkg/test"
2324
)
2425

26+
func makeHTTPGetRequest(url string, resultPtr interface{}, client *resty.Client) error {
27+
resp, err := client.R().Get(url)
28+
if err != nil {
29+
return err
30+
}
31+
32+
if resp.StatusCode() != http.StatusOK {
33+
log.Printf("unable to make GET request on %s, response status code: %d", url, resp.StatusCode())
34+
35+
return errors.New(string(resp.Body())) //nolint: goerr113
36+
}
37+
38+
err = json.Unmarshal(resp.Body(), resultPtr)
39+
if err != nil {
40+
return err
41+
}
42+
43+
return nil
44+
}
45+
46+
func makeHTTPDeleteRequest(url string, client *resty.Client) error {
47+
resp, err := client.R().Delete(url)
48+
if err != nil {
49+
return err
50+
}
51+
52+
if resp.StatusCode() != http.StatusAccepted {
53+
log.Printf("unable to make DELETE request on %s, response status code: %d", url, resp.StatusCode())
54+
55+
return errors.New(string(resp.Body())) //nolint: goerr113
56+
}
57+
58+
return nil
59+
}
60+
2561
func deleteTestRepo(repos []string, url string, client *resty.Client) error {
2662
for _, repo := range repos {
27-
resp, err := client.R().Delete((fmt.Sprintf("%s/v2/%s/", url, repo)))
63+
var tags api.ImageTags
64+
65+
// get tags
66+
err := makeHTTPGetRequest(fmt.Sprintf("%s/v2/%s/tags/list", url, repo), &tags, client)
2867
if err != nil {
2968
return err
3069
}
3170

32-
// request specific check
33-
statusCode := resp.StatusCode()
34-
if statusCode != http.StatusAccepted {
35-
return errors.ErrUnknownCode
71+
for _, tag := range tags.Tags {
72+
var manifest ispec.Manifest
73+
74+
// first get tag manifest to get containing blobs
75+
err := makeHTTPGetRequest(fmt.Sprintf("%s/v2/%s/manifests/%s", url, repo, tag), &manifest, client)
76+
if err != nil {
77+
return err
78+
}
79+
80+
// delete blobs
81+
for _, blob := range manifest.Layers {
82+
err := makeHTTPDeleteRequest(fmt.Sprintf("%s/v2/%s/blobs/%s", url, repo, blob.Digest.String()), client)
83+
if err != nil {
84+
return err
85+
}
86+
}
87+
88+
// delete config blob
89+
err = makeHTTPDeleteRequest(fmt.Sprintf("%s/v2/%s/blobs/%s", url, repo, manifest.Config.Digest.String()), client)
90+
if err != nil {
91+
return err
92+
}
93+
94+
// delete manifest
95+
err = makeHTTPDeleteRequest(fmt.Sprintf("%s/v2/%s/manifests/%s", url, repo, tag), client)
96+
if err != nil {
97+
return err
98+
}
3699
}
37100
}
38101

@@ -273,7 +336,7 @@ func pushMonolithImage(workdir, url, trepo string, repos []string, config testCo
273336
// request specific check
274337
statusCode = resp.StatusCode()
275338
if statusCode != http.StatusAccepted {
276-
return nil, repos, errors.ErrUnknownCode
339+
return nil, repos, errors.New(string(resp.Body())) //nolint: goerr113
277340
}
278341

279342
loc := test.Location(url, resp)
@@ -311,7 +374,7 @@ func pushMonolithImage(workdir, url, trepo string, repos []string, config testCo
311374
// request specific check
312375
statusCode = resp.StatusCode()
313376
if statusCode != http.StatusCreated {
314-
return nil, repos, errors.ErrUnknownCode
377+
return nil, repos, errors.New(string(resp.Body())) //nolint: goerr113
315378
}
316379

317380
// upload image config blob
@@ -325,7 +388,7 @@ func pushMonolithImage(workdir, url, trepo string, repos []string, config testCo
325388
// request specific check
326389
statusCode = resp.StatusCode()
327390
if statusCode != http.StatusAccepted {
328-
return nil, repos, errors.ErrUnknownCode
391+
return nil, repos, errors.New(string(resp.Body())) //nolint: goerr113
329392
}
330393

331394
loc = test.Location(url, resp)
@@ -345,7 +408,7 @@ func pushMonolithImage(workdir, url, trepo string, repos []string, config testCo
345408
// request specific check
346409
statusCode = resp.StatusCode()
347410
if statusCode != http.StatusCreated {
348-
return nil, repos, errors.ErrUnknownCode
411+
return nil, repos, errors.New(string(resp.Body())) //nolint: goerr113
349412
}
350413

351414
// create a manifest
@@ -388,7 +451,7 @@ func pushMonolithImage(workdir, url, trepo string, repos []string, config testCo
388451
// request specific check
389452
statusCode = resp.StatusCode()
390453
if statusCode != http.StatusCreated {
391-
return nil, repos, errors.ErrUnknownCode
454+
return nil, repos, errors.New(string(resp.Body())) //nolint: goerr113
392455
}
393456

394457
manifestHash[repo] = manifestTag

‎cmd/zb/main.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ func NewPerfRootCmd() *cobra.Command {
1818

1919
var concurrency, requests int
2020

21+
var skipCleanup bool
22+
2123
rootCmd := &cobra.Command{
2224
Use: "zb <url>",
2325
Short: "`zb`",
@@ -46,7 +48,7 @@ func NewPerfRootCmd() *cobra.Command {
4648

4749
requests = concurrency * (requests / concurrency)
4850

49-
Perf(workdir, url, auth, repo, concurrency, requests, outFmt, srcIPs, srcCIDR)
51+
Perf(workdir, url, auth, repo, concurrency, requests, outFmt, srcIPs, srcCIDR, skipCleanup)
5052
},
5153
}
5254

@@ -66,6 +68,8 @@ func NewPerfRootCmd() *cobra.Command {
6668
"Number of requests to perform")
6769
rootCmd.Flags().StringVarP(&outFmt, "output-format", "o", "",
6870
"Output format of test results: stdout (default), json, ci-cd")
71+
rootCmd.Flags().BoolVar(&skipCleanup, "skip-cleanup", false,
72+
"Clean up pushed repos from remote registry after running benchmark (default true)")
6973

7074
// "version"
7175
rootCmd.Flags().BoolVarP(&showVersion, "version", "v", false, "Show the version and exit")

‎cmd/zb/perf.go‎

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ type testFunc func(
270270
config testConfig,
271271
statsCh chan statsRecord,
272272
client *resty.Client,
273+
skipCleanup bool,
273274
) error
274275

275276
//nolint:gosec
@@ -279,6 +280,7 @@ func GetCatalog(
279280
config testConfig,
280281
statsCh chan statsRecord,
281282
client *resty.Client,
283+
skipCleanup bool,
282284
) error {
283285
var repos []string
284286

@@ -336,9 +338,11 @@ func GetCatalog(
336338
}
337339

338340
// clean up
339-
err = deleteTestRepo(repos, url, client)
340-
if err != nil {
341-
return err
341+
if !skipCleanup {
342+
err = deleteTestRepo(repos, url, client)
343+
if err != nil {
344+
return err
345+
}
342346
}
343347

344348
return nil
@@ -350,6 +354,7 @@ func PushMonolithStreamed(
350354
config testConfig,
351355
statsCh chan statsRecord,
352356
client *resty.Client,
357+
skipCleanup bool,
353358
) error {
354359
var repos []string
355360

@@ -363,9 +368,11 @@ func PushMonolithStreamed(
363368
}
364369

365370
// clean up
366-
err := deleteTestRepo(repos, url, client)
367-
if err != nil {
368-
return err
371+
if !skipCleanup {
372+
err := deleteTestRepo(repos, url, client)
373+
if err != nil {
374+
return err
375+
}
369376
}
370377

371378
return nil
@@ -377,6 +384,7 @@ func PushChunkStreamed(
377384
config testConfig,
378385
statsCh chan statsRecord,
379386
client *resty.Client,
387+
skipCleanup bool,
380388
) error {
381389
var repos []string
382390

@@ -390,9 +398,11 @@ func PushChunkStreamed(
390398
}
391399

392400
// clean up
393-
err := deleteTestRepo(repos, url, client)
394-
if err != nil {
395-
return err
401+
if !skipCleanup {
402+
err := deleteTestRepo(repos, url, client)
403+
if err != nil {
404+
return err
405+
}
396406
}
397407

398408
return nil
@@ -404,6 +414,7 @@ func Pull(
404414
config testConfig,
405415
statsCh chan statsRecord,
406416
client *resty.Client,
417+
skipCleanup bool,
407418
) error {
408419
var repos []string
409420

@@ -472,9 +483,11 @@ func Pull(
472483
}
473484

474485
// clean up
475-
err := deleteTestRepo(repos, url, client)
476-
if err != nil {
477-
return err
486+
if !skipCleanup {
487+
err := deleteTestRepo(repos, url, client)
488+
if err != nil {
489+
return err
490+
}
478491
}
479492

480493
return nil
@@ -486,6 +499,7 @@ func MixedPullAndPush(
486499
config testConfig,
487500
statsCh chan statsRecord,
488501
client *resty.Client,
502+
skipCleanup bool,
489503
) error {
490504
var repos []string
491505

@@ -519,9 +533,11 @@ func MixedPullAndPush(
519533
}
520534

521535
// clean up
522-
err = deleteTestRepo(repos, url, client)
523-
if err != nil {
524-
return err
536+
if !skipCleanup {
537+
err = deleteTestRepo(repos, url, client)
538+
if err != nil {
539+
return err
540+
}
525541
}
526542

527543
return nil
@@ -633,7 +649,7 @@ var testSuite = []testConfig{ //nolint:gochecknoglobals // used only in this tes
633649
func Perf(
634650
workdir, url, auth, repo string,
635651
concurrency int, requests int,
636-
outFmt string, srcIPs string, srcCIDR string,
652+
outFmt string, srcIPs string, srcCIDR string, skipCleanup bool,
637653
) {
638654
json := jsoniter.ConfigCompatibleWithStandardLibrary
639655
// logging
@@ -702,7 +718,10 @@ func Perf(
702718
log.Fatal(err)
703719
}
704720

705-
_ = tconfig.tfunc(workdir, url, repo, requests/concurrency, tconfig, statsCh, httpClient)
721+
err = tconfig.tfunc(workdir, url, repo, requests/concurrency, tconfig, statsCh, httpClient, skipCleanup)
722+
if err != nil {
723+
log.Fatal(err)
724+
}
706725
}()
707726
}
708727
wg.Wait()

‎pkg/storage/local/local.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,13 @@ retry:
10451045
}
10461046
}
10471047

1048+
// also put dedupe blob in cache
1049+
if err := is.cache.PutBlob(dstDigest, dst); err != nil {
1050+
is.log.Error().Err(err).Str("blobPath", dst).Msg("dedupe: unable to insert blob record")
1051+
1052+
return err
1053+
}
1054+
10481055
if err := os.Remove(src); err != nil {
10491056
is.log.Error().Err(err).Str("src", src).Msg("dedupe: uname to remove blob")
10501057

0 commit comments

Comments
 (0)