Operator Update Logic: add hash based comparison (#44332)

* Operator Update Logic: add hash based comparation

Fixes #44280

Signed-off-by: Pedro Ruivo <1492066+pruivo@users.noreply.github.com>

* refinements to the update logic

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

---------

Signed-off-by: Pedro Ruivo <1492066+pruivo@users.noreply.github.com>
Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Co-authored-by: Pedro Ruivo <1492066+pruivo@users.noreply.github.com>
Co-authored-by: Steve Hawkins <shawkins@redhat.com>

operator/src/main/java/org/keycloak/operator/Constants.java

@@ -39,6 +39,7 @@ public final class Constants {
     public static final String KEYCLOAK_RECREATE_UPDATE_ANNOTATION = "operator.keycloak.org/recreate-update";
     public static final String KEYCLOAK_UPDATE_REASON_ANNOTATION = "operator.keycloak.org/update-reason";
     public static final String KEYCLOAK_UPDATE_REVISION_ANNOTATION = "operator.keycloak.org/update-revision";
+    public static final String KEYCLOAK_UPDATE_HASH_ANNOTATION = "operator.keycloak.org/update-hash";
     public static final String APP_LABEL = "app";
 
     public static final String DEFAULT_LABELS_AS_STRING = "app=keycloak,app.kubernetes.io/managed-by=keycloak-operator";

operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java

@@ -90,6 +90,8 @@ public class KeycloakController implements Reconciler<Keycloak> {
     @Inject
     KeycloakUpdateJobDependentResource updateJobDependentResource;
 
+    KeycloakDeploymentDependentResource keycloakDeploymentDependentResource = new KeycloakDeploymentDependentResource();
+
     @Override
     public List<EventSource<?, Keycloak>> prepareEventSources(EventSourceContext<Keycloak> context) {
         return EventSourceUtils.dependentEventSources(context, updateJobDependentResource);
@@ -139,7 +141,7 @@ public class KeycloakController implements Reconciler<Keycloak> {
         ContextUtils.storeWatchedResources(context, watchedResources);
         ContextUtils.storeDistConfigurator(context, distConfigurator);
         ContextUtils.storeCurrentStatefulSet(context, existingDeployment);
-        ContextUtils.storeDesiredStatefulSet(context, new KeycloakDeploymentDependentResource().desired(kc, context));
+        ContextUtils.storeDesiredStatefulSet(context, keycloakDeploymentDependentResource.initialDesired(kc, context));
 
         var updateLogic = updateLogicFactory.create(kc, context);
         var updateLogicControl = updateLogic.decideUpdate();

operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java

@@ -127,8 +127,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
         this.useServiceCaCrt = useServiceCaCrt;
     }
 
-    @Override
+    public StatefulSet initialDesired(Keycloak primary, Context<Keycloak> context) {
-    public StatefulSet desired(Keycloak primary, Context<Keycloak> context) {
         Config operatorConfig = ContextUtils.getOperatorConfig(context);
         WatchedResources watchedResources = ContextUtils.getWatchedResources(context);
 
@@ -150,6 +149,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
 
         // default to the new revision - will be overriden to the old one if needed
         UpdateSpec.getRevision(primary).ifPresent(rev -> addUpdateRevisionAnnotation(rev, baseDeployment));
+        addUpdateHashAnnotation(KeycloakUpdateJobDependentResource.keycloakHash(primary), baseDeployment);
 
         var existingDeployment = ContextUtils.getCurrentStatefulSet(context).orElse(null);
 
@@ -164,6 +164,13 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
         }
 
         baseDeployment.getSpec().setServiceName(serviceName);
+        return baseDeployment;
+    }
+
+    @Override
+    public StatefulSet desired(Keycloak primary, Context<Keycloak> context) {
+        StatefulSet baseDeployment = ContextUtils.getDesiredStatefulSet(context);
+        var existingDeployment = ContextUtils.getCurrentStatefulSet(context).orElse(null);
 
         var updateType = ContextUtils.getUpdateType(context);
 
@@ -181,7 +188,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
 
         return switch (updateType.get()) {
             case ROLLING -> handleRollingUpdate(baseDeployment);
-            case RECREATE -> handleRecreateUpdate(existingDeployment, baseDeployment, kcContainer);
+            case RECREATE -> handleRecreateUpdate(existingDeployment, baseDeployment, CRDUtils.firstContainerOf(baseDeployment).orElseThrow());
         };
     }
 
@@ -627,8 +634,9 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
         } else {
             Log.debug("Performing a recreate update - scaling down the stateful set");
 
-            // keep the old revision and image, mark as migrating, and scale down
+            // keep the old revision, image, and hash, then mark as migrating, and scale down
-            CRDUtils.getRevision(actual).ifPresent(rev -> addUpdateRevisionAnnotation(rev, desired));
+            addOrRemoveAnnotation(CRDUtils.getRevision(actual).orElse(null), Constants.KEYCLOAK_UPDATE_REVISION_ANNOTATION, desired);
+            addOrRemoveAnnotation(CRDUtils.getUpdateHash(actual).orElse(null), Constants.KEYCLOAK_UPDATE_HASH_ANNOTATION, desired);
             desired.getMetadata().getAnnotations().put(Constants.KEYCLOAK_MIGRATING_ANNOTATION, Boolean.TRUE.toString());
             desired.getSpec().setReplicas(0);
             var currentImage = RecreateOnImageChangeUpdateLogic.extractImage(actual);
@@ -641,6 +649,14 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
         toUpdate.getMetadata().getAnnotations().put(Constants.KEYCLOAK_UPDATE_REVISION_ANNOTATION, revision);
     }
 
+    private static void addUpdateHashAnnotation(String hash, StatefulSet toUpdate) {
+        toUpdate.getMetadata().getAnnotations().put(Constants.KEYCLOAK_UPDATE_HASH_ANNOTATION, hash);
+    }
+
+    private static void addOrRemoveAnnotation(String value, String annotation, StatefulSet toUpdate) {
+        toUpdate.getMetadata().getAnnotations().compute(annotation, (k, v) -> value);
+    }
+
     record ManagementEndpoint(String relativePath, String protocol, int port, String portName) {}
 
     static ManagementEndpoint managementEndpoint(Keycloak keycloakCR, Context<Keycloak> context, boolean health) {

operator/src/main/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResource.java

@@ -224,7 +224,7 @@ public class KeycloakUpdateJobDependentResource extends CRUDKubernetesDependentR
         return Stream.concat(updateArgs.stream(), currentArgs.stream().filter(arg -> !arg.equals("start"))).toList();
     }
 
-    static String keycloakHash(Keycloak keycloak) {
+    public static String keycloakHash(Keycloak keycloak) {
         return Utils.hash(
                 List.of(new KeycloakSpecBuilder(keycloak.getSpec()).withInstances(null).withLivenessProbeSpec(null)
                         .withStartupProbeSpec(null).withReadinessProbeSpec(null).withResourceRequirements(null)

operator/src/main/java/org/keycloak/operator/crds/v2alpha1/CRDUtils.java

@@ -123,4 +123,11 @@ public final class CRDUtils {
                 .map(ObjectMeta::getAnnotations)
                 .map(annotations -> annotations.get(Constants.KEYCLOAK_UPDATE_REVISION_ANNOTATION));
     }
+
+    public static Optional<String> getUpdateHash(StatefulSet statefulSet) {
+        return Optional.ofNullable(statefulSet)
+                .map(StatefulSet::getMetadata)
+                .map(ObjectMeta::getAnnotations)
+                .map(annotations -> annotations.get(Constants.KEYCLOAK_UPDATE_HASH_ANNOTATION));
+    }
 }

operator/src/main/java/org/keycloak/operator/update/impl/BaseUpdateLogic.java

@@ -65,8 +65,15 @@ abstract class BaseUpdateLogic implements UpdateLogic {
         }
         copyStatusFromExistStatefulSet(existing.get());
 
+        Optional<String> storedHash = CRDUtils.getUpdateHash(existing.get());
         var desiredStatefulSet = ContextUtils.getDesiredStatefulSet(context);
         var desiredContainer = CRDUtils.firstContainerOf(desiredStatefulSet).orElseThrow(BaseUpdateLogic::containerNotFound);
+
+        if (Objects.equals(CRDUtils.getUpdateHash(desiredStatefulSet).orElseThrow(), storedHash.orElse(null))) {
+            Log.debug("Hash is equals - skipping update logic");
+            return Optional.empty();
+        }
+
         var actualContainer = CRDUtils.firstContainerOf(existing.get()).orElseThrow(BaseUpdateLogic::containerNotFound);
 
         if (isContainerEquals(actualContainer, desiredContainer)) {

operator/src/test/java/org/keycloak/operator/testsuite/unit/PodTemplateTest.java

@@ -123,7 +123,7 @@ public class PodTemplateTest {
 
         //noinspection unchecked
         Context context = mockContext(null);
-        return deployment.desired(kc, context);
+        return deployment.initialDesired(kc, context);
     }
 
     private Keycloak createKeycloak(PodTemplateSpec podTemplate, Consumer<KeycloakSpecBuilder> additionalSpec) {