#27 Restart Jenkins when any user plugin version has changed

pkg/controller/jenkins/configuration/base/reconcile.go

@@ -88,13 +88,12 @@ func (r *ReconcileJenkinsBaseConfiguration) Reconcile() (reconcile.Result, jenki
 	}
 	r.logger.V(log.VDebug).Info("Jenkins API client set")
 
-	ok, err := r.verifyPlugins(jenkinsClient)
+	ok, err := r.verifyPlugins(jenkinsClient, plugins.BasePluginsMap)
 	if err != nil {
 		return reconcile.Result{}, nil, err
 	}
 	if !ok {
-		r.logger.V(log.VWarn).Info("Please correct Jenkins CR(spec.master.OperatorPlugins or spec.master.plugins)")
+		r.logger.Info("Some plugins have changed, restarting Jenkins")
-		// TODO inform user via Admin Monitor and don't restart Jenkins
 		return reconcile.Result{Requeue: true}, nil, r.restartJenkinsMasterPod(metaObject)
 	}
 
@@ -150,7 +149,7 @@ func (r *ReconcileJenkinsBaseConfiguration) ensureResourcesRequiredForJenkinsPod
 	return nil
 }
 
-func (r *ReconcileJenkinsBaseConfiguration) verifyPlugins(jenkinsClient jenkinsclient.Jenkins) (bool, error) {
+func (r *ReconcileJenkinsBaseConfiguration) verifyPlugins(jenkinsClient jenkinsclient.Jenkins, basePlugins map[string][]plugins.Plugin) (bool, error) {
 	allPluginsInJenkins, err := jenkinsClient.GetPlugins(fetchAllPlugins)
 	if err != nil {
 		return false, stackerr.WithStack(err)
@@ -158,7 +157,7 @@ func (r *ReconcileJenkinsBaseConfiguration) verifyPlugins(jenkinsClient jenkinsc
 
 	var installedPlugins []string
 	for _, jenkinsPlugin := range allPluginsInJenkins.Raw.Plugins {
-		if !jenkinsPlugin.Deleted {
+		if isValidPlugin(jenkinsPlugin) {
 			installedPlugins = append(installedPlugins, plugins.Plugin{Name: jenkinsPlugin.ShortName, Version: jenkinsPlugin.Version}.String())
 		}
 	}
@@ -178,7 +177,7 @@ func (r *ReconcileJenkinsBaseConfiguration) verifyPlugins(jenkinsClient jenkinsc
 	}
 
 	status := true
-	allRequiredPlugins := []map[string][]plugins.Plugin{plugins.BasePluginsMap, userPlugins}
+	allRequiredPlugins := []map[string][]plugins.Plugin{basePlugins, userPlugins}
 	for _, requiredPlugins := range allRequiredPlugins {
 		for rootPluginName, p := range requiredPlugins {
 			rootPlugin, _ := plugins.New(rootPluginName)
@@ -195,16 +194,43 @@ func (r *ReconcileJenkinsBaseConfiguration) verifyPlugins(jenkinsClient jenkinsc
 		}
 	}
 
+	for rootPluginName, p := range userPlugins {
+		rootPlugin, _ := plugins.New(rootPluginName)
+		if found, ok := isPluginVersionCompatible(allPluginsInJenkins, *rootPlugin); !ok {
+			r.logger.V(log.VWarn).Info(fmt.Sprintf("Incompatible plugin '%s' version, actual '%+v'", rootPlugin, found))
+			status = false
+		}
+		for _, requiredPlugin := range p {
+			if found, ok := isPluginInstalled(allPluginsInJenkins, requiredPlugin); !ok {
+				r.logger.V(log.VWarn).Info(fmt.Sprintf("Incompatible plugin '%s' version, actual '%+v'", requiredPlugin, found))
+				status = false
+			}
+		}
+	}
+
 	return status, nil
 }
 
+func isPluginVersionCompatible(plugins *gojenkins.Plugins, plugin plugins.Plugin) (gojenkins.Plugin, bool) {
+	p := plugins.Contains(plugin.Name)
+	if p == nil {
+		return gojenkins.Plugin{}, false
+	}
+
+	return *p, p.Version == plugin.Version
+}
+
+func isValidPlugin(plugin gojenkins.Plugin) bool {
+	return plugin.Active && plugin.Enabled && !plugin.Deleted
+}
+
 func isPluginInstalled(plugins *gojenkins.Plugins, requiredPlugin plugins.Plugin) (gojenkins.Plugin, bool) {
 	p := plugins.Contains(requiredPlugin.Name)
 	if p == nil {
 		return gojenkins.Plugin{}, false
 	}
 
-	return *p, p.Active && p.Enabled && !p.Deleted
+	return *p, isValidPlugin(*p)
 }
 
 func (r *ReconcileJenkinsBaseConfiguration) createOperatorCredentialsSecret(meta metav1.ObjectMeta) error {

pkg/controller/jenkins/configuration/base/reconcile_test.go

@@ -4,8 +4,13 @@ import (
 	"testing"
 
 	"github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkinsio/v1alpha1"
+	"github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/client"
 	"github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/base/resources"
+	"github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/plugins"
+	"github.com/jenkinsci/kubernetes-operator/pkg/log"
 
+	"github.com/bndr/gojenkins"
+	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 )
@@ -149,3 +154,216 @@ func TestCompareVolumes(t *testing.T) {
 		assert.True(t, got)
 	})
 }
+
+func TestReconcileJenkinsBaseConfiguration_verifyPlugins(t *testing.T) {
+	log.SetupLogger(true)
+
+	t.Run("happy, empty base and user plugins", func(t *testing.T) {
+		jenkins := &v1alpha1.Jenkins{}
+		r := ReconcileJenkinsBaseConfiguration{
+			logger:  log.Log,
+			jenkins: jenkins,
+		}
+		pluginsInJenkins := &gojenkins.Plugins{
+			Raw: &gojenkins.PluginResponse{},
+		}
+		basePlugins := map[string][]plugins.Plugin{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		jenkinsClient := client.NewMockJenkins(ctrl)
+		jenkinsClient.EXPECT().GetPlugins(fetchAllPlugins).Return(pluginsInJenkins, nil)
+
+		got, err := r.verifyPlugins(jenkinsClient, basePlugins)
+
+		assert.NoError(t, err)
+		assert.True(t, got)
+	})
+	t.Run("happy, not empty base and empty user plugins", func(t *testing.T) {
+		jenkins := &v1alpha1.Jenkins{}
+		r := ReconcileJenkinsBaseConfiguration{
+			logger:  log.Log,
+			jenkins: jenkins,
+		}
+		pluginsInJenkins := &gojenkins.Plugins{
+			Raw: &gojenkins.PluginResponse{
+				Plugins: []gojenkins.Plugin{
+					{
+						ShortName: "plugin-name",
+						Active:    true,
+						Deleted:   false,
+						Enabled:   true,
+						Version:   "0.0.1",
+					},
+				},
+			},
+		}
+		basePlugins := map[string][]plugins.Plugin{
+			"plugin-name:0.0.1": {},
+		}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		jenkinsClient := client.NewMockJenkins(ctrl)
+		jenkinsClient.EXPECT().GetPlugins(fetchAllPlugins).Return(pluginsInJenkins, nil)
+
+		got, err := r.verifyPlugins(jenkinsClient, basePlugins)
+
+		assert.NoError(t, err)
+		assert.True(t, got)
+	})
+	t.Run("happy, empty base and not empty user plugins", func(t *testing.T) {
+		jenkins := &v1alpha1.Jenkins{
+			Spec: v1alpha1.JenkinsSpec{
+				Master: v1alpha1.JenkinsMaster{
+					Plugins: map[string][]string{"plugin-name:0.0.1": {}},
+				},
+			},
+		}
+		r := ReconcileJenkinsBaseConfiguration{
+			logger:  log.Log,
+			jenkins: jenkins,
+		}
+		pluginsInJenkins := &gojenkins.Plugins{
+			Raw: &gojenkins.PluginResponse{
+				Plugins: []gojenkins.Plugin{
+					{
+						ShortName: "plugin-name",
+						Active:    true,
+						Deleted:   false,
+						Enabled:   true,
+						Version:   "0.0.1",
+					},
+				},
+			},
+		}
+		basePlugins := map[string][]plugins.Plugin{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		jenkinsClient := client.NewMockJenkins(ctrl)
+		jenkinsClient.EXPECT().GetPlugins(fetchAllPlugins).Return(pluginsInJenkins, nil)
+
+		got, err := r.verifyPlugins(jenkinsClient, basePlugins)
+
+		assert.NoError(t, err)
+		assert.True(t, got)
+	})
+	t.Run("happy, plugin version doesn't matter for base plugins", func(t *testing.T) {
+		jenkins := &v1alpha1.Jenkins{}
+		r := ReconcileJenkinsBaseConfiguration{
+			logger:  log.Log,
+			jenkins: jenkins,
+		}
+		pluginsInJenkins := &gojenkins.Plugins{
+			Raw: &gojenkins.PluginResponse{
+				Plugins: []gojenkins.Plugin{
+					{
+						ShortName: "plugin-name",
+						Active:    true,
+						Deleted:   false,
+						Enabled:   true,
+						Version:   "0.0.2",
+					},
+				},
+			},
+		}
+		basePlugins := map[string][]plugins.Plugin{
+			"plugin-name:0.0.1": {},
+		}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		jenkinsClient := client.NewMockJenkins(ctrl)
+		jenkinsClient.EXPECT().GetPlugins(fetchAllPlugins).Return(pluginsInJenkins, nil)
+
+		got, err := r.verifyPlugins(jenkinsClient, basePlugins)
+
+		assert.NoError(t, err)
+		assert.True(t, got)
+	})
+	t.Run("plugin version matter for user plugins", func(t *testing.T) {
+		jenkins := &v1alpha1.Jenkins{
+			Spec: v1alpha1.JenkinsSpec{
+				Master: v1alpha1.JenkinsMaster{
+					Plugins: map[string][]string{"plugin-name:0.0.2": {}},
+				},
+			},
+		}
+		r := ReconcileJenkinsBaseConfiguration{
+			logger:  log.Log,
+			jenkins: jenkins,
+		}
+		pluginsInJenkins := &gojenkins.Plugins{
+			Raw: &gojenkins.PluginResponse{
+				Plugins: []gojenkins.Plugin{
+					{
+						ShortName: "plugin-name",
+						Active:    true,
+						Deleted:   false,
+						Enabled:   true,
+						Version:   "0.0.1",
+					},
+				},
+			},
+		}
+		basePlugins := map[string][]plugins.Plugin{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		jenkinsClient := client.NewMockJenkins(ctrl)
+		jenkinsClient.EXPECT().GetPlugins(fetchAllPlugins).Return(pluginsInJenkins, nil)
+
+		got, err := r.verifyPlugins(jenkinsClient, basePlugins)
+
+		assert.NoError(t, err)
+		assert.False(t, got)
+	})
+	t.Run("missing base plugin", func(t *testing.T) {
+		jenkins := &v1alpha1.Jenkins{}
+		r := ReconcileJenkinsBaseConfiguration{
+			logger:  log.Log,
+			jenkins: jenkins,
+		}
+		pluginsInJenkins := &gojenkins.Plugins{
+			Raw: &gojenkins.PluginResponse{
+				Plugins: []gojenkins.Plugin{},
+			},
+		}
+		basePlugins := map[string][]plugins.Plugin{
+			"plugin-name:0.0.2": {},
+		}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		jenkinsClient := client.NewMockJenkins(ctrl)
+		jenkinsClient.EXPECT().GetPlugins(fetchAllPlugins).Return(pluginsInJenkins, nil)
+
+		got, err := r.verifyPlugins(jenkinsClient, basePlugins)
+
+		assert.NoError(t, err)
+		assert.False(t, got)
+	})
+	t.Run("missing user plugin", func(t *testing.T) {
+		jenkins := &v1alpha1.Jenkins{
+			Spec: v1alpha1.JenkinsSpec{
+				Master: v1alpha1.JenkinsMaster{
+					Plugins: map[string][]string{"plugin-name:0.0.2": {}},
+				},
+			},
+		}
+		r := ReconcileJenkinsBaseConfiguration{
+			logger:  log.Log,
+			jenkins: jenkins,
+		}
+		pluginsInJenkins := &gojenkins.Plugins{
+			Raw: &gojenkins.PluginResponse{
+				Plugins: []gojenkins.Plugin{},
+			},
+		}
+		basePlugins := map[string][]plugins.Plugin{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		jenkinsClient := client.NewMockJenkins(ctrl)
+		jenkinsClient.EXPECT().GetPlugins(fetchAllPlugins).Return(pluginsInJenkins, nil)
+
+		got, err := r.verifyPlugins(jenkinsClient, basePlugins)
+
+		assert.NoError(t, err)
+		assert.False(t, got)
+	})
+}