From 15a04021e0d1d4331bade51a1ac8844ce56e5e78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?V=C3=A1clav=20Muzik=C3=A1=C5=99?= <vmuzikar@redhat.com>
Date: Tue, 19 Jul 2022 12:24:09 +0200
Subject: [PATCH] Operator doesn't reconcile annotations specified in pod
 template

---
 .../controllers/KeycloakDeployment.java       |  5 ++-
 .../testsuite/unit/PodTemplateTest.java       | 37 ++++++++++++++++++-
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeployment.java b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeployment.java
index 53eeb7a860..725c7bcd7a 100644
--- a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeployment.java
+++ b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeployment.java
@@ -93,11 +93,12 @@ public class KeycloakDeployment extends OperatorManagedResource implements Statu
             // don't overwrite metadata, just specs
             reconciledDeployment.setSpec(baseDeployment.getSpec());
 
-            // don't overwrite annotations in pod templates to support rolling restarts
+            // don't fully overwrite annotations in pod templates to support rolling restarts (K8s sets some extra annotation to track restart)
+            // instead, merge it
             if (existingDeployment.getSpec() != null && existingDeployment.getSpec().getTemplate() != null) {
                 mergeMaps(
-                        Optional.ofNullable(reconciledDeployment.getSpec().getTemplate().getMetadata()).map(m -> m.getAnnotations()).orElse(null),
                         Optional.ofNullable(existingDeployment.getSpec().getTemplate().getMetadata()).map(m -> m.getAnnotations()).orElse(null),
+                        Optional.ofNullable(reconciledDeployment.getSpec().getTemplate().getMetadata()).map(m -> m.getAnnotations()).orElse(null),
                         annotations -> reconciledDeployment.getSpec().getTemplate().getMetadata().setAnnotations(annotations));
             }
         }
diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java
index e7d2bb6e46..89609b2f4e 100644
--- a/operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java
+++ b/operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java
@@ -22,6 +22,7 @@ import io.fabric8.kubernetes.api.model.PodTemplateSpec;
 import io.fabric8.kubernetes.api.model.PodTemplateSpecBuilder;
 import io.fabric8.kubernetes.api.model.ProbeBuilder;
 import io.fabric8.kubernetes.api.model.apps.StatefulSet;
+import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder;
 import io.quarkus.test.junit.QuarkusTest;
 import org.junit.jupiter.api.Test;
 import org.keycloak.operator.Config;
@@ -30,13 +31,14 @@ import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpec;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpecUnsupported;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @QuarkusTest
 public class PodTemplateTest {
 
-    StatefulSet getDeployment(PodTemplateSpec podTemplate) {
+    private StatefulSet getDeployment(PodTemplateSpec podTemplate, StatefulSet existingDeployment) {
         var config = new Config(){
             @Override
             public Keycloak keycloak() {
@@ -58,10 +60,14 @@ public class PodTemplateTest {
         spec.setHostname("example.com");
         spec.setTlsSecret("example-tls-secret");
         kc.setSpec(spec);
-        var deployment = new KeycloakDeployment(null, config, kc, new StatefulSet(), "dummy-admin");
+        var deployment = new KeycloakDeployment(null, config, kc, existingDeployment, "dummy-admin");
         return (StatefulSet) deployment.getReconciledResource().get();
     }
 
+    private StatefulSet getDeployment(PodTemplateSpec podTemplate) {
+        return getDeployment(podTemplate, new StatefulSet());
+    }
+
     @Test
     public void testEmpty() {
         // Arrange
@@ -235,4 +241,31 @@ public class PodTemplateTest {
         assertEquals(env, envVar.getName());
         assertEquals(value, envVar.getValue());
     }
+
+    @Test
+    public void testAnnotationsAreMerged() {
+        // Arrange
+        var existingDeployment = new StatefulSetBuilder()
+                .withNewSpec()
+                .withNewTemplate()
+                .withNewMetadata()
+                .addToAnnotations("one", "1")
+                .addToAnnotations("two", "3")
+                .endMetadata()
+                .endTemplate()
+                .endSpec()
+                .build();
+        var additionalPodTemplate = new PodTemplateSpecBuilder()
+                .withNewMetadata()
+                .addToAnnotations("two", "2")
+                .endMetadata()
+                .build();
+
+        // Act
+        var podTemplate = getDeployment(additionalPodTemplate, existingDeployment).getSpec().getTemplate();
+
+        // Assert
+        assertThat(podTemplate.getMetadata().getAnnotations()).containsEntry("one", "1");
+        assertThat(podTemplate.getMetadata().getAnnotations()).containsEntry("two", "2");
+    }
 }