From 3aacf995363e8161b6573d4a721ae7afcec5fe81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20S=C4=99k?= Date: Wed, 16 Jan 2019 10:05:09 +0100 Subject: [PATCH] Small refactoring --- .../jenkins/configuration/base/reconcile.go | 72 +++++++++---------- .../jenkins/configuration/user/reconcile.go | 33 +++++---- pkg/controller/jenkins/jenkins_controller.go | 10 +-- 3 files changed, 60 insertions(+), 55 deletions(-) diff --git a/pkg/controller/jenkins/configuration/base/reconcile.go b/pkg/controller/jenkins/configuration/base/reconcile.go index 925adbc2..043053e1 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile.go +++ b/pkg/controller/jenkins/configuration/base/reconcile.go @@ -53,80 +53,80 @@ func New(client client.Client, scheme *runtime.Scheme, logger logr.Logger, } // Reconcile takes care of base configuration -func (r *ReconcileJenkinsBaseConfiguration) Reconcile() (*reconcile.Result, jenkinsclient.Jenkins, error) { +func (r *ReconcileJenkinsBaseConfiguration) Reconcile() (reconcile.Result, jenkinsclient.Jenkins, error) { metaObject := resources.NewResourceObjectMeta(r.jenkins) if err := r.createOperatorCredentialsSecret(metaObject); err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } r.logger.V(log.VDebug).Info("Operator credentials secret is present") if err := r.createScriptsConfigMap(metaObject); err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } r.logger.V(log.VDebug).Info("Scripts config map is present") if err := r.createInitConfigurationConfigMap(metaObject); err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } r.logger.V(log.VDebug).Info("Init configuration config map is present") if err := r.createBaseConfigurationConfigMap(metaObject); err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } r.logger.V(log.VDebug).Info("Base configuration config map is present") if err := r.createUserConfigurationConfigMap(metaObject); err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } r.logger.V(log.VDebug).Info("User configuration config map is present") if err := r.createRBAC(metaObject); err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } r.logger.V(log.VDebug).Info("User configuration config map is present") if err := r.createService(metaObject); err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } r.logger.V(log.VDebug).Info("Service is present") result, err := r.createJenkinsMasterPod(metaObject) if err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } - if result != nil { + if result.Requeue { return result, nil, nil } r.logger.V(log.VDebug).Info("Jenkins master pod is present") result, err = r.waitForJenkins(metaObject) if err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } - if result != nil { + if result.Requeue { return result, nil, nil } r.logger.V(log.VDebug).Info("Jenkins master pod is ready") jenkinsClient, err := r.getJenkinsClient(metaObject) if err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } r.logger.V(log.VDebug).Info("Jenkins API client set") ok, err := r.verifyBasePlugins(jenkinsClient) if err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } if !ok { r.logger.V(log.VWarn).Info("Please correct Jenkins CR (spec.master.plugins)") currentJenkinsMasterPod, err := r.getJenkinsMasterPod(metaObject) if err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } if err := r.k8sClient.Delete(context.TODO(), currentJenkinsMasterPod); err != nil { - return &reconcile.Result{}, nil, err + return reconcile.Result{}, nil, err } } @@ -271,7 +271,7 @@ func (r *ReconcileJenkinsBaseConfiguration) getJenkinsMasterPod(meta metav1.Obje return currentJenkinsMasterPod, nil } -func (r *ReconcileJenkinsBaseConfiguration) createJenkinsMasterPod(meta metav1.ObjectMeta) (*reconcile.Result, error) { +func (r *ReconcileJenkinsBaseConfiguration) createJenkinsMasterPod(meta metav1.ObjectMeta) (reconcile.Result, error) { // Check if this Pod already exists currentJenkinsMasterPod, err := r.getJenkinsMasterPod(meta) if err != nil && errors.IsNotFound(err) { @@ -279,16 +279,16 @@ func (r *ReconcileJenkinsBaseConfiguration) createJenkinsMasterPod(meta metav1.O r.logger.Info(fmt.Sprintf("Creating a new Jenkins Master Pod %s/%s", jenkinsMasterPod.Namespace, jenkinsMasterPod.Name)) err = r.createResource(jenkinsMasterPod) if err != nil { - return nil, err + return reconcile.Result{}, err } r.jenkins.Status = virtuslabv1alpha1.JenkinsStatus{} err = r.updateResource(r.jenkins) if err != nil { - return nil, err + return reconcile.Result{}, err } - return nil, nil + return reconcile.Result{}, nil } else if err != nil && !errors.IsNotFound(err) { - return nil, err + return reconcile.Result{}, err } // Recreate pod @@ -323,38 +323,38 @@ func (r *ReconcileJenkinsBaseConfiguration) createJenkinsMasterPod(meta metav1.O if currentJenkinsMasterPod != nil && recreatePod && currentJenkinsMasterPod.ObjectMeta.DeletionTimestamp == nil { r.logger.Info(fmt.Sprintf("Terminating Jenkins Master Pod %s/%s", currentJenkinsMasterPod.Namespace, currentJenkinsMasterPod.Name)) if err := r.k8sClient.Delete(context.TODO(), currentJenkinsMasterPod); err != nil { - return nil, err + return reconcile.Result{}, err } - return &reconcile.Result{Requeue: true}, nil + return reconcile.Result{Requeue: true}, nil } - return nil, nil + return reconcile.Result{}, nil } -func (r *ReconcileJenkinsBaseConfiguration) waitForJenkins(meta metav1.ObjectMeta) (*reconcile.Result, error) { +func (r *ReconcileJenkinsBaseConfiguration) waitForJenkins(meta metav1.ObjectMeta) (reconcile.Result, error) { jenkinsMasterPodStatus, err := r.getJenkinsMasterPod(meta) if err != nil { - return nil, err + return reconcile.Result{}, err } if jenkinsMasterPodStatus.ObjectMeta.DeletionTimestamp != nil { r.logger.V(log.VDebug).Info("Jenkins master pod is terminating") - return &reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil } if jenkinsMasterPodStatus.Status.Phase != corev1.PodRunning { r.logger.V(log.VDebug).Info("Jenkins master pod not ready") - return &reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil } for _, containerStatus := range jenkinsMasterPodStatus.Status.ContainerStatuses { if !containerStatus.Ready { r.logger.V(log.VDebug).Info("Jenkins master pod not ready, readiness probe failed") - return &reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil } } - return nil, nil + return reconcile.Result{}, nil } func (r *ReconcileJenkinsBaseConfiguration) getJenkinsClient(meta metav1.ObjectMeta) (jenkinsclient.Jenkins, error) { @@ -418,28 +418,28 @@ func (r *ReconcileJenkinsBaseConfiguration) getJenkinsClient(meta metav1.ObjectM string(credentialsSecret.Data[resources.OperatorCredentialsSecretTokenKey])) } -func (r *ReconcileJenkinsBaseConfiguration) baseConfiguration(jenkinsClient jenkinsclient.Jenkins) (*reconcile.Result, error) { +func (r *ReconcileJenkinsBaseConfiguration) baseConfiguration(jenkinsClient jenkinsclient.Jenkins) (reconcile.Result, error) { groovyClient := groovy.New(jenkinsClient, r.k8sClient, r.logger, fmt.Sprintf("%s-base-configuration", constants.OperatorName), resources.JenkinsBaseConfigurationVolumePath) err := groovyClient.ConfigureGroovyJob() if err != nil { - return &reconcile.Result{}, err + return reconcile.Result{}, err } configuration := &corev1.ConfigMap{} namespaceName := types.NamespacedName{Namespace: r.jenkins.Namespace, Name: resources.GetBaseConfigurationConfigMapName(r.jenkins)} err = r.k8sClient.Get(context.TODO(), namespaceName, configuration) if err != nil { - return &reconcile.Result{}, err + return reconcile.Result{}, err } done, err := groovyClient.EnsureGroovyJob(configuration.Data, r.jenkins) if err != nil { - return &reconcile.Result{}, err + return reconcile.Result{}, err } if !done { - return &reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil } - return nil, nil + return reconcile.Result{}, nil } diff --git a/pkg/controller/jenkins/configuration/user/reconcile.go b/pkg/controller/jenkins/configuration/user/reconcile.go index 301aa426..48bf7537 100644 --- a/pkg/controller/jenkins/configuration/user/reconcile.go +++ b/pkg/controller/jenkins/configuration/user/reconcile.go @@ -40,61 +40,64 @@ 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) { +func (r *ReconcileUserConfiguration) Reconcile() (reconcile.Result, error) { // reconcile seed jobs result, err := r.reconcileSeedJobs() - if err != nil || result != nil { - return result, err + if err != nil { + return reconcile.Result{}, err + } + if result.Requeue { + return result, nil } return r.userConfiguration(r.jenkinsClient) } -func (r *ReconcileUserConfiguration) reconcileSeedJobs() (*reconcile.Result, error) { +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 + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil } // build failed and cannot be recovered if err == jobs.ErrorUnrecoverableBuildFailed { - return nil, nil + return reconcile.Result{}, nil } // unexpected error - requeue reconciliation loop - return &reconcile.Result{}, err + 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 + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil } - return nil, nil + return reconcile.Result{}, nil } -func (r *ReconcileUserConfiguration) userConfiguration(jenkinsClient jenkinsclient.Jenkins) (*reconcile.Result, error) { +func (r *ReconcileUserConfiguration) userConfiguration(jenkinsClient jenkinsclient.Jenkins) (reconcile.Result, error) { groovyClient := groovy.New(jenkinsClient, r.k8sClient, r.logger, fmt.Sprintf("%s-user-configuration", constants.OperatorName), resources.JenkinsUserConfigurationVolumePath) err := groovyClient.ConfigureGroovyJob() if err != nil { - return &reconcile.Result{}, err + return reconcile.Result{}, err } configuration := &corev1.ConfigMap{} namespaceName := types.NamespacedName{Namespace: r.jenkins.Namespace, Name: resources.GetUserConfigurationConfigMapName(r.jenkins)} err = r.k8sClient.Get(context.TODO(), namespaceName, configuration) if err != nil { - return &reconcile.Result{}, err + return reconcile.Result{}, err } done, err := groovyClient.EnsureGroovyJob(configuration.Data, r.jenkins) if err != nil { - return &reconcile.Result{}, err + return reconcile.Result{}, err } if !done { - return &reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil } - return nil, nil + return reconcile.Result{}, nil } diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index 1cd49833..16b06ad6 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -139,9 +139,10 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg if err != nil { return reconcile.Result{}, err } - if result != nil { - return *result, nil + if result.Requeue { + return result, nil } + if jenkins.Status.BaseConfigurationCompletedTime == nil { logger.Info("Base configuration phase is complete") now := metav1.Now() @@ -169,9 +170,10 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg if err != nil { return reconcile.Result{}, err } - if result != nil { - return *result, nil + if result.Requeue { + return result, nil } + if jenkins.Status.UserConfigurationCompletedTime == nil { logger.Info("User configuration phase is complete") now := metav1.Now()