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]>

Author: Michal Minář

pkg/image/registry/imagestreammapping/rest.go

@@ -1,6 +1,8 @@
 package imagestreammapping
 
 import (
+	"github.com/golang/glog"
+
 	kapi "k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/rest"
@@ -64,13 +66,27 @@ func (s *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, err
 		tag = api.DefaultImageTag
 	}
 
-	if err := s.imageRegistry.CreateImage(ctx, &image); err != nil && !errors.IsAlreadyExists(err) {
-		return nil, err
+	imageCreateErr := s.imageRegistry.CreateImage(ctx, &image)
+	if imageCreateErr != nil && !errors.IsAlreadyExists(imageCreateErr) {
+		return nil, imageCreateErr
+	}
+
+	// prefer dockerImageReference set on image for the tagEvent if the image is new
+	ref := image.DockerImageReference
+	if errors.IsAlreadyExists(imageCreateErr) && image.Annotations[api.ManagedByOpenShiftAnnotation] == "true" {
+		// the image is managed by us and, most probably, tagged in some other image stream
+		// let's make the reference local to this stream
+		if streamRef, err := api.DockerImageReferenceForStream(stream); err == nil {
+			streamRef.ID = image.Name
+			ref = streamRef.Exact()
+		} else {
+			glog.V(4).Infof("Failed to get dockerImageReference for stream %s/%s: %v", stream.Namespace, stream.Name, err)
+		}
 	}
 
 	next := api.TagEvent{
 		Created:              unversioned.Now(),
-		DockerImageReference: image.DockerImageReference,
+		DockerImageReference: ref,
 		Image:                image.Name,
 	}
 

pkg/image/registry/imagestreammapping/rest_test.go

@@ -34,7 +34,9 @@ import (
 	_ "github.com/openshift/origin/pkg/api/install"
 )
 
-var testDefaultRegistry = api.DefaultRegistryFunc(func() (string, bool) { return "defaultregistry:5000", true })
+const testDefaultRegistryURL = "defaultregistry:5000"
+
+var testDefaultRegistry = api.DefaultRegistryFunc(func() (string, bool) { return testDefaultRegistryURL, true })
 
 type fakeSubjectAccessReviewRegistry struct {
 }
@@ -74,6 +76,8 @@ func validImageStream() *api.ImageStream {
 	}
 }
 
+const testImageID = "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4"
+
 func validNewMappingWithName() *api.ImageStreamMapping {
 	return &api.ImageStreamMapping{
 		ObjectMeta: kapi.ObjectMeta{
@@ -82,9 +86,10 @@ func validNewMappingWithName() *api.ImageStreamMapping {
 		},
 		Image: api.Image{
 			ObjectMeta: kapi.ObjectMeta{
-				Name: "imageID1",
+				Name:        testImageID,
+				Annotations: map[string]string{api.ManagedByOpenShiftAnnotation: "true"},
 			},
-			DockerImageReference: "localhost:5000/default/somerepo:imageID1",
+			DockerImageReference: "localhost:5000/default/somerepo@" + testImageID,
 			DockerImageMetadata: api.DockerImage{
 				Config: &api.DockerConfig{
 					Cmd:          []string{"ls", "/"},
@@ -171,9 +176,9 @@ func TestCreateSuccessWithName(t *testing.T) {
 		t.Fatalf("Unexpected error creating mapping: %#v", err)
 	}
 
-	image, err := storage.imageRegistry.GetImage(ctx, "imageID1")
+	image, err := storage.imageRegistry.GetImage(ctx, testImageID)
 	if err != nil {
-		t.Errorf("Unexpected error retrieving image: %#v", err)
+		t.Fatalf("Unexpected error retrieving image: %#v", err)
 	}
 	if e, a := mapping.Image.DockerImageReference, image.DockerImageReference; e != a {
 		t.Errorf("Expected %s, got %s", e, a)
@@ -186,43 +191,33 @@ func TestCreateSuccessWithName(t *testing.T) {
 	if err != nil {
 		t.Errorf("Unexpected non-nil err: %#v", err)
 	}
-	if e, a := "imageID1", repo.Status.Tags["latest"].Items[0].Image; e != a {
+	if e, a := testImageID, repo.Status.Tags["latest"].Items[0].Image; e != a {
 		t.Errorf("Expected %s, got %s", e, a)
 	}
 }
 
 func TestAddExistingImageWithNewTag(t *testing.T) {
-	imageID := "8d812da98d6dd61620343f1a5bf6585b34ad6ed16e5c5f7c7216a525d6aeb772"
+	imageID := "sha256:8d812da98d6dd61620343f1a5bf6585b34ad6ed16e5c5f7c7216a525d6aeb772"
 	existingRepo := &api.ImageStream{
 		ObjectMeta: kapi.ObjectMeta{
 			Name:      "somerepo",
 			Namespace: "default",
 		},
 		Spec: api.ImageStreamSpec{
-			DockerImageRepository: "localhost:5000/someproject/somerepo",
-			/*
-				Tags: map[string]api.TagReference{
-					"existingTag": {
-						From: &kapi.ObjectReference{
-							Kind: "ImageStreamTag",
-
-						Tag: "existingTag", Reference: imageID},
-				},
-			*/
+			DockerImageRepository: "localhost:5000/default/somerepo",
 		},
 		Status: api.ImageStreamStatus{
 			Tags: map[string]api.TagEventList{
-				"existingTag": {Items: []api.TagEvent{{DockerImageReference: "localhost:5000/someproject/somerepo:" + imageID}}},
+				"existingTag": {Items: []api.TagEvent{{DockerImageReference: "localhost:5000/somens/somerepo@" + imageID}}},
 			},
 		},
 	}
 
 	existingImage := &api.Image{
 		ObjectMeta: kapi.ObjectMeta{
-			Name:      imageID,
-			Namespace: "default",
+			Name: imageID,
 		},
-		DockerImageReference: "localhost:5000/someproject/somerepo:" + imageID,
+		DockerImageReference: "localhost:5000/somens/somerepo@" + imageID,
 		DockerImageMetadata: api.DockerImage{
 			Config: &api.DockerConfig{
 				Cmd:          []string{"ls", "/"},
@@ -249,19 +244,153 @@ func TestAddExistingImageWithNewTag(t *testing.T) {
 
 	_, err = client.Put(
 		context.TODO(),
-		etcdtest.AddPrefix("/images/default/"+imageID), runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(v1.SchemeGroupVersion), existingImage),
+		etcdtest.AddPrefix("/images/"+imageID), runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(v1.SchemeGroupVersion), existingImage),
 	)
 	if err != nil {
 		t.Fatalf("Unexpected error: %v", err)
 	}
 
 	mapping := api.ImageStreamMapping{
+		ObjectMeta: kapi.ObjectMeta{
+			Name: "somerepo",
+		},
 		Image: *existingImage,
 		Tag:   "latest",
 	}
-	_, err = storage.Create(kapi.NewDefaultContext(), &mapping)
-	if !errors.IsInvalid(err) {
-		t.Fatalf("Unexpected non-error creating mapping: %#v", err)
+	ctx := kapi.NewDefaultContext()
+	_, err = storage.Create(ctx, &mapping)
+	if err != nil {
+		t.Errorf("Unexpected error creating image stream mapping%v", err)
+	}
+
+	image, err := storage.imageRegistry.GetImage(ctx, imageID)
+	if err != nil {
+		t.Errorf("Unexpected error retrieving image: %#v", err)
+	}
+	if e, a := mapping.Image.DockerImageReference, image.DockerImageReference; e != a {
+		t.Errorf("Expected %s, got %s", e, a)
+	}
+	if !reflect.DeepEqual(mapping.Image.DockerImageMetadata, image.DockerImageMetadata) {
+		t.Errorf("Expected %#v, got %#v", mapping.Image, image)
+	}
+
+	repo, err := storage.imageStreamRegistry.GetImageStream(ctx, "somerepo")
+	if err != nil {
+		t.Fatalf("Unexpected non-nil err: %#v", err)
+	}
+	if e, a := imageID, repo.Status.Tags["latest"].Items[0].Image; e != a {
+		t.Errorf("Expected %s, got %s", e, a)
+	}
+	tagEvent := api.LatestTaggedImage(repo, "latest")
+	if e, a := image.DockerImageReference, tagEvent.DockerImageReference; e != a {
+		t.Errorf("Unexpected tracking dockerImageReference: %q != %q", a, e)
+	}
+
+	pullSpec, ok := api.ResolveLatestTaggedImage(repo, "latest")
+	if !ok {
+		t.Fatalf("Failed to resolv latest tagged image")
+	}
+	if e, a := image.DockerImageReference, pullSpec; e != a {
+		t.Errorf("Expected %s, got %s", e, a)
+	}
+}
+
+func TestAddExistingImageOverridingDockerImageReference(t *testing.T) {
+	imageID := "sha256:8d812da98d6dd61620343f1a5bf6585b34ad6ed16e5c5f7c7216a525d6aeb772"
+	newRepo := &api.ImageStream{
+		ObjectMeta: kapi.ObjectMeta{
+			Namespace: "default",
+			Name:      "newrepo",
+		},
+		Spec: api.ImageStreamSpec{
+			DockerImageRepository: "localhost:5000/default/newrepo",
+		},
+		Status: api.ImageStreamStatus{
+			DockerImageRepository: "localhost:5000/default/newrepo",
+		},
+	}
+	existingImage := &api.Image{
+		ObjectMeta: kapi.ObjectMeta{
+			Name:        imageID,
+			Annotations: map[string]string{api.ManagedByOpenShiftAnnotation: "true"},
+		},
+		DockerImageReference: "localhost:5000/someproject/somerepo@" + imageID,
+		DockerImageMetadata: api.DockerImage{
+			Config: &api.DockerConfig{
+				Cmd:          []string{"ls", "/"},
+				Env:          []string{"a=1"},
+				ExposedPorts: map[string]struct{}{"1234/tcp": {}},
+				Memory:       1234,
+				CPUShares:    99,
+				WorkingDir:   "/workingDir",
+			},
+		},
+	}
+
+	client, server, storage := setup(t)
+	defer server.Terminate(t)
+
+	_, err := client.Put(
+		context.TODO(),
+		etcdtest.AddPrefix("/imagestreams/default/newrepo"),
+		runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(v1.SchemeGroupVersion), newRepo),
+	)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	_, err = client.Put(
+		context.TODO(),
+		etcdtest.AddPrefix("/images/"+imageID), runtime.EncodeOrDie(kapi.Codecs.LegacyCodec(v1.SchemeGroupVersion), existingImage),
+	)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	mapping := api.ImageStreamMapping{
+		ObjectMeta: kapi.ObjectMeta{
+			Name: "newrepo",
+		},
+		Image: *existingImage,
+		Tag:   "latest",
+	}
+	ctx := kapi.NewDefaultContext()
+	_, err = storage.Create(ctx, &mapping)
+	if err != nil {
+		t.Fatalf("Unexpected error creating mapping: %#v", err)
+	}
+
+	image, err := storage.imageRegistry.GetImage(ctx, imageID)
+	if err != nil {
+		t.Errorf("Unexpected error retrieving image: %#v", err)
+	}
+	if e, a := mapping.Image.DockerImageReference, image.DockerImageReference; e != a {
+		t.Errorf("Expected %s, got %s", e, a)
+	}
+	if !reflect.DeepEqual(mapping.Image.DockerImageMetadata, image.DockerImageMetadata) {
+		t.Errorf("Expected %#v, got %#v", mapping.Image, image)
+	}
+
+	repo, err := storage.imageStreamRegistry.GetImageStream(ctx, "newrepo")
+	if err != nil {
+		t.Fatalf("Unexpected non-nil err: %#v", err)
+	}
+	if e, a := imageID, repo.Status.Tags["latest"].Items[0].Image; e != a {
+		t.Errorf("Expected %s, got %s", e, a)
+	}
+	tagEvent := api.LatestTaggedImage(repo, "latest")
+	if e, a := testDefaultRegistryURL+"/default/newrepo@"+imageID, tagEvent.DockerImageReference; e != a {
+		t.Errorf("Expected %s, got %s", e, a)
+	}
+	if tagEvent.DockerImageReference == image.DockerImageReference {
+		t.Errorf("Expected image stream to have dockerImageReference other than %q", image.DockerImageReference)
+	}
+
+	pullSpec, ok := api.ResolveLatestTaggedImage(repo, "latest")
+	if !ok {
+		t.Fatalf("Failed to resolv latest tagged image")
+	}
+	if e, a := testDefaultRegistryURL+"/default/newrepo@"+imageID, pullSpec; e != a {
+		t.Errorf("Expected %s, got %s", e, a)
 	}
 }
 
@@ -291,7 +420,7 @@ func TestAddExistingImageAndTag(t *testing.T) {
 			Name:      "existingImage",
 			Namespace: "default",
 		},
-		DockerImageReference: "localhost:5000/someproject/somerepo:imageID1",
+		DockerImageReference: "localhost:5000/someproject/somerepo@" + testImageID,
 		DockerImageMetadata: api.DockerImage{
 			Config: &api.DockerConfig{
 				Cmd:          []string{"ls", "/"},