Skip to content

Commit 5aefe9c

Browse files
author
Michal Minář
committed
Existing images are tagged with correct reference on push
Prior to this patch, when docker pushed an image with manifest of schema 2 where the image already existed in the integrated registry, the tagevent used to point to the original image stream. With this patch, the dockerImageReference of the tag event will be rewritten to the target image stream of image stream mapping. Signed-off-by: Michal Minář <[email protected]>
1 parent 9b125cb commit 5aefe9c

File tree

2 files changed

+169
-28
lines changed

2 files changed

+169
-28
lines changed

‎pkg/image/registry/imagestreammapping/rest.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,25 @@ func (s *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, err
6464
tag = api.DefaultImageTag
6565
}
6666

67-
if err := s.imageRegistry.CreateImage(ctx, &image); err != nil && !errors.IsAlreadyExists(err) {
67+
imageCreateErr := s.imageRegistry.CreateImage(ctx, &image)
68+
if imageCreateErr != nil && !errors.IsAlreadyExists(imageCreateErr) {
6869
return nil, err
6970
}
7071

72+
// prefer dockerImageReference set on image for the tagEvent if the image is new
73+
ref := image.DockerImageReference
74+
if errors.IsAlreadyExists(imageCreateErr) && image.Annotations[api.ManagedByOpenShiftAnnotation] == "true" {
75+
// the image is managed by us and, most probably, tagged in some other image stream
76+
// let's make the reference local to this stream
77+
if streamRef, err := api.DockerImageReferenceForStream(stream); err == nil {
78+
streamRef.ID = image.Name
79+
ref = streamRef.Exact()
80+
}
81+
}
82+
7183
next := api.TagEvent{
7284
Created: unversioned.Now(),
73-
DockerImageReference: image.DockerImageReference,
85+
DockerImageReference: ref,
7486
Image: image.Name,
7587
}
7688

‎pkg/image/registry/imagestreammapping/rest_test.go

Lines changed: 155 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ import (
3434
_ "github.com/openshift/origin/pkg/api/install"
3535
)
3636

37-
var testDefaultRegistry = api.DefaultRegistryFunc(func() (string, bool) { return "defaultregistry:5000", true })
37+
const testDefaultRegistryURL = "defaultregistry:5000"
38+
39+
var testDefaultRegistry = api.DefaultRegistryFunc(func() (string, bool) { return testDefaultRegistryURL, true })
3840

3941
type fakeSubjectAccessReviewRegistry struct {
4042
}
@@ -74,6 +76,8 @@ func validImageStream() *api.ImageStream {
7476
}
7577
}
7678

79+
const testImageID = "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4"
80+
7781
func validNewMappingWithName() *api.ImageStreamMapping {
7882
return &api.ImageStreamMapping{
7983
ObjectMeta: kapi.ObjectMeta{
@@ -82,9 +86,10 @@ func validNewMappingWithName() *api.ImageStreamMapping {
8286
},
8387
Image: api.Image{
8488
ObjectMeta: kapi.ObjectMeta{
85-
Name: "imageID1",
89+
Name: testImageID,
90+
Annotations: map[string]string{api.ManagedByOpenShiftAnnotation: "true"},
8691
},
87-
DockerImageReference: "localhost:5000/default/somerepo:imageID1",
92+
DockerImageReference: "localhost:5000/default/somerepo@" + testImageID,
8893
DockerImageMetadata: api.DockerImage{
8994
Config: &api.DockerConfig{
9095
Cmd: []string{"ls", "/"},
@@ -171,9 +176,9 @@ func TestCreateSuccessWithName(t *testing.T) {
171176
t.Fatalf("Unexpected error creating mapping: %#v", err)
172177
}
173178

174-
image, err := storage.imageRegistry.GetImage(ctx, "imageID1")
179+
image, err := storage.imageRegistry.GetImage(ctx, testImageID)
175180
if err != nil {
176-
t.Errorf("Unexpected error retrieving image: %#v", err)
181+
t.Fatalf("Unexpected error retrieving image: %#v", err)
177182
}
178183
if e, a := mapping.Image.DockerImageReference, image.DockerImageReference; e != a {
179184
t.Errorf("Expected %s, got %s", e, a)
@@ -186,43 +191,33 @@ func TestCreateSuccessWithName(t *testing.T) {
186191
if err != nil {
187192
t.Errorf("Unexpected non-nil err: %#v", err)
188193
}
189-
if e, a := "imageID1", repo.Status.Tags["latest"].Items[0].Image; e != a {
194+
if e, a := testImageID, repo.Status.Tags["latest"].Items[0].Image; e != a {
190195
t.Errorf("Expected %s, got %s", e, a)
191196
}
192197
}
193198

194199
func TestAddExistingImageWithNewTag(t *testing.T) {
195-
imageID := "8d812da98d6dd61620343f1a5bf6585b34ad6ed16e5c5f7c7216a525d6aeb772"
200+
imageID := "sha256:8d812da98d6dd61620343f1a5bf6585b34ad6ed16e5c5f7c7216a525d6aeb772"
196201
existingRepo := &api.ImageStream{
197202
ObjectMeta: kapi.ObjectMeta{
198203
Name: "somerepo",
199204
Namespace: "default",
200205
},
201206
Spec: api.ImageStreamSpec{
202-
DockerImageRepository: "localhost:5000/someproject/somerepo",
203-
/*
204-
Tags: map[string]api.TagReference{
205-
"existingTag": {
206-
From: &kapi.ObjectReference{
207-
Kind: "ImageStreamTag",
208-
209-
Tag: "existingTag", Reference: imageID},
210-
},
211-
*/
207+
DockerImageRepository: "localhost:5000/default/somerepo",
212208
},
213209
Status: api.ImageStreamStatus{
214210
Tags: map[string]api.TagEventList{
215-
"existingTag": {Items: []api.TagEvent{{DockerImageReference: "localhost:5000/someproject/somerepo:" + imageID}}},
211+
"existingTag": {Items: []api.TagEvent{{DockerImageReference: "localhost:5000/somens/somerepo@" + imageID}}},
216212
},
217213
},
218214
}
219215

220216
existingImage := &api.Image{
221217
ObjectMeta: kapi.ObjectMeta{
222-
Name: imageID,
223-
Namespace: "default",
218+
Name: imageID,
224219
},
225-
DockerImageReference: "localhost:5000/someproject/somerepo:" + imageID,
220+
DockerImageReference: "localhost:5000/somens/somerepo@" + imageID,
226221
DockerImageMetadata: api.DockerImage{
227222
Config: &api.DockerConfig{
228223
Cmd: []string{"ls", "/"},
@@ -249,19 +244,153 @@ func TestAddExistingImageWithNewTag(t *testing.T) {
249244

250245
_, err = client.Put(
251246
context.TODO(),
252-
etcdtest.AddPrefix("/images/default/"+imageID), runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(v1.SchemeGroupVersion), existingImage),
247+
etcdtest.AddPrefix("/images/"+imageID), runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(v1.SchemeGroupVersion), existingImage),
253248
)
254249
if err != nil {
255250
t.Fatalf("Unexpected error: %v", err)
256251
}
257252

258253
mapping := api.ImageStreamMapping{
254+
ObjectMeta: kapi.ObjectMeta{
255+
Name: "somerepo",
256+
},
259257
Image: *existingImage,
260258
Tag: "latest",
261259
}
262-
_, err = storage.Create(kapi.NewDefaultContext(), &mapping)
263-
if !errors.IsInvalid(err) {
264-
t.Fatalf("Unexpected non-error creating mapping: %#v", err)
260+
ctx := kapi.NewDefaultContext()
261+
_, err = storage.Create(ctx, &mapping)
262+
if err != nil {
263+
t.Errorf("Unexpected error creating image stream mapping%v", err)
264+
}
265+
266+
image, err := storage.imageRegistry.GetImage(ctx, imageID)
267+
if err != nil {
268+
t.Errorf("Unexpected error retrieving image: %#v", err)
269+
}
270+
if e, a := mapping.Image.DockerImageReference, image.DockerImageReference; e != a {
271+
t.Errorf("Expected %s, got %s", e, a)
272+
}
273+
if !reflect.DeepEqual(mapping.Image.DockerImageMetadata, image.DockerImageMetadata) {
274+
t.Errorf("Expected %#v, got %#v", mapping.Image, image)
275+
}
276+
277+
repo, err := storage.imageStreamRegistry.GetImageStream(ctx, "somerepo")
278+
if err != nil {
279+
t.Fatalf("Unexpected non-nil err: %#v", err)
280+
}
281+
if e, a := imageID, repo.Status.Tags["latest"].Items[0].Image; e != a {
282+
t.Errorf("Expected %s, got %s", e, a)
283+
}
284+
tagEvent := api.LatestTaggedImage(repo, "latest")
285+
if e, a := image.DockerImageReference, tagEvent.DockerImageReference; e != a {
286+
t.Errorf("Unexpected tracking dockerImageReference: %q != %q", a, e)
287+
}
288+
289+
pullSpec, ok := api.ResolveLatestTaggedImage(repo, "latest")
290+
if !ok {
291+
t.Fatalf("Failed to resolv latest tagged image")
292+
}
293+
if e, a := image.DockerImageReference, pullSpec; e != a {
294+
t.Errorf("Expected %s, got %s", e, a)
295+
}
296+
}
297+
298+
func TestAddExistingImageOverridingDockerImageReference(t *testing.T) {
299+
imageID := "sha256:8d812da98d6dd61620343f1a5bf6585b34ad6ed16e5c5f7c7216a525d6aeb772"
300+
newRepo := &api.ImageStream{
301+
ObjectMeta: kapi.ObjectMeta{
302+
Namespace: "default",
303+
Name: "newrepo",
304+
},
305+
Spec: api.ImageStreamSpec{
306+
DockerImageRepository: "localhost:5000/default/newrepo",
307+
},
308+
Status: api.ImageStreamStatus{
309+
DockerImageRepository: "localhost:5000/default/newrepo",
310+
},
311+
}
312+
existingImage := &api.Image{
313+
ObjectMeta: kapi.ObjectMeta{
314+
Name: imageID,
315+
Annotations: map[string]string{api.ManagedByOpenShiftAnnotation: "true"},
316+
},
317+
DockerImageReference: "localhost:5000/someproject/somerepo@" + imageID,
318+
DockerImageMetadata: api.DockerImage{
319+
Config: &api.DockerConfig{
320+
Cmd: []string{"ls", "/"},
321+
Env: []string{"a=1"},
322+
ExposedPorts: map[string]struct{}{"1234/tcp": {}},
323+
Memory: 1234,
324+
CPUShares: 99,
325+
WorkingDir: "/workingDir",
326+
},
327+
},
328+
}
329+
330+
client, server, storage := setup(t)
331+
defer server.Terminate(t)
332+
333+
_, err := client.Put(
334+
context.TODO(),
335+
etcdtest.AddPrefix("/imagestreams/default/newrepo"),
336+
runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(v1.SchemeGroupVersion), newRepo),
337+
)
338+
if err != nil {
339+
t.Fatalf("Unexpected error: %v", err)
340+
}
341+
_, err = client.Put(
342+
context.TODO(),
343+
etcdtest.AddPrefix("/images/"+imageID), runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(v1.SchemeGroupVersion), existingImage),
344+
)
345+
if err != nil {
346+
t.Fatalf("Unexpected error: %v", err)
347+
}
348+
349+
mapping := api.ImageStreamMapping{
350+
ObjectMeta: kapi.ObjectMeta{
351+
Name: "newrepo",
352+
},
353+
Image: *existingImage,
354+
Tag: "latest",
355+
}
356+
ctx := kapi.NewDefaultContext()
357+
_, err = storage.Create(ctx, &mapping)
358+
if err != nil {
359+
t.Fatalf("Unexpected error creating mapping: %#v", err)
360+
}
361+
362+
image, err := storage.imageRegistry.GetImage(ctx, imageID)
363+
if err != nil {
364+
t.Errorf("Unexpected error retrieving image: %#v", err)
365+
}
366+
if e, a := mapping.Image.DockerImageReference, image.DockerImageReference; e != a {
367+
t.Errorf("Expected %s, got %s", e, a)
368+
}
369+
if !reflect.DeepEqual(mapping.Image.DockerImageMetadata, image.DockerImageMetadata) {
370+
t.Errorf("Expected %#v, got %#v", mapping.Image, image)
371+
}
372+
373+
repo, err := storage.imageStreamRegistry.GetImageStream(ctx, "newrepo")
374+
if err != nil {
375+
t.Fatalf("Unexpected non-nil err: %#v", err)
376+
}
377+
if e, a := imageID, repo.Status.Tags["latest"].Items[0].Image; e != a {
378+
t.Errorf("Expected %s, got %s", e, a)
379+
}
380+
tagEvent := api.LatestTaggedImage(repo, "latest")
381+
if e, a := testDefaultRegistryURL+"/default/newrepo@"+imageID, tagEvent.DockerImageReference; e != a {
382+
t.Errorf("Expected %s, got %s", e, a)
383+
}
384+
if tagEvent.DockerImageReference == image.DockerImageReference {
385+
t.Errorf("Expected image stream to have dockerImageReference other than %q", image.DockerImageReference)
386+
}
387+
388+
pullSpec, ok := api.ResolveLatestTaggedImage(repo, "latest")
389+
if !ok {
390+
t.Fatalf("Failed to resolv latest tagged image")
391+
}
392+
if e, a := testDefaultRegistryURL+"/default/newrepo@"+imageID, pullSpec; e != a {
393+
t.Errorf("Expected %s, got %s", e, a)
265394
}
266395
}
267396

@@ -291,7 +420,7 @@ func TestAddExistingImageAndTag(t *testing.T) {
291420
Name: "existingImage",
292421
Namespace: "default",
293422
},
294-
DockerImageReference: "localhost:5000/someproject/somerepo:imageID1",
423+
DockerImageReference: "localhost:5000/someproject/somerepo@" + testImageID,
295424
DockerImageMetadata: api.DockerImage{
296425
Config: &api.DockerConfig{
297426
Cmd: []string{"ls", "/"},

0 commit comments

Comments
 (0)