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

@@ -62,7 +62,7 @@ func (c *ImageChangeController) HandleImageRepo(imageRepo *imageapi.ImageReposit
 			if len(tag) == 0 {
 				tag = buildapi.DefaultImageTag
 			}
-			latest, err := imageapi.LatestTaggedImage(*imageRepo, tag)
+			latest, err := imageapi.LatestTaggedImage(imageRepo, tag)
 			if err != nil {
 				glog.V(2).Info(err)
 				continue

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{}

pkg/image/controller/controller.go

@@ -21,21 +21,36 @@ type ImportController struct {
 	client       dockerregistry.Client
 }
 
+// needsImport returns true if the provided repository should have its tags imported.
+func needsImport(repo *api.ImageRepository) bool {
+	if len(repo.DockerImageRepository) == 0 {
+		return false
+	}
+	if repo.Annotations != nil && len(repo.Annotations[dockerImageRepositoryCheckAnnotation]) != 0 {
+		return false
+	}
+	if len(repo.Tags) == 0 {
+		return true
+	}
+	emptyTags := 0
+	for _, v := range repo.Tags {
+		if len(v) == 0 {
+			emptyTags++
+		}
+	}
+	return emptyTags > 0
+}
+
 // Next processes the given image repository, looking for repos that have DockerImageRepository
 // set but have not yet been marked as "ready". If transient errors occur, err is returned but
 // the image repository is not modified (so it will be tried again later). If a permanent
-// failure occurs the image is marked with an annotation.
+// failure occurs the image is marked with an annotation. The tags of the original spec image
+// are left as is (those are updated through status).
 func (c *ImportController) Next(repo *api.ImageRepository) error {
-	name := repo.DockerImageRepository
-	if len(name) == 0 {
-		return nil
-	}
-	if repo.Annotations == nil {
-		repo.Annotations = make(map[string]string)
-	}
-	if len(repo.Annotations[dockerImageRepositoryCheckAnnotation]) != 0 {
+	if !needsImport(repo) {
 		return nil
 	}
+	name := repo.DockerImageRepository
 
 	ref, err := api.ParseDockerImageReference(name)
 	if err != nil {
@@ -85,9 +100,6 @@ func (c *ImportController) Next(repo *api.ImageRepository) error {
 		}
 	}
 
-	// whether we ignore or succeed, ensure the most recent mappings are recorded
-	repo.Tags = newTags
-
 	// nothing to tag - no images in the upstream repo, or we're in sync
 	if len(imageToTag) == 0 {
 		return c.done(repo, "")
@@ -156,6 +168,9 @@ func (c *ImportController) done(repo *api.ImageRepository, reason string) error
 	if len(reason) == 0 {
 		reason = util.Now().UTC().Format(time.RFC3339)
 	}
+	if repo.Annotations == nil {
+		repo.Annotations = make(map[string]string)
+	}
 	repo.Annotations[dockerImageRepositoryCheckAnnotation] = reason
 	if _, err := c.repositories.ImageRepositories(repo.Namespace).Update(repo); err != nil && !errors.IsNotFound(err) {
 		return err

pkg/image/controller/controller_test.go

@@ -132,11 +132,11 @@ func TestControllerRepoTagsAlreadySet(t *testing.T) {
 		ObjectMeta:            kapi.ObjectMeta{Name: "test", Namespace: "other"},
 		DockerImageRepository: "foo/bar",
 		Tags: map[string]string{
-			"test": "value",
+			"test": "",
 		},
 	}
 	if err := c.Next(&repo); err != nil {
-		t.Errorf("unexpected error: %v", err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 	if len(repo.Annotations["openshift.io/image.dockerRepositoryCheck"]) == 0 {
 		t.Errorf("did not set annotation: %#v", repo)