Consolidate image reference generation/lookup

pkg/build/api/v1beta1/conversion.go

@@ -90,9 +90,13 @@ func init() {
 			out.Tag = in.Tag
 			if len(in.DockerImageReference) > 0 {
 				out.DockerImageReference = in.DockerImageReference
-				registry, namespace, name, tag, _ := image.SplitDockerPullSpec(in.DockerImageReference)
-				out.Registry = registry
-				out.ImageTag = image.JoinDockerPullSpec("", namespace, name, tag)
+				ref, err := image.ParseDockerImageReference(in.DockerImageReference)
+				if err != nil {
+					return err
+				}
+				out.Registry = ref.Registry
+				ref.Registry = ""
+				out.ImageTag = ref.String()
 			}
 			return nil
 		},
@@ -106,11 +110,12 @@ func init() {
 				return nil
 			}
 			if len(in.ImageTag) != 0 {
-				_, namespace, name, tag, err := image.SplitDockerPullSpec(in.ImageTag)
+				ref, err := image.ParseDockerImageReference(in.ImageTag)
 				if err != nil {
 					return err
 				}
-				out.DockerImageReference = image.JoinDockerPullSpec(in.Registry, namespace, name, tag)
+				ref.Registry = in.Registry
+				out.DockerImageReference = ref.String()
 			}
 			return nil
 		},

pkg/build/api/validation/validation.go

@@ -126,7 +126,7 @@ func validateOutput(output *buildapi.BuildOutput) errs.ValidationErrorList {
 	}
 
 	if len(output.DockerImageReference) != 0 {
-		if _, _, _, _, err := imageapi.SplitDockerPullSpec(output.DockerImageReference); err != nil {
+		if _, err := imageapi.ParseDockerImageReference(output.DockerImageReference); err != nil {
 			allErrs = append(allErrs, errs.NewFieldInvalid("dockerImageReference", output.DockerImageReference, err.Error()))
 		}
 	}

pkg/build/builder/cmd/builder.go

@@ -49,11 +49,11 @@ func run(builderFactory factoryFunc) {
 		output = false
 	}
 	if output {
-		registry, _, _, _, err := image.SplitDockerPullSpec(build.Parameters.Output.DockerImageReference)
+		ref, err := image.ParseDockerImageReference(build.Parameters.Output.DockerImageReference)
 		if err != nil {
 			glog.Fatalf("Build output does not have a valid Docker image reference: %v", err)
 		}
-		authcfg, authPresent = dockercfg.NewHelper().GetDockerAuth(registry)
+		authcfg, authPresent = dockercfg.NewHelper().GetDockerAuth(ref.Registry)
 	}
 	b := builderFactory(client, endpoint, authcfg, authPresent, &build)
 	if err = b.Build(); err != nil {

pkg/build/controller/image_change_controller.go

@@ -62,16 +62,17 @@ func (c *ImageChangeController) HandleImageRepo(imageRepo *imageapi.ImageReposit
 			if len(tag) == 0 {
 				tag = buildapi.DefaultImageTag
 			}
-			imageID, hasTag := imageRepo.Tags[tag]
-			if !hasTag {
+			latest, err := imageapi.LatestTaggedImage(*imageRepo, tag)
+			if err != nil {
+				glog.V(2).Info(err)
 				continue
 			}
 
 			// (must be different) to trigger a build
-			if icTrigger.LastTriggeredImageID != imageID {
-				imageSubstitutions[icTrigger.Image] = imageRepo.Status.DockerImageRepository + ":" + imageID
+			if icTrigger.LastTriggeredImageID != latest.Image {
+				imageSubstitutions[icTrigger.Image] = latest.DockerImageReference
 				shouldTriggerBuild = true
-				icTrigger.LastTriggeredImageID = imageID
+				icTrigger.LastTriggeredImageID = latest.Image
 			}
 		}
 

pkg/build/controller/image_change_controller_test.go

@@ -73,12 +73,25 @@ func appendTrigger(buildcfg *buildapi.BuildConfig, triggerImage, repoName, repoT
 }
 
 func mockImageRepo(repoName, dockerImageRepo string, tags map[string]string) *imageapi.ImageRepository {
+	tagHistory := make(map[string]imageapi.TagEventList)
+	for tag, imageID := range tags {
+		tagHistory[tag] = imageapi.TagEventList{
+			Items: []imageapi.TagEvent{
+				{
+					Image:                imageID,
+					DockerImageReference: fmt.Sprintf("%s:%s", dockerImageRepo, imageID),
+				},
+			},
+		}
+	}
+
 	return &imageapi.ImageRepository{
 		ObjectMeta: kapi.ObjectMeta{
 			Name: repoName,
 		},
 		Status: imageapi.ImageRepositoryStatus{
 			DockerImageRepository: dockerImageRepo,
+			Tags: tagHistory,
 		},
 		Tags: tags,
 	}
@@ -105,7 +118,7 @@ func TestNewImageID(t *testing.T) {
 		t.Errorf("Unexpected error %v from HandleImageRepo", err)
 	}
 	if buildCreator.build == nil {
-		t.Error("Expected new build when new image was created!")
+		t.Fatal("Expected new build when new image was created!")
 	}
 	if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename:newImageID123" {
 		t.Errorf("Image substitutions not properly setup for new build.  Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.Image)
@@ -131,7 +144,7 @@ func TestNewImageIDDefaultTag(t *testing.T) {
 		t.Errorf("Unexpected error %v from HandleImageRepo", err)
 	}
 	if buildCreator.build == nil {
-		t.Error("Expected new build when new image was created!")
+		t.Fatal("Expected new build when new image was created!")
 	}
 	if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename:newImageID123" {
 		t.Errorf("Image substitutions not properly setup for new build.  Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.Image)
@@ -241,7 +254,7 @@ func TestMultipleTriggers(t *testing.T) {
 		t.Errorf("Unexpected error %v from HandleImageRepo", err)
 	}
 	if buildCreator.build == nil {
-		t.Error("Expected new build when new image was created!")
+		t.Fatal("Expected new build when new image was created!")
 	}
 	if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename1" {
 		t.Errorf("Image substitutions not properly setup for new build.  Expected %s, got %s |", "registry.com/namespace/imagename1", buildCreator.build.Parameters.Strategy.DockerStrategy.Image)
@@ -315,7 +328,7 @@ func TestBuildCreateError(t *testing.T) {
 		t.Error("Expected retryable error from HandleImageRepo")
 	}
 	if buildCreator.build == nil {
-		t.Error("Expected new build when new image was created!")
+		t.Fatal("Expected new build when new image was created!")
 	}
 	if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename:newImageID123" {
 		t.Errorf("Image substitutions not properly setup for new build.  Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.Image)
@@ -339,7 +352,7 @@ func TestBuildUpdateError(t *testing.T) {
 		t.Error("Expected fatal error from HandleImageRepo")
 	}
 	if buildCreator.build == nil {
-		t.Error("Expected new build when new image was created!")
+		t.Fatal("Expected new build when new image was created!")
 	}
 	if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename:newImageID123" {
 		t.Errorf("Image substitutions not properly setup for new build.  Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.Image)

pkg/build/controller/strategy/util.go

@@ -77,13 +77,13 @@ func setupBuildEnv(build *buildapi.Build, pod *kapi.Pod) error {
 		// Do nothing for unknown source types
 	}
 
-	registry, namespace, name, tag, err := imageapi.SplitDockerPullSpec(build.Parameters.Output.DockerImageReference)
+	ref, err := imageapi.ParseDockerImageReference(build.Parameters.Output.DockerImageReference)
 	if err != nil {
 		return err
 	}
-	outputImage := imageapi.JoinDockerPullSpec("", namespace, name, tag)
-	vars = append(vars, kapi.EnvVar{"OUTPUT_IMAGE", outputImage})
-	vars = append(vars, kapi.EnvVar{"OUTPUT_REGISTRY", registry})
+	vars = append(vars, kapi.EnvVar{"OUTPUT_REGISTRY", ref.Registry})
+	ref.Registry = ""
+	vars = append(vars, kapi.EnvVar{"OUTPUT_IMAGE", ref.String()})
 
 	if len(pod.Spec.Containers) > 0 {
 		pod.Spec.Containers[0].Env = append(pod.Spec.Containers[0].Env, vars...)

pkg/build/util/generate.go

@@ -102,12 +102,13 @@ func GenerateBuildWithImageTag(config *buildapi.BuildConfig, revision *buildapi.
 		if len(tag) == 0 {
 			tag = buildapi.DefaultImageTag
 		}
-		imageID, hasTag := imageRepo.Tags[tag]
-		if !hasTag {
+		latest, err := imageapi.LatestTaggedImage(*imageRepo, tag)
+		if err != nil {
 			continue
 		}
-		glog.V(4).Infof("Adding substitution %s with %s:%s", icTrigger.Image, imageRepo.Status.DockerImageRepository, imageID)
-		imageSubstitutions[icTrigger.Image] = imageRepo.Status.DockerImageRepository + ":" + imageID
+		imageRef := latest.DockerImageReference
+		glog.V(4).Infof("Adding substitution %s with %s", icTrigger.Image, imageRef)
+		imageSubstitutions[icTrigger.Image] = imageRef
 	}
 	glog.V(4).Infof("Generating build from config for build config %s", config.Name)
 	build := GenerateBuildFromConfig(config, revision, imageSubstitutions)

pkg/build/util/generate_test.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -552,6 +553,16 @@ func (m *mockImageRepositoryNamespaceGetter) GetByNamespace(namespace, name stri
 		ObjectMeta: kapi.ObjectMeta{Name: imageRepoName},
 		Status: imageapi.ImageRepositoryStatus{
 			DockerImageRepository: originalImage,
+			Tags: map[string]imageapi.TagEventList{
+				tagName: {
+					Items: []imageapi.TagEvent{
+						{
+							DockerImageReference: fmt.Sprintf("%s:%s", originalImage, newTag),
+							Image:                newTag,
+						},
+					},
+				},
+			},
 		},
 		Tags: map[string]string{tagName: newTag},
 	}, nil

pkg/cmd/experimental/generate/generate.go

@@ -152,9 +152,9 @@ func newImageResolver(namespace string, osClient osclient.Interface, dockerClien
 		}
 		namespaces = append(namespaces, "default")
 		imageStreamResolver := &genapp.ImageStreamResolver{
-			Client:     osClient,
-			Images:     osClient,
-			Namespaces: namespaces,
+			Client:            osClient,
+			ImageStreamImages: osClient,
+			Namespaces:        namespaces,
 		}
 		resolver = append(resolver, genapp.WeightedResolver{imageStreamResolver, 0.0})
 	}

pkg/deploy/controller/imagechange/controller.go

@@ -58,14 +58,25 @@ func (c *ImageChangeController) Handle(imageRepo *imageapi.ImageRepository) erro
 					continue
 				}
 
-				// The container image's tag name is by convention the same as the image ID it references
-				_, _, _, containerImageID, err := imageapi.SplitDockerPullSpec(container.Image)
+				ref, err := imageapi.ParseDockerImageReference(container.Image)
 				if err != nil {
 					glog.V(4).Infof("Skipping container %s for config %s; container's image is invalid: %v", container.Name, labelFor(config), err)
 					continue
 				}
 
-				if repoImageID, repoHasTag := imageRepo.Tags[params.Tag]; repoHasTag && repoImageID != containerImageID {
+				latest, err := imageapi.LatestTaggedImage(*imageRepo, params.Tag)
+				if err != nil {
+					glog.V(4).Infof("Skipping container %s for config %s; %s", container.Name, labelFor(config), err)
+					continue
+				}
+
+				containerImageID := ref.ID
+				if len(containerImageID) == 0 {
+					// For v1 images, the container image's tag name is by convention the same as the image ID it references
+					containerImageID = ref.Tag
+				}
+				if latest.Image != containerImageID {
+					glog.V(4).Infof("Container %s for config %s: image id changed from %q to %q; regenerating config", container.Name, labelFor(config), containerImageID, latest.Image)
 					configsToGenerate = append(configsToGenerate, config)
 					firedTriggersForConfig[config.Name] = append(firedTriggersForConfig[config.Name], params)
 				}
@@ -135,11 +146,11 @@ func (c *ImageChangeController) regenerate(imageRepo *imageapi.ImageRepository,
 				continue
 			}
 
-			id, ok := imageRepo.Tags[trigger.Tag]
-			if !ok {
-				// TODO: not really sure what to do here
+			latest, err := imageapi.LatestTaggedImage(*imageRepo, trigger.Tag)
+			if err != nil {
+				return fmt.Errorf("error generating new version of deploymentConfig: %s: %s", labelFor(config), err)
 			}
-			repoName = fmt.Sprintf("%s:%s", imageRepo.Status.DockerImageRepository, id)
+			repoName = latest.DockerImageReference
 		}
 
 		causes = append(causes,

pkg/deploy/controller/imagechange/controller_test.go

@@ -1,6 +1,7 @@
 package imagechange
 
 import (
+	"fmt"
 	"testing"
 
 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -132,25 +133,50 @@ func TestHande_matchScenarios(t *testing.T) {
 		},
 	}
 
+	tagHistoryFor := func(repo, tag, value string) map[string]imageapi.TagEventList {
+		return map[string]imageapi.TagEventList{
+			tag: {
+				Items: []imageapi.TagEvent{
+					{
+						DockerImageReference: fmt.Sprintf("%s:%s", repo, value),
+						Image:                value,
+					},
+				},
+			},
+		}
+	}
+
 	updates := map[string]*imageapi.ImageRepository{
 		"repo.1": {
 			ObjectMeta: kapi.ObjectMeta{Name: "repoA", Namespace: kapi.NamespaceDefault},
-			Status:     imageapi.ImageRepositoryStatus{DockerImageRepository: "registry:8080/openshift/test-image"},
-			Tags:       map[string]string{"test-tag": "ref-2"},
+			Status: imageapi.ImageRepositoryStatus{
+				DockerImageRepository: "registry:8080/openshift/test-image",
+				Tags: tagHistoryFor("registry:8080/openshift/test-image", "test-tag", "ref-2"),
+			},
+			Tags: map[string]string{"test-tag": "ref-2"},
 		},
 		"repo.2": {
 			ObjectMeta: kapi.ObjectMeta{Name: "repoB", Namespace: kapi.NamespaceDefault},
-			Status:     imageapi.ImageRepositoryStatus{DockerImageRepository: "registry:8080/openshift/test-image"},
-			Tags:       map[string]string{"test-tag": "ref-3"},
+			Status: imageapi.ImageRepositoryStatus{
+				DockerImageRepository: "registry:8080/openshift/test-image",
+				Tags: tagHistoryFor("registry:8080/openshift/test-image", "test-tag", "ref-3"),
+			},
+			Tags: map[string]string{"test-tag": "ref-3"},
 		},
 		"repo.3": {
 			ObjectMeta: kapi.ObjectMeta{Name: "repoC", Namespace: kapi.NamespaceDefault},
-			Status:     imageapi.ImageRepositoryStatus{DockerImageRepository: "registry:8080/openshift/test-image-B"},
-			Tags:       map[string]string{"test-tag": "ref-2"},
+			Status: imageapi.ImageRepositoryStatus{
+				DockerImageRepository: "registry:8080/openshift/test-image-B",
+				Tags: tagHistoryFor("registry:8080/openshift/test-image-B", "test-tag", "ref-2"),
+			},
+			Tags: map[string]string{"test-tag": "ref-2"},
 		},
 		"repo.4": {
 			ObjectMeta: kapi.ObjectMeta{Name: "repoA", Namespace: kapi.NamespaceDefault},
-			Tags:       map[string]string{"test-tag": "ref-2"},
+			Status: imageapi.ImageRepositoryStatus{
+				Tags: tagHistoryFor("default/repoA", "test-tag", "ref-2"),
+			},
+			Tags: map[string]string{"test-tag": "ref-2"},
 		},
 	}
 

pkg/deploy/generator/config_generator.go

@@ -213,6 +213,10 @@ func referencesByIndex(refs triggersByRef, legacy triggersByName) reposByIndex {
 func replaceReferences(dc *deployapi.DeploymentConfig, repos reposByIndex) (changed bool, errs errors.ValidationErrorList) {
 	template := dc.Template.ControllerTemplate.Template
 	for i, repo := range repos {
+		if len(repo.Status.DockerImageRepository) == 0 {
+			errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, fmt.Sprintf("image repository %s/%s does not have a Docker image repository reference set and can't be used in a deployment config trigger", repo.Namespace, repo.Name)))
+			continue
+		}
 		params := dc.Triggers[i].ImageChangeParams
 
 		// lookup image id
@@ -221,17 +225,14 @@ func replaceReferences(dc *deployapi.DeploymentConfig, repos reposByIndex) (chan
 			// TODO: replace with "preferred tag" from repo
 			tag = "latest"
 		}
-		id, ok := repo.Tags[tag]
-		if !ok {
-			errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, fmt.Sprintf("image repository %s/%s does not have tag %q", repo.Namespace, repo.Name, tag)))
-			continue
-		}
-		if len(repo.Status.DockerImageRepository) == 0 {
-			errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, fmt.Sprintf("image repository %s/%s does not have a Docker image repository reference set and can't be used in a deployment config trigger", repo.Namespace, repo.Name)))
+
+		// get the image ref from the repo's tag history
+		latest, err := imageapi.LatestTaggedImage(*repo, tag)
+		if err != nil {
+			errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, err.Error()))
 			continue
 		}
-		// TODO: this assumes that tag value is the image id
-		image := fmt.Sprintf("%s:%s", repo.Status.DockerImageRepository, id)
+		image := latest.DockerImageReference
 
 		// update containers
 		names := util.NewStringSet(params.ContainerNames...)