From e05c2d7a975e4e647498c6215badc55541b292aa Mon Sep 17 00:00:00 2001 From: jkhelil Date: Mon, 11 May 2020 19:23:04 +0200 Subject: [PATCH 1/2] refactor log handeling --- .../jenkins/configuration/base/reconcile.go | 4 +- .../configuration/base/reconcile_test.go | 14 ++-- .../configuration/base/validate_test.go | 71 ++++++++++--------- .../jenkins/configuration/user/reconcile.go | 8 +-- pkg/controller/jenkins/jenkins_controller.go | 38 +++++----- test/e2e/jenkins.go | 2 +- 6 files changed, 70 insertions(+), 67 deletions(-) diff --git a/pkg/controller/jenkins/configuration/base/reconcile.go b/pkg/controller/jenkins/configuration/base/reconcile.go index 13f8335b..1b0029d9 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile.go +++ b/pkg/controller/jenkins/configuration/base/reconcile.go @@ -39,10 +39,10 @@ type ReconcileJenkinsBaseConfiguration struct { } // New create structure which takes care of base configuration -func New(config configuration.Configuration, logger logr.Logger, jenkinsAPIConnectionSettings jenkinsclient.JenkinsAPIConnectionSettings) *ReconcileJenkinsBaseConfiguration { +func New(config configuration.Configuration, jenkinsAPIConnectionSettings jenkinsclient.JenkinsAPIConnectionSettings) *ReconcileJenkinsBaseConfiguration { return &ReconcileJenkinsBaseConfiguration{ Configuration: config, - logger: logger, + logger: log.Log.WithValues("cr", config.Jenkins.Name), jenkinsAPIConnectionSettings: jenkinsAPIConnectionSettings, } } diff --git a/pkg/controller/jenkins/configuration/base/reconcile_test.go b/pkg/controller/jenkins/configuration/base/reconcile_test.go index 52f3ecbe..2bc4fe8f 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile_test.go +++ b/pkg/controller/jenkins/configuration/base/reconcile_test.go @@ -105,7 +105,7 @@ func TestCompareVolumes(t *testing.T) { Volumes: resources.GetJenkinsMasterPodBaseVolumes(jenkins), }, } - reconciler := New(configuration.Configuration{Jenkins: jenkins}, nil, client.JenkinsAPIConnectionSettings{}) + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := reconciler.compareVolumes(pod) @@ -129,7 +129,7 @@ func TestCompareVolumes(t *testing.T) { Volumes: resources.GetJenkinsMasterPodBaseVolumes(jenkins), }, } - reconciler := New(configuration.Configuration{Jenkins: jenkins}, nil, client.JenkinsAPIConnectionSettings{}) + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := reconciler.compareVolumes(pod) @@ -153,7 +153,7 @@ func TestCompareVolumes(t *testing.T) { Volumes: append(resources.GetJenkinsMasterPodBaseVolumes(jenkins), corev1.Volume{Name: "added"}), }, } - reconciler := New(configuration.Configuration{Jenkins: jenkins}, nil, client.JenkinsAPIConnectionSettings{}) + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := reconciler.compareVolumes(pod) @@ -636,7 +636,7 @@ func TestEnsureExtraRBAC(t *testing.T) { Jenkins: jenkins, Scheme: scheme.Scheme, } - reconciler := New(config, log.Log, client.JenkinsAPIConnectionSettings{}) + reconciler := New(config, client.JenkinsAPIConnectionSettings{}) metaObject := resources.NewResourceObjectMeta(jenkins) // when @@ -677,7 +677,7 @@ func TestEnsureExtraRBAC(t *testing.T) { Jenkins: jenkins, Scheme: scheme.Scheme, } - reconciler := New(config, log.Log, client.JenkinsAPIConnectionSettings{}) + reconciler := New(config, client.JenkinsAPIConnectionSettings{}) metaObject := resources.NewResourceObjectMeta(jenkins) // when @@ -724,7 +724,7 @@ func TestEnsureExtraRBAC(t *testing.T) { Jenkins: jenkins, Scheme: scheme.Scheme, } - reconciler := New(config, log.Log, client.JenkinsAPIConnectionSettings{}) + reconciler := New(config, client.JenkinsAPIConnectionSettings{}) metaObject := resources.NewResourceObjectMeta(jenkins) // when @@ -772,7 +772,7 @@ func TestEnsureExtraRBAC(t *testing.T) { Jenkins: jenkins, Scheme: scheme.Scheme, } - reconciler := New(config, log.Log, client.JenkinsAPIConnectionSettings{}) + reconciler := New(config, client.JenkinsAPIConnectionSettings{}) metaObject := resources.NewResourceObjectMeta(jenkins) // when diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index 7fd85531..2f455f8e 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/pkg/controller/jenkins/configuration/base/validate_test.go @@ -18,14 +18,14 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client/fake" - logf "sigs.k8s.io/controller-runtime/pkg/log/zap" ) const defaultNamespace = "default" func TestValidatePlugins(t *testing.T) { log.SetupLogger(true) - baseReconcileLoop := New(configuration.Configuration{}, log.Log, client.JenkinsAPIConnectionSettings{}) + jenkins := &v1alpha2.Jenkins{ObjectMeta: metav1.ObjectMeta{Name: "example"}} + baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) t.Run("empty", func(t *testing.T) { var requiredBasePlugins []plugins.Plugin var basePlugins []v1alpha2.Plugin @@ -168,7 +168,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateImagePullSecrets() fmt.Println(got) @@ -192,7 +192,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: &jenkins, - }, nil, client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, _ := baseReconcileLoop.validateImagePullSecrets() @@ -228,7 +228,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: &jenkins, - }, nil, client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, _ := baseReconcileLoop.validateImagePullSecrets() @@ -264,7 +264,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, _ := baseReconcileLoop.validateImagePullSecrets() @@ -300,7 +300,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, _ := baseReconcileLoop.validateImagePullSecrets() @@ -336,7 +336,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, _ := baseReconcileLoop.validateImagePullSecrets() @@ -369,7 +369,7 @@ func TestValidateJenkinsMasterPodEnvs(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterPodEnvs() assert.Nil(t, got) }) @@ -392,7 +392,7 @@ func TestValidateJenkinsMasterPodEnvs(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterPodEnvs() assert.Equal(t, got, []string{"Jenkins Master container env 'JAVA_OPTS' doesn't have required flag '-Djava.awt.headless=true'"}) @@ -416,7 +416,7 @@ func TestValidateJenkinsMasterPodEnvs(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterPodEnvs() assert.Equal(t, got, []string{"Jenkins Master container env 'JAVA_OPTS' doesn't have required flag '-Djenkins.install.runSetupWizard=false'"}) @@ -438,7 +438,7 @@ func TestValidateReservedVolumes(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateReservedVolumes() assert.Nil(t, got) }) @@ -456,7 +456,7 @@ func TestValidateReservedVolumes(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateReservedVolumes() assert.Equal(t, got, []string{"Jenkins Master pod volume 'jenkins-home' is reserved please choose different one"}) @@ -472,7 +472,7 @@ func TestValidateContainerVolumeMounts(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateContainerVolumeMounts(v1alpha2.Container{}) assert.Nil(t, got) }) @@ -500,7 +500,7 @@ func TestValidateContainerVolumeMounts(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateContainerVolumeMounts(jenkins.Spec.Master.Containers[0]) assert.Nil(t, got) }) @@ -528,7 +528,7 @@ func TestValidateContainerVolumeMounts(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateContainerVolumeMounts(jenkins.Spec.Master.Containers[0]) assert.Equal(t, got, []string{"mountPath not set for 'example' volume mount in container ''"}) }) @@ -551,7 +551,7 @@ func TestValidateContainerVolumeMounts(t *testing.T) { } baseReconcileLoop := New(configuration.Configuration{ Jenkins: &jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateContainerVolumeMounts(jenkins.Spec.Master.Containers[0]) assert.Equal(t, got, []string{"Not found volume for 'missing-volume' volume mount in container ''"}) @@ -570,9 +570,11 @@ func TestValidateConfigMapVolume(t *testing.T) { }, } fakeClient := fake.NewFakeClient() + baseReconcileLoop := New(configuration.Configuration{ + Jenkins: &v1alpha2.Jenkins{ObjectMeta: metav1.ObjectMeta{Name: "example"}}, Client: fakeClient, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateConfigMapVolume(volume) @@ -600,7 +602,7 @@ func TestValidateConfigMapVolume(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateConfigMapVolume(volume) @@ -626,7 +628,7 @@ func TestValidateConfigMapVolume(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateConfigMapVolume(volume) @@ -649,8 +651,9 @@ func TestValidateSecretVolume(t *testing.T) { } fakeClient := fake.NewFakeClient() baseReconcileLoop := New(configuration.Configuration{ + Jenkins: &v1alpha2.Jenkins{ObjectMeta: metav1.ObjectMeta{Name: "example"}}, Client: fakeClient, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateSecretVolume(volume) @@ -676,7 +679,7 @@ func TestValidateSecretVolume(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateSecretVolume(volume) @@ -700,7 +703,7 @@ func TestValidateSecretVolume(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Client: fakeClient, Jenkins: jenkins, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateSecretVolume(volume) assert.NoError(t, err) @@ -723,7 +726,7 @@ func TestValidateCustomization(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Jenkins: jenkins, Client: fakeClient, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateCustomization(customization, "spec.groovyScripts") @@ -745,7 +748,7 @@ func TestValidateCustomization(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Jenkins: jenkins, Client: fakeClient, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) err := fakeClient.Create(context.TODO(), secret) require.NoError(t, err) @@ -776,7 +779,7 @@ func TestValidateCustomization(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Jenkins: jenkins, Client: fakeClient, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) err := fakeClient.Create(context.TODO(), secret) require.NoError(t, err) err = fakeClient.Create(context.TODO(), configMap) @@ -803,7 +806,7 @@ func TestValidateCustomization(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Jenkins: jenkins, Client: fakeClient, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) err := fakeClient.Create(context.TODO(), configMap) require.NoError(t, err) @@ -828,7 +831,7 @@ func TestValidateCustomization(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Jenkins: jenkins, Client: fakeClient, - }, logf.Logger(false), client.JenkinsAPIConnectionSettings{}) + }, client.JenkinsAPIConnectionSettings{}) err := fakeClient.Create(context.TODO(), secret) require.NoError(t, err) @@ -844,7 +847,7 @@ func TestValidateJenkinsMasterContainerCommand(t *testing.T) { log.SetupLogger(true) t.Run("no Jenkins master container", func(t *testing.T) { jenkins := &v1alpha2.Jenkins{} - baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{}) + baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterContainerCommand() @@ -862,7 +865,7 @@ func TestValidateJenkinsMasterContainerCommand(t *testing.T) { }, }, } - baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{}) + baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterContainerCommand() @@ -885,7 +888,7 @@ func TestValidateJenkinsMasterContainerCommand(t *testing.T) { }, }, } - baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{}) + baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterContainerCommand() @@ -904,7 +907,7 @@ func TestValidateJenkinsMasterContainerCommand(t *testing.T) { }, }, } - baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{}) + baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterContainerCommand() @@ -928,7 +931,7 @@ func TestValidateJenkinsMasterContainerCommand(t *testing.T) { }, }, } - baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{}) + baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterContainerCommand() @@ -952,7 +955,7 @@ func TestValidateJenkinsMasterContainerCommand(t *testing.T) { }, }, } - baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{}) + baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) got := baseReconcileLoop.validateJenkinsMasterContainerCommand() diff --git a/pkg/controller/jenkins/configuration/user/reconcile.go b/pkg/controller/jenkins/configuration/user/reconcile.go index acb8c81e..a4c8851a 100644 --- a/pkg/controller/jenkins/configuration/user/reconcile.go +++ b/pkg/controller/jenkins/configuration/user/reconcile.go @@ -3,6 +3,7 @@ package user import ( "strings" + "github.com/go-logr/logr" jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/client" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/backuprestore" @@ -10,8 +11,7 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/user/casc" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/user/seedjobs" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/groovy" - - "github.com/go-logr/logr" + "github.com/jenkinsci/kubernetes-operator/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -23,11 +23,11 @@ type ReconcileUserConfiguration struct { } // New create structure which takes care of user configuration -func New(configuration configuration.Configuration, jenkinsClient jenkinsclient.Jenkins, logger logr.Logger) *ReconcileUserConfiguration { +func New(configuration configuration.Configuration, jenkinsClient jenkinsclient.Jenkins) *ReconcileUserConfiguration { return &ReconcileUserConfiguration{ Configuration: configuration, jenkinsClient: jenkinsClient, - logger: logger, + logger: log.Log.WithValues("cr", configuration.Jenkins.Name), } } diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index b852d015..4272a3ed 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -18,8 +18,6 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/notifications/reason" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/plugins" "github.com/jenkinsci/kubernetes-operator/pkg/log" - - "github.com/go-logr/logr" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -44,6 +42,8 @@ type reconcileError struct { var reconcileErrors = map[string]reconcileError{} +var logx = log.Log + // Add creates a new Jenkins Controller and adds it to the Manager. The Manager will set fields on the Controller // and Start it when the Manager is Started. func Add(mgr manager.Manager, jenkinsAPIConnectionSettings jenkinsclient.JenkinsAPIConnectionSettings, clientSet kubernetes.Clientset, config rest.Config, notificationEvents *chan event.Event) error { @@ -124,10 +124,10 @@ type ReconcileJenkins struct { // 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) { reconcileFailLimit := uint64(10) - logger := r.buildLogger(request.Name) + logger := logx.WithValues("cr", request.Name) logger.V(log.VDebug).Info("Reconciling Jenkins") - result, jenkins, err := r.reconcile(request, logger) + result, jenkins, err := r.reconcile(request) if err != nil && apierrors.IsConflict(err) { return reconcile.Result{Requeue: true}, nil } else if err != nil { @@ -148,7 +148,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul reconcileErrors[request.Name] = lastErrors if lastErrors.counter >= reconcileFailLimit { if log.Debug { - logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %+v", reconcileFailLimit, err)) + logger.V(log.VDebug).Info(fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %+v", reconcileFailLimit, err)) } else { logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %s", reconcileFailLimit, err)) } @@ -166,7 +166,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul } if log.Debug { - logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed: %+v", err)) + logger.V(log.VDebug).Info(fmt.Sprintf("Reconcile loop failed: %+v", err)) } else if err.Error() != fmt.Sprintf("Operation cannot be fulfilled on jenkins.jenkins.io \"%s\": the object has been modified; please apply your changes to the latest version and try again", request.Name) { logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed: %s", err)) } @@ -192,7 +192,8 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul return result, nil } -func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logger) (reconcile.Result, *v1alpha2.Jenkins, error) { +func (r *ReconcileJenkins) reconcile(request reconcile.Request) (reconcile.Result, *v1alpha2.Jenkins, error) { + logger := logx.WithValues("cr", request.Name) // Fetch the Jenkins instance jenkins := &v1alpha2.Jenkins{} var err error @@ -208,7 +209,7 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg return reconcile.Result{}, nil, errors.WithStack(err) } var requeue bool - requeue, err = r.setDefaults(jenkins, logger) + requeue, err = r.setDefaults(jenkins) if err != nil { return reconcile.Result{}, jenkins, err } @@ -216,7 +217,7 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg return reconcile.Result{Requeue: true}, jenkins, nil } - requeue, err = r.handleDeprecatedData(jenkins, logger) + requeue, err = r.handleDeprecatedData(jenkins) if err != nil { return reconcile.Result{}, jenkins, err } @@ -235,7 +236,7 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg } // Reconcile base configuration - baseConfiguration := base.New(config, logger, r.jenkinsAPIConnectionSettings) + baseConfiguration := base.New(config, r.jenkinsAPIConnectionSettings) var baseMessages []string baseMessages, err = baseConfiguration.Validate(jenkins) @@ -289,7 +290,7 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg logger.Info(message) } // Reconcile user configuration - userConfiguration := user.New(config, jenkinsClient, logger) + userConfiguration := user.New(config, jenkinsClient) var messages []string messages, err = userConfiguration.Validate(jenkins) @@ -341,12 +342,9 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg return reconcile.Result{}, jenkins, nil } -func (r *ReconcileJenkins) buildLogger(jenkinsName string) logr.Logger { - return log.Log.WithValues("cr", jenkinsName) -} - -func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha2.Jenkins, logger logr.Logger) (requeue bool, err error) { +func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha2.Jenkins) (requeue bool, err error) { changed := false + logger := logx.WithValues("cr", jenkins.Name) var jenkinsContainer v1alpha2.Container if len(jenkins.Spec.Master.Containers) == 0 { @@ -455,7 +453,7 @@ func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha2.Jenkins, logger logr.Lo } if len(jenkins.Spec.Master.Containers) > 1 { for i, container := range jenkins.Spec.Master.Containers[1:] { - if setDefaultsForContainer(jenkins, i+1, logger.WithValues("container", container.Name)) { + if r.setDefaultsForContainer(jenkins, container.Name, i+1) { changed = true } } @@ -502,8 +500,9 @@ func isJavaOpsVariableNotSet(container v1alpha2.Container) bool { return true } -func setDefaultsForContainer(jenkins *v1alpha2.Jenkins, containerIndex int, logger logr.Logger) bool { +func (r *ReconcileJenkins) setDefaultsForContainer(jenkins *v1alpha2.Jenkins, containerName string, containerIndex int) bool { changed := false + logger := logx.WithValues("cr", jenkins.Name, "container", containerName) if len(jenkins.Spec.Master.Containers[containerIndex].ImagePullPolicy) == 0 { logger.Info(fmt.Sprintf("Setting default container image pull policy: %s", corev1.PullAlways)) @@ -539,8 +538,9 @@ func basePlugins() (result []v1alpha2.Plugin) { return } -func (r *ReconcileJenkins) handleDeprecatedData(jenkins *v1alpha2.Jenkins, logger logr.Logger) (requeue bool, err error) { +func (r *ReconcileJenkins) handleDeprecatedData(jenkins *v1alpha2.Jenkins) (requeue bool, err error) { changed := false + logger := logx.WithValues("cr", jenkins.Name) if len(jenkins.Spec.Master.AnnotationsDeprecated) > 0 { changed = true diff --git a/test/e2e/jenkins.go b/test/e2e/jenkins.go index 28cf81d8..5b42738b 100644 --- a/test/e2e/jenkins.go +++ b/test/e2e/jenkins.go @@ -163,7 +163,7 @@ func createJenkinsAPIClientFromServiceAccount(t *testing.T, jenkins *v1alpha2.Je return nil, err } config := configuration.Configuration{Jenkins: jenkins, ClientSet: *clientSet, Config: framework.Global.KubeConfig} - r := base.New(config, nil, jenkinsclient.JenkinsAPIConnectionSettings{}) + r := base.New(config, jenkinsclient.JenkinsAPIConnectionSettings{}) token, _, err := r.Configuration.Exec(podName, resources.JenkinsMasterContainerName, []string{"cat", "/var/run/secrets/kubernetes.io/serviceaccount/token"}) if err != nil { From 196d208b9f1ff5a052e146f3e4e3cb4c8e79678d Mon Sep 17 00:00:00 2001 From: jkhelil Date: Wed, 13 May 2020 16:04:01 +0200 Subject: [PATCH 2/2] Trigger ci --- pkg/controller/jenkins/jenkins_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index 4272a3ed..fa4262c2 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -148,7 +148,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul reconcileErrors[request.Name] = lastErrors if lastErrors.counter >= reconcileFailLimit { if log.Debug { - logger.V(log.VDebug).Info(fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %+v", reconcileFailLimit, err)) + logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %+v", reconcileFailLimit, err)) } else { logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %s", reconcileFailLimit, err)) } @@ -166,7 +166,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul } if log.Debug { - logger.V(log.VDebug).Info(fmt.Sprintf("Reconcile loop failed: %+v", err)) + logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed: %+v", err)) } else if err.Error() != fmt.Sprintf("Operation cannot be fulfilled on jenkins.jenkins.io \"%s\": the object has been modified; please apply your changes to the latest version and try again", request.Name) { logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed: %s", err)) }