Small refactoring

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

@@ -2,9 +2,9 @@ package user
 
 import (
 	virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1"
-	jenkins "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/client"
+	jenkinsclient "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/client"
 	"github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/configuration/user/seedjobs"
-	"github.com/VirtusLab/jenkins-operator/pkg/log"
+
 	"github.com/go-logr/logr"
 	k8s "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -13,13 +13,13 @@ import (
 // ReconcileUserConfiguration defines values required for Jenkins user configuration
 type ReconcileUserConfiguration struct {
 	k8sClient     k8s.Client
-	jenkinsClient jenkins.Jenkins
+	jenkinsClient jenkinsclient.Jenkins
 	logger        logr.Logger
 	jenkins       *virtuslabv1alpha1.Jenkins
 }
 
 // New create structure which takes care of user configuration
-func New(k8sClient k8s.Client, jenkinsClient jenkins.Jenkins, logger logr.Logger,
+func New(k8sClient k8s.Client, jenkinsClient jenkinsclient.Jenkins, logger logr.Logger,
 	jenkins *virtuslabv1alpha1.Jenkins) *ReconcileUserConfiguration {
 	return &ReconcileUserConfiguration{
 		k8sClient:     k8sClient,
@@ -31,11 +31,6 @@ func New(k8sClient k8s.Client, jenkinsClient jenkins.Jenkins, logger logr.Logger
 
 // Reconcile it's a main reconciliation loop for user supplied configuration
 func (r *ReconcileUserConfiguration) Reconcile() (*reconcile.Result, error) {
-	if !r.validate(r.k8sClient, r.jenkins) {
-		r.logger.V(log.VWarn).Info("Please correct Jenkins CR")
-		return &reconcile.Result{}, nil
-	}
-
 	err := seedjobs.EnsureSeedJobs(r.jenkinsClient, r.k8sClient, r.jenkins)
 	if err != nil {
 		return &reconcile.Result{}, err

pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go

@@ -1,14 +1,15 @@
 package seedjobs
 
 import (
-	virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1"
-	jenkins "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/client"
-	k8s "sigs.k8s.io/controller-runtime/pkg/client"
-
 	"context"
 	"fmt"
+
+	virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1"
+	jenkinsclient "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/client"
+
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	k8s "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 const (
@@ -24,7 +25,7 @@ const (
 )
 
 // EnsureSeedJobs configures seed job and runs it for every entry from Jenkins.Spec.SeedJobs
-func EnsureSeedJobs(jenkinsClient jenkins.Jenkins, k8sClient k8s.Client, jenkins *virtuslabv1alpha1.Jenkins) error {
+func EnsureSeedJobs(jenkinsClient jenkinsclient.Jenkins, k8sClient k8s.Client, jenkins *virtuslabv1alpha1.Jenkins) error {
 	err := configureSeedJob(jenkinsClient)
 	if err != nil {
 		return err
@@ -36,7 +37,7 @@ func EnsureSeedJobs(jenkinsClient jenkins.Jenkins, k8sClient k8s.Client, jenkins
 	return nil
 }
 
-func configureSeedJob(jenkinsClient jenkins.Jenkins) error {
+func configureSeedJob(jenkinsClient jenkinsclient.Jenkins) error {
 	_, err := jenkinsClient.CreateOrUpdateJob(seedJobConfigXML, ConfigureSeedJobsName)
 	if err != nil {
 		return err
@@ -44,7 +45,7 @@ func configureSeedJob(jenkinsClient jenkins.Jenkins) error {
 	return nil
 }
 
-func buildAndVerifySeedJobs(jenkinsClient jenkins.Jenkins, k8sClient k8s.Client, jenkins *virtuslabv1alpha1.Jenkins) error {
+func buildAndVerifySeedJobs(jenkinsClient jenkinsclient.Jenkins, k8sClient k8s.Client, jenkins *virtuslabv1alpha1.Jenkins) error {
 	seedJobs := jenkins.Spec.SeedJobs
 	for _, seedJob := range seedJobs {
 		privateKey, err := privateKeyFromSecret(k8sClient, jenkins.Namespace, seedJob)
@@ -64,7 +65,7 @@ func buildAndVerifySeedJobs(jenkinsClient jenkins.Jenkins, k8sClient k8s.Client,
 	return nil
 }
 
-func buildAndVerifySeedJob(jenkinsClient jenkins.Jenkins, deployKeyID, privateKey, repositoryURL, repositoryBranch, targets, displayName string) error {
+func buildAndVerifySeedJob(jenkinsClient jenkinsclient.Jenkins, deployKeyID, privateKey, repositoryURL, repositoryBranch, targets, displayName string) error {
 	// FIXME this function should build job and verify job status when finished (state in cr status)
 	// requeue when job is running and check job status next time
 	options := map[string]string{

pkg/controller/jenkins/configuration/user/validate.go

@@ -2,28 +2,34 @@ package user
 
 import (
 	"context"
+	"fmt"
+	"strings"
+
 	virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1"
+	"github.com/VirtusLab/jenkins-operator/pkg/log"
+
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	k8s "sigs.k8s.io/controller-runtime/pkg/client"
-	"strings"
 )
 
-func (r *ReconcileUserConfiguration) validate(k8sClient k8s.Client, jenkins *virtuslabv1alpha1.Jenkins) bool {
+// Validate validates Jenkins CR Spec section
+func (r *ReconcileUserConfiguration) Validate(k8sClient k8s.Client, jenkins *virtuslabv1alpha1.Jenkins) bool {
 	// validate jenkins.Spec.SeedJobs
 	if jenkins.Spec.SeedJobs != nil {
 		for _, seedJob := range jenkins.Spec.SeedJobs {
+			logger := r.logger.WithValues("seedJob", fmt.Sprintf("%+v", seedJob)).V(log.VWarn)
 
 			// validate seed job id is not empty
 			if len(seedJob.ID) == 0 {
-				r.logger.V(0).Info("seed job id can't be empty")
+				logger.Info("seed job id can't be empty")
 				return false
 			}
 
 			// validate repository url match private key
 			if strings.Contains(seedJob.RepositoryURL, "git@") {
 				if seedJob.PrivateKey.SecretKeyRef == nil {
-					r.logger.V(0).Info("private key can't be empty while using ssh repository url")
+					logger.Info("private key can't be empty while using ssh repository url")
 					return false
 				}
 			}
@@ -33,23 +39,26 @@ func (r *ReconcileUserConfiguration) validate(k8sClient k8s.Client, jenkins *vir
 				deployKeySecret := &v1.Secret{}
 				namespaceName := types.NamespacedName{Namespace: jenkins.Namespace, Name: seedJob.PrivateKey.SecretKeyRef.Name}
 				err := k8sClient.Get(context.TODO(), namespaceName, deployKeySecret)
+				//TODO(bantoniak) handle error properly
 				if err != nil {
-					r.logger.V(0).Info("couldn't read private key secret")
+					logger.Info("couldn't read private key secret")
 					return false
 				}
 
 				privateKey := string(deployKeySecret.Data[seedJob.PrivateKey.SecretKeyRef.Key])
-				if privateKey != "" {
+				if privateKey == "" {
-					r.logger.V(0).Info("private key is empty")
+					logger.Info("private key is empty")
 					return false
 				}
 
+				//TODO(bantoniak) load private key to validate it
 				if !strings.HasPrefix(privateKey, "-----BEGIN RSA PRIVATE KEY-----") {
-					r.logger.V(0).Info("private key has wrong prefix")
+					logger.Info("private key has wrong prefix")
 					return false
 				}
 			}
 		}
 	}
+
 	return true
 }

pkg/controller/jenkins/jenkins_controller.go

@@ -125,6 +125,10 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul
 
 	// Reconcile user configuration
 	userConfiguration := user.New(r.client, jenkinsClient, logger, jenkins)
+	if !userConfiguration.Validate(r.client, jenkins) {
+		logger.V(log.VWarn).Info("Please correct Jenkins CR")
+		return reconcile.Result{}, nil // don't requeue
+	}
 	result, err = userConfiguration.Reconcile()
 	if err != nil {
 		return reconcile.Result{}, err

test/e2e/jenkins.go

@@ -120,6 +120,7 @@ func createJenkinsCRWithSeedJob(t *testing.T, namespace string) *virtuslabv1alph
 					},
 				},
 			},
+			//TODO(bantoniak) add seed job with private key
 			SeedJobs: []virtuslabv1alpha1.SeedJob{
 				{
 					ID:               "jenkins-operator-e2e",

test/e2e/main_test.go

@@ -6,12 +6,10 @@ import (
 	"github.com/VirtusLab/jenkins-operator/pkg/apis"
 	virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	f "github.com/operator-framework/operator-sdk/pkg/test"
-
 	framework "github.com/operator-framework/operator-sdk/pkg/test"
 	"github.com/operator-framework/operator-sdk/pkg/test/e2eutil"
-
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	f "github.com/operator-framework/operator-sdk/pkg/test"
 )
 
 const (

test/e2e/user_configuration_test.go

@@ -1,11 +1,13 @@
 package e2e
 
 import (
-	"github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/configuration/user/seedjobs"
-	"github.com/bndr/gojenkins"
-	"k8s.io/apimachinery/pkg/util/wait"
 	"testing"
 	"time"
+
+	"github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/configuration/user/seedjobs"
+
+	"github.com/bndr/gojenkins"
+	"k8s.io/apimachinery/pkg/util/wait"
 )
 
 func TestUserConfiguration(t *testing.T) {
@@ -37,4 +39,7 @@ func verifyJenkinsSeedJobs(t *testing.T, client *gojenkins.Jenkins) {
 	if err != nil {
 		t.Fatalf("couldn't get seed job '%v'", err)
 	}
+
+	//TODO(bantoniak) verify if seed jobs have been created
+	//TODO(bantoniak) verify if jobs created by seed jobs have been created
 }