From fc216263e8a291236390d38a10314df7f35725b5 Mon Sep 17 00:00:00 2001 From: Akram Ben Aissi Date: Mon, 25 May 2020 12:34:58 +0200 Subject: [PATCH] Minor refactor of the jenkins reconcile - Extract resources limits creation - Start to extract the reconcilier structure into a separate file --- cmd/manager/main.go | 6 +- pkg/configuration/base/plugin.go | 2 +- pkg/configuration/base/pod.go | 2 +- pkg/configuration/base/reconcile.go | 2 +- pkg/configuration/base/resources/probe.go | 26 ++++ pkg/configuration/base/resources/resource.go | 19 +++ pkg/configuration/base/validate_test.go | 2 +- pkg/configuration/user/seedjobs/seedjobs.go | 2 +- pkg/controller/jenkins/jenkins_controller.go | 124 +++++-------------- pkg/controller/jenkins/reconciler.go | 49 ++++++++ pkg/notifications/sender.go | 4 +- 11 files changed, 132 insertions(+), 106 deletions(-) create mode 100644 pkg/configuration/base/resources/probe.go create mode 100644 pkg/configuration/base/resources/resource.go create mode 100644 pkg/controller/jenkins/reconciler.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 4e669c2f..320fdb7f 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -11,14 +11,14 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" "github.com/jenkinsci/kubernetes-operator/pkg/apis" - "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins" "github.com/jenkinsci/kubernetes-operator/pkg/client" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/constants" - "github.com/jenkinsci/kubernetes-operator/pkg/notifications" - e "github.com/jenkinsci/kubernetes-operator/pkg/notifications/event" + "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins" "github.com/jenkinsci/kubernetes-operator/pkg/event" "github.com/jenkinsci/kubernetes-operator/pkg/log" + "github.com/jenkinsci/kubernetes-operator/pkg/notifications" + e "github.com/jenkinsci/kubernetes-operator/pkg/notifications/event" "github.com/jenkinsci/kubernetes-operator/version" "github.com/operator-framework/operator-sdk/pkg/k8sutil" diff --git a/pkg/configuration/base/plugin.go b/pkg/configuration/base/plugin.go index f8a80a3d..4847f6c5 100644 --- a/pkg/configuration/base/plugin.go +++ b/pkg/configuration/base/plugin.go @@ -5,8 +5,8 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/client" - "github.com/jenkinsci/kubernetes-operator/pkg/plugins" "github.com/jenkinsci/kubernetes-operator/pkg/log" + "github.com/jenkinsci/kubernetes-operator/pkg/plugins" "github.com/bndr/gojenkins" stackerr "github.com/pkg/errors" diff --git a/pkg/configuration/base/pod.go b/pkg/configuration/base/pod.go index 9c1c0d40..873f367d 100644 --- a/pkg/configuration/base/pod.go +++ b/pkg/configuration/base/pod.go @@ -8,9 +8,9 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/backuprestore" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" + "github.com/jenkinsci/kubernetes-operator/pkg/log" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/event" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" - "github.com/jenkinsci/kubernetes-operator/pkg/log" "github.com/jenkinsci/kubernetes-operator/version" stackerr "github.com/pkg/errors" diff --git a/pkg/configuration/base/reconcile.go b/pkg/configuration/base/reconcile.go index 020ffa72..8ebf5351 100644 --- a/pkg/configuration/base/reconcile.go +++ b/pkg/configuration/base/reconcile.go @@ -14,8 +14,8 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/configuration" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/groovy" - "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" "github.com/jenkinsci/kubernetes-operator/pkg/log" + "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" "github.com/go-logr/logr" stackerr "github.com/pkg/errors" diff --git a/pkg/configuration/base/resources/probe.go b/pkg/configuration/base/resources/probe.go new file mode 100644 index 00000000..545933ae --- /dev/null +++ b/pkg/configuration/base/resources/probe.go @@ -0,0 +1,26 @@ +package resources + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func NewSimpleProbe(uri string, port string, scheme corev1.URIScheme, initialDelaySeconds int32) *corev1.Probe { + return &corev1.Probe{ + Handler: corev1.Handler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: uri, + Port: intstr.FromString(port), + Scheme: corev1.URISchemeHTTP, + }, + }, + InitialDelaySeconds: initialDelaySeconds, + } +} + +func NewProbe(uri string, port string, scheme corev1.URIScheme, initialDelaySeconds, timeoutSeconds, failureThreshold int32) *corev1.Probe { + p := NewSimpleProbe(uri, port, scheme, initialDelaySeconds) + p.TimeoutSeconds = timeoutSeconds + p.FailureThreshold = failureThreshold + return p +} diff --git a/pkg/configuration/base/resources/resource.go b/pkg/configuration/base/resources/resource.go new file mode 100644 index 00000000..5be1889c --- /dev/null +++ b/pkg/configuration/base/resources/resource.go @@ -0,0 +1,19 @@ +package resources + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func NewResourceRequirements(cpuRequest, memoryRequest, cpuLimit, memoryLimit string) corev1.ResourceRequirements { + return corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpuRequest), + corev1.ResourceMemory: resource.MustParse(memoryRequest), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpuLimit), + corev1.ResourceMemory: resource.MustParse(memoryLimit), + }, + } +} diff --git a/pkg/configuration/base/validate_test.go b/pkg/configuration/base/validate_test.go index 4735934b..96eb0183 100644 --- a/pkg/configuration/base/validate_test.go +++ b/pkg/configuration/base/validate_test.go @@ -10,8 +10,8 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/configuration" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/constants" - "github.com/jenkinsci/kubernetes-operator/pkg/plugins" "github.com/jenkinsci/kubernetes-operator/pkg/log" + "github.com/jenkinsci/kubernetes-operator/pkg/plugins" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/configuration/user/seedjobs/seedjobs.go b/pkg/configuration/user/seedjobs/seedjobs.go index 24328e9a..0744529a 100644 --- a/pkg/configuration/user/seedjobs/seedjobs.go +++ b/pkg/configuration/user/seedjobs/seedjobs.go @@ -16,8 +16,8 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/constants" "github.com/jenkinsci/kubernetes-operator/pkg/groovy" - "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" "github.com/jenkinsci/kubernetes-operator/pkg/log" + "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" stackerr "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index 49430f4f..35bfbc42 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -9,25 +9,20 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/client" - "github.com/jenkinsci/kubernetes-operator/pkg/configuration" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/user" "github.com/jenkinsci/kubernetes-operator/pkg/constants" + "github.com/jenkinsci/kubernetes-operator/pkg/log" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/event" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" "github.com/jenkinsci/kubernetes-operator/pkg/plugins" - "github.com/jenkinsci/kubernetes-operator/pkg/log" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -40,32 +35,29 @@ type reconcileError struct { counter uint64 } +const ( + APIVersion = "core/v1" + PodKind = "Pod" + SecretKind = "Secret" + ConfigMapKind = "ConfigMap" + containerProbeURI = "login" + containerProbePortName = "http" +) + var reconcileErrors = map[string]reconcileError{} - var logx = log.Log +var _ reconcile.Reconciler = &ReconcileJenkins{} -// Add creates a new Jenkins Controller and adds it to the Manager. The Manager will set fields on the Controller +// Add creates a newReconcilierConfiguration 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 { reconciler := newReconciler(mgr, jenkinsAPIConnectionSettings, clientSet, config, notificationEvents) return add(mgr, reconciler) } -// newReconciler returns a new reconcile.Reconciler. -func newReconciler(mgr manager.Manager, jenkinsAPIConnectionSettings jenkinsclient.JenkinsAPIConnectionSettings, clientSet kubernetes.Clientset, config rest.Config, notificationEvents *chan event.Event) reconcile.Reconciler { - return &ReconcileJenkins{ - client: mgr.GetClient(), - scheme: mgr.GetScheme(), - jenkinsAPIConnectionSettings: jenkinsAPIConnectionSettings, - clientSet: clientSet, - config: config, - notificationEvents: notificationEvents, - } -} - -// add adds a new Controller to mgr with r as the reconcile.Reconciler. +// add adds a newReconcilierConfiguration Controller to mgr with r as the reconcile.Reconciler. func add(mgr manager.Manager, r reconcile.Reconciler) error { - // Create a new controller + // Create a newReconcilierConfiguration controller c, err := controller.New("jenkins-controller", mgr, controller.Options{Reconciler: r}) if err != nil { return errors.WithStack(err) @@ -79,7 +71,9 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { } // Watch for changes to secondary resource Pods and requeue the owner Jenkins - err = c.Watch(&source.Kind{Type: &corev1.Pod{TypeMeta: metav1.TypeMeta{APIVersion: "core/v1", Kind: "Pod"}}}, &handler.EnqueueRequestForOwner{ + + podResource := &source.Kind{Type: &corev1.Pod{TypeMeta: metav1.TypeMeta{APIVersion: APIVersion, Kind: PodKind}}} + err = c.Watch(podResource, &handler.EnqueueRequestForOwner{ IsController: true, OwnerType: &v1alpha2.Jenkins{}, }) @@ -87,7 +81,8 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { return errors.WithStack(err) } - err = c.Watch(&source.Kind{Type: &corev1.Secret{TypeMeta: metav1.TypeMeta{APIVersion: "core/v1", Kind: "Secret"}}}, &handler.EnqueueRequestForOwner{ + secretResource := &source.Kind{Type: &corev1.Secret{TypeMeta: metav1.TypeMeta{APIVersion: APIVersion, Kind: SecretKind}}} + err = c.Watch(secretResource, &handler.EnqueueRequestForOwner{ IsController: true, OwnerType: &v1alpha2.Jenkins{}, }) @@ -96,31 +91,19 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { } jenkinsHandler := &enqueueRequestForJenkins{} - err = c.Watch(&source.Kind{Type: &corev1.Secret{TypeMeta: metav1.TypeMeta{APIVersion: "core/v1", Kind: "Secret"}}}, jenkinsHandler) + err = c.Watch(secretResource, jenkinsHandler) if err != nil { return errors.WithStack(err) } - err = c.Watch(&source.Kind{Type: &corev1.ConfigMap{TypeMeta: metav1.TypeMeta{APIVersion: "core/v1", Kind: "ConfigMap"}}}, jenkinsHandler) + configMapResource := &source.Kind{Type: &corev1.ConfigMap{TypeMeta: metav1.TypeMeta{APIVersion: APIVersion, Kind: ConfigMapKind}}} + err = c.Watch(configMapResource, jenkinsHandler) if err != nil { return errors.WithStack(err) } - return nil } -var _ reconcile.Reconciler = &ReconcileJenkins{} - -// ReconcileJenkins reconciles a Jenkins object. -type ReconcileJenkins struct { - client client.Client - scheme *runtime.Scheme - jenkinsAPIConnectionSettings jenkinsclient.JenkinsAPIConnectionSettings - clientSet kubernetes.Clientset - config rest.Config - notificationEvents *chan event.Event -} - // 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) @@ -225,16 +208,7 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request) (reconcile.Resul return reconcile.Result{Requeue: true}, jenkins, nil } - config := configuration.Configuration{ - Client: r.client, - ClientSet: r.clientSet, - Notifications: r.notificationEvents, - Jenkins: jenkins, - Scheme: r.scheme, - Config: &r.config, - JenkinsAPIConnectionSettings: r.jenkinsAPIConnectionSettings, - } - + config := r.newReconcilierConfiguration(jenkins) // Reconcile base configuration baseConfiguration := base.New(config, r.jenkinsAPIConnectionSettings) @@ -349,7 +323,6 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request) (reconcile.Resul } logger.Info(message) } - return reconcile.Result{}, jenkins, nil } @@ -379,35 +352,16 @@ func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha2.Jenkins) (requeue bool, changed = true jenkinsContainer.ImagePullPolicy = corev1.PullAlways } + if jenkinsContainer.ReadinessProbe == nil { logger.Info("Setting default Jenkins readinessProbe") changed = true - jenkinsContainer.ReadinessProbe = &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/login", - Port: intstr.FromString("http"), - Scheme: corev1.URISchemeHTTP, - }, - }, - InitialDelaySeconds: int32(30), - } + jenkinsContainer.ReadinessProbe = resources.NewSimpleProbe(containerProbeURI, containerProbePortName, corev1.URISchemeHTTP, 30) } if jenkinsContainer.LivenessProbe == nil { logger.Info("Setting default Jenkins livenessProbe") changed = true - jenkinsContainer.LivenessProbe = &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/login", - Port: intstr.FromString("http"), - Scheme: corev1.URISchemeHTTP, - }, - }, - InitialDelaySeconds: int32(80), - TimeoutSeconds: int32(5), - FailureThreshold: int32(12), - } + jenkinsContainer.LivenessProbe = resources.NewProbe(containerProbeURI, containerProbePortName, corev1.URISchemeHTTP, 80, 5, 12) } if len(jenkinsContainer.Command) == 0 { logger.Info("Setting default Jenkins container command") @@ -430,21 +384,11 @@ func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha2.Jenkins) (requeue bool, if isResourceRequirementsNotSet(jenkinsContainer.Resources) { logger.Info("Setting default Jenkins master container resource requirements") changed = true - jenkinsContainer.Resources = corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - corev1.ResourceMemory: resource.MustParse("500Mi"), - }, - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1500m"), - corev1.ResourceMemory: resource.MustParse("3Gi"), - }, - } + jenkinsContainer.Resources = resources.NewResourceRequirements("1", "500Mi", "1500m", "3Gi") } if reflect.DeepEqual(jenkins.Spec.Service, v1alpha2.Service{}) { logger.Info("Setting default Jenkins master service") changed = true - var serviceType = corev1.ServiceTypeClusterIP if r.jenkinsAPIConnectionSettings.UseNodePort { serviceType = corev1.ServiceTypeNodePort @@ -523,18 +467,8 @@ func (r *ReconcileJenkins) setDefaultsForContainer(jenkins *v1alpha2.Jenkins, co if isResourceRequirementsNotSet(jenkins.Spec.Master.Containers[containerIndex].Resources) { logger.Info("Setting default container resource requirements") changed = true - jenkins.Spec.Master.Containers[containerIndex].Resources = corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("50m"), - corev1.ResourceMemory: resource.MustParse("50Mi"), - }, - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("100m"), - corev1.ResourceMemory: resource.MustParse("100Mi"), - }, - } + jenkins.Spec.Master.Containers[containerIndex].Resources = resources.NewResourceRequirements("50m", "50Mi", "100m", "100Mi") } - return changed } @@ -552,14 +486,12 @@ func basePlugins() (result []v1alpha2.Plugin) { 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 jenkins.Spec.Master.Annotations = jenkins.Spec.Master.AnnotationsDeprecated jenkins.Spec.Master.AnnotationsDeprecated = map[string]string{} logger.V(log.VWarn).Info("spec.master.masterAnnotations is deprecated, the annotations have been moved to spec.master.annotations") } - if changed { return changed, errors.WithStack(r.client.Update(context.TODO(), jenkins)) } diff --git a/pkg/controller/jenkins/reconciler.go b/pkg/controller/jenkins/reconciler.go new file mode 100644 index 00000000..9f9be6e6 --- /dev/null +++ b/pkg/controller/jenkins/reconciler.go @@ -0,0 +1,49 @@ +package jenkins + +import ( + "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" + jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/client" + "github.com/jenkinsci/kubernetes-operator/pkg/configuration" + "github.com/jenkinsci/kubernetes-operator/pkg/notifications/event" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// ReconcileJenkins reconciles a Jenkins object. +type ReconcileJenkins struct { + client client.Client + scheme *runtime.Scheme + jenkinsAPIConnectionSettings jenkinsclient.JenkinsAPIConnectionSettings + clientSet kubernetes.Clientset + config rest.Config + notificationEvents *chan event.Event +} + +func (r *ReconcileJenkins) newReconcilierConfiguration(jenkins *v1alpha2.Jenkins) configuration.Configuration { + config := configuration.Configuration{ + Client: r.client, + ClientSet: r.clientSet, + Notifications: r.notificationEvents, + Jenkins: jenkins, + Scheme: r.scheme, + Config: &r.config, + JenkinsAPIConnectionSettings: r.jenkinsAPIConnectionSettings, + } + return config +} + +// newReconciler returns a newReconcilierConfiguration reconcile.Reconciler. +func newReconciler(mgr manager.Manager, jenkinsAPIConnectionSettings jenkinsclient.JenkinsAPIConnectionSettings, clientSet kubernetes.Clientset, config rest.Config, notificationEvents *chan event.Event) reconcile.Reconciler { + return &ReconcileJenkins{ + client: mgr.GetClient(), + scheme: mgr.GetScheme(), + jenkinsAPIConnectionSettings: jenkinsAPIConnectionSettings, + clientSet: clientSet, + config: config, + notificationEvents: notificationEvents, + } +} diff --git a/pkg/notifications/sender.go b/pkg/notifications/sender.go index 850ef252..4e100c88 100644 --- a/pkg/notifications/sender.go +++ b/pkg/notifications/sender.go @@ -7,13 +7,13 @@ import ( "strings" "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" + k8sevent "github.com/jenkinsci/kubernetes-operator/pkg/event" + "github.com/jenkinsci/kubernetes-operator/pkg/log" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/event" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/mailgun" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/msteams" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/slack" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/smtp" - k8sevent "github.com/jenkinsci/kubernetes-operator/pkg/event" - "github.com/jenkinsci/kubernetes-operator/pkg/log" "github.com/pkg/errors" k8sclient "sigs.k8s.io/controller-runtime/pkg/client"