Change how tags and status events are recorded

TagEvent is set via PUT /imageRepositories/foo/status in
ImageRepositoryMappings. This means that users and admins/infra
components can set spec tags and status tags independently.

User set tags are true tag mappings, with eventual support for
digests within the repository.  They still require the user have
access to the image in order to get the pull spec.

pkg/build/controller/image_change_controller.go

@@ -3,9 +3,11 @@ package controller
 import (
 	"fmt"
 
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache"
 	"github.com/golang/glog"
 
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+
 	buildapi "github.com/openshift/origin/pkg/build/api"
 	buildclient "github.com/openshift/origin/pkg/build/client"
 	buildutil "github.com/openshift/origin/pkg/build/util"
@@ -33,63 +35,60 @@ type ImageChangeController struct {
 }
 
 // HandleImageRepo processes the next ImageRepository event.
-func (c *ImageChangeController) HandleImageRepo(imageRepo *imageapi.ImageRepository) error {
-	glog.V(4).Infof("Build image change controller detected imagerepo change %s", imageRepo.DockerImageRepository)
-	imageSubstitutions := make(map[string]string)
+func (c *ImageChangeController) HandleImageRepo(repo *imageapi.ImageRepository) error {
+	glog.V(4).Infof("Build image change controller detected imagerepo change %s", repo.Status.DockerImageRepository)
+	subs := make(map[string]string)
 
 	// TODO: this is inefficient
 	for _, bc := range c.BuildConfigStore.List() {
 		config := bc.(*buildapi.BuildConfig)
-		glog.V(4).Infof("Detecting changed images for buildConfig %s", config.Name)
 
-		// Extract relevant triggers for this imageRepo for this config
-		shouldTriggerBuild := false
+		// Extract relevant triggers for this repo for this config
+		shouldBuild := false
 		for _, trigger := range config.Triggers {
 			if trigger.Type != buildapi.ImageChangeBuildTriggerType {
 				continue
 			}
-			icTrigger := trigger.ImageChange
+			change := trigger.ImageChange
 			// only trigger a build if this image repo matches the name and namespace of the ref in the build trigger
 			// also do not trigger if the imagerepo does not have a valid DockerImageRepository value for us to pull
 			// the image from
-			if imageRepo.Status.DockerImageRepository == "" || icTrigger.From.Name != imageRepo.Name || (len(icTrigger.From.Namespace) != 0 && icTrigger.From.Namespace != imageRepo.Namespace) {
+			if repo.Status.DockerImageRepository == "" || change.From.Name != repo.Name || (len(change.From.Namespace) != 0 && change.From.Namespace != repo.Namespace) {
 				continue
 			}
-			// for every ImageChange trigger, record the image it substitutes for and get the latest
-			// image id from the imagerepository.  We will substitute all images in the buildconfig
-			// with the latest values from the imagerepositories.
-			tag := icTrigger.Tag
-			if len(tag) == 0 {
-				tag = buildapi.DefaultImageTag
-			}
-			latest, err := imageapi.LatestTaggedImage(*imageRepo, tag)
+			latest, err := imageapi.LatestTaggedImage(repo, change.Tag)
 			if err != nil {
-				glog.V(2).Info(err)
+				util.HandleError(fmt.Errorf("unable to find tagged image: %v", err))
 				continue
 			}
 
 			// (must be different) to trigger a build
-			if icTrigger.LastTriggeredImageID != latest.Image {
-				imageSubstitutions[icTrigger.Image] = latest.DockerImageReference
-				shouldTriggerBuild = true
-				icTrigger.LastTriggeredImageID = latest.Image
+			last := change.LastTriggeredImageID
+			next := latest.Image
+			if len(next) == 0 {
+				// tags without images should still trigger builds (when going from a pure tag to an image
+				// based tag, we should rebuild)
+				next = latest.DockerImageReference
+			}
+			if len(last) == 0 || next != last {
+				subs[change.Image] = latest.DockerImageReference
+				change.LastTriggeredImageID = next
+				shouldBuild = true
 			}
 		}
 
-		if shouldTriggerBuild {
+		if shouldBuild {
 			glog.V(4).Infof("Running build for buildConfig %s in namespace %s", config.Name, config.Namespace)
-			b := buildutil.GenerateBuildFromConfig(config, nil, imageSubstitutions)
+			b := buildutil.GenerateBuildFromConfig(config, nil, subs)
 			if err := c.BuildCreator.Create(config.Namespace, b); err != nil {
-				return fmt.Errorf("Error starting build for buildConfig %s: %v", config.Name, err)
-			} else {
-				if err := c.BuildConfigUpdater.Update(config); err != nil {
-					// This is not a retryable error because the build has been created.  The worst case
-					// outcome of not updating the buildconfig is that we might rerun a build for the
-					// same "new" imageid change in the future, which is better than guaranteeing we
-					// run the build 2+ times by retrying it here.
-					glog.V(2).Infof("Error updating buildConfig %v: %v", config.Name, err)
-					return ImageChangeControllerFatalError{Reason: fmt.Sprintf("Error updating buildConfig %s with new LastTriggeredImageID", config.Name), Err: err}
-				}
+				return fmt.Errorf("error starting build for buildConfig %s: %v", config.Name, err)
+			}
+			if err := c.BuildConfigUpdater.Update(config); err != nil {
+				// This is not a retryable error because the build has been created.  The worst case
+				// outcome of not updating the buildconfig is that we might rerun a build for the
+				// same "new" imageid change in the future, which is better than running the build
+				// 2+ times by retrying it here.
+				return ImageChangeControllerFatalError{Reason: fmt.Sprintf("Error updating buildConfig %s with new LastTriggeredImageID", config.Name), Err: err}
 			}
 		}
 	}

pkg/build/util/generate.go

@@ -102,7 +102,7 @@ func GenerateBuildWithImageTag(config *buildapi.BuildConfig, revision *buildapi.
 		if len(tag) == 0 {
 			tag = buildapi.DefaultImageTag
 		}
-		latest, err := imageapi.LatestTaggedImage(*imageRepo, tag)
+		latest, err := imageapi.LatestTaggedImage(imageRepo, tag)
 		if err != nil {
 			continue
 		}

pkg/cmd/server/origin/master.go

@@ -202,8 +202,8 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin
 
 	imageStorage := imageetcd.NewREST(c.EtcdHelper)
 	imageRegistry := image.NewRegistry(imageStorage)
-	imageRepositoryStorage := imagerepositoryetcd.NewREST(c.EtcdHelper, imagerepository.DefaultRegistryFunc(defaultRegistryFunc))
-	imageRepositoryRegistry := imagerepository.NewRegistry(imageRepositoryStorage)
+	imageRepositoryStorage, imageRepositoryStatus := imagerepositoryetcd.NewREST(c.EtcdHelper, imagerepository.DefaultRegistryFunc(defaultRegistryFunc))
+	imageRepositoryRegistry := imagerepository.NewRegistry(imageRepositoryStorage, imageRepositoryStatus)
 	imageRepositoryMappingStorage := imagerepositorymapping.NewREST(imageRegistry, imageRepositoryRegistry)
 	imageRepositoryTagStorage := imagerepositorytag.NewREST(imageRegistry, imageRepositoryRegistry)
 	imageStreamImageStorage := imagestreamimage.NewREST(imageRegistry, imageRepositoryRegistry)
@@ -232,14 +232,15 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin
 		"buildConfigs": buildconfigregistry.NewREST(buildEtcd),
 		"buildLogs":    buildlogregistry.NewREST(buildEtcd, c.BuildLogClient()),
 
-		"images":                  imageStorage,
-		"imageStreams":            imageRepositoryStorage,
-		"imageStreamImages":       imageStreamImageStorage,
-		"imageStreamMappings":     imageRepositoryMappingStorage,
-		"imageStreamTags":         imageRepositoryTagStorage,
-		"imageRepositories":       imageRepositoryStorage,
-		"imageRepositoryMappings": imageRepositoryMappingStorage,
-		"imageRepositoryTags":     imageRepositoryTagStorage,
+		"images":                   imageStorage,
+		"imageStreams":             imageRepositoryStorage,
+		"imageStreamImages":        imageStreamImageStorage,
+		"imageStreamMappings":      imageRepositoryMappingStorage,
+		"imageStreamTags":          imageRepositoryTagStorage,
+		"imageRepositories":        imageRepositoryStorage,
+		"imageRepositories/status": imageRepositoryStatus,
+		"imageRepositoryMappings":  imageRepositoryMappingStorage,
+		"imageRepositoryTags":      imageRepositoryTagStorage,
 
 		"deployments":               deployregistry.NewREST(deployEtcd),
 		"deploymentConfigs":         deployconfigregistry.NewREST(deployEtcd),

pkg/deploy/controller/imagechange/controller.go

@@ -64,7 +64,7 @@ func (c *ImageChangeController) Handle(imageRepo *imageapi.ImageRepository) erro
 					continue
 				}
 
-				latest, err := imageapi.LatestTaggedImage(*imageRepo, params.Tag)
+				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
@@ -146,7 +146,7 @@ func (c *ImageChangeController) regenerate(imageRepo *imageapi.ImageRepository,
 				continue
 			}
 
-			latest, err := imageapi.LatestTaggedImage(*imageRepo, trigger.Tag)
+			latest, err := imageapi.LatestTaggedImage(imageRepo, trigger.Tag)
 			if err != nil {
 				return fmt.Errorf("error generating new version of deploymentConfig: %s: %s", labelFor(config), err)
 			}

pkg/deploy/generator/config_generator.go

@@ -219,15 +219,8 @@ func replaceReferences(dc *deployapi.DeploymentConfig, repos reposByIndex) (chan
 		}
 		params := dc.Triggers[i].ImageChangeParams
 
-		// lookup image id
-		tag := params.Tag
-		if len(tag) == 0 {
-			// TODO: replace with "preferred tag" from repo
-			tag = "latest"
-		}
-
 		// get the image ref from the repo's tag history
-		latest, err := imageapi.LatestTaggedImage(*repo, tag)
+		latest, err := imageapi.LatestTaggedImage(repo, params.Tag)
 		if err != nil {
 			errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, err.Error()))
 			continue

pkg/generate/app/imagelookup.go

@@ -271,18 +271,17 @@ func (r ImageStreamResolver) Resolve(value string) (*ComponentMatch, error) {
 			return nil, err
 		}
 		searchTag := ref.Tag
-		// TODO: move to a lookup function on repo, or better yet, have the repo.Status.Tags field automatically infer latest
 		if len(searchTag) == 0 {
 			searchTag = "latest"
 		}
-		latest, err := imageapi.LatestTaggedImage(*repo, searchTag)
+		latest, err := imageapi.LatestTaggedImage(repo, searchTag)
 		if err != nil {
 			return nil, ErrNoMatch{value: value, qualifier: err.Error()}
 		}
 		imageData, err := r.ImageStreamImages.ImageStreamImages(namespace).Get(ref.Name, latest.Image)
 		if err != nil {
 			if errors.IsNotFound(err) {
-				return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("tag %q is set, but image %q has been removed", ref.Tag, latest.Image)}
+				return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("tag %q is set, but image %q has been removed", searchTag, latest.Image)}
 			}
 			return nil, err
 		}

pkg/image/api/helper.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
 // DockerDefaultNamespace is the value for namespace when a single segment name is provided.
@@ -18,38 +20,31 @@ type DockerImageReference struct {
 	ID        string
 }
 
-// COPIED from upstream
-// TODO remove
-func parseRepositoryTag(repos string) (string, string) {
+// TODO remove (base, tag, id)
+func parseRepositoryTag(repos string) (string, string, string) {
 	n := strings.Index(repos, "@")
 	if n >= 0 {
 		parts := strings.Split(repos, "@")
-		return parts[0], parts[1]
+		return parts[0], "", parts[1]
 	}
 	n = strings.LastIndex(repos, ":")
 	if n < 0 {
-		return repos, ""
+		return repos, "", ""
 	}
 	if tag := repos[n+1:]; !strings.Contains(tag, "/") {
-		return repos[:n], tag
+		return repos[:n], tag, ""
 	}
-	return repos, ""
+	return repos, "", ""
 }
 
 // ParseDockerImageReference parses a Docker pull spec string into a
 // DockerImageReference.
 func ParseDockerImageReference(spec string) (DockerImageReference, error) {
 	var (
-		ref     DockerImageReference
-		tag, id string
+		ref DockerImageReference
 	)
 	// TODO replace with docker version once docker/docker PR11109 is merged upstream
-	repo, tagOrID := parseRepositoryTag(spec)
-	if strings.Contains(tagOrID, ":") {
-		id = tagOrID
-	} else {
-		tag = tagOrID
-	}
+	repo, tag, id := parseRepositoryTag(spec)
 
 	repoParts := strings.Split(repo, "/")
 	switch len(repoParts) {
@@ -159,21 +154,108 @@ func ImageWithMetadata(image Image) (*Image, error) {
 	return &image, nil
 }
 
+func TagValueToTagEvent(repo *ImageRepository, value string) (*TagEvent, error) {
+	if strings.Contains(value, "@") {
+		segs := strings.SplitN(value, "@", 2)
+		if len(segs[1]) == 0 {
+			return nil, fmt.Errorf("%q may not end with a @", value)
+		}
+		ref, err := DockerImageReferenceForRepository(repo)
+		if err != nil {
+			return nil, err
+		}
+		ref.ID = segs[1]
+		return &TagEvent{
+			Created:              util.Now(),
+			DockerImageReference: ref.String(),
+			Image:                ref.ID,
+		}, nil
+	}
+	return LatestTaggedImage(repo, value)
+}
+
+// DockerImageReferenceForRepository returns a DockerImageReference that represents
+// the ImageRepository or false, if no valid reference exists.
+func DockerImageReferenceForRepository(repo *ImageRepository) (DockerImageReference, error) {
+	spec := repo.Status.DockerImageRepository
+	if len(spec) == 0 {
+		spec = repo.DockerImageRepository
+	}
+	if len(spec) == 0 {
+		return DockerImageReference{}, fmt.Errorf("no possible pull spec for %s/%s", repo.Namespace, repo.Name)
+	}
+	return ParseDockerImageReference(spec)
+}
+
 // LatestTaggedImage returns the most recent TagEvent for the specified image
-// repository and tag.
-func LatestTaggedImage(repo ImageRepository, tag string) (*TagEvent, error) {
-	if _, ok := repo.Tags[tag]; !ok {
-		return nil, fmt.Errorf("image repository %s/%s: tag %q not found", repo.Namespace, repo.Name, tag)
+// repository and tag. Will resolve lookups for the empty tag.
+func LatestTaggedImage(repo *ImageRepository, tag string) (*TagEvent, error) {
+	if len(tag) == 0 {
+		tag = "latest"
+	}
+	// find the most recent tag event with an image reference
+	if repo.Status.Tags != nil {
+		if history, ok := repo.Status.Tags[tag]; ok {
+			for _, item := range history.Items {
+				if len(item.DockerImageReference) > 0 {
+					return &item, nil
+				}
+			}
+		}
+	}
+
+	// infer a pull spec given the pull locations - requires the tag
+	// to have a value, for .status.DIR or .DIR to be set, and for
+	// one of those values to be valid.
+	if value, ok := repo.Tags[tag]; ok && len(value) > 0 {
+		ref, err := DockerImageReferenceForRepository(repo)
+		if err != nil {
+			return nil, err
+		}
+		ref.Tag = value
+		return &TagEvent{
+			Created:              util.Now(),
+			DockerImageReference: ref.String(),
+		}, nil
+	}
+
+	return nil, fmt.Errorf("no image recorded for %s/%s:%s", repo.Namespace, repo.Name, tag)
+}
+
+// AddTagEventToImageRepository attempts to update the given image repository with a tag event. It will
+// collapse duplicate entries - returning true if a change was made or false if no change
+// occurred.
+func AddTagEventToImageRepository(repo *ImageRepository, tag string, next TagEvent) bool {
+	if repo.Status.Tags == nil {
+		repo.Status.Tags = make(map[string]TagEventList)
+	}
+
+	tags, ok := repo.Status.Tags[tag]
+	if !ok || len(tags.Items) == 0 {
+		repo.Status.Tags[tag] = TagEventList{Items: []TagEvent{next}}
+		return true
 	}
 
-	tagHistory, ok := repo.Status.Tags[tag]
-	if !ok {
-		return nil, fmt.Errorf("image repository %s/%s: tag %q not found in tag history", repo.Namespace, repo.Name, tag)
+	previous := &tags.Items[0]
+
+	// image reference has not changed
+	if previous.DockerImageReference == next.DockerImageReference {
+		if next.Image == previous.Image {
+			return false
+		}
+		previous.Image = next.Image
+		repo.Status.Tags[tag] = tags
+		return true
 	}
 
-	if len(tagHistory.Items) == 0 {
-		return nil, fmt.Errorf("image repository %s/%s: tag %q has 0 history items", repo.Namespace, repo.Name, tag)
+	// image has not changed, but image reference has
+	if next.Image == previous.Image {
+		previous.DockerImageReference = next.DockerImageReference
+		repo.Status.Tags[tag] = tags
+		return true
 	}
 
-	return &tagHistory.Items[0], nil
+	tags.Items = append([]TagEvent{next}, tags.Items...)
+	repo.Status.Tags[tag] = tags
+	return true
 }

pkg/image/api/validation/validation.go

@@ -57,6 +57,14 @@ func ValidateImageRepositoryUpdate(newRepo, oldRepo *api.ImageRepository) errors
 	return result
 }
 
+func ValidateImageRepositoryStatusUpdate(newRepo, oldRepo *api.ImageRepository) errors.ValidationErrorList {
+	result := errors.ValidationErrorList{}
+	result = append(result, validation.ValidateObjectMetaUpdate(&oldRepo.ObjectMeta, &newRepo.ObjectMeta).Prefix("metadata")...)
+	newRepo.Tags = oldRepo.Tags
+	newRepo.DockerImageRepository = oldRepo.DockerImageRepository
+	return result
+}
+
 // ValidateImageRepositoryMapping tests required fields for an ImageRepositoryMapping.
 func ValidateImageRepositoryMapping(mapping *api.ImageRepositoryMapping) errors.ValidationErrorList {
 	result := errors.ValidationErrorList{}