From c42477d2752628926baea2d7230c90d8519630a1 Mon Sep 17 00:00:00 2001 From: jkhelil Date: Thu, 14 May 2020 15:04:52 +0200 Subject: [PATCH] inject abstration not implementation --- .../jenkins/configuration/base/reconcile.go | 2 +- .../configuration/base/validate_test.go | 4 +- .../jenkins/configuration/user/casc/casc.go | 17 +++-- .../jenkins/configuration/user/reconcile.go | 45 +++++++----- .../configuration/user/seedjobs/seedjobs.go | 48 ++++++++----- .../user/seedjobs/seedjobs_test.go | 14 ++-- .../configuration/user/seedjobs/validate.go | 10 +-- .../user/seedjobs/validate_test.go | 68 ++++++++++++------- .../jenkins/configuration/user/validate.go | 4 +- pkg/controller/jenkins/groovy/groovy.go | 4 +- pkg/controller/jenkins/groovy/groovy_test.go | 30 ++++---- pkg/controller/jenkins/jenkins_controller.go | 15 +++- 12 files changed, 161 insertions(+), 100 deletions(-) diff --git a/pkg/controller/jenkins/configuration/base/reconcile.go b/pkg/controller/jenkins/configuration/base/reconcile.go index 1b0029d93..75fdf7004 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile.go +++ b/pkg/controller/jenkins/configuration/base/reconcile.go @@ -371,7 +371,7 @@ func (r *ReconcileJenkinsBaseConfiguration) ensureBaseConfiguration(jenkinsClien Configurations: []v1alpha2.ConfigMapRef{{Name: resources.GetBaseConfigurationConfigMapName(r.Configuration.Jenkins)}}, }, } - groovyClient := groovy.New(jenkinsClient, r.Client, r.logger, r.Configuration.Jenkins, "base-groovy", customization.Customization) + groovyClient := groovy.New(jenkinsClient, r.Client, r.Configuration.Jenkins, "base-groovy", customization.Customization) requeue, err := groovyClient.Ensure(func(name string) bool { return strings.HasSuffix(name, ".groovy") }, func(groovyScript string) string { diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index 2f455f8e1..c08b38366 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/pkg/controller/jenkins/configuration/base/validate_test.go @@ -573,7 +573,7 @@ func TestValidateConfigMapVolume(t *testing.T) { baseReconcileLoop := New(configuration.Configuration{ Jenkins: &v1alpha2.Jenkins{ObjectMeta: metav1.ObjectMeta{Name: "example"}}, - Client: fakeClient, + Client: fakeClient, }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateConfigMapVolume(volume) @@ -652,7 +652,7 @@ func TestValidateSecretVolume(t *testing.T) { fakeClient := fake.NewFakeClient() baseReconcileLoop := New(configuration.Configuration{ Jenkins: &v1alpha2.Jenkins{ObjectMeta: metav1.ObjectMeta{Name: "example"}}, - Client: fakeClient, + Client: fakeClient, }, client.JenkinsAPIConnectionSettings{}) got, err := baseReconcileLoop.validateSecretVolume(volume) diff --git a/pkg/controller/jenkins/configuration/user/casc/casc.go b/pkg/controller/jenkins/configuration/user/casc/casc.go index 51f7050dd..d92cf31fb 100644 --- a/pkg/controller/jenkins/configuration/user/casc/casc.go +++ b/pkg/controller/jenkins/configuration/user/casc/casc.go @@ -9,26 +9,29 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/groovy" - "github.com/go-logr/logr" k8s "sigs.k8s.io/controller-runtime/pkg/client" ) const groovyUtf8MaxStringLength = 65535 -// ConfigurationAsCode defines API which configures Jenkins with help Configuration as a code plugin -type ConfigurationAsCode struct { +// ConfigurationAsCode defines client for configurationAsCode +type ConfigurationAsCode interface { + Ensure(jenkins *v1alpha2.Jenkins) (requeue bool, err error) +} + +type configurationAsCode struct { groovyClient *groovy.Groovy } // New creates new instance of ConfigurationAsCode -func New(jenkinsClient jenkinsclient.Jenkins, k8sClient k8s.Client, logger logr.Logger, jenkins *v1alpha2.Jenkins) *ConfigurationAsCode { - return &ConfigurationAsCode{ - groovyClient: groovy.New(jenkinsClient, k8sClient, logger, jenkins, "user-casc", jenkins.Spec.ConfigurationAsCode.Customization), +func New(jenkinsClient jenkinsclient.Jenkins, k8sClient k8s.Client, jenkins *v1alpha2.Jenkins) ConfigurationAsCode { + return &configurationAsCode{ + groovyClient: groovy.New(jenkinsClient, k8sClient, jenkins, "user-casc", jenkins.Spec.ConfigurationAsCode.Customization), } } // Ensure configures Jenkins with help Configuration as a code plugin -func (c *ConfigurationAsCode) Ensure(jenkins *v1alpha2.Jenkins) (requeue bool, err error) { +func (c *configurationAsCode) Ensure(jenkins *v1alpha2.Jenkins) (requeue bool, err error) { requeue, err = c.groovyClient.WaitForSecretSynchronization(resources.ConfigurationAsCodeSecretVolumePath) if err != nil || requeue { return requeue, err diff --git a/pkg/controller/jenkins/configuration/user/reconcile.go b/pkg/controller/jenkins/configuration/user/reconcile.go index a4c8851ac..c52b914bc 100644 --- a/pkg/controller/jenkins/configuration/user/reconcile.go +++ b/pkg/controller/jenkins/configuration/user/reconcile.go @@ -3,6 +3,8 @@ package user import ( "strings" + "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" + "github.com/go-logr/logr" jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/client" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration" @@ -15,27 +17,31 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -// ReconcileUserConfiguration defines values required for Jenkins user configuration -type ReconcileUserConfiguration struct { +// ReconcileUserConfiguration defines API client for reconcile User Configuration +type ReconcileUserConfiguration interface { + ReconcileCasc() (reconcile.Result, error) + ReconcileOthers() (reconcile.Result, error) + Validate(jenkins *v1alpha2.Jenkins) ([]string, error) +} + +type reconcileUserConfiguration struct { configuration.Configuration jenkinsClient jenkinsclient.Jenkins logger logr.Logger } -// New create structure which takes care of user configuration -func New(configuration configuration.Configuration, jenkinsClient jenkinsclient.Jenkins) *ReconcileUserConfiguration { - return &ReconcileUserConfiguration{ +// New create structure which takes care of user configuration. +func New(configuration configuration.Configuration, jenkinsClient jenkinsclient.Jenkins) ReconcileUserConfiguration { + return &reconcileUserConfiguration{ Configuration: configuration, jenkinsClient: jenkinsClient, logger: log.Log.WithValues("cr", configuration.Jenkins.Name), } } -// Reconcile it's a main reconciliation loop for user supplied configuration -func (r *ReconcileUserConfiguration) Reconcile() (reconcile.Result, error) { - backupAndRestore := backuprestore.New(r.Configuration, r.logger) - - result, err := r.ensureUserConfiguration(r.jenkinsClient) +// ReconcileCasc is a reconcile loop for casc. +func (r *reconcileUserConfiguration) ReconcileCasc() (reconcile.Result, error) { + result, err := r.ensureCasc(r.jenkinsClient) if err != nil { return reconcile.Result{}, err } @@ -43,7 +49,14 @@ func (r *ReconcileUserConfiguration) Reconcile() (reconcile.Result, error) { return result, nil } - result, err = r.ensureSeedJobs() + return reconcile.Result{}, nil +} + +// Reconcile it's a main reconciliation loop for user supplied configuration +func (r *reconcileUserConfiguration) ReconcileOthers() (reconcile.Result, error) { + backupAndRestore := backuprestore.New(r.Configuration, r.logger) + + result, err := r.ensureSeedJobs() if err != nil { return reconcile.Result{}, err } @@ -65,8 +78,8 @@ func (r *ReconcileUserConfiguration) Reconcile() (reconcile.Result, error) { return reconcile.Result{}, nil } -func (r *ReconcileUserConfiguration) ensureSeedJobs() (reconcile.Result, error) { - seedJobs := seedjobs.New(r.jenkinsClient, r.Configuration, r.logger) +func (r *reconcileUserConfiguration) ensureSeedJobs() (reconcile.Result, error) { + seedJobs := seedjobs.New(r.jenkinsClient, r.Configuration) done, err := seedJobs.EnsureSeedJobs(r.Configuration.Jenkins) if err != nil { return reconcile.Result{}, err @@ -77,8 +90,8 @@ func (r *ReconcileUserConfiguration) ensureSeedJobs() (reconcile.Result, error) return reconcile.Result{}, nil } -func (r *ReconcileUserConfiguration) ensureUserConfiguration(jenkinsClient jenkinsclient.Jenkins) (reconcile.Result, error) { - configurationAsCodeClient := casc.New(jenkinsClient, r.Client, r.logger, r.Configuration.Jenkins) +func (r *reconcileUserConfiguration) ensureCasc(jenkinsClient jenkinsclient.Jenkins) (reconcile.Result, error) { + configurationAsCodeClient := casc.New(jenkinsClient, r.Client, r.Configuration.Jenkins) requeue, err := configurationAsCodeClient.Ensure(r.Configuration.Jenkins) if err != nil { return reconcile.Result{}, err @@ -87,7 +100,7 @@ func (r *ReconcileUserConfiguration) ensureUserConfiguration(jenkinsClient jenki return reconcile.Result{Requeue: true}, nil } - groovyClient := groovy.New(jenkinsClient, r.Client, r.logger, r.Configuration.Jenkins, "user-groovy", r.Configuration.Jenkins.Spec.GroovyScripts.Customization) + groovyClient := groovy.New(jenkinsClient, r.Client, r.Configuration.Jenkins, "user-groovy", r.Configuration.Jenkins.Spec.GroovyScripts.Customization) requeue, err = groovyClient.WaitForSecretSynchronization(resources.GroovyScriptsSecretVolumePath) if err != nil { return reconcile.Result{}, err diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go b/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go index e5237f0e4..6e9d2fa86 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go +++ b/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs.go @@ -8,6 +8,7 @@ import ( "reflect" "text/template" + "github.com/go-logr/logr" "github.com/jenkinsci/kubernetes-operator/internal/render" "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" jenkinsclient "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/client" @@ -16,8 +17,7 @@ import ( "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/constants" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/groovy" "github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/notifications/reason" - - "github.com/go-logr/logr" + "github.com/jenkinsci/kubernetes-operator/pkg/log" stackerr "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -142,24 +142,40 @@ jobRef.setAssignedLabel(new LabelAtom("{{ .AgentName }}")) jenkins.getQueue().schedule(jobRef) `)) -// SeedJobs defines API for configuring and ensuring Jenkins Seed Jobs and Deploy Keys -type SeedJobs struct { +// SeedJobs defines client interface to SeedJobs +type SeedJobs interface { + EnsureSeedJobs(jenkins *v1alpha2.Jenkins) (done bool, err error) + waitForSeedJobAgent(agentName string) (requeue bool, err error) + createJobs(jenkins *v1alpha2.Jenkins) (requeue bool, err error) + ensureLabelsForSecrets(jenkins v1alpha2.Jenkins) error + credentialValue(namespace string, seedJob v1alpha2.SeedJob) (string, error) + getAllSeedJobIDs(jenkins v1alpha2.Jenkins) []string + isRecreatePodNeeded(jenkins v1alpha2.Jenkins) bool + createAgent(jenkinsClient jenkinsclient.Jenkins, k8sClient client.Client, jenkinsManifest *v1alpha2.Jenkins, namespace string, agentName string) error + ValidateSeedJobs(jenkins v1alpha2.Jenkins) ([]string, error) + validateSchedule(job v1alpha2.SeedJob, str string, key string) []string + validateGitHubPushTrigger(jenkins v1alpha2.Jenkins) []string + validateBitbucketPushTrigger(jenkins v1alpha2.Jenkins) []string + validateIfIDIsUnique(seedJobs []v1alpha2.SeedJob) []string +} + +type seedJobs struct { configuration.Configuration jenkinsClient jenkinsclient.Jenkins logger logr.Logger } // New creates SeedJobs object -func New(jenkinsClient jenkinsclient.Jenkins, config configuration.Configuration, logger logr.Logger) *SeedJobs { - return &SeedJobs{ +func New(jenkinsClient jenkinsclient.Jenkins, config configuration.Configuration) SeedJobs { + return &seedJobs{ Configuration: config, jenkinsClient: jenkinsClient, - logger: logger, + logger: log.Log.WithValues("cr", config.Jenkins.Name), } } // EnsureSeedJobs configures seed job and runs it for every entry from Jenkins.Spec.SeedJobs -func (s *SeedJobs) EnsureSeedJobs(jenkins *v1alpha2.Jenkins) (done bool, err error) { +func (s *seedJobs) EnsureSeedJobs(jenkins *v1alpha2.Jenkins) (done bool, err error) { if s.isRecreatePodNeeded(*jenkins) { message := "Some seed job has been deleted, recreating pod" s.logger.Info(message) @@ -218,7 +234,7 @@ func (s *SeedJobs) EnsureSeedJobs(jenkins *v1alpha2.Jenkins) (done bool, err err return true, nil } -func (s SeedJobs) waitForSeedJobAgent(agentName string) (requeue bool, err error) { +func (s *seedJobs) waitForSeedJobAgent(agentName string) (requeue bool, err error) { agent := appsv1.Deployment{} err = s.Client.Get(context.TODO(), types.NamespacedName{Name: agentDeploymentName(*s.Jenkins, agentName), Namespace: s.Jenkins.Namespace}, &agent) if apierrors.IsNotFound(err) { @@ -237,8 +253,8 @@ func (s SeedJobs) waitForSeedJobAgent(agentName string) (requeue bool, err error } // createJob is responsible for creating jenkins job which configures jenkins seed jobs and deploy keys -func (s *SeedJobs) createJobs(jenkins *v1alpha2.Jenkins) (requeue bool, err error) { - groovyClient := groovy.New(s.jenkinsClient, s.Client, s.logger, jenkins, "seed-jobs", jenkins.Spec.GroovyScripts.Customization) +func (s *seedJobs) createJobs(jenkins *v1alpha2.Jenkins) (requeue bool, err error) { + groovyClient := groovy.New(s.jenkinsClient, s.Client, jenkins, "seed-jobs", jenkins.Spec.GroovyScripts.Customization) for _, seedJob := range jenkins.Spec.SeedJobs { credentialValue, err := s.credentialValue(jenkins.Namespace, seedJob) if err != nil { @@ -269,7 +285,7 @@ func (s *SeedJobs) createJobs(jenkins *v1alpha2.Jenkins) (requeue bool, err erro // ensureLabelsForSecrets adds labels to Kubernetes secrets where are Jenkins credentials used for seed jobs, // thanks to them kubernetes-credentials-provider-plugin will create Jenkins credentials in Jenkins and // Operator will able to watch any changes made to them -func (s *SeedJobs) ensureLabelsForSecrets(jenkins v1alpha2.Jenkins) error { +func (s *seedJobs) ensureLabelsForSecrets(jenkins v1alpha2.Jenkins) error { for _, seedJob := range jenkins.Spec.SeedJobs { if seedJob.JenkinsCredentialType == v1alpha2.BasicSSHCredentialType || seedJob.JenkinsCredentialType == v1alpha2.UsernamePasswordCredentialType { requiredLabels := resources.BuildLabelsForWatchedResources(jenkins) @@ -294,7 +310,7 @@ func (s *SeedJobs) ensureLabelsForSecrets(jenkins v1alpha2.Jenkins) error { return nil } -func (s *SeedJobs) credentialValue(namespace string, seedJob v1alpha2.SeedJob) (string, error) { +func (s *seedJobs) credentialValue(namespace string, seedJob v1alpha2.SeedJob) (string, error) { if seedJob.JenkinsCredentialType == v1alpha2.BasicSSHCredentialType || seedJob.JenkinsCredentialType == v1alpha2.UsernamePasswordCredentialType { secret := &corev1.Secret{} namespaceName := types.NamespacedName{Namespace: namespace, Name: seedJob.CredentialID} @@ -311,7 +327,7 @@ func (s *SeedJobs) credentialValue(namespace string, seedJob v1alpha2.SeedJob) ( return "", nil } -func (s *SeedJobs) getAllSeedJobIDs(jenkins v1alpha2.Jenkins) []string { +func (s *seedJobs) getAllSeedJobIDs(jenkins v1alpha2.Jenkins) []string { var ids []string for _, seedJob := range jenkins.Spec.SeedJobs { ids = append(ids, seedJob.ID) @@ -319,7 +335,7 @@ func (s *SeedJobs) getAllSeedJobIDs(jenkins v1alpha2.Jenkins) []string { return ids } -func (s *SeedJobs) isRecreatePodNeeded(jenkins v1alpha2.Jenkins) bool { +func (s *seedJobs) isRecreatePodNeeded(jenkins v1alpha2.Jenkins) bool { for _, createdSeedJob := range jenkins.Status.CreatedSeedJobs { found := false for _, seedJob := range jenkins.Spec.SeedJobs { @@ -336,7 +352,7 @@ func (s *SeedJobs) isRecreatePodNeeded(jenkins v1alpha2.Jenkins) bool { } // createAgent deploys Jenkins agent to Kubernetes cluster -func (s SeedJobs) createAgent(jenkinsClient jenkinsclient.Jenkins, k8sClient client.Client, jenkinsManifest *v1alpha2.Jenkins, namespace string, agentName string) error { +func (s *seedJobs) createAgent(jenkinsClient jenkinsclient.Jenkins, k8sClient client.Client, jenkinsManifest *v1alpha2.Jenkins, namespace string, agentName string) error { _, err := jenkinsClient.GetNode(agentName) // Create node if not exists diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go b/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go index 7e678c40d..03f3aafbc 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go +++ b/pkg/controller/jenkins/configuration/user/seedjobs/seedjobs_test.go @@ -22,7 +22,6 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" - logf "sigs.k8s.io/controller-runtime/pkg/log/zap" ) const agentSecret = "test-secret" @@ -70,7 +69,6 @@ func jenkinsCustomResource() *v1alpha2.Jenkins { func TestEnsureSeedJobs(t *testing.T) { t.Run("happy", func(t *testing.T) { // given - logger := logf.Logger(false) ctrl := gomock.NewController(t) ctx := context.TODO() defer ctrl.Finish() @@ -105,7 +103,7 @@ func TestEnsureSeedJobs(t *testing.T) { jenkinsClient.EXPECT().GetNodeSecret(AgentName).Return(agentSecret, nil).AnyTimes() jenkinsClient.EXPECT().ExecuteScript(seedJobCreatingScript).AnyTimes() - seedJobClient := New(jenkinsClient, config, logger) + seedJobClient := New(jenkinsClient, config) // when _, err = seedJobClient.EnsureSeedJobs(jenkins) @@ -120,7 +118,6 @@ func TestEnsureSeedJobs(t *testing.T) { t.Run("delete agent deployment when no seed jobs", func(t *testing.T) { // given - logger := logf.Logger(false) ctrl := gomock.NewController(t) ctx := context.TODO() defer ctrl.Finish() @@ -144,7 +141,7 @@ func TestEnsureSeedJobs(t *testing.T) { jenkinsClient.EXPECT().CreateNode(AgentName, 1, "The jenkins-operator generated agent", "/home/jenkins", AgentName).AnyTimes() jenkinsClient.EXPECT().GetNodeSecret(AgentName).Return(agentSecret, nil).AnyTimes() - seedJobsClient := New(jenkinsClient, config, logger) + seedJobsClient := New(jenkinsClient, config) err = fakeClient.Create(ctx, &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -170,7 +167,6 @@ func TestEnsureSeedJobs(t *testing.T) { func TestCreateAgent(t *testing.T) { t.Run("don't fail when deployment is already created", func(t *testing.T) { // given - logger := logf.Logger(false) ctrl := gomock.NewController(t) ctx := context.TODO() defer ctrl.Finish() @@ -190,9 +186,10 @@ func TestCreateAgent(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobsClient := New(jenkinsClient, config, logger) + seedJobsClient := New(jenkinsClient, config) err = fakeClient.Create(ctx, &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -215,8 +212,9 @@ func TestSeedJobs_isRecreatePodNeeded(t *testing.T) { Client: nil, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobsClient := New(nil, config, nil) + seedJobsClient := New(nil, config) t.Run("empty", func(t *testing.T) { jenkins := v1alpha2.Jenkins{} diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/validate.go b/pkg/controller/jenkins/configuration/user/seedjobs/validate.go index 30d501e0f..8e52529f8 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/validate.go +++ b/pkg/controller/jenkins/configuration/user/seedjobs/validate.go @@ -16,7 +16,7 @@ import ( ) // ValidateSeedJobs verify seed jobs configuration -func (s *SeedJobs) ValidateSeedJobs(jenkins v1alpha2.Jenkins) ([]string, error) { +func (s *seedJobs) ValidateSeedJobs(jenkins v1alpha2.Jenkins) ([]string, error) { var messages []string if msg := s.validateIfIDIsUnique(jenkins.Spec.SeedJobs); len(msg) > 0 { @@ -118,7 +118,7 @@ func (s *SeedJobs) ValidateSeedJobs(jenkins v1alpha2.Jenkins) ([]string, error) return messages, nil } -func (s *SeedJobs) validateSchedule(job v1alpha2.SeedJob, str string, key string) []string { +func (s *seedJobs) validateSchedule(job v1alpha2.SeedJob, str string, key string) []string { var messages []string _, err := cron.Parse(str) if err != nil { @@ -127,7 +127,7 @@ func (s *SeedJobs) validateSchedule(job v1alpha2.SeedJob, str string, key string return messages } -func (s *SeedJobs) validateGitHubPushTrigger(jenkins v1alpha2.Jenkins) []string { +func (s *seedJobs) validateGitHubPushTrigger(jenkins v1alpha2.Jenkins) []string { var messages []string if err := checkPluginExists(jenkins, "github"); err != nil { return append(messages, fmt.Sprintf("githubPushTrigger cannot be enabled: %s", err)) @@ -135,7 +135,7 @@ func (s *SeedJobs) validateGitHubPushTrigger(jenkins v1alpha2.Jenkins) []string return messages } -func (s *SeedJobs) validateBitbucketPushTrigger(jenkins v1alpha2.Jenkins) []string { +func (s *seedJobs) validateBitbucketPushTrigger(jenkins v1alpha2.Jenkins) []string { var messages []string if err := checkPluginExists(jenkins, "bitbucket"); err != nil { return append(messages, fmt.Sprintf("bitbucketPushTrigger cannot be enabled: %s", err)) @@ -164,7 +164,7 @@ func checkPluginExists(jenkins v1alpha2.Jenkins, name string) error { return nil } -func (s *SeedJobs) validateIfIDIsUnique(seedJobs []v1alpha2.SeedJob) []string { +func (s *seedJobs) validateIfIDIsUnique(seedJobs []v1alpha2.SeedJob) []string { var messages []string ids := map[string]bool{} for _, seedJob := range seedJobs { diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go b/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go index 5acfff3be..7e8ecf468 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go +++ b/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go @@ -12,8 +12,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client/fake" - - logf "sigs.k8s.io/controller-runtime/pkg/log/zap" ) var fakePrivateKey = `-----BEGIN RSA PRIVATE KEY----- @@ -81,9 +79,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -109,9 +108,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -149,9 +149,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -188,9 +189,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -228,9 +230,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -259,9 +262,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -288,9 +292,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -317,9 +322,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -346,9 +352,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -386,9 +393,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -425,9 +433,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -465,9 +474,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -504,9 +514,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -543,9 +554,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -575,9 +587,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -608,9 +621,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -639,9 +653,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -676,9 +691,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -707,9 +723,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -744,9 +761,10 @@ func TestValidateSeedJobs(t *testing.T) { Client: fakeClient, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - seedJobs := New(nil, config, logf.Logger(false)) + seedJobs := New(nil, config) result, err := seedJobs.ValidateSeedJobs(jenkins) assert.NoError(t, err) @@ -764,9 +782,10 @@ func TestValidateIfIDIsUnique(t *testing.T) { Client: nil, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - ctrl := New(nil, config, logf.Logger(false)) + ctrl := New(nil, config) got := ctrl.validateIfIDIsUnique(seedJobs) assert.Nil(t, got) }) @@ -779,9 +798,10 @@ func TestValidateIfIDIsUnique(t *testing.T) { Client: nil, ClientSet: kubernetes.Clientset{}, Notifications: nil, + Jenkins: &v1alpha2.Jenkins{}, } - ctrl := New(nil, config, logf.Logger(false)) + ctrl := New(nil, config) got := ctrl.validateIfIDIsUnique(seedJobs) assert.Equal(t, got, []string{"'first' seed job ID is not unique"}) diff --git a/pkg/controller/jenkins/configuration/user/validate.go b/pkg/controller/jenkins/configuration/user/validate.go index 1a163d70c..5039ac041 100644 --- a/pkg/controller/jenkins/configuration/user/validate.go +++ b/pkg/controller/jenkins/configuration/user/validate.go @@ -7,12 +7,12 @@ import ( ) // Validate validates Jenkins CR Spec section -func (r *ReconcileUserConfiguration) Validate(jenkins *v1alpha2.Jenkins) ([]string, error) { +func (r *reconcileUserConfiguration) Validate(jenkins *v1alpha2.Jenkins) ([]string, error) { backupAndRestore := backuprestore.New(r.Configuration, r.logger) if msg := backupAndRestore.Validate(); msg != nil { return msg, nil } - seedJobs := seedjobs.New(r.jenkinsClient, r.Configuration, r.logger) + seedJobs := seedjobs.New(r.jenkinsClient, r.Configuration) return seedJobs.ValidateSeedJobs(*jenkins) } diff --git a/pkg/controller/jenkins/groovy/groovy.go b/pkg/controller/jenkins/groovy/groovy.go index dd092b04e..6c68357e0 100644 --- a/pkg/controller/jenkins/groovy/groovy.go +++ b/pkg/controller/jenkins/groovy/groovy.go @@ -30,14 +30,14 @@ type Groovy struct { } // New creates new instance of Groovy -func New(jenkinsClient jenkinsclient.Jenkins, k8sClient k8s.Client, logger logr.Logger, jenkins *v1alpha2.Jenkins, configurationType string, customization v1alpha2.Customization) *Groovy { +func New(jenkinsClient jenkinsclient.Jenkins, k8sClient k8s.Client, jenkins *v1alpha2.Jenkins, configurationType string, customization v1alpha2.Customization) *Groovy { return &Groovy{ jenkinsClient: jenkinsClient, k8sClient: k8sClient, - logger: logger, jenkins: jenkins, configurationType: configurationType, customization: customization, + logger: log.Log.WithValues("cr", jenkins.Name), } } diff --git a/pkg/controller/jenkins/groovy/groovy_test.go b/pkg/controller/jenkins/groovy/groovy_test.go index 043a308e0..fd12fb6d0 100644 --- a/pkg/controller/jenkins/groovy/groovy_test.go +++ b/pkg/controller/jenkins/groovy/groovy_test.go @@ -52,7 +52,7 @@ func TestGroovy_EnsureSingle(t *testing.T) { jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, emptyCustomization) // when requeue, err := groovyClient.EnsureSingle(source, groovyScriptName, hash, groovyScript) @@ -97,7 +97,7 @@ func TestGroovy_EnsureSingle(t *testing.T) { defer ctrl.Finish() jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, emptyCustomization) // when requeue, err := groovyClient.EnsureSingle(source, groovyScriptName, hash, groovyScript) @@ -137,7 +137,7 @@ func TestGroovy_EnsureSingle(t *testing.T) { jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, emptyCustomization) // when requeue, err := groovyClient.EnsureSingle(source, groovyScriptName, hash, groovyScript) @@ -209,7 +209,7 @@ func TestGroovy_EnsureSingle(t *testing.T) { jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, emptyCustomization) requeue, err := groovyClient.EnsureSingle(source, firstGroovyScriptName, firstGroovyScriptHash, groovyScript) @@ -268,7 +268,7 @@ func TestGroovy_EnsureSingle(t *testing.T) { jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, emptyCustomization) requeue, err := groovyClient.EnsureSingle("test-conf1", "test.groovy", hash, groovyScript) require.NoError(t, err) @@ -299,13 +299,13 @@ func TestGroovy_EnsureSingle(t *testing.T) { jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, emptyCustomization) requeue, err := groovyClient.EnsureSingle(source, "test.groovy", hash, groovyScript) require.NoError(t, err) assert.True(t, requeue) - groovyClient = New(jenkinsClient, fakeClient, log.Log, jenkins, "another-test-configuration-type", emptyCustomization) + groovyClient = New(jenkinsClient, fakeClient, jenkins, "another-test-configuration-type", emptyCustomization) requeue, err = groovyClient.EnsureSingle(source, "test.groovy", "anotherHash", groovyScript) require.NoError(t, err) @@ -332,7 +332,7 @@ func TestGroovy_EnsureSingle(t *testing.T) { jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("fail logs", &jenkinsclient.GroovyScriptExecutionFailed{}) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, emptyCustomization) // when requeue, err := groovyClient.EnsureSingle(source, groovyScriptName, hash, groovyScript) @@ -403,7 +403,7 @@ func TestGroovy_Ensure(t *testing.T) { jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, customization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, customization) onlyGroovyFilesFunc := func(name string) bool { return strings.HasSuffix(name, groovyScriptExtension) } @@ -463,7 +463,7 @@ func TestGroovy_Ensure(t *testing.T) { jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) jenkinsClient.EXPECT().ExecuteScript(groovyScript+groovyScriptSuffix).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, customization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, customization) updateGroovyFunc := func(groovyScript string) string { return groovyScript + groovyScriptSuffix } @@ -522,7 +522,7 @@ func TestGroovy_Ensure(t *testing.T) { jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, customization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, customization) // when requeue, err := groovyClient.Ensure(allGroovyScriptsFunc, noUpdateGroovyScript) @@ -590,7 +590,7 @@ func TestGroovy_Ensure(t *testing.T) { jenkinsClient := jenkinsclient.NewMockJenkins(ctrl) jenkinsClient.EXPECT().ExecuteScript(groovyScript).Return("logs", nil) - groovyClient := New(jenkinsClient, fakeClient, log.Log, jenkins, configurationType, customization) + groovyClient := New(jenkinsClient, fakeClient, jenkins, configurationType, customization) // when requeue, err := groovyClient.Ensure(allGroovyScriptsFunc, noUpdateGroovyScript) @@ -628,7 +628,7 @@ func TestGroovy_isGroovyScriptAlreadyApplied(t *testing.T) { }, }, } - groovyClient := New(nil, nil, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(nil, nil, jenkins, configurationType, emptyCustomization) got := groovyClient.isGroovyScriptAlreadyApplied("source", "name", "hash") @@ -647,7 +647,7 @@ func TestGroovy_isGroovyScriptAlreadyApplied(t *testing.T) { }, }, } - groovyClient := New(nil, nil, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(nil, nil, jenkins, configurationType, emptyCustomization) got := groovyClient.isGroovyScriptAlreadyApplied("source", "not-exist", "hash") @@ -655,7 +655,7 @@ func TestGroovy_isGroovyScriptAlreadyApplied(t *testing.T) { }) t.Run("empty Jenkins status", func(t *testing.T) { jenkins := &v1alpha2.Jenkins{} - groovyClient := New(nil, nil, log.Log, jenkins, configurationType, emptyCustomization) + groovyClient := New(nil, nil, jenkins, configurationType, emptyCustomization) got := groovyClient.isGroovyScriptAlreadyApplied("source", "name", "hash") diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index fa4262c2b..2d0e75068 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -289,7 +289,8 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request) (reconcile.Resul } logger.Info(message) } - // Reconcile user configuration + + // Reconcile casc, seedjobs and backups userConfiguration := user.New(config, jenkinsClient) var messages []string @@ -313,7 +314,17 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request) (reconcile.Resul return reconcile.Result{}, jenkins, nil // don't requeue } - result, err = userConfiguration.Reconcile() + // Reconcile casc + result, err = userConfiguration.ReconcileCasc() + if err != nil { + return reconcile.Result{}, jenkins, err + } + if result.Requeue { + return result, jenkins, nil + } + + // Reconcile seedjobs, backups + result, err = userConfiguration.ReconcileOthers() if err != nil { return reconcile.Result{}, jenkins, err }