keycloak · vmuzikar · Jun 28, 2023 · Jun 24, 2023
@@ -349,4 +349,8 @@ their corresponding replacements.
 * `Streams` interfaces in federated storage provider classes were deprecated.
 * `KeycloakModelUtils#getClientScopeMappings` was removed.
 * Deprecated methods from `KeycloakSession` were removed.
-* `UserQueryProvider#getUsersStream` methods were removed.
+* `UserQueryProvider#getUsersStream` methods were removed.
+
+= Multiple Keycloak instances
+
+Multiple Keycloak CRs may be created in the same namespace and will be managed independently by the operator.  To allow for this StatefulSets created by older versions of the operator must be re-created.  This will happen automatically when the operator is upgraded and lead to small amount of downtime.
@@ -22,14 +22,14 @@
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
-import java.util.stream.Collectors;
 
 public final class Constants {
     public static final String CRDS_GROUP = "k8s.keycloak.org";
     public static final String CRDS_VERSION = "v2alpha1";
     public static final String SHORT_NAME = "kc";
     public static final String NAME = "keycloak";
     public static final String PLURAL_NAME = "keycloaks";
+    public static final String INSTANCE_LABEL = "app.kubernetes.io/instance";
     public static final String MANAGED_BY_LABEL = "app.kubernetes.io/managed-by";
     public static final String MANAGED_BY_VALUE = "keycloak-operator";
     public static final String COMPONENT_LABEL = "app.kubernetes.io/component";
@@ -40,9 +40,7 @@ public final class Constants {
             MANAGED_BY_LABEL, MANAGED_BY_VALUE
     )));
 
-    public static final String DEFAULT_LABELS_AS_STRING = DEFAULT_LABELS.entrySet().stream()
-            .map(e -> e.getKey() + "=" + e.getValue())
-            .collect(Collectors.joining(","));
+    public static final String DEFAULT_LABELS_AS_STRING = Utils.toSelectorString(DEFAULT_LABELS);
 
     public static final List<ValueOrSecret> DEFAULT_DIST_CONFIG_LIST = List.of(
             new ValueOrSecret("health-enabled", "true"),

@@ -24,6 +24,8 @@
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.Base64;
+import java.util.Map;
+import java.util.stream.Collectors;
 
 /**
  * @author Vaclav Muzikar <vmuzikar@redhat.com>
@@ -45,4 +47,12 @@ public static String asBase64(String toEncode) {
         return Base64.getEncoder().encodeToString(toEncode.getBytes(StandardCharsets.UTF_8));
     }
 
+    public static String toSelectorString(Map<String, String> labels) {
+        if (labels == null || labels.isEmpty()) {
+            return null;
+        }
+        return labels.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue())
+                .collect(Collectors.joining(","));
+    }
+
 }
@@ -33,6 +33,7 @@
 import org.keycloak.common.util.CollectionUtil;
 import org.keycloak.operator.Config;
 import org.keycloak.operator.Constants;
+import org.keycloak.operator.Utils;
 import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
 import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret;
@@ -42,7 +43,6 @@
 import java.util.Base64;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -71,15 +71,7 @@ public KeycloakDeployment(KubernetesClient client, Config config, Keycloak keycl
         this.operatorConfig = config;
         this.keycloakCR = keycloakCR;
         this.adminSecretName = adminSecretName;
-
-        if (existingDeployment != null) {
-            Log.info("Existing Deployment provided by controller");
-            this.existingDeployment = existingDeployment;
-        } else {
-            Log.info("Trying to fetch existing Deployment from the API");
-            this.existingDeployment = fetchExistingDeployment();
-        }
-
+        this.existingDeployment = existingDeployment;
         this.baseDeployment = createBaseDeployment();
         this.distConfigurator = configureDist();
         mergePodTemplate(this.baseDeployment.getSpec().getTemplate());
@@ -93,18 +85,18 @@ public Optional<HasMetadata> getReconciledResource() {
         else {
             Log.info("Existing Deployment found, handling migration");
 
+            if (!existingDeployment.isMarkedForDeletion() && !hasExpectedMatchLabels(existingDeployment)) {
+                client.resource(existingDeployment).lockResourceVersion().delete();
+                Log.info("Existing Deployment found with old label selector, it will be recreated");
+            }
+
             migrateDeployment(existingDeployment, baseDeployment);
         }
         return Optional.of(baseDeployment);
     }
 
-    private StatefulSet fetchExistingDeployment() {
-        return client
-                .apps()
-                .statefulSets()
-                .inNamespace(getNamespace())
-                .withName(getName())
-                .get();
+    private boolean hasExpectedMatchLabels(StatefulSet statefulSet) {
+        return Optional.ofNullable(statefulSet).map(s -> getInstanceLabels().equals(s.getSpec().getSelector().getMatchLabels())).orElse(true);
     }
 
     public void validatePodTemplate(KeycloakStatusAggregator status) {
@@ -361,10 +353,10 @@ private StatefulSet createBaseDeployment() {
 
         baseDeployment.getMetadata().setName(getName());
         baseDeployment.getMetadata().setNamespace(getNamespace());
-        baseDeployment.getSpec().getSelector().setMatchLabels(Constants.DEFAULT_LABELS);
+        baseDeployment.getSpec().getSelector().setMatchLabels(getInstanceLabels());
         baseDeployment.getSpec().setReplicas(keycloakCR.getSpec().getInstances());
 
-        Map<String, String> labels = new LinkedHashMap<>(Constants.DEFAULT_LABELS);
+        Map<String, String> labels = getInstanceLabels();
         if (operatorConfig.keycloak().podLabels() != null) {
             labels.putAll(operatorConfig.keycloak().podLabels());
         }
@@ -483,7 +475,7 @@ private List<EnvVar> getEnvVars() {
 
     @Override
     public void updateStatus(KeycloakStatusAggregator status) {
-        status.apply(b -> b.withSelector(Constants.DEFAULT_LABELS_AS_STRING));
+        status.apply(b -> b.withSelector(Utils.toSelectorString(getInstanceLabels())));
         validatePodTemplate(status);
         if (existingDeployment == null) {
             status.addNotReadyMessage("No existing StatefulSet found, waiting for creating a new one");

@@ -43,7 +43,7 @@ private ServiceSpec getServiceSpec() {
               .withProtocol("TCP")
               .withPort(Constants.KEYCLOAK_DISCOVERY_SERVICE_PORT)
               .endPort()
-              .withSelector(Constants.DEFAULT_LABELS)
+              .withSelector(getInstanceLabels())
               .withClusterIP("None")
               .build();
     }

@@ -49,7 +49,7 @@ private ServiceSpec getServiceSpec() {
               .withPort(getServicePort(keycloak))
               .withProtocol(Constants.KEYCLOAK_SERVICE_PROTOCOL)
               .endPort()
-              .withSelector(Constants.DEFAULT_LABELS)
+              .withSelector(getInstanceLabels())
               .build();
     }
 

@@ -31,7 +31,7 @@
 import org.keycloak.operator.Constants;
 
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Optional;
 
@@ -55,7 +55,7 @@ public OperatorManagedResource(KubernetesClient client, CustomResource<?, ?> cr)
     public void createOrUpdateReconciled() {
         getReconciledResource().ifPresent(resource -> {
             try {
-                setDefaultLabels(resource);
+                setInstanceLabels(resource);
                 setOwnerReferences(resource);
 
                 Log.debugf("Creating or updating resource: %s", resource);
@@ -88,10 +88,19 @@ public void createOrUpdateReconciled() {
         });
     }
 
-    protected void setDefaultLabels(HasMetadata resource) {
-        Map<String, String> labels = Optional.ofNullable(resource.getMetadata().getLabels()).orElse(new HashMap<>());
+    protected void setInstanceLabels(HasMetadata resource) {
+        resource.getMetadata().setLabels(updateWithInstanceLabels(resource.getMetadata().getLabels(), cr.getMetadata().getName()));
+    }
+
+    protected Map<String, String> getInstanceLabels() {
+        return updateWithInstanceLabels(null, cr.getMetadata().getName());
+    }
+
+    public static Map<String, String> updateWithInstanceLabels(Map<String, String> labels, String instanceName) {
+        labels = Optional.ofNullable(labels).orElse(new LinkedHashMap<>());
         labels.putAll(Constants.DEFAULT_LABELS);
-        resource.getMetadata().setLabels(labels);
+        labels.put(Constants.INSTANCE_LABEL, instanceName);
+        return labels;
     }
 
     protected void setOwnerReferences(HasMetadata resource) {

@@ -90,8 +90,8 @@ protected Optional<HasMetadata> getReconciledResource() {
     }
 
     @Override
-    protected void setDefaultLabels(HasMetadata resource) {
-        super.setDefaultLabels(resource);
+    protected void setInstanceLabels(HasMetadata resource) {
+        super.setInstanceLabels(resource);
         resource.getMetadata().getLabels().put(Constants.COMPONENT_LABEL, COMPONENT);
     }
 

@@ -24,6 +24,7 @@
 import io.fabric8.kubernetes.client.ConfigBuilder;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.KubernetesClientBuilder;
+import io.fabric8.kubernetes.client.NamespacedKubernetesClient;
 import io.javaoperatorsdk.operator.Operator;
 import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
@@ -166,6 +167,8 @@ private static void createOperator() {
   private static void createNamespace() {
     Log.info("Creating Namespace " + namespace);
     k8sclient.resource(new NamespaceBuilder().withNewMetadata().addToLabels("app","keycloak-test").withName(namespace).endMetadata().build()).create();
+    // ensure that the client defaults to the namespace - eventually most of the test code usage of inNamespace can be removed
+    k8sclient = k8sclient.adapt(NamespacedKubernetesClient.class).inNamespace(namespace);
   }
 
   private static void calculateNamespace() {

@@ -17,6 +17,12 @@
 
 package org.keycloak.operator.testsuite.integration;
 
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.Secret;
+import io.fabric8.kubernetes.api.model.Service;
+import io.fabric8.kubernetes.api.model.apps.StatefulSet;
+import io.fabric8.kubernetes.api.model.networking.v1.Ingress;
+import io.fabric8.kubernetes.client.readiness.Readiness;
 import io.fabric8.kubernetes.client.utils.Serialization;
 import io.quarkus.logging.Log;
 import io.quarkus.test.junit.QuarkusTest;
@@ -45,22 +51,74 @@
 @QuarkusTest
 public class ClusteringTest extends BaseOperatorTest {
 
+    @Test
+    public void testMultipleDeployments() throws InterruptedException {
+        // given
+        var kc = K8sUtils.getDefaultKeycloakDeployment();
+
+        // another instance running off the same database
+        // - should eventually give this a separate schema
+        var kc1 = K8sUtils.getDefaultKeycloakDeployment();
+        kc1.getMetadata().setName("another-example");
+        kc1.getSpec().getHostnameSpec().setHostname("another-example.com");
+        // this is using the wrong tls-secret, but simply removing http spec renders the pod unstartable
+
+        try {
+            K8sUtils.deployKeycloak(k8sclient, kc, true);
+            K8sUtils.deployKeycloak(k8sclient, kc1, true);
+        } catch (Exception e) {
+            k8sclient.resources(Keycloak.class).list().getItems().stream().forEach(k -> {
+                Log.infof("Keycloak %s status: %s", k.getMetadata().getName(), Serialization.asYaml(k.getStatus()));
+            });
+            k8sclient.pods().list().getItems().stream().filter(p -> !Readiness.isPodReady(p)).forEach(p -> {
+                Log.infof("Pod %s not ready: %s", p.getMetadata().getName(), Serialization.asYaml(p.getStatus()));
+            });
+            throw e;
+        }
+
+        assertThat(k8sclient.resources(Keycloak.class).list().getItems().size()).isEqualTo(2);
+
+        // get the current version for the uid
+        kc = k8sclient.resource(kc).get();
+        kc1 = k8sclient.resource(kc1).get();
+
+        // the main resources are ready, check for the expected dependents
+        checkInstanceCount(1, StatefulSet.class, kc, kc1);
+        checkInstanceCount(2, Secret.class, kc, kc1);
+        checkInstanceCount(1, Ingress.class, kc, kc1);
+        checkInstanceCount(2, Service.class, kc, kc1);
+
+        // ensure they don't see each other's pods
+        assertThat(k8sclient.resource(kc).scale().getStatus().getReplicas()).isEqualTo(1);
+        assertThat(k8sclient.resource(kc1).scale().getStatus().getReplicas()).isEqualTo(1);
+
+        // could also scale one instance to zero end ensure the services are no longer reachable
+    }
+
+    private void checkInstanceCount(int count, Class<? extends HasMetadata> type, HasMetadata... toCheck) {
+        var instances = k8sclient.resources(type).list().getItems();
+
+        for (HasMetadata hasMetadata : toCheck) {
+            assertThat(instances.stream()
+                    .filter(h -> h.getOwnerReferenceFor(hasMetadata).isPresent() && hasMetadata.getMetadata()
+                            .getName().equals(h.getMetadata().getLabels().get(Constants.INSTANCE_LABEL)))
+                    .count()).isEqualTo(count);
+        }
+    }
+
     @Test
     public void testKeycloakScaleAsExpected() {
         // given
         var kc = K8sUtils.getDefaultKeycloakDeployment();
-        var crSelector = k8sclient
-                .resources(Keycloak.class)
-                .inNamespace(kc.getMetadata().getNamespace())
-                .withName(kc.getMetadata().getName());
+        var crSelector = k8sclient.resource(kc);
         K8sUtils.deployKeycloak(k8sclient, kc, true);
 
         var kcPodsSelector = k8sclient.pods().inNamespace(namespace).withLabel("app", "keycloak");
 
         var scale = crSelector.scale();
         assertThat(scale.getSpec().getReplicas()).isEqualTo(1);
         assertThat(scale.getStatus().getReplicas()).isEqualTo(1);
-        assertThat(scale.getStatus().getSelector()).isEqualTo(Constants.DEFAULT_LABELS_AS_STRING);
+        assertThat(scale.getStatus().getSelector()).isEqualTo("app=keycloak,app.kubernetes.io/managed-by=keycloak-operator,app.kubernetes.io/instance=example-kc");
 
         // when scale it to 0
         Keycloak scaled = crSelector.scale(0);
@@ -139,16 +197,10 @@ public void testKeycloakCacheIsConnected() throws Exception {
         // given
         Log.info("Setup");
         var kc = K8sUtils.getDefaultKeycloakDeployment();
-        var crSelector = k8sclient
-                .resources(Keycloak.class)
-                .inNamespace(kc.getMetadata().getNamespace())
-                .withName(kc.getMetadata().getName());
+        var crSelector = k8sclient.resource(kc);
         K8sUtils.deployKeycloak(k8sclient, kc, false);
         var targetInstances = 3;
-        crSelector.accept(keycloak -> {
-        	keycloak.getMetadata().setResourceVersion(null);
-        	keycloak.getSpec().setInstances(targetInstances);
-        });
+        crSelector.scale(targetInstances);
         K8sUtils.set(k8sclient, getClass().getResourceAsStream("/token-test-realm.yaml"));
         var realmImportSelector = k8sclient.resources(KeycloakRealmImport.class).inNamespace(namespace).withName("example-token-test-kc");
 

@@ -23,9 +23,12 @@
 import io.fabric8.kubernetes.api.model.Secret;
 import io.fabric8.kubernetes.api.model.SecretBuilder;
 import io.fabric8.kubernetes.api.model.SecretKeySelectorBuilder;
+import io.fabric8.kubernetes.api.model.apps.StatefulSet;
+import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder;
 import io.fabric8.kubernetes.api.model.apps.StatefulSetSpecBuilder;
 import io.quarkus.logging.Log;
 import io.quarkus.test.junit.QuarkusTest;
+
 import org.awaitility.Awaitility;
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.Test;
@@ -204,7 +207,21 @@ public void testDeploymentDurability() {
         try {
             var kc = getDefaultKeycloakDeployment();
             var deploymentName = kc.getMetadata().getName();
-            deployKeycloak(k8sclient, kc, true);
+
+            // create a dummy StatefulSet representing the pre-multiinstance state that we'll be forced to delete
+            StatefulSet statefulSet = new StatefulSetBuilder().withMetadata(kc.getMetadata()).editMetadata()
+                    .addToLabels(Constants.DEFAULT_LABELS).endMetadata().withNewSpec().withNewSelector()
+                    .withMatchLabels(Constants.DEFAULT_LABELS).endSelector().withServiceName("foo").withReplicas(0)
+                    .withNewTemplate().withNewMetadata().withLabels(Constants.DEFAULT_LABELS).endMetadata()
+                    .withNewSpec().addNewContainer().withName("pause").withImage("registry.k8s.io/pause:3.1")
+                    .endContainer().endSpec().endTemplate().endSpec().build();
+            k8sclient.resource(statefulSet).create();
+
+            // start will not be successful because the statefulSet is in the way
+            deployKeycloak(k8sclient, kc, false);
+            // once the statefulset is owned by the keycloak it will be picked up by the informer
+            k8sclient.resource(statefulSet).accept(s -> s.addOwnerReference(k8sclient.resource(kc).get()));
+            waitForKeycloakToBeReady(k8sclient, kc);
 
             Log.info("Trying to delete deployment");
             assertThat(k8sclient.apps().statefulSets().withName(deploymentName).delete()).isNotNull();