Minor refactoring

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

@@ -34,23 +34,32 @@ func New(k8sClient k8s.Client, jenkinsClient jenkinsclient.Jenkins, logger logr.
 
 // Reconcile it's a main reconciliation loop for user supplied configuration
 func (r *ReconcileUserConfiguration) Reconcile() (*reconcile.Result, error) {
-	return r.reconcileSeedJobs()
+	// reconcile seed jobs
+	result, err := r.reconcileSeedJobs()
+	if err != nil {
+		return result, err
+	}
+	return result, nil
+
+	return nil, nil
 }
 
 func (r *ReconcileUserConfiguration) reconcileSeedJobs() (*reconcile.Result, error) {
 	seedJobs := seedjobs.New(r.jenkinsClient, r.k8sClient, r.logger)
 	done, err := seedJobs.EnsureSeedJobs(r.jenkins)
 	if err != nil {
+		// build failed and can be recovered - retry build and requeue reconciliation loop with timeout
 		if err == jobs.ErrorBuildFailed {
 			return &reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil
 		}
-
+		// build failed and cannot be recovered
-		if err == jobs.ErrorEmptyJenkinsCR || err == jobs.ErrorUncoverBuildFailed {
+		if err == jobs.ErrorUncoverBuildFailed {
 			return nil, nil
 		}
-
+		// unexpected error - requeue reconciliation loop
 		return &reconcile.Result{}, err
 	}
+	// build not finished yet - requeue reconciliation loop with timeout
 	if !done {
 		return &reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil
 	}

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

@@ -10,6 +10,7 @@ import (
 	jenkinsclient "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/client"
 
 	"github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/jobs"
+	"github.com/VirtusLab/jenkins-operator/pkg/log"
 	"github.com/go-logr/logr"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -35,7 +36,7 @@ type SeedJobs struct {
 	logger        logr.Logger
 }
 
-// New creates seedJobs client
+// New creates SeedJobs object
 func New(jenkinsClient jenkinsclient.Jenkins, k8sClient k8s.Client, logger logr.Logger) *SeedJobs {
 	return &SeedJobs{
 		jenkinsClient: jenkinsClient,
@@ -46,18 +47,21 @@ func New(jenkinsClient jenkinsclient.Jenkins, k8sClient k8s.Client, logger logr.
 
 // EnsureSeedJobs configures seed job and runs it for every entry from Jenkins.Spec.SeedJobs
 func (s *SeedJobs) EnsureSeedJobs(jenkins *virtuslabv1alpha1.Jenkins) (done bool, err error) {
-	err = s.configureSeedJob()
+	err = s.createJob()
 	if err != nil {
+		s.logger.V(log.VWarn).Info("Couldn't create jenkins seed job")
 		return false, err
 	}
-	done, err = s.buildSeedJobs(jenkins)
+	done, err = s.buildJobs(jenkins)
 	if err != nil {
+		s.logger.V(log.VWarn).Info("Couldn't build jenkins seed job")
 		return false, err
 	}
 	return done, nil
 }
 
-func (s *SeedJobs) configureSeedJob() error {
+// createJob is responsible for creating jenkins job which configures jenkins seed jobs and deploy keys
+func (s *SeedJobs) createJob() error {
 	_, err := s.jenkinsClient.CreateOrUpdateJob(seedJobConfigXML, ConfigureSeedJobsName)
 	if err != nil {
 		return err
@@ -65,7 +69,8 @@ func (s *SeedJobs) configureSeedJob() error {
 	return nil
 }
 
-func (s *SeedJobs) buildSeedJobs(jenkins *virtuslabv1alpha1.Jenkins) (done bool, err error) {
+// buildJobs is responsible for running jenkins builds which configures jenkins seed jobs and deploy keys
+func (s *SeedJobs) buildJobs(jenkins *virtuslabv1alpha1.Jenkins) (done bool, err error) {
 	allDone := true
 	seedJobs := jenkins.Spec.SeedJobs
 	for _, seedJob := range seedJobs {
@@ -96,15 +101,14 @@ func (s *SeedJobs) buildSeedJobs(jenkins *virtuslabv1alpha1.Jenkins) (done bool,
 		if err != nil {
 			return false, err
 		}
-
 		if !done {
 			allDone = false
 		}
 	}
-
 	return allDone, nil
 }
 
+// privateKeyFromSecret it's utility function which extracts deploy key from the kubernetes secret
 func (s *SeedJobs) privateKeyFromSecret(namespace string, seedJob virtuslabv1alpha1.SeedJob) (string, error) {
 	if seedJob.PrivateKey.SecretKeyRef != nil {
 		deployKeySecret := &v1.Secret{}
@@ -118,7 +122,7 @@ func (s *SeedJobs) privateKeyFromSecret(namespace string, seedJob virtuslabv1alp
 	return "", nil
 }
 
-// FIXME consider to use mask-password plugin for params.PRIVATE_KEY
+// seedJobConfigXML this is the XML representation of seed job
 var seedJobConfigXML = `
 <flow-definition plugin="workflow-job@2.30">
   <actions/>

pkg/controller/jenkins/jenkins_controller.go

@@ -52,7 +52,6 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}
 
-	// TODO(user): Modify this to be the types you create that are owned by the primary resource
 	// Watch for changes to secondary resource Pods and requeue the owner Jenkins
 	err = c.Watch(&source.Kind{Type: &corev1.Pod{}}, &handler.EnqueueRequestForOwner{
 		IsController: true,
@@ -69,15 +68,12 @@ var _ reconcile.Reconciler = &ReconcileJenkins{}
 
 // ReconcileJenkins reconciles a Jenkins object
 type ReconcileJenkins struct {
-	// This client, initialized using mgr.Client() above, is a split client
-	// that reads objects from the cache and writes to the apiserver
 	client          client.Client
 	scheme          *runtime.Scheme
 	local, minikube bool
 }
 
-// Reconcile it's a main reconciliation loop which maintain desired state for on Jenkins.Spec
+// Reconcile it's a main reconciliation loop which maintain desired state based on Jenkins.Spec
-// including base and user supplied configuration
 func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Result, error) {
 	logger := r.buildLogger(request.Name)
 	logger.Info("Reconciling Jenkins")
@@ -104,7 +100,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul
 	// Reconcile base configuration
 	baseConfiguration := base.New(r.client, r.scheme, logger, jenkins, r.local, r.minikube)
 	if !baseConfiguration.Validate(jenkins) {
-		logger.V(log.VWarn).Info("Please correct Jenkins CR")
+		logger.V(log.VWarn).Info("Validation of base configuration failed, please correct Jenkins CR")
 		return reconcile.Result{}, nil // don't requeue
 	}
 	result, jenkinsClient, err := baseConfiguration.Reconcile()
@@ -114,7 +110,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul
 	if result != nil {
 		return *result, nil
 	}
-	if err == nil && result == nil && jenkins.Status.BaseConfigurationCompletedTime == nil {
+	if jenkins.Status.BaseConfigurationCompletedTime == nil {
 		now := metav1.Now()
 		jenkins.Status.BaseConfigurationCompletedTime = &now
 		err = r.client.Update(context.TODO(), jenkins)
@@ -126,7 +122,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul
 	// Reconcile user configuration
 	userConfiguration := user.New(r.client, jenkinsClient, logger, jenkins)
 	if !userConfiguration.Validate(jenkins) {
-		logger.V(log.VWarn).Info("Please correct Jenkins CR")
+		logger.V(log.VWarn).Info("Validation of user configuration failed, please correct Jenkins CR")
 		return reconcile.Result{}, nil // don't requeue
 	}
 	result, err = userConfiguration.Reconcile()
@@ -136,7 +132,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul
 	if result != nil {
 		return *result, nil
 	}
-	if err == nil && result == nil && jenkins.Status.UserConfigurationCompletedTime == nil {
+	if jenkins.Status.UserConfigurationCompletedTime == nil {
 		now := metav1.Now()
 		jenkins.Status.UserConfigurationCompletedTime = &now
 		err = r.client.Update(context.TODO(), jenkins)