Merge pull request #1036 from weaveworks/issue/1026-look-at-git-not-c…

…luster

Make auto-release commits and events more accurate

cluster/cluster.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 
 	"github.com/weaveworks/flux"
+	"github.com/weaveworks/flux/resource"
 	"github.com/weaveworks/flux/ssh"
 )
 
@@ -37,26 +38,18 @@ type Controller struct {
 	Containers ContainersOrExcuse
 }
 
-// A Container represents a container specification in a pod. The Name
-// identifies it within the pod, and the Image says which image it's
-// configured to run.
-type Container struct {
-	Name  string
-	Image string
-}
-
 // Sometimes we care if we can't find the containers for a service,
 // sometimes we just want the information we can get.
 type ContainersOrExcuse struct {
 	Excuse     string
-	Containers []Container
+	Containers []resource.Container
 }
 
-func (s Controller) ContainersOrNil() []Container {
+func (s Controller) ContainersOrNil() []resource.Container {
 	return s.Containers.Containers
 }
 
-func (s Controller) ContainersOrError() ([]Container, error) {
+func (s Controller) ContainersOrError() ([]resource.Container, error) {
 	var err error
 	if s.Containers.Excuse != "" {
 		err = errors.New(s.Containers.Excuse)

cluster/kubernetes/resource/cronjob.go

@@ -1,5 +1,22 @@
 package resource
 
+import (
+	"github.com/weaveworks/flux/resource"
+)
+
 type CronJob struct {
 	baseObject
+	Spec CronJobSpec
+}
+
+type CronJobSpec struct {
+	JobTemplate struct {
+		Spec struct {
+			Template PodTemplate
+		}
+	}
+}
+
+func (c CronJob) Containers() []resource.Container {
+	return c.Spec.JobTemplate.Spec.Template.Containers()
 }

cluster/kubernetes/resource/daemonset.go

@@ -1,5 +1,9 @@
 package resource
 
+import (
+	"github.com/weaveworks/flux/resource"
+)
+
 type DaemonSet struct {
 	baseObject
 	Spec DaemonSetSpec
@@ -8,3 +12,7 @@ type DaemonSet struct {
 type DaemonSetSpec struct {
 	Template PodTemplate
 }
+
+func (ds DaemonSet) Containers() []resource.Container {
+	return ds.Spec.Template.Containers()
+}

cluster/kubernetes/resource/deployment.go

@@ -1,5 +1,9 @@
 package resource
 
+import (
+	"github.com/weaveworks/flux/resource"
+)
+
 type Deployment struct {
 	baseObject
 	Spec DeploymentSpec
@@ -9,3 +13,7 @@ type DeploymentSpec struct {
 	Replicas int
 	Template PodTemplate
 }
+
+func (d Deployment) Containers() []resource.Container {
+	return d.Spec.Template.Containers()
+}

cluster/kubernetes/resource/spec.go

@@ -1,5 +1,10 @@
 package resource
 
+import (
+	"github.com/weaveworks/flux/image"
+	"github.com/weaveworks/flux/resource"
+)
+
 // Types that daemonsets, deployments, and other things have in
 // common.
 
@@ -13,6 +18,16 @@ type PodTemplate struct {
 	Spec     PodSpec
 }
 
+func (t PodTemplate) Containers() []resource.Container {
+	var result []resource.Container
+	for _, c := range t.Spec.Containers {
+		// FIXME(michael): account for possible errors here
+		im, _ := image.ParseRef(c.Image)
+		result = append(result, resource.Container{Name: c.Name, Image: im})
+	}
+	return result
+}
+
 type PodSpec struct {
 	ImagePullSecrets []struct{ Name string }
 	Volumes          []Volume

cluster/kubernetes/resource/statefulset.go

@@ -1,5 +1,9 @@
 package resource
 
+import (
+	"github.com/weaveworks/flux/resource"
+)
+
 type StatefulSet struct {
 	baseObject
 	Spec StatefulSetSpec
@@ -9,3 +13,7 @@ type StatefulSetSpec struct {
 	Replicas int
 	Template PodTemplate
 }
+
+func (ss StatefulSet) Containers() []resource.Container {
+	return ss.Spec.Template.Containers()
+}

cluster/kubernetes/resourcekinds.go

@@ -11,6 +11,8 @@ import (
 
 	"github.com/weaveworks/flux"
 	"github.com/weaveworks/flux/cluster"
+	"github.com/weaveworks/flux/image"
+	"github.com/weaveworks/flux/resource"
 )
 
 /////////////////////////////////////////////////////////////////////////////
@@ -42,15 +44,22 @@ type podController struct {
 }
 
 func (pc podController) toClusterController(resourceID flux.ResourceID) cluster.Controller {
-	var clusterContainers []cluster.Container
+	var clusterContainers []resource.Container
+	var excuse string
 	for _, container := range pc.podTemplate.Spec.Containers {
-		clusterContainers = append(clusterContainers, cluster.Container{Name: container.Name, Image: container.Image})
+		ref, err := image.ParseRef(container.Image)
+		if err != nil {
+			clusterContainers = nil
+			excuse = err.Error()
+			break
+		}
+		clusterContainers = append(clusterContainers, resource.Container{Name: container.Name, Image: ref})
 	}
 
 	return cluster.Controller{
 		ID:         resourceID,
 		Status:     pc.status,
-		Containers: cluster.ContainersOrExcuse{Containers: clusterContainers},
+		Containers: cluster.ContainersOrExcuse{Containers: clusterContainers, Excuse: excuse},
 	}
 }
 

cluster/manifests.go

@@ -16,6 +16,7 @@ import (
 type Manifests interface {
 	// Given a directory with manifest files, find which files define
 	// which services.
+	// FIXME(michael): remove when redundant
 	FindDefinedServices(path string) (map[flux.ResourceID][]string, error)
 	// Update the definitions in a manifests bytes according to the
 	// spec given.

daemon/daemon.go

@@ -24,6 +24,7 @@ import (
 	"github.com/weaveworks/flux/policy"
 	"github.com/weaveworks/flux/registry"
 	"github.com/weaveworks/flux/release"
+	"github.com/weaveworks/flux/resource"
 	"github.com/weaveworks/flux/update"
 )
 
@@ -118,6 +119,16 @@ func (d *Daemon) ListServices(ctx context.Context, namespace string) ([]v6.Contr
 	return res, nil
 }
 
+type clusterContainers []cluster.Controller
+
+func (cs clusterContainers) Len() int {
+	return len(cs)
+}
+
+func (cs clusterContainers) Containers(i int) []resource.Container {
+	return cs[i].ContainersOrNil()
+}
+
 // List the images available for set of services
 func (d *Daemon) ListImages(ctx context.Context, spec update.ResourceSpec) ([]v6.ImageStatus, error) {
 	var services []cluster.Controller
@@ -132,7 +143,7 @@ func (d *Daemon) ListImages(ctx context.Context, spec update.ResourceSpec) ([]v6
 		services, err = d.Cluster.SomeControllers([]flux.ResourceID{id})
 	}
 
-	images, err := update.CollectAvailableImages(d.Registry, services, d.Logger)
+	images, err := update.CollectAvailableImages(d.Registry, clusterContainers(services), d.Logger)
 	if err != nil {
 		return nil, errors.Wrap(err, "getting images for services")
 	}
@@ -337,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 {
@@ -520,14 +531,13 @@ func (d *Daemon) LogEvent(ev event.Event) error {
 
 // vvv helpers vvv
 
-func containers2containers(cs []cluster.Container) []v6.Container {
+func containers2containers(cs []resource.Container) []v6.Container {
 	res := make([]v6.Container, len(cs))
 	for i, c := range cs {
-		id, _ := image.ParseRef(c.Image)
 		res[i] = v6.Container{
 			Name: c.Name,
 			Current: image.Info{
-				ID: id,
+				ID: c.Image,
 			},
 		}
 	}
@@ -536,16 +546,15 @@ func containers2containers(cs []cluster.Container) []v6.Container {
 
 func containersWithAvailable(service cluster.Controller, images update.ImageMap) (res []v6.Container) {
 	for _, c := range service.ContainersOrNil() {
-		im, _ := image.ParseRef(c.Image)
-		available := images.Available(im.Name)
+		available := images.Available(c.Image.Name)
 		availableErr := ""
 		if available == nil {
 			availableErr = registry.ErrNoImageData.Error()
 		}
 		res = append(res, v6.Container{
 			Name: c.Name,
 			Current: image.Info{
-				ID: im,
+				ID: c.Image,
 			},
 			Available:      available,
 			AvailableError: availableErr,

daemon/daemon_test.go

@@ -344,8 +344,15 @@ func TestDaemon_JobStatusWithNoCache(t *testing.T) {
 }
 
 func makeImageInfo(ref string, t time.Time) image.Info {
-	r, _ := image.ParseRef(ref)
-	return image.Info{ID: r, CreatedAt: t}
+	return image.Info{ID: mustParseImageRef(ref), CreatedAt: t}
+}
+
+func mustParseImageRef(ref string) image.Ref {
+	r, err := image.ParseRef(ref)
+	if err != nil {
+		panic(err)
+	}
+	return r
 }
 
 func mockDaemon(t *testing.T) (*Daemon, func(), func(), *cluster.Mock, *mockEventWriter) {
@@ -354,10 +361,10 @@ func mockDaemon(t *testing.T) (*Daemon, func(), func(), *cluster.Mock, *mockEven
 	singleService := cluster.Controller{
 		ID: flux.MustParseResourceID(svc),
 		Containers: cluster.ContainersOrExcuse{
-			Containers: []cluster.Container{
+			Containers: []resource.Container{
 				{
 					Name:  container,
-					Image: currentHelloImage,
+					Image: mustParseImageRef(currentHelloImage),
 				},
 			},
 		},
@@ -367,10 +374,10 @@ func mockDaemon(t *testing.T) (*Daemon, func(), func(), *cluster.Mock, *mockEven
 		cluster.Controller{
 			ID: flux.MakeResourceID("another", "deployment", "service"),
 			Containers: cluster.ContainersOrExcuse{
-				Containers: []cluster.Container{
+				Containers: []resource.Container{
 					{
-						Name:  "it doesn't matter",
-						Image: "another/service:latest",
+						Name:  "it-doesn't-matter",
+						Image: mustParseImageRef("another/service:latest"),
 					},
 				},
 			},

daemon/images.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/weaveworks/flux"
 	"github.com/weaveworks/flux/git"
-	"github.com/weaveworks/flux/image"
 	"github.com/weaveworks/flux/policy"
 	"github.com/weaveworks/flux/update"
 )
@@ -35,7 +34,7 @@ func (d *Daemon) pollForNewImages(logger log.Logger) {
 		return
 	}
 	// Check the latest available image(s) for each service
-	imageMap, err := update.CollectAvailableImages(d.Registry, services, logger)
+	imageMap, err := update.CollectAvailableImages(d.Registry, clusterContainers(services), logger)
 	if err != nil {
 		logger.Log("error", errors.Wrap(err, "fetching image updates"))
 		return
@@ -46,7 +45,7 @@ func (d *Daemon) pollForNewImages(logger log.Logger) {
 		for _, container := range service.ContainersOrNil() {
 			logger := log.With(logger, "service", service.ID, "container", container.Name, "currentimage", container.Image)
 
-			currentImageID, err := image.ParseRef(container.Image)
+			currentImageID := container.Image
 			if err != nil {
 				logger.Log("error", err)
 				continue

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