From b5fca5950a79599db07b259c2f60b8f400e57a1c Mon Sep 17 00:00:00 2001 From: antoniaklja Date: Wed, 2 Jan 2019 18:59:09 +0100 Subject: [PATCH] Minor refactoring --- .../jenkins/configuration/user/reconcile.go | 17 ++++++++++++---- .../configuration/user/seedjobs/seedjobs.go | 20 +++++++++++-------- pkg/controller/jenkins/jenkins_controller.go | 14 +++++-------- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/pkg/controller/jenkins/configuration/user/reconcile.go b/pkg/controller/jenkins/configuration/user/reconcile.go index 66ca11ea..8b242995 100644 --- a/pkg/controller/jenkins/configuration/user/reconcile.go +++ b/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 } - - if err == jobs.ErrorEmptyJenkinsCR || err == jobs.ErrorUncoverBuildFailed { + // build failed and cannot be recovered + 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 } diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go b/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go index ac44c516..387847c4 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go +++ b/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 = ` diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index 672c5449..a906a965 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/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 -// including base and user supplied configuration +// Reconcile it's a main reconciliation loop which maintain desired state based on Jenkins.Spec 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)