Skip to content

Commit 39e4f8c

Browse files
author
OpenShift Bot
authored
Merge pull request #12525 from miminar/existing-images-pushed-with-correct-ref
Merged by openshift-bot
2 parents 249ec86 + 4b5918e commit 39e4f8c

File tree

2 files changed

+174
-29
lines changed

2 files changed

+174
-29
lines changed

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

+19-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package imagestreammapping
22

33
import (
4+
"github.com/golang/glog"
5+
46
kapi "k8s.io/kubernetes/pkg/api"
57
"k8s.io/kubernetes/pkg/api/errors"
68
"k8s.io/kubernetes/pkg/api/rest"
@@ -64,13 +66,27 @@ func (s *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, err
6466
tag = api.DefaultImageTag
6567
}
6668

67-
if err := s.imageRegistry.CreateImage(ctx, &image); err != nil && !errors.IsAlreadyExists(err) {
68-
return nil, err
69+
imageCreateErr := s.imageRegistry.CreateImage(ctx, &image)
70+
if imageCreateErr != nil && !errors.IsAlreadyExists(imageCreateErr) {
71+
return nil, imageCreateErr
72+
}
73+
74+
// prefer dockerImageReference set on image for the tagEvent if the image is new
75+
ref := image.DockerImageReference
76+
if errors.IsAlreadyExists(imageCreateErr) && image.Annotations[api.ManagedByOpenShiftAnnotation] == "true" {
77+
// the image is managed by us and, most probably, tagged in some other image stream
78+
// let's make the reference local to this stream
79+
if streamRef, err := api.DockerImageReferenceForStream(stream); err == nil {
80+
streamRef.ID = image.Name
81+
ref = streamRef.Exact()
82+
} else {
83+
glog.V(4).Infof("Failed to get dockerImageReference for stream %s/%s: %v", stream.Namespace, stream.Name, err)
84+
}
6985
}
7086

7187
next := api.TagEvent{
7288
Created: unversioned.Now(),
73-
DockerImageReference: image.DockerImageReference,
89+
DockerImageReference: ref,
7490
Image: image.Name,
7591
}
7692

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

+155-26
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)