Skip to content

Commit 4684bcf

Browse files
andaaronandaaron
committed
fix(dedupe): lock repositories sharing the same digest when blobs are pushed or deleted
Signed-off-by: Andrei Aaron <[email protected]>
1 parent 1dce3d4 commit 4684bcf

File tree

13 files changed

+801
-740
lines changed

13 files changed

+801
-740
lines changed

‎pkg/api/routes.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -956,12 +956,12 @@ func (rh *RouteHandler) CheckBlob(response http.ResponseWriter, request *http.Re
956956
if userCanMount {
957957
ok, blen, err = imgStore.CheckBlob(name, digest)
958958
} else {
959-
var lockLatency time.Time
959+
err = imgStore.WithRepoReadLock(name, func() error {
960+
var err error
961+
ok, blen, _, err = imgStore.StatBlob(name, digest)
960962

961-
imgStore.RLockRepo(name, &lockLatency)
962-
defer imgStore.RUnlockRepo(name, &lockLatency)
963-
964-
ok, blen, _, err = imgStore.StatBlob(name, digest)
963+
return err
964+
})
965965
}
966966

967967
if err != nil {

‎pkg/extensions/sync/destination.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"os"
1212
"path"
1313
"strings"
14-
"time"
1514

1615
"github.com/containers/image/v5/types"
1716
"github.com/opencontainers/go-digest"
@@ -99,8 +98,6 @@ func (registry *DestinationRegistry) CommitImage(imageReference types.ImageRefer
9998
registry.log.Info().Str("syncTempDir", path.Join(tempImageStore.RootDir(), repo)).Str("reference", reference).
10099
Msg("pushing synced local image to local registry")
101100

102-
var lockLatency time.Time
103-
104101
manifestBlob, manifestDigest, mediaType, err := tempImageStore.GetImageManifest(repo, reference)
105102
if err != nil {
106103
registry.log.Error().Str("errorType", common.TypeOf(err)).
@@ -136,10 +133,14 @@ func (registry *DestinationRegistry) CommitImage(imageReference types.ImageRefer
136133
}
137134

138135
for _, manifest := range indexManifest.Manifests {
139-
tempImageStore.RLockRepo(repo, &lockLatency)
140-
manifestBuf, err := tempImageStore.GetBlobContent(repo, manifest.Digest)
141-
tempImageStore.RUnlockRepo(repo, &lockLatency)
136+
var manifestBuf []byte
137+
138+
err := tempImageStore.WithRepoReadLock(repo, func() error {
139+
var err error
140+
manifestBuf, err = tempImageStore.GetBlobContent(repo, manifest.Digest)
142141

142+
return err
143+
})
143144
if err != nil {
144145
registry.log.Error().Str("errorType", common.TypeOf(err)).
145146
Err(err).Str("dir", path.Join(tempImageStore.RootDir(), repo)).Str("digest", manifest.Digest.String()).

‎pkg/meta/parse.go

Lines changed: 78 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"errors"
7-
"time"
87

98
godigest "github.com/opencontainers/go-digest"
109
ispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -108,66 +107,65 @@ func getReposToBeDeleted(allStorageRepos []string, allMetaDBRepos []string) []st
108107
func ParseRepo(repo string, metaDB mTypes.MetaDB, storeController stypes.StoreController, log log.Logger) error {
109108
imageStore := storeController.GetImageStore(repo)
110109

111-
var lockLatency time.Time
110+
err := imageStore.WithRepoReadLock(repo, func() error {
111+
indexBlob, err := imageStore.GetIndexContent(repo)
112+
if err != nil {
113+
log.Error().Err(err).Str("repository", repo).Msg("failed to read index.json for repo")
112114

113-
imageStore.RLockRepo(repo, &lockLatency)
114-
defer imageStore.RUnlockRepo(repo, &lockLatency)
115+
return err
116+
}
115117

116-
indexBlob, err := imageStore.GetIndexContent(repo)
117-
if err != nil {
118-
log.Error().Err(err).Str("repository", repo).Msg("failed to read index.json for repo")
118+
var indexContent ispec.Index
119119

120-
return err
121-
}
120+
err = json.Unmarshal(indexBlob, &indexContent)
121+
if err != nil {
122+
log.Error().Err(err).Str("repository", repo).Msg("failed to unmarshal index.json for repo")
122123

123-
var indexContent ispec.Index
124+
return err
125+
}
124126

125-
err = json.Unmarshal(indexBlob, &indexContent)
126-
if err != nil {
127-
log.Error().Err(err).Str("repository", repo).Msg("failed to unmarshal index.json for repo")
127+
err = metaDB.ResetRepoReferences(repo)
128+
if err != nil && !errors.Is(err, zerr.ErrRepoMetaNotFound) {
129+
log.Error().Err(err).Str("repository", repo).Msg("failed to reset tag field in RepoMetadata for repo")
128130

129-
return err
130-
}
131+
return err
132+
}
131133

132-
err = metaDB.ResetRepoReferences(repo)
133-
if err != nil && !errors.Is(err, zerr.ErrRepoMetaNotFound) {
134-
log.Error().Err(err).Str("repository", repo).Msg("failed to reset tag field in RepoMetadata for repo")
134+
for _, manifest := range indexContent.Manifests {
135+
tag := manifest.Annotations[ispec.AnnotationRefName]
135136

136-
return err
137-
}
137+
if zcommon.IsReferrersTag(tag) {
138+
continue
139+
}
138140

139-
for _, manifest := range indexContent.Manifests {
140-
tag := manifest.Annotations[ispec.AnnotationRefName]
141+
manifestBlob, _, _, err := imageStore.GetImageManifest(repo, manifest.Digest.String())
142+
if err != nil {
143+
log.Error().Err(err).Str("repository", repo).Str("digest", manifest.Digest.String()).
144+
Msg("failed to get blob for image")
141145

142-
if zcommon.IsReferrersTag(tag) {
143-
continue
144-
}
146+
return err
147+
}
145148

146-
manifestBlob, _, _, err := imageStore.GetImageManifest(repo, manifest.Digest.String())
147-
if err != nil {
148-
log.Error().Err(err).Str("repository", repo).Str("digest", manifest.Digest.String()).
149-
Msg("failed to get blob for image")
149+
reference := tag
150150

151-
return err
152-
}
151+
if tag == "" {
152+
reference = manifest.Digest.String()
153+
}
153154

154-
reference := tag
155+
err = SetImageMetaFromInput(context.Background(), repo, reference, manifest.MediaType, manifest.Digest, manifestBlob,
156+
imageStore, metaDB, log)
157+
if err != nil {
158+
log.Error().Err(err).Str("repository", repo).Str("tag", tag).
159+
Msg("failed to set metadata for image")
155160

156-
if tag == "" {
157-
reference = manifest.Digest.String()
161+
return err
162+
}
158163
}
159164

160-
err = SetImageMetaFromInput(context.Background(), repo, reference, manifest.MediaType, manifest.Digest, manifestBlob,
161-
imageStore, metaDB, log)
162-
if err != nil {
163-
log.Error().Err(err).Str("repository", repo).Str("tag", tag).
164-
Msg("failed to set metadata for image")
165-
166-
return err
167-
}
168-
}
165+
return nil
166+
})
169167

170-
return nil
168+
return err
171169
}
172170

173171
func getAllRepos(storeController stypes.StoreController, log log.Logger) ([]string, error) {
@@ -222,34 +220,33 @@ func getCosignSignatureLayersInfo(
222220
return layers, err
223221
}
224222

225-
var lockLatency time.Time
226-
227-
imageStore.RLockRepo(repo, &lockLatency)
228-
defer imageStore.RUnlockRepo(repo, &lockLatency)
223+
err := imageStore.WithRepoReadLock(repo, func() error {
224+
for _, layer := range manifestContent.Layers {
225+
layerContent, err := imageStore.GetBlobContent(repo, layer.Digest)
226+
if err != nil {
227+
log.Error().Err(err).Str("repository", repo).Str("reference", tag).Str("layerDigest", layer.Digest.String()).
228+
Msg("failed to get cosign signature layer content")
229229

230-
for _, layer := range manifestContent.Layers {
231-
layerContent, err := imageStore.GetBlobContent(repo, layer.Digest)
232-
if err != nil {
233-
log.Error().Err(err).Str("repository", repo).Str("reference", tag).Str("layerDigest", layer.Digest.String()).Msg(
234-
"failed to get cosign signature layer content")
230+
return err
231+
}
235232

236-
return layers, err
237-
}
233+
layerSigKey, ok := layer.Annotations[zcommon.CosignSigKey]
234+
if !ok {
235+
log.Error().Err(err).Str("repository", repo).Str("reference", tag).Str("layerDigest", layer.Digest.String()).
236+
Msg("failed to get specific annotation of cosign signature")
237+
}
238238

239-
layerSigKey, ok := layer.Annotations[zcommon.CosignSigKey]
240-
if !ok {
241-
log.Error().Err(err).Str("repository", repo).Str("reference", tag).Str("layerDigest", layer.Digest.String()).Msg(
242-
"failed to get specific annotation of cosign signature")
239+
layers = append(layers, mTypes.LayerInfo{
240+
LayerDigest: layer.Digest.String(),
241+
LayerContent: layerContent,
242+
SignatureKey: layerSigKey,
243+
})
243244
}
244245

245-
layers = append(layers, mTypes.LayerInfo{
246-
LayerDigest: layer.Digest.String(),
247-
LayerContent: layerContent,
248-
SignatureKey: layerSigKey,
249-
})
250-
}
246+
return nil
247+
})
251248

252-
return layers, nil
249+
return layers, err
253250
}
254251

255252
func getNotationSignatureLayersInfo(
@@ -279,28 +276,27 @@ func getNotationSignatureLayersInfo(
279276

280277
layer := manifestContent.Layers[0].Digest
281278

282-
var lockLatency time.Time
279+
err := imageStore.WithRepoReadLock(repo, func() error {
280+
layerContent, err := imageStore.GetBlobContent(repo, layer)
281+
if err != nil {
282+
log.Error().Err(err).Str("repository", repo).Str("reference", manifestDigest).Str("layerDigest", layer.String()).
283+
Msg("failed to get notation signature blob content")
283284

284-
imageStore.RLockRepo(repo, &lockLatency)
285-
defer imageStore.RUnlockRepo(repo, &lockLatency)
285+
return err
286+
}
286287

287-
layerContent, err := imageStore.GetBlobContent(repo, layer)
288-
if err != nil {
289-
log.Error().Err(err).Str("repository", repo).Str("reference", manifestDigest).Str("layerDigest", layer.String()).Msg(
290-
"failed to get notation signature blob content")
288+
layerSigKey := manifestContent.Layers[0].MediaType
291289

292-
return layers, err
293-
}
294-
295-
layerSigKey := manifestContent.Layers[0].MediaType
290+
layers = append(layers, mTypes.LayerInfo{
291+
LayerDigest: layer.String(),
292+
LayerContent: layerContent,
293+
SignatureKey: layerSigKey,
294+
})
296295

297-
layers = append(layers, mTypes.LayerInfo{
298-
LayerDigest: layer.String(),
299-
LayerContent: layerContent,
300-
SignatureKey: layerSigKey,
296+
return nil
301297
})
302298

303-
return layers, nil
299+
return layers, err
304300
}
305301

306302
// SetMetadataFromInput tries to set manifest metadata and update repo metadata by adding the current tag

‎pkg/storage/gc/gc.go

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -101,62 +101,61 @@ func (gc GarbageCollect) CleanRepo(ctx context.Context, repo string) error {
101101
}
102102

103103
func (gc GarbageCollect) cleanRepo(ctx context.Context, repo string) error {
104-
var lockLatency time.Time
105-
106104
dir := path.Join(gc.imgStore.RootDir(), repo)
107105
if !gc.imgStore.DirExists(dir) {
108106
return zerr.ErrRepoNotFound
109107
}
110108

111-
gc.imgStore.LockRepo(repo, &lockLatency)
112-
defer gc.imgStore.UnlockRepo(repo, &lockLatency)
109+
err := gc.imgStore.WithRepoLock(repo, func() error {
110+
/* this index (which represents the index.json of this repo) is the root point from which we
111+
search for dangling manifests/blobs
112+
so this index is passed by reference in all functions that modifies it
113113
114-
/* this index (which represents the index.json of this repo) is the root point from which we
115-
search for dangling manifests/blobs
116-
so this index is passed by reference in all functions that modifies it
114+
Instead of removing manifests one by one with storage APIs we just remove manifests descriptors
115+
from index.Manifests[] list and update repo's index.json afterwards.
117116
118-
Instead of removing manifests one by one with storage APIs we just remove manifests descriptors
119-
from index.Manifests[] list and update repo's index.json afterwards.
117+
After updating repo's index.json we clean all unreferenced blobs (manifests included).
118+
*/
119+
index, err := common.GetIndex(gc.imgStore, repo, gc.log)
120+
if err != nil {
121+
gc.log.Error().Err(err).Str("module", "gc").Str("repository", repo).Msg("failed to read index.json in repo")
120122

121-
After updating repo's index.json we clean all unreferenced blobs (manifests included).
122-
*/
123-
index, err := common.GetIndex(gc.imgStore, repo, gc.log)
124-
if err != nil {
125-
gc.log.Error().Err(err).Str("module", "gc").Str("repository", repo).Msg("failed to read index.json in repo")
123+
return err
124+
}
126125

127-
return err
128-
}
126+
// apply tags retention
127+
if err := gc.removeTagsPerRetentionPolicy(ctx, repo, &index); err != nil {
128+
return err
129+
}
129130

130-
// apply tags retention
131-
if err := gc.removeTagsPerRetentionPolicy(ctx, repo, &index); err != nil {
132-
return err
133-
}
131+
// gc referrers manifests with missing subject and untagged manifests
132+
if err := gc.removeManifestsPerRepoPolicy(ctx, repo, &index); err != nil {
133+
return err
134+
}
134135

135-
// gc referrers manifests with missing subject and untagged manifests
136-
if err := gc.removeManifestsPerRepoPolicy(ctx, repo, &index); err != nil {
137-
return err
138-
}
136+
// update repos's index.json in storage
137+
if !gc.opts.ImageRetention.DryRun {
138+
/* this will update the index.json with manifests deleted above
139+
and the manifests blobs will be removed by gc.removeUnreferencedBlobs()*/
140+
if err := gc.imgStore.PutIndexContent(repo, index); err != nil {
141+
return err
142+
}
143+
}
139144

140-
// update repos's index.json in storage
141-
if !gc.opts.ImageRetention.DryRun {
142-
/* this will update the index.json with manifests deleted above
143-
and the manifests blobs will be removed by gc.removeUnreferencedBlobs()*/
144-
if err := gc.imgStore.PutIndexContent(repo, index); err != nil {
145+
// gc unreferenced blobs
146+
if err := gc.removeUnreferencedBlobs(repo, gc.opts.Delay, gc.log); err != nil {
145147
return err
146148
}
147-
}
148149

149-
// gc unreferenced blobs
150-
if err := gc.removeUnreferencedBlobs(repo, gc.opts.Delay, gc.log); err != nil {
151-
return err
152-
}
150+
// gc old blob uploads
151+
if err := gc.removeBlobUploads(repo, gc.opts.Delay); err != nil {
152+
return err
153+
}
153154

154-
// gc old blob uploads
155-
if err := gc.removeBlobUploads(repo, gc.opts.Delay); err != nil {
156-
return err
157-
}
155+
return nil
156+
})
158157

159-
return nil
158+
return err
160159
}
161160

162161
func (gc GarbageCollect) removeManifestsPerRepoPolicy(ctx context.Context, repo string, index *ispec.Index) error {

0 commit comments

Comments
 (0)