Skip to content

Commit 81543fa

Browse files
legionuslegionus
committed
pullthrough not optional
Signed-off-by: Gladkov Alexey <[email protected]>
1 parent c0ab4d3 commit 81543fa

File tree

9 files changed

+54
-187
lines changed

9 files changed

+54
-187
lines changed

‎pkg/dockerregistry/server/blobdescriptorservice.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (bs *blobDescriptorService) Stat(ctx context.Context, dgst digest.Digest) (
5252
// ensure it's referenced inside of corresponding image stream
5353
if bs.repo.cache.ContainsRepository(dgst, bs.repo.imageStream.Reference()) {
5454
context.GetLogger(ctx).Debugf("found cached blob %q in repository %s", dgst.String(), bs.repo.imageStream.Reference())
55-
} else if image := bs.repo.imageStream.HasBlob(ctx, dgst, !bs.repo.app.config.Pullthrough.Enabled); image != nil {
55+
} else if image := bs.repo.imageStream.HasBlob(ctx, dgst); image != nil {
5656
// remember all the layers of matching image
5757
RememberLayersOfImage(ctx, bs.repo.cache, image, bs.repo.imageStream.Reference())
5858
} else {

‎pkg/dockerregistry/server/configuration/configuration.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,12 @@ func migratePullthroughSection(cfg *Configuration, options configuration.Paramet
440440
if err != nil {
441441
err = fmt.Errorf("configuration error in openshift.pullthrough.mirror: %v", err)
442442
}
443+
444+
if !cfg.Pullthrough.Enabled {
445+
log.Warnf("pullthrough can't be disabled anymore")
446+
cfg.Pullthrough.Enabled = true
447+
}
448+
443449
return
444450
}
445451

‎pkg/dockerregistry/server/registry_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,14 @@ func newTestRegistry(
3636
osClient registryclient.Interface,
3737
storageDriver driver.StorageDriver,
3838
blobrepositorycachettl time.Duration,
39-
pullthrough bool,
4039
useBlobDescriptorCacheProvider bool,
4140
) (distribution.Namespace, error) {
4241
cfg := &configuration.Configuration{
4342
Server: &configuration.Server{
4443
Addr: "localhost:5000",
4544
},
4645
Pullthrough: &configuration.Pullthrough{
47-
Enabled: pullthrough,
46+
Enabled: true,
4847
},
4948
Cache: &configuration.Cache{
5049
BlobRepositoryTTL: blobrepositorycachettl,

‎pkg/dockerregistry/server/repository.go

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,11 @@ func (app *App) Repository(ctx context.Context, repo distribution.Repository, cr
7777
},
7878
}
7979

80-
if app.config.Pullthrough.Enabled {
81-
r.remoteBlobGetter = NewBlobGetterService(
82-
r.imageStream,
83-
r.imageStream.GetSecrets,
84-
r.cache,
85-
)
86-
}
80+
r.remoteBlobGetter = NewBlobGetterService(
81+
r.imageStream,
82+
r.imageStream.GetSecrets,
83+
r.cache,
84+
)
8785

8886
bdsf := blobDescriptorServiceFactoryFunc(r.BlobDescriptorService)
8987

@@ -109,17 +107,15 @@ func (r *repository) Manifests(ctx context.Context, options ...distribution.Mani
109107
acceptSchema2: r.app.config.Compatibility.AcceptSchema2,
110108
}
111109

112-
if r.app.config.Pullthrough.Enabled {
113-
ms = &pullthroughManifestService{
114-
ManifestService: ms,
115-
newLocalManifestService: func(ctx context.Context) (distribution.ManifestService, error) {
116-
return r.Repository.Manifests(ctx, opts...)
117-
},
118-
imageStream: r.imageStream,
119-
cache: r.cache,
120-
mirror: r.app.config.Pullthrough.Mirror,
121-
registryAddr: r.app.config.Server.Addr,
122-
}
110+
ms = &pullthroughManifestService{
111+
ManifestService: ms,
112+
newLocalManifestService: func(ctx context.Context) (distribution.ManifestService, error) {
113+
return r.Repository.Manifests(ctx, opts...)
114+
},
115+
imageStream: r.imageStream,
116+
cache: r.cache,
117+
mirror: r.app.config.Pullthrough.Mirror,
118+
registryAddr: r.app.config.Server.Addr,
123119
}
124120

125121
ms = newPendingErrorsManifestService(ms, r)
@@ -147,15 +143,13 @@ func (r *repository) Blobs(ctx context.Context) distribution.BlobStore {
147143
}
148144
}
149145

150-
if r.app.config.Pullthrough.Enabled {
151-
bs = &pullthroughBlobStore{
152-
BlobStore: bs,
146+
bs = &pullthroughBlobStore{
147+
BlobStore: bs,
153148

154-
remoteBlobGetter: r.remoteBlobGetter,
155-
writeLimiter: r.app.writeLimiter,
156-
mirror: r.app.config.Pullthrough.Mirror,
157-
newLocalBlobStore: r.Repository.Blobs,
158-
}
149+
remoteBlobGetter: r.remoteBlobGetter,
150+
writeLimiter: r.app.writeLimiter,
151+
mirror: r.app.config.Pullthrough.Mirror,
152+
newLocalBlobStore: r.Repository.Blobs,
159153
}
160154

161155
bs = newPendingErrorsBlobStore(bs, r)
@@ -176,9 +170,8 @@ func (r *repository) Tags(ctx context.Context) distribution.TagService {
176170
ts := r.Repository.Tags(ctx)
177171

178172
ts = &tagService{
179-
TagService: ts,
180-
imageStream: r.imageStream,
181-
pullthroughEnabled: r.app.config.Pullthrough.Enabled,
173+
TagService: ts,
174+
imageStream: r.imageStream,
182175
}
183176

184177
ts = newPendingErrorsTagService(ts, r)

‎pkg/dockerregistry/server/repository_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ func TestRepositoryBlobStat(t *testing.T) {
9999
stat string
100100
images []imageapiv1.Image
101101
imageStreams []imageapiv1.ImageStream
102-
pullthrough bool
103102
skipAuth bool
104103
deferredErrors deferredErrors
105104
expectedDescriptor distribution.Descriptor
@@ -165,7 +164,6 @@ func TestRepositoryBlobStat(t *testing.T) {
165164
},
166165
},
167166
},
168-
pullthrough: true,
169167
expectedDescriptor: testNewDescriptorForLayer(testImages["nm/unmanaged:missing-layer-links"][0].DockerImageLayers[1]),
170168
expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "images"}},
171169
},
@@ -205,7 +203,7 @@ func TestRepositoryBlobStat(t *testing.T) {
205203
},
206204
},
207205
expectedError: distribution.ErrBlobUnknown,
208-
expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "images"}},
206+
expectedActions: []clientAction{{"get", "imagestreams"}, {"get", "images"}, {"list", "imagestreams"}},
209207
},
210208

211209
{
@@ -232,7 +230,8 @@ func TestRepositoryBlobStat(t *testing.T) {
232230
},
233231
},
234232
},
235-
expectedError: distribution.ErrBlobUnknown,
233+
expectedError: distribution.ErrBlobUnknown,
234+
expectedActions: []clientAction{{"get", "imagestreams"}, {"list", "imagestreams"}},
236235
},
237236

238237
{
@@ -259,7 +258,8 @@ func TestRepositoryBlobStat(t *testing.T) {
259258
},
260259
},
261260
},
262-
expectedError: distribution.ErrBlobUnknown,
261+
expectedError: distribution.ErrBlobUnknown,
262+
expectedActions: []clientAction{{"get", "imagestreams"}, {"list", "imagestreams"}},
263263
},
264264

265265
{
@@ -312,7 +312,7 @@ func TestRepositoryBlobStat(t *testing.T) {
312312
}
313313
}
314314

315-
reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, cfg.Cache.BlobRepositoryTTL, tc.pullthrough, true)
315+
reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, cfg.Cache.BlobRepositoryTTL, true)
316316
if err != nil {
317317
t.Fatalf("unexpected error: %v", err)
318318
}
@@ -374,7 +374,7 @@ func TestRepositoryBlobStatCacheEviction(t *testing.T) {
374374
testutil.AddImageStream(t, fos, "nm", "is", nil)
375375
testutil.AddImage(t, fos, testImage, "nm", "is", "latest")
376376

377-
reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, blobRepoCacheTTL, false, false)
377+
reg, err := newTestRegistry(ctx, registryclient.NewFakeRegistryAPIClient(nil, imageClient), driver, blobRepoCacheTTL, false)
378378
if err != nil {
379379
t.Fatalf("unexpected error: %v", err)
380380
}

‎pkg/dockerregistry/server/tagservice.go

Lines changed: 4 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ import (
1010
type tagService struct {
1111
distribution.TagService
1212

13-
imageStream imagestream.ImageStream
14-
pullthroughEnabled bool
13+
imageStream imagestream.ImageStream
1514
}
1615

1716
func (t tagService) Get(ctx context.Context, tag string) (distribution.Descriptor, error) {
@@ -33,17 +32,6 @@ func (t tagService) Get(ctx context.Context, tag string) (distribution.Descripto
3332
return distribution.Descriptor{}, distribution.ErrTagUnknown{Tag: tag}
3433
}
3534

36-
if !t.pullthroughEnabled {
37-
image, err := t.imageStream.GetImageOfImageStream(ctx, dgst)
38-
if err != nil {
39-
return distribution.Descriptor{}, err
40-
}
41-
42-
if !imagestream.IsImageManaged(image) {
43-
return distribution.Descriptor{}, distribution.ErrTagUnknown{Tag: tag}
44-
}
45-
}
46-
4735
return distribution.Descriptor{Digest: dgst}, nil
4836
}
4937

@@ -62,30 +50,10 @@ func (t tagService) All(ctx context.Context) ([]string, error) {
6250
}
6351

6452
tagList := []string{}
65-
managedImages := make(map[string]bool)
66-
for tag, dgst := range tags {
67-
if t.pullthroughEnabled {
68-
tagList = append(tagList, tag)
69-
continue
70-
}
71-
72-
managed, found := managedImages[dgst.String()]
73-
if !found {
74-
image, err := t.imageStream.GetImageOfImageStream(ctx, dgst)
75-
if err != nil {
76-
context.GetLogger(ctx).Errorf("unable to get image %s %s: %v", t.imageStream.Reference(), dgst.String(), err)
77-
continue
78-
}
79-
managed = imagestream.IsImageManaged(image)
80-
managedImages[dgst.String()] = managed
81-
}
82-
83-
if !managed {
84-
continue
85-
}
86-
53+
for tag := range tags {
8754
tagList = append(tagList, tag)
8855
}
56+
8957
return tagList, nil
9058
}
9159

@@ -104,34 +72,14 @@ func (t tagService) Lookup(ctx context.Context, desc distribution.Descriptor) ([
10472
}
10573

10674
tagList := []string{}
107-
managedImages := make(map[string]bool)
10875
for tag, dgst := range tags {
10976
if dgst != desc.Digest {
11077
continue
11178
}
11279

113-
if t.pullthroughEnabled {
114-
tagList = append(tagList, tag)
115-
continue
116-
}
117-
118-
managed, found := managedImages[dgst.String()]
119-
if !found {
120-
image, err := t.imageStream.GetImageOfImageStream(ctx, dgst)
121-
if err != nil {
122-
context.GetLogger(ctx).Errorf("unable to get image %s %s: %v", t.imageStream.Reference(), dgst.String(), err)
123-
continue
124-
}
125-
managed = imagestream.IsImageManaged(image)
126-
managedImages[dgst.String()] = managed
127-
}
128-
129-
if !managed {
130-
continue
131-
}
132-
13380
tagList = append(tagList, tag)
13481
}
82+
13583
return tagList, nil
13684
}
13785

0 commit comments

Comments
 (0)