From 2a21ee7c2c3186d69802feeb36cc9270e1650f83 Mon Sep 17 00:00:00 2001 From: Benjamin Herbert Date: Mon, 5 May 2025 13:21:41 +0200 Subject: [PATCH 1/6] feat: allow to ignore volumes during reconciliation --- api/v1alpha2/jenkins_types.go | 4 +++ pkg/configuration/base/reconcile_test.go | 41 ++++++++++++++++++++++++ pkg/configuration/base/reconciler.go | 15 +++++++++ 3 files changed, 60 insertions(+) diff --git a/api/v1alpha2/jenkins_types.go b/api/v1alpha2/jenkins_types.go index f71b32d3..dcd7554f 100644 --- a/api/v1alpha2/jenkins_types.go +++ b/api/v1alpha2/jenkins_types.go @@ -400,6 +400,10 @@ type JenkinsMaster struct { // Defaults to 30 seconds. // +optional TerminationGracePeriodSeconds *int64 `json:"terminationGracePeriodSeconds,omitempty"` + + // IgnoredVolumes defines the list of volume names that should be excluded from processing or consideration. + // +optional + IgnoredVolumes []string `json:"ignoredVolumes,omitempty"` } // Service defines Kubernetes service attributes diff --git a/pkg/configuration/base/reconcile_test.go b/pkg/configuration/base/reconcile_test.go index 2c39c175..fcbf3b59 100644 --- a/pkg/configuration/base/reconcile_test.go +++ b/pkg/configuration/base/reconcile_test.go @@ -160,6 +160,47 @@ func TestCompareVolumes(t *testing.T) { assert.True(t, got) }) + + t.Run("different - additional workspace identity volume", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{}, + }, + } + pod := corev1.Pod{ + Spec: corev1.PodSpec{ + ServiceAccountName: "service-account-name", + Volumes: append(resources.GetJenkinsMasterPodBaseVolumes(jenkins), corev1.Volume{Name: "azure-identity-token"}), + }, + } + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) + + got := reconciler.compareVolumes(pod) + + assert.False(t, got) + }) + + t.Run("additional workspace identity volume but ignored", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + IgnoredVolumes: []string{"azure-identity-token"}, + }, + }, + } + pod := corev1.Pod{ + Spec: corev1.PodSpec{ + ServiceAccountName: "service-account-name", + Volumes: append(resources.GetJenkinsMasterPodBaseVolumes(jenkins), corev1.Volume{Name: "azure-identity-token"}), + }, + } + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) + + got := reconciler.compareVolumes(pod) + + assert.True(t, got) + }) + } func TestJenkinsBaseConfigurationReconciler_verifyPlugins(t *testing.T) { diff --git a/pkg/configuration/base/reconciler.go b/pkg/configuration/base/reconciler.go index e5096513..940e77d0 100644 --- a/pkg/configuration/base/reconciler.go +++ b/pkg/configuration/base/reconciler.go @@ -294,6 +294,11 @@ func CompareContainerVolumeMounts(expected corev1.Container, actual corev1.Conta func (r *JenkinsBaseConfigurationReconciler) compareVolumes(actualPod corev1.Pod) bool { var toCompare []corev1.Volume for _, volume := range actualPod.Spec.Volumes { + + if r.isVolumeIgnored(volume.Name) { + continue + } + // filter out service account if strings.HasPrefix(volume.Name, actualPod.Spec.ServiceAccountName) { continue @@ -421,3 +426,13 @@ func (r *JenkinsBaseConfigurationReconciler) ensureBaseConfiguration(jenkinsClie }) return reconcile.Result{Requeue: requeue}, err } + +// isVolumeIgnored checks if the given volume name is in the list of ignored volumes +func (r *JenkinsBaseConfigurationReconciler) isVolumeIgnored(volumeName string) bool { + for _, ignoredVolume := range r.Jenkins.Spec.Master.IgnoredVolumes { + if ignoredVolume == volumeName { + return true + } + } + return false +} From dcf1b261225496fac2cf423cfdcd115c31f42233 Mon Sep 17 00:00:00 2001 From: Benjamin Herbert Date: Thu, 22 May 2025 19:55:16 +0200 Subject: [PATCH 2/6] refactor: define and use struct for lifecycle-ignore --- api/v1alpha2/jenkins_types.go | 27 +++++++++++++++++++++ pkg/configuration/base/reconcile_test.go | 31 ++++++++++++++++++++++-- pkg/configuration/base/reconciler.go | 2 +- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/api/v1alpha2/jenkins_types.go b/api/v1alpha2/jenkins_types.go index dcd7554f..e3626b3b 100644 --- a/api/v1alpha2/jenkins_types.go +++ b/api/v1alpha2/jenkins_types.go @@ -73,6 +73,21 @@ type JenkinsSpec struct { // JenkinsAPISettings defines configuration used by the operator to gain admin access to the Jenkins API JenkinsAPISettings JenkinsAPISettings `json:"jenkinsAPISettings"` + + // +optional + Lifecycle JenkinsLifecycle `json:"lifecycle,omitempty"` +} + +type JenkinsLifecycle struct { + // +optional + Ignore JenkinsLifecycleIgnore `json:"ignore,omitempty"` +} + +type JenkinsLifecycleIgnore struct { + IgnoredVolumes []string `json:"volumes,omitempty"` + IgnoredEnvs []string `json:"envs,omitempty"` + IgnoredAnnotations []string `json:"annotations,omitempty"` + IgnoredLabels []string `json:"labels,omitempty"` } // AuthorizationStrategy defines authorization strategy of the operator for the Jenkins API @@ -404,6 +419,18 @@ type JenkinsMaster struct { // IgnoredVolumes defines the list of volume names that should be excluded from processing or consideration. // +optional IgnoredVolumes []string `json:"ignoredVolumes,omitempty"` + + // IgnoredAnnotations specifies a list of annotation keys that should be ignored during configuration updates. + // +optional + IgnoredAnnotations []string `json:"ignoredAnnotations,omitempty"` + + // IgnoredEnvVars defines the list of environment variable names that should be excluded from the Jenkins master pod. + // +optional + IgnoredEnvVars []string `json:"ignoredEnvVars,omitempty"` + + // IgnoredLabels specifies the list of labels to be excluded from configuration or processing in the Jenkins master. + // +optional + IgnoredLabels []string `json:"ignoredLabels,omitempty"` } // Service defines Kubernetes service attributes diff --git a/pkg/configuration/base/reconcile_test.go b/pkg/configuration/base/reconcile_test.go index fcbf3b59..daac66da 100644 --- a/pkg/configuration/base/reconcile_test.go +++ b/pkg/configuration/base/reconcile_test.go @@ -182,9 +182,12 @@ func TestCompareVolumes(t *testing.T) { t.Run("additional workspace identity volume but ignored", func(t *testing.T) { jenkins := &v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ - Master: v1alpha2.JenkinsMaster{ - IgnoredVolumes: []string{"azure-identity-token"}, + Lifecycle: v1alpha2.JenkinsLifecycle{ + Ignore: v1alpha2.JenkinsLifecycleIgnore{ + IgnoredVolumes: []string{"azure-identity-token"}, + }, }, }, } @@ -201,6 +204,30 @@ func TestCompareVolumes(t *testing.T) { assert.True(t, got) }) + t.Run("additional multiple volumes added but ignored", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + + Spec: v1alpha2.JenkinsSpec{ + Lifecycle: v1alpha2.JenkinsLifecycle{ + Ignore: v1alpha2.JenkinsLifecycleIgnore{ + IgnoredVolumes: []string{"volume-present", "volume-absent"}, + }, + }, + }, + } + pod := corev1.Pod{ + Spec: corev1.PodSpec{ + ServiceAccountName: "service-account-name", + Volumes: append(resources.GetJenkinsMasterPodBaseVolumes(jenkins), corev1.Volume{Name: "volume-present"}), + }, + } + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) + + got := reconciler.compareVolumes(pod) + + assert.True(t, got) + }) + } func TestJenkinsBaseConfigurationReconciler_verifyPlugins(t *testing.T) { diff --git a/pkg/configuration/base/reconciler.go b/pkg/configuration/base/reconciler.go index 940e77d0..53112074 100644 --- a/pkg/configuration/base/reconciler.go +++ b/pkg/configuration/base/reconciler.go @@ -429,7 +429,7 @@ func (r *JenkinsBaseConfigurationReconciler) ensureBaseConfiguration(jenkinsClie // isVolumeIgnored checks if the given volume name is in the list of ignored volumes func (r *JenkinsBaseConfigurationReconciler) isVolumeIgnored(volumeName string) bool { - for _, ignoredVolume := range r.Jenkins.Spec.Master.IgnoredVolumes { + for _, ignoredVolume := range r.Jenkins.Spec.Lifecycle.Ignore.IgnoredVolumes { if ignoredVolume == volumeName { return true } From 94b9a2eaac6c9f0e197362b3c5ffbe3f7da99213 Mon Sep 17 00:00:00 2001 From: Benjamin Herbert Date: Fri, 23 May 2025 00:14:09 +0200 Subject: [PATCH 3/6] chore: remove old attempt --- api/v1alpha2/jenkins_types.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/api/v1alpha2/jenkins_types.go b/api/v1alpha2/jenkins_types.go index e3626b3b..190802b4 100644 --- a/api/v1alpha2/jenkins_types.go +++ b/api/v1alpha2/jenkins_types.go @@ -415,22 +415,6 @@ type JenkinsMaster struct { // Defaults to 30 seconds. // +optional TerminationGracePeriodSeconds *int64 `json:"terminationGracePeriodSeconds,omitempty"` - - // IgnoredVolumes defines the list of volume names that should be excluded from processing or consideration. - // +optional - IgnoredVolumes []string `json:"ignoredVolumes,omitempty"` - - // IgnoredAnnotations specifies a list of annotation keys that should be ignored during configuration updates. - // +optional - IgnoredAnnotations []string `json:"ignoredAnnotations,omitempty"` - - // IgnoredEnvVars defines the list of environment variable names that should be excluded from the Jenkins master pod. - // +optional - IgnoredEnvVars []string `json:"ignoredEnvVars,omitempty"` - - // IgnoredLabels specifies the list of labels to be excluded from configuration or processing in the Jenkins master. - // +optional - IgnoredLabels []string `json:"ignoredLabels,omitempty"` } // Service defines Kubernetes service attributes From a334db7eadbdad9672fb46108cfb4d6f8b1c65eb Mon Sep 17 00:00:00 2001 From: Benjamin Herbert Date: Fri, 23 May 2025 00:16:00 +0200 Subject: [PATCH 4/6] feat: implement compare annotations It allows to specify ignores values. Note that additional values are ignored anyways, this ignore annotation only handles different values or additional annotations. --- pkg/configuration/base/pod.go | 20 ++++- pkg/configuration/base/reconcile_test.go | 108 +++++++++++++++++++++++ 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/pkg/configuration/base/pod.go b/pkg/configuration/base/pod.go index 579e085f..337ca0f3 100644 --- a/pkg/configuration/base/pod.go +++ b/pkg/configuration/base/pod.go @@ -3,14 +3,14 @@ package base import ( "context" "fmt" - "reflect" - "github.com/jenkinsci/kubernetes-operator/api/v1alpha2" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/backuprestore" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/event" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" "github.com/jenkinsci/kubernetes-operator/version" + "reflect" + "slices" stackerr "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -82,7 +82,7 @@ func (r *JenkinsBaseConfigurationReconciler) checkForPodRecreation(currentJenkin currentJenkinsMasterPod.Labels, r.Configuration.Jenkins.Spec.Master.Labels)) } - if !compareMap(r.Configuration.Jenkins.Spec.Master.Annotations, currentJenkinsMasterPod.ObjectMeta.Annotations) { + if !r.compareAnnotations(currentJenkinsMasterPod) { messages = append(messages, "Jenkins pod annotations have changed") verbose = append(verbose, fmt.Sprintf("Jenkins pod annotations have changed, actual '%+v' required '%+v'", currentJenkinsMasterPod.ObjectMeta.Annotations, r.Configuration.Jenkins.Spec.Master.Annotations)) @@ -146,6 +146,20 @@ func (r *JenkinsBaseConfigurationReconciler) checkForPodRecreation(currentJenkin return reason.NewPodRestart(reason.OperatorSource, messages, verbose...) } +func (r *JenkinsBaseConfigurationReconciler) compareAnnotations(currentJenkinsMasterPod corev1.Pod) bool { + ignoredAnnotations := r.Jenkins.Spec.Lifecycle.Ignore.IgnoredAnnotations + annotations := r.Configuration.Jenkins.Spec.Master.Annotations + + res := make(map[string]string) + for key, val := range annotations { + if slices.Contains(ignoredAnnotations, key) { + continue + } + res[key] = val + } + return compareMap(res, currentJenkinsMasterPod.ObjectMeta.Annotations) +} + func (r *JenkinsBaseConfigurationReconciler) ensureJenkinsMasterPod(meta metav1.ObjectMeta) (reconcile.Result, error) { userAndPasswordHash, err := r.calculateUserAndPasswordHash() if err != nil { diff --git a/pkg/configuration/base/reconcile_test.go b/pkg/configuration/base/reconcile_test.go index daac66da..49c6b174 100644 --- a/pkg/configuration/base/reconcile_test.go +++ b/pkg/configuration/base/reconcile_test.go @@ -97,6 +97,114 @@ func TestCompareContainerVolumeMounts(t *testing.T) { }) } +func TestCompareAnnotations(t *testing.T) { + type testCase struct { + name string + jenkinsAnnotations map[string]string + ignoredAnnotations []string + podAnnotations map[string]string + expectedShouldMatch bool + } + + runTest := func(t *testing.T, tc testCase) { + + t.Run(tc.name, func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + Annotations: tc.jenkinsAnnotations, + }, + }, + } + + if len(tc.ignoredAnnotations) > 0 { + jenkins.Spec.Lifecycle = v1alpha2.JenkinsLifecycle{ + Ignore: v1alpha2.JenkinsLifecycleIgnore{ + IgnoredAnnotations: tc.ignoredAnnotations, + }, + } + } + + pod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: tc.podAnnotations, + }, + } + + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) + result := reconciler.compareAnnotations(pod) + + assert.Equal(t, tc.expectedShouldMatch, result, + "Expected compareAnnotations to return %v but got %v", tc.expectedShouldMatch, result) + }) + } + + testCases := []testCase{ + { + name: "no annotation - additional annotations - not different", + jenkinsAnnotations: map[string]string{}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"one": "two"}, + expectedShouldMatch: true, + }, + { + name: "one additional annotation - change not ignored - not different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"one": "two", "additional": "annotation"}, + expectedShouldMatch: true, + }, + { + + name: "annotations different - different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"two": "three"}, + expectedShouldMatch: false, + }, + + { + name: "annotations different - ignored - not different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: []string{"one"}, + podAnnotations: map[string]string{"two": "three"}, + expectedShouldMatch: true, + }, + { + name: "one annotation different - change not ignored - different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"one": "different"}, + expectedShouldMatch: false, + }, + { + name: "one annotation different - change ignored - not different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: []string{"one"}, + podAnnotations: map[string]string{"one": "different"}, + expectedShouldMatch: true, + }, + { + name: "one additional annotation - different", + jenkinsAnnotations: map[string]string{"one": "two", "ignore": "me"}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"one": "two", "ignore": "this"}, + expectedShouldMatch: false, + }, + { + name: "one additional annotation - change ignored - not different", + jenkinsAnnotations: map[string]string{"one": "two", "ignore": "me"}, + ignoredAnnotations: []string{"ignore"}, + podAnnotations: map[string]string{"one": "two", "ignore": "this"}, + expectedShouldMatch: true, + }, + } + + for _, tc := range testCases { + runTest(t, tc) + } +} + func TestCompareVolumes(t *testing.T) { t.Run("defaults", func(t *testing.T) { jenkins := &v1alpha2.Jenkins{} From 9d4ce8c3192b34311e2a031f396efedac55532ac Mon Sep 17 00:00:00 2001 From: Benjamin Herbert Date: Wed, 28 May 2025 13:43:11 +0200 Subject: [PATCH 5/6] chore: reorder imports --- pkg/configuration/base/pod.go | 5 +++-- pkg/configuration/base/reconciler.go | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/configuration/base/pod.go b/pkg/configuration/base/pod.go index 337ca0f3..2f68f879 100644 --- a/pkg/configuration/base/pod.go +++ b/pkg/configuration/base/pod.go @@ -3,14 +3,15 @@ package base import ( "context" "fmt" + "reflect" + "slices" + "github.com/jenkinsci/kubernetes-operator/api/v1alpha2" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/backuprestore" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/base/resources" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/event" "github.com/jenkinsci/kubernetes-operator/pkg/notifications/reason" "github.com/jenkinsci/kubernetes-operator/version" - "reflect" - "slices" stackerr "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" diff --git a/pkg/configuration/base/reconciler.go b/pkg/configuration/base/reconciler.go index 53112074..8d550838 100644 --- a/pkg/configuration/base/reconciler.go +++ b/pkg/configuration/base/reconciler.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "fmt" "reflect" + "slices" "strings" "time" From c40f3b894130a9bd6c811f81b2939623c8c45d99 Mon Sep 17 00:00:00 2001 From: Benjamin Herbert Date: Thu, 12 Jun 2025 11:02:48 +0200 Subject: [PATCH 6/6] fix: unused import --- pkg/configuration/base/reconciler.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/configuration/base/reconciler.go b/pkg/configuration/base/reconciler.go index 8d550838..53112074 100644 --- a/pkg/configuration/base/reconciler.go +++ b/pkg/configuration/base/reconciler.go @@ -6,7 +6,6 @@ import ( "encoding/base64" "fmt" "reflect" - "slices" "strings" "time"