From f21c486dfb3d32f2797d6c19ae1b207a4d92ec83 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Mon, 14 Apr 2025 12:05:44 -0400 Subject: [PATCH] making the update reason and recreate annotations stable closes: #38487 Signed-off-by: Steve Hawkins --- .../KeycloakDeploymentDependentResource.java | 73 +++++++++---------- .../testsuite/integration/UpdateTest.java | 1 + .../operator/testsuite/utils/CRAssert.java | 3 +- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java index 28959f8c7c2..f8734084a0a 100644 --- a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java +++ b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java @@ -22,7 +22,6 @@ import io.fabric8.kubernetes.api.model.EnvVar; import io.fabric8.kubernetes.api.model.EnvVarBuilder; import io.fabric8.kubernetes.api.model.EnvVarSource; import io.fabric8.kubernetes.api.model.EnvVarSourceBuilder; -import io.fabric8.kubernetes.api.model.ObjectMetaFluent; import io.fabric8.kubernetes.api.model.PodSpecFluent; import io.fabric8.kubernetes.api.model.PodTemplateSpec; import io.fabric8.kubernetes.api.model.Secret; @@ -130,7 +129,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent public StatefulSet desired(Keycloak primary, Context context) { Config operatorConfig = ContextUtils.getOperatorConfig(context); WatchedResources watchedResources = ContextUtils.getWatchedResources(context); - + StatefulSet baseDeployment = createBaseDeployment(primary, context, operatorConfig); TreeSet allSecrets = new TreeSet<>(); if (isTlsConfigured(primary)) { @@ -147,16 +146,24 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent watchedResources.annotateDeployment(new ArrayList<>(allSecrets), Secret.class, baseDeployment, context.getClient()); } - // add revision from CR if set + // default to the new revision - will be overriden to the old one if needed UpdateSpec.getRevision(primary).ifPresent(rev -> addUpdateRevisionAnnotation(rev, baseDeployment)); - var updateType = ContextUtils.getUpdateType(context); - // empty means no existing stateful set. - if (updateType.isEmpty()) { - return baseDeployment; + var existingDeployment = ContextUtils.getCurrentStatefulSet(context).orElse(null); + + if (existingDeployment != null) { + // copy the existing annotations to keep the status consistent + CRDUtils.findUpdateReason(existingDeployment).ifPresent(r -> baseDeployment.getMetadata().getAnnotations() + .put(Constants.KEYCLOAK_UPDATE_REASON_ANNOTATION, r)); + CRDUtils.fetchIsRecreateUpdate(existingDeployment).ifPresent(b -> baseDeployment.getMetadata() + .getAnnotations().put(Constants.KEYCLOAK_RECREATE_UPDATE_ANNOTATION, b.toString())); } - var existingDeployment = ContextUtils.getCurrentStatefulSet(context).orElseThrow(); + var updateType = ContextUtils.getUpdateType(context); + + if (existingDeployment == null || updateType.isEmpty()) { + return baseDeployment; + } // version 22 changed the match labels, account for older versions if (!existingDeployment.isMarkedForDeletion() && !hasExpectedMatchLabels(existingDeployment, primary)) { @@ -164,6 +171,8 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent Log.info("Existing Deployment found with old label selector, it will be recreated"); } + baseDeployment.getMetadata().getAnnotations().put(Constants.KEYCLOAK_UPDATE_REASON_ANNOTATION, ContextUtils.getUpdateReason(context)); + return switch (updateType.get()) { case ROLLING -> handleRollingUpdate(baseDeployment, context, primary); case RECREATE -> handleRecreateUpdate(existingDeployment, baseDeployment, context); @@ -551,43 +560,31 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent private static StatefulSet handleRollingUpdate(StatefulSet desired, Context context, Keycloak primary) { // return the desired stateful set since Kubernetes does a rolling in-place update by default. Log.debug("Performing a rolling update"); - var builder = desired.toBuilder() - .editMetadata() - .addToAnnotations(Constants.KEYCLOAK_RECREATE_UPDATE_ANNOTATION, "false") - .addToAnnotations(Constants.KEYCLOAK_UPDATE_REASON_ANNOTATION, ContextUtils.getUpdateReason(context)); - return builder.endMetadata().build(); + desired.getMetadata().getAnnotations().put(Constants.KEYCLOAK_RECREATE_UPDATE_ANNOTATION, Boolean.FALSE.toString()); + return desired; } - private static StatefulSet handleRecreateUpdate(StatefulSet actual, StatefulSet desired, Context context) { - if (actual.getStatus().getReplicas() == 0) { + private static StatefulSet handleRecreateUpdate(StatefulSet actual, StatefulSet desired, + Context context) { + desired.getMetadata().getAnnotations().put(Constants.KEYCLOAK_RECREATE_UPDATE_ANNOTATION, Boolean.TRUE.toString()); + + if (Optional.ofNullable(actual.getStatus().getReplicas()).orElse(0) == 0) { Log.debug("Performing a recreate update - scaling up the stateful set"); - return desired; + + // desired state correct as is + } else { + Log.debug("Performing a recreate update - scaling down the stateful set"); + + // keep the old revision, mark as migrating, and scale down + CRDUtils.getRevision(actual).ifPresent(rev -> addUpdateRevisionAnnotation(rev, desired)); + desired.getMetadata().getAnnotations().put(Constants.KEYCLOAK_MIGRATING_ANNOTATION, Boolean.TRUE.toString()); + desired.getSpec().setReplicas(0); } - Log.debug("Performing a recreate update - scaling down the stateful set"); - // return the existing stateful set, but set replicas to zero - var builder = actual.toBuilder(); - builder.editSpec() - .withReplicas(0) - .endSpec(); - // update metadata from the new stateful set, it is safe to do so. - var metadataBuilder = desired.getMetadata().edit() - .addToAnnotations(Constants.KEYCLOAK_MIGRATING_ANNOTATION, Boolean.TRUE.toString()) - .addToAnnotations(Constants.KEYCLOAK_RECREATE_UPDATE_ANNOTATION, Boolean.TRUE.toString()) - .addToAnnotations(Constants.KEYCLOAK_UPDATE_REASON_ANNOTATION, ContextUtils.getUpdateReason(context)); - // copy revision number from the previous stateful set. - CRDUtils.getRevision(actual).ifPresent(rev -> addUpdateRevisionAnnotation(rev, metadataBuilder)); - return builder - .withMetadata(metadataBuilder.build()) - .build(); + return desired; } private static void addUpdateRevisionAnnotation(String revision, StatefulSet toUpdate) { - var metadataBuilder = toUpdate.getMetadata().edit(); - addUpdateRevisionAnnotation(revision, metadataBuilder); - toUpdate.setMetadata(metadataBuilder.build()); + toUpdate.getMetadata().getAnnotations().put(Constants.KEYCLOAK_UPDATE_REVISION_ANNOTATION, revision); } - private static void addUpdateRevisionAnnotation(String revision, ObjectMetaFluent builder) { - builder.addToAnnotations(Constants.KEYCLOAK_UPDATE_REVISION_ANNOTATION, revision); - } } diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/integration/UpdateTest.java b/operator/src/test/java/org/keycloak/operator/testsuite/integration/UpdateTest.java index 38e1dde35dc..9629a80ff64 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/integration/UpdateTest.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/integration/UpdateTest.java @@ -174,6 +174,7 @@ public class UpdateTest extends BaseOperatorTest { @Test public void testExplicitStrategy() throws InterruptedException { var kc = createInitialDeployment(UpdateStrategy.EXPLICIT); + disableProbes(kc); var updateCondition = assertUnknownUpdateTypeStatus(kc); deployKeycloak(k8sclient, kc, true); diff --git a/operator/src/test/java/org/keycloak/operator/testsuite/utils/CRAssert.java b/operator/src/test/java/org/keycloak/operator/testsuite/utils/CRAssert.java index 95e4798c538..e2f6f4d3388 100644 --- a/operator/src/test/java/org/keycloak/operator/testsuite/utils/CRAssert.java +++ b/operator/src/test/java/org/keycloak/operator/testsuite/utils/CRAssert.java @@ -311,7 +311,8 @@ public final class CRAssert { public static CompletableFuture eventuallyRecreateUpdateStatus(KubernetesClient client, Keycloak keycloak, String reason) { var cf1 = client.resource(keycloak).informOnCondition(kcs -> { try { - assertKeycloakStatusCondition(kcs.get(0), KeycloakStatusCondition.READY, false, "Performing Keycloak update"); + // could be not ready "Performing Keycloak update", or "Waiting for more replicas" + assertKeycloakStatusCondition(kcs.get(0), KeycloakStatusCondition.READY, false, null); return true; } catch (AssertionError e) { return false;