From a275ed62d5a3e01ef5ed5e333ce6860b84a8f05f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20S=C4=99k?= Date: Sun, 27 Jan 2019 23:21:12 +0100 Subject: [PATCH] Code improvements --- pkg/controller/jenkins/jenkins_controller.go | 33 ++++++++++---------- pkg/event/event.go | 12 ++++--- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index cfc7953b..9fab9fa7 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -13,7 +13,6 @@ import ( "github.com/VirtusLab/jenkins-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" "k8s.io/apimachinery/pkg/api/resource" @@ -28,9 +27,12 @@ import ( ) const ( - ReasonBaseConfigurationSuccess event.Reason = "BaseConfigurationSuccess" - ReasonBaseConfigurationFailure event.Reason = "BaseConfigurationFailure" - ReasonCRValidationFailure event.Reason = "CRValidationFailure" + // reasonBaseConfigurationSuccess is the event which informs base configuration has been completed successfully + reasonBaseConfigurationSuccess event.Reason = "BaseConfigurationSuccess" + // reasonUserConfigurationSuccess is the event which informs user configuration has been completed successfully + reasonUserConfigurationSuccess event.Reason = "BaseConfigurationFailure" + // reasonCRValidationFailure is the event which informs user has provided invalid configuration in Jenkins CR + reasonCRValidationFailure event.Reason = "CRValidationFailure" ) // Add creates a new Jenkins Controller and adds it to the Manager. The Manager will set fields on the Controller @@ -138,34 +140,31 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg valid, err := baseConfiguration.Validate(jenkins) if err != nil { - r.events.Emitf(jenkins, event.TypeWarning, ReasonBaseConfigurationFailure, "Base configuration failed: %s", err) - return reconcile.Result{}, errors.Wrap(err, "Base configuration failed") + return reconcile.Result{}, err } if !valid { - r.events.Emit(jenkins, event.TypeWarning, ReasonCRValidationFailure, "Base CR validation failed") + r.events.Emit(jenkins, event.TypeWarning, reasonCRValidationFailure, "Base CR validation failed") 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() if err != nil { - r.events.Emitf(jenkins, event.TypeWarning, ReasonBaseConfigurationFailure, "Base configuration failed: %s", err) - return reconcile.Result{}, errors.Wrap(err, "Base configuration failed") + return reconcile.Result{}, err } if result.Requeue { return result, nil } if jenkins.Status.BaseConfigurationCompletedTime == nil { - logger.Info("Base configuration phase is complete") now := metav1.Now() jenkins.Status.BaseConfigurationCompletedTime = &now err = r.client.Update(context.TODO(), jenkins) if err != nil { return reconcile.Result{}, err } - r.events.Emit(jenkins, event.TypeNormal, ReasonBaseConfigurationSuccess, "Base configuration completed") - logger.Info("Base configuration completed time has been updated") + logger.Info("Base configuration phase is complete") + r.events.Emit(jenkins, event.TypeNormal, reasonBaseConfigurationSuccess, "Base configuration completed") } // Reconcile user configuration userConfiguration := user.New(r.client, jenkinsClient, logger, jenkins) @@ -176,33 +175,33 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg } if !valid { logger.V(log.VWarn).Info("Validation of user configuration failed, please correct Jenkins CR") - r.events.Emit(jenkins, event.TypeWarning, ReasonCRValidationFailure, "User CR validation failed") + r.events.Emit(jenkins, event.TypeWarning, reasonCRValidationFailure, "User CR validation failed") return reconcile.Result{}, nil // don't requeue } result, err = userConfiguration.Reconcile() if err != nil { - return reconcile.Result{}, errors.Wrap(err, "Base configuration failed") + return reconcile.Result{}, err } if result.Requeue { return result, nil } if jenkins.Status.UserConfigurationCompletedTime == nil { - logger.Info("User configuration phase is complete") now := metav1.Now() jenkins.Status.UserConfigurationCompletedTime = &now err = r.client.Update(context.TODO(), jenkins) if err != nil { return reconcile.Result{}, err } - logger.Info("User configuration completed time has been updated") + logger.Info("User configuration phase is complete") + r.events.Emit(jenkins, event.TypeNormal, reasonUserConfigurationSuccess, "User configuration completed") } return reconcile.Result{}, nil } -func (*ReconcileJenkins) buildLogger(jenkinsName string) logr.Logger { +func (r *ReconcileJenkins) buildLogger(jenkinsName string) logr.Logger { return log.Log.WithValues("cr", jenkinsName) } diff --git a/pkg/event/event.go b/pkg/event/event.go index 763cc3aa..614e053f 100644 --- a/pkg/event/event.go +++ b/pkg/event/event.go @@ -5,7 +5,6 @@ import ( "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/constants" - "github.com/golang/glog" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" @@ -16,15 +15,19 @@ import ( ) const ( - // Information only and will not cause any problems + // TypeNormal is the information event type TypeNormal = Type("Normal") - // These events are to warn that something might go wrong + // TypeWarning is the warning event type, informs that something went wrong TypeWarning = Type("Warning") ) +// Type is the type of event type Type string + +// Reason is the type of reason message, used in evant type Reason string +// Recorder is the interface used to emit events type Recorder interface { Emit(object runtime.Object, eventType Type, reason Reason, message string) Emitf(object runtime.Object, eventType Type, reason Reason, format string, args ...interface{}) @@ -34,6 +37,7 @@ type recorder struct { recorder record.EventRecorder } +// New returns recorder used to emit events func New(config *rest.Config) (Recorder, error) { eventRecorder, err := initializeEventRecorder(config) if err != nil { @@ -51,7 +55,7 @@ func initializeEventRecorder(config *rest.Config) (record.EventRecorder, error) return nil, err } eventBroadcaster := record.NewBroadcaster() - eventBroadcaster.StartLogging(glog.Infof) + //eventBroadcaster.StartLogging(glog.Infof) TODO integrate with proper logger eventBroadcaster.StartRecordingToSink( &typedcorev1.EventSinkImpl{ Interface: client.CoreV1().Events("")})