adds an instance label to support multiple instances (#20906)

Closes #10562 #14220
This commit is contained in:
Steven Hawkins 2023-06-28 12:05:23 -04:00 committed by GitHub
parent e80107757f
commit e9c9f80e8d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 141 additions and 55 deletions

View file

@ -350,3 +350,7 @@ their corresponding replacements.
* `KeycloakModelUtils#getClientScopeMappings` was removed.
* Deprecated methods from `KeycloakSession` 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.

View file

@ -22,7 +22,6 @@ import java.util.Collections;
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";
@ -30,6 +29,7 @@ public final class Constants {
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"),

View file

@ -24,6 +24,8 @@ import java.time.ZoneOffset;
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 final class Utils {
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(","));
}
}

View file

@ -33,6 +33,7 @@ import io.quarkus.logging.Log;
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.ArrayList;
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 class KeycloakDeployment extends OperatorManagedResource implements Statu
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 class KeycloakDeployment extends OperatorManagedResource implements Statu
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 @@ public class KeycloakDeployment extends OperatorManagedResource implements Statu
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 @@ public class KeycloakDeployment extends OperatorManagedResource implements Statu
@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");

View file

@ -43,7 +43,7 @@ public class KeycloakDiscoveryService extends OperatorManagedResource implements
.withProtocol("TCP")
.withPort(Constants.KEYCLOAK_DISCOVERY_SERVICE_PORT)
.endPort()
.withSelector(Constants.DEFAULT_LABELS)
.withSelector(getInstanceLabels())
.withClusterIP("None")
.build();
}

View file

@ -49,7 +49,7 @@ public class KeycloakService extends OperatorManagedResource implements StatusUp
.withPort(getServicePort(keycloak))
.withProtocol(Constants.KEYCLOAK_SERVICE_PROTOCOL)
.endPort()
.withSelector(Constants.DEFAULT_LABELS)
.withSelector(getInstanceLabels())
.build();
}

View file

@ -31,7 +31,7 @@ import io.quarkus.logging.Log;
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 abstract class OperatorManagedResource {
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 abstract class OperatorManagedResource {
});
}
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) {

View file

@ -90,8 +90,8 @@ public class WatchedSecretsStore extends OperatorManagedResource {
}
@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);
}

View file

@ -24,6 +24,7 @@ import io.fabric8.kubernetes.client.Config;
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 @@ public abstract class BaseOperatorTest {
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() {

View file

@ -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,14 +51,66 @@ import static org.assertj.core.api.Assertions.assertThat;
@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");
@ -60,7 +118,7 @@ public class ClusteringTest extends BaseOperatorTest {
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 class ClusteringTest extends BaseOperatorTest {
// 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");

View file

@ -23,9 +23,12 @@ import io.fabric8.kubernetes.api.model.LocalObjectReferenceBuilder;
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 class KeycloakDeploymentTest extends BaseOperatorTest {
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();

View file

@ -28,7 +28,8 @@ import io.quarkus.test.junit.QuarkusTest;
import org.junit.jupiter.api.Test;
import org.keycloak.operator.Config;
import org.keycloak.operator.controllers.KeycloakDeployment;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import org.keycloak.operator.controllers.OperatorManagedResource;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakBuilder;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpecBuilder;
import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder;
@ -67,7 +68,10 @@ public class PodTemplateTest {
};
}
};
var kc = new Keycloak();
var kc = new KeycloakBuilder().withNewMetadata().withName("instance").endMetadata().build();
existingDeployment = new StatefulSetBuilder(existingDeployment).editOrNewSpec().editOrNewSelector()
.addToMatchLabels(OperatorManagedResource.updateWithInstanceLabels(null, kc.getMetadata().getName()))
.endSelector().endSpec().build();
var httpSpec = new HttpSpecBuilder().withTlsSecret("example-tls-secret").build();
var hostnameSpec = new HostnameSpecBuilder().withHostname("example.com").build();
@ -84,6 +88,7 @@ public class PodTemplateTest {
kc.setSpec(keycloakSpecBuilder.build());
var deployment = new KeycloakDeployment(null, config, kc, existingDeployment, "dummy-admin");
return (StatefulSet) deployment.getReconciledResource().get();
}

View file

@ -101,11 +101,7 @@ public final class K8sUtils {
.timeout(5, TimeUnit.MINUTES)
.ignoreExceptions()
.untilAsserted(() -> {
var currentKc = client
.resources(Keycloak.class)
.inNamespace(kc.getMetadata().getNamespace())
.withName(kc.getMetadata().getName())
.get();
var currentKc = client.resource(kc).get();
CRAssert.assertKeycloakStatusCondition(currentKc, KeycloakStatusCondition.READY, true);
CRAssert.assertKeycloakStatusCondition(currentKc, KeycloakStatusCondition.HAS_ERRORS, false);