Omit changes that aren't, in auto-releases

Part of the problem being addressed here is that releases decide what
to do based on the images used in the cluster, and that does not
always correspond to changes in the repo. This commit removes the
possibility for automated releases, by

 - skipping any containers that _already_ use the supposedly new image

 - building the commit message by looking only at the images that were
   successfully changed.

daemon/daemon.go

@@ -348,7 +348,7 @@ func (d *Daemon) release(spec update.Spec, c release.Changes) daemonJobFunc {
 		if c.ReleaseKind() == update.ReleaseKindExecute {
 			commitMsg := spec.Cause.Message
 			if commitMsg == "" {
-				commitMsg = c.CommitMessage()
+				commitMsg = c.CommitMessage(result)
 			}
 			commitAuthor := ""
 			if d.GitConfig.SetAuthor {

event/event.go

@@ -91,7 +91,7 @@ func (e Event) String() string {
 	switch e.Type {
 	case EventRelease:
 		metadata := e.Metadata.(*ReleaseEventMetadata)
-		strImageIDs := metadata.Result.ImageIDs()
+		strImageIDs := metadata.Result.ChangedImages()
 		if len(strImageIDs) == 0 {
 			strImageIDs = []string{"no image changes"}
 		}
@@ -121,7 +121,7 @@ func (e Event) String() string {
 		)
 	case EventAutoRelease:
 		metadata := e.Metadata.(*AutoReleaseEventMetadata)
-		strImageIDs := metadata.Result.ImageIDs()
+		strImageIDs := metadata.Result.ChangedImages()
 		if len(strImageIDs) == 0 {
 			strImageIDs = []string{"no image changes"}
 		}

release/releaser.go

@@ -12,7 +12,7 @@ type Changes interface {
 	CalculateRelease(update.ReleaseContext, log.Logger) ([]*update.ControllerUpdate, update.Result, error)
 	ReleaseKind() update.ReleaseKind
 	ReleaseType() update.ReleaseType
-	CommitMessage() string
+	CommitMessage(update.Result) string
 }
 
 func Release(rc *ReleaseContext, changes Changes, logger log.Logger) (results update.Result, err error) {

update/automated.go

@@ -1,8 +1,8 @@
 package update
 
 import (
+	"bytes"
 	"fmt"
-	"strings"
 
 	"github.com/go-kit/kit/log"
 	"github.com/weaveworks/flux"
@@ -52,24 +52,24 @@ func (a *Automated) ReleaseKind() ReleaseKind {
 	return ReleaseKindExecute
 }
 
-func (a *Automated) CommitMessage() string {
-	var images []string
-	for _, image := range a.Images() {
-		images = append(images, image.String())
+func (a *Automated) CommitMessage(result Result) string {
+	images := result.ChangedImages()
+	buf := &bytes.Buffer{}
+	prefix := ""
+	switch len(images) {
+	case 0: // FIXME(michael): can we get here?
+		fmt.Fprintln(buf, "Auto-release (no images)")
+	case 1:
+		fmt.Fprint(buf, "Auto-release ")
+	default:
+		fmt.Fprintln(buf, "Auto-release multiple images")
+		fmt.Fprintln(buf)
+		prefix = " - "
 	}
-	return fmt.Sprintf("Release %s to automated", strings.Join(images, ", "))
-}
-
-func (a *Automated) Images() []image.Ref {
-	imageMap := map[image.Ref]struct{}{}
-	for _, change := range a.Changes {
-		imageMap[change.ImageID] = struct{}{}
+	for _, im := range images {
+		fmt.Fprintf(buf, "%s%s\n", prefix, im)
 	}
-	var images []image.Ref
-	for image, _ := range imageMap {
-		images = append(images, image)
-	}
-	return images
+	return buf.String()
 }
 
 func (a *Automated) markSkipped(results Result) {
@@ -88,15 +88,7 @@ func (a *Automated) calculateImageUpdates(rc ReleaseContext, candidates []*Contr
 
 	serviceMap := a.serviceMap()
 	for _, u := range candidates {
-		containers, err := u.Controller.ContainersOrError()
-		if err != nil {
-			result[u.ResourceID] = ControllerResult{
-				Status: ReleaseStatusFailed,
-				Error:  err.Error(),
-			}
-			continue
-		}
-
+		containers := u.Resource.Containers()
 		changes := serviceMap[u.ResourceID]
 		containerUpdates := []ContainerUpdate{}
 		for _, container := range containers {
@@ -106,7 +98,16 @@ func (a *Automated) calculateImageUpdates(rc ReleaseContext, candidates []*Contr
 					continue
 				}
 
+				// It turns out this isn't a change after all; skip this container
+				if change.ImageID.CanonicalRef() == container.Image.CanonicalRef() {
+					continue
+				}
+
+				// We transplant the tag here, to make sure we keep
+				// the format of the image name as it is in the
+				// resource (e.g., to avoid canonicalising it)
 				newImageID := currentImageID.WithNewTag(change.ImageID.Tag)
+				var err error
 				u.ManifestBytes, err = rc.Manifests().UpdateDefinition(u.ManifestBytes, container.Name, newImageID)
 				if err != nil {
 					return nil, err
@@ -129,15 +130,16 @@ func (a *Automated) calculateImageUpdates(rc ReleaseContext, candidates []*Contr
 			}
 		} else {
 			result[u.ResourceID] = ControllerResult{
-				Status: ReleaseStatusIgnored,
-				Error:  DoesNotUseImage,
+				Status: ReleaseStatusSkipped,
+				Error:  ImageUpToDate,
 			}
 		}
 	}
 
 	return updates, nil
 }
 
+// serviceMap transposes the changes so they can be looked up by ID
 func (a *Automated) serviceMap() map[flux.ResourceID][]Change {
 	set := map[flux.ResourceID][]Change{}
 	for _, change := range a.Changes {

update/release.go

@@ -96,7 +96,7 @@ func (s ReleaseSpec) ReleaseKind() ReleaseKind {
 	return s.Kind
 }
 
-func (s ReleaseSpec) CommitMessage() string {
+func (s ReleaseSpec) CommitMessage(result Result) string {
 	image := strings.Trim(s.ImageSpec.String(), "<>")
 	var services []string
 	for _, spec := range s.ServiceSpecs {

update/result.go

@@ -30,9 +30,12 @@ func (r Result) ServiceIDs() []string {
 	return result
 }
 
-func (r Result) ImageIDs() []string {
+func (r Result) ChangedImages() []string {
 	images := map[image.Ref]struct{}{}
 	for _, serviceResult := range r {
+		if serviceResult.Status != ReleaseStatusSuccess {
+			continue
+		}
 		for _, containerResult := range serviceResult.PerContainer {
 			images[containerResult.Target] = struct{}{}
 		}