add test and semver dependency

Gopkg.toml

@@ -45,6 +45,10 @@ required = [
   # branch = "v0.2.x" #osdk_branch_annotation
   version = "=v0.2.1" #osdk_version_annotation
 
+[[constraint]]
+  name = "github.com/blang/semver"
+  version = "v3.6.1"
+
 [prune]
   go-tests = true
   non-go = true

Makefile

@@ -1,7 +1,7 @@
-ORG=pb82
+ORG=integr8ly
 NAMESPACE=application-monitoring
 PROJECT=grafana-operator
-REG=docker.io
+REG=quay.io
 SHELL=/bin/bash
 TAG=latest
 PKG=github.com/integr8ly/grafana-operator

pkg/apis/integreatly/v1alpha1/pluginsList.go

@@ -1,5 +1,9 @@
 package v1alpha1
 
+import (
+	"github.com/blang/semver"
+)
+
 type PluginList []GrafanaPlugin
 
 // Returns true if the list contains the same plugin in the exact or a different version
@@ -12,6 +16,16 @@ func (l PluginList) HasSomeVersionOf(plugin *GrafanaPlugin) bool {
 	return false
 }
 
+// Get the plugin from the list regardless of the version
+func (l PluginList) GetInstalledVersionOf(plugin *GrafanaPlugin) *GrafanaPlugin {
+	for _, listedPlugin := range l {
+		if listedPlugin.Name == plugin.Name {
+			return &listedPlugin
+		}
+	}
+	return nil
+}
+
 // Returns true if the list contains the same plugin in the same version
 func (l PluginList) HasExactVersionOf(plugin *GrafanaPlugin) bool {
 	for _, listedPlugin := range l {
@@ -22,6 +36,30 @@ func (l PluginList) HasExactVersionOf(plugin *GrafanaPlugin) bool {
 	return false
 }
 
+// Returns true if the list contains the same plugin but in a newer version
+func (l PluginList) HasNewerVersionOf(plugin *GrafanaPlugin) (bool, error) {
+	for _, listedPlugin := range l {
+		if listedPlugin.Name != plugin.Name {
+			continue
+		}
+
+		listedVersion, err := semver.Make(listedPlugin.Version)
+		if err != nil {
+			return false, err
+		}
+
+		requestedVersion, err := semver.Make(plugin.Version)
+		if err != nil {
+			return false, err
+		}
+
+		if listedVersion.Compare(requestedVersion) == 1 {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 // Returns the number of different versions of a given plugin in the list
 func (l PluginList) VersionsOf(plugin *GrafanaPlugin) int {
 	i := 0
@@ -33,7 +71,7 @@ func (l PluginList) VersionsOf(plugin *GrafanaPlugin) int {
 	return i
 }
 
-// Returns the number of different versions of a given plugin in the list
+// Set the originating dashboard for every plugin in the list
 func (l PluginList) SetOrigin(dashboard *GrafanaDashboard) {
 	for i := range l {
 		l[i].Origin = dashboard

pkg/controller/grafana/grafana_controller.go

@@ -128,7 +128,7 @@ func (r *ReconcileGrafana) ReconcileNamespaces(cr *integreatly.Grafana) (reconci
 						dashboardCopy := d.DeepCopy()
 						dashboardCopy.Spec.Plugins.SetOrigin(dashboardCopy)
 						requestedPlugins = append(requestedPlugins, dashboardCopy.Spec.Plugins...)
-						r.ReconcileDashboards(cr, dashboardCopy)
+						r.ReconcileDashboardConfigMap(cr, dashboardCopy)
 					}
 				}
 			}
@@ -137,14 +137,27 @@ func (r *ReconcileGrafana) ReconcileNamespaces(cr *integreatly.Grafana) (reconci
 		filteredPlugins, updated := r.plugins.FilterPlugins(cr, requestedPlugins)
 		if updated {
 			r.ReconcilePlugins(cr, filteredPlugins)
+
+			// Update the dashboards that had their plugins modified
+			// to let the owners know about the status
+			err = r.ReconcileDashboards(filteredPlugins)
 		}
 	}
 
-	return reconcile.Result{RequeueAfter: time.Second * 10}, nil
+	return reconcile.Result{RequeueAfter: time.Second * 10}, err
 }
 
+func (r *ReconcileGrafana) ReconcileDashboards(plugins integreatly.PluginList) error {
+	for _, plugin := range plugins {
+		err := r.client.Update(context.TODO(), plugin.Origin)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
 
-func (r *ReconcileGrafana) ReconcileDashboards(cr *integreatly.Grafana, d *integreatly.GrafanaDashboard) {
+func (r *ReconcileGrafana) ReconcileDashboardConfigMap(cr *integreatly.Grafana, d *integreatly.GrafanaDashboard) {
 	err := r.helper.updateDashboard(cr.Namespace, d.Namespace, d)
 	if err != nil {
 		log.Error(err, "Error updating dashboard config")
@@ -170,16 +183,6 @@ func (r *ReconcileGrafana) ReconcilePlugins(cr *integreatly.Grafana, plugins []i
 
 	newEnv := r.plugins.BuildEnv(cr)
 	err = r.helper.updateGrafanaDeployment(cr.Namespace, newEnv)
-	if err != nil {
-		return err
-	}
-
-	for _, plugin := range plugins {
-		log.Info(fmt.Sprintf("== Updating dashboard for plugin"))
-		log.Info(fmt.Sprintf("%v", plugin.Origin.Status))
-		r.ReconcileDashboards(cr, plugin.Origin)
-	}
-
 	return err
 }
 

pkg/controller/grafana/pluginsHelper.go

@@ -51,13 +51,39 @@ func (h *PluginsHelperImpl) BuildEnv(cr *integreatly.Grafana) string {
 	return strings.Join(env, ",")
 }
 
-func (h *PluginsHelperImpl) AppendMessage(message string, d *integreatly.GrafanaDashboard) {
+// Append a status message to the origin dashboard of a plugin
+func (h *PluginsHelperImpl) AppendMessage(message string, dashboard *integreatly.GrafanaDashboard) {
+	if dashboard == nil {
+		return
+	}
+
 	status := integreatly.GrafanaDashboardStatusMessage{
 		Message:   message,
 		Timestamp: time.Now().Format(time.RFC850),
 	}
 
-	d.Status.Messages = append(d.Status.Messages, status)
+	dashboard.Status.Messages = append(dashboard.Status.Messages, status)
+}
+
+// Append a status message to the origin dashboard of a plugin
+func (h *PluginsHelperImpl) PickLatestVersions(requested integreatly.PluginList) (integreatly.PluginList, error) {
+	var latestVersions integreatly.PluginList
+	for _, plugin := range requested {
+		result, err := requested.HasNewerVersionOf(&plugin)
+
+		// Errors might happen if plugins don't use semver
+		// In that case fall back to whichever comes first
+		if err != nil {
+			return requested, err
+		}
+
+		// Skip this version if there is a more recent one
+		if result {
+			continue
+		}
+		latestVersions = append(latestVersions, plugin)
+	}
+	return latestVersions, nil
 }
 
 // Creates the list of plugins that can be added or updated
@@ -67,6 +93,12 @@ func (h *PluginsHelperImpl) FilterPlugins(cr *integreatly.Grafana, requested int
 	filteredPlugins := integreatly.PluginList{}
 	pluginsUpdated := false
 
+	// Try to pick the latest versions of all plugins
+	requested, err := h.PickLatestVersions(requested)
+	if err != nil {
+		log.Error(err, "Unable to pick latest plugin versions")
+	}
+
 	// Remove all plugins
 	if len(requested) == 0 && len(cr.Status.InstalledPlugins) > 0 {
 		return filteredPlugins, true
@@ -75,7 +107,8 @@ func (h *PluginsHelperImpl) FilterPlugins(cr *integreatly.Grafana, requested int
 	for _, plugin := range requested {
 		// Don't allow to install multiple versions of the same plugin
 		if filteredPlugins.HasSomeVersionOf(&plugin) == true {
-			h.AppendMessage(fmt.Sprintf("Another version of %s is already installed", plugin.Name), plugin.Origin)
+			installedVersion := filteredPlugins.GetInstalledVersionOf(&plugin)
+			h.AppendMessage(fmt.Sprintf("Not installing version %s of %s because %s is already installed", plugin.Version, plugin.Name, installedVersion.Version), plugin.Origin)
 			continue
 		}
 
@@ -94,12 +127,16 @@ func (h *PluginsHelperImpl) FilterPlugins(cr *integreatly.Grafana, requested int
 		}
 
 		// Plugin update: allow to update a plugin if only one dashboard requests it
+		// The condition is: some version of the plugin is aleady installed, but it's not the exact same version
+		// and there is only one dashboard that requires this plugin
 		// If multiple dashboards request different versions of the same plugin, then we can't upgrade because
 		// there is no way to decide which version is the correct one
 		if cr.Status.InstalledPlugins.HasSomeVersionOf(&plugin) == true &&
 			cr.Status.InstalledPlugins.HasExactVersionOf(&plugin) == false &&
 			requested.VersionsOf(&plugin) == 1 {
+			installedVersion := cr.Status.InstalledPlugins.GetInstalledVersionOf(&plugin)
 			filteredPlugins = append(filteredPlugins, plugin)
+			h.AppendMessage(fmt.Sprintf("Changing version of plugin %s form %s to %s", plugin.Name, installedVersion.Version, plugin.Version), plugin.Origin)
 			pluginsUpdated = true
 			continue
 		}

pkg/controller/grafana/pluginsHelper_test.go

@@ -0,0 +1,88 @@
+package grafana
+
+import (
+	"testing"
+)
+
+func TestPluginsList(t *testing.T) {
+	result := true
+	result = result && MockPluginList.HasSomeVersionOf(&Mockplugina100)
+	result = result && MockPluginList.HasSomeVersionOf(&Mockplugina101)
+	result = result && MockPluginList.HasSomeVersionOf(&Mockpluginb100)
+	result = result && MockPluginList.HasSomeVersionOf(&Mockplugina102)
+
+	if !result {
+		t.Errorf("Error in `HasSomeVersionOf`")
+	}
+
+	result = result && MockPluginList.HasExactVersionOf(&Mockplugina100)
+	result = result && MockPluginList.HasExactVersionOf(&Mockplugina101)
+	result = result && MockPluginList.HasExactVersionOf(&Mockpluginb100)
+	result = result && (MockPluginList.HasExactVersionOf(&Mockplugina102) == false)
+	result = result && (MockPluginList.HasExactVersionOf(&Mockpluginc100) == false)
+
+	if !result {
+		t.Errorf("Error in `HasExactVersionOf`")
+	}
+
+	result, err := MockPluginList.HasNewerVersionOf(&Mockplugina100)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if !result {
+		t.Errorf("Error in `HasNewerVersionOf`")
+	}
+
+	result, err = MockPluginList.HasNewerVersionOf(&Mockplugina101)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if result {
+		t.Errorf("Error in `HasNewerVersionOf`")
+	}
+}
+
+func TestPluginsHelperImpl_PickLatestVersions(t *testing.T) {
+	var h PluginsHelperImpl
+
+	latestVersions, err := h.PickLatestVersions(MockPluginList)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if latestVersions.HasExactVersionOf(&Mockplugina100) {
+		t.Errorf("Expected %s but got %s", Mockplugina101.Version, Mockplugina100.Version)
+	}
+
+	result, err := latestVersions.HasNewerVersionOf(&Mockplugina101)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if result {
+		t.Errorf("Expected no newer version than %s", Mockplugina101.Version)
+	}
+}
+
+func TestPluginsHelperImpl_FilterPlugins(t *testing.T) {
+	var h PluginsHelperImpl
+
+	MockPluginList.SetOrigin(&MockDashboard)
+	installed, updated := h.FilterPlugins(&MockGrafana, MockPluginList)
+
+	if !updated {
+		t.Errorf("Expected plugins to be installed")
+	}
+
+	result := true
+	result = result && installed.HasExactVersionOf(&Mockplugina101)
+	result = result && installed.HasExactVersionOf(&Mockpluginb100)
+	result = result && (len(MockDashboard.Status.Messages) == 2)
+	result = result && (len(installed) == 2)
+
+	if !result {
+		t.Errorf("Unexpected plugins got installed")
+	}
+}

pkg/controller/grafana/testing_shared.go

@@ -0,0 +1,43 @@
+package grafana
+
+import "github.com/integr8ly/grafana-operator/pkg/apis/integreatly/v1alpha1"
+
+var Mockplugina100 = v1alpha1.GrafanaPlugin{
+	Name:    "a",
+	Version: "1.0.0",
+}
+
+var Mockplugina101 = v1alpha1.GrafanaPlugin{
+	Name:    "a",
+	Version: "1.0.1",
+}
+
+var Mockplugina102 = v1alpha1.GrafanaPlugin{
+	Name:    "a",
+	Version: "1.0.2",
+}
+
+var Mockpluginb100 = v1alpha1.GrafanaPlugin{
+	Name:    "b",
+	Version: "1.0.0",
+}
+
+var Mockpluginc100 = v1alpha1.GrafanaPlugin{
+	Name:    "c",
+	Version: "1.0.0",
+}
+
+var MockPluginList = v1alpha1.PluginList{Mockplugina100, Mockplugina101, Mockpluginb100}
+
+var MockDashboard = v1alpha1.GrafanaDashboard{
+	Status: v1alpha1.GrafanaDashboardStatus{
+		Messages: []v1alpha1.GrafanaDashboardStatusMessage{},
+	},
+}
+
+var MockGrafana = v1alpha1.Grafana{
+	Status: v1alpha1.GrafanaStatus{
+		Phase:            0,
+		InstalledPlugins: v1alpha1.PluginList{},
+	},
+}