From a334db7eadbdad9672fb46108cfb4d6f8b1c65eb Mon Sep 17 00:00:00 2001 From: Benjamin Herbert Date: Fri, 23 May 2025 00:16:00 +0200 Subject: [PATCH] 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{}