From 31062d126932def3da28a7b66616393a2e818c42 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Mon, 1 Jul 2024 14:25:45 -0400 Subject: [PATCH] fix: adds affinity and other scheduling to the operator closes: #29258 Signed-off-by: Steve Hawkins --- .../release_notes/topics/26_0_0.adoc | 7 + .../topics/changes/changes-26_0_0.adoc | 4 + .../operator/advanced-configuration.adoc | 42 ++++++ .../KeycloakDeploymentDependentResource.java | 43 ++++++ .../v2alpha1/deployment/KeycloakSpec.java | 13 ++ .../deployment/spec/SchedulingSpec.java | 56 ++++++++ .../testsuite/unit/PodTemplateTest.java | 125 ++++++++++++++++++ 7 files changed, 290 insertions(+) create mode 100644 operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/spec/SchedulingSpec.java diff --git a/docs/documentation/release_notes/topics/26_0_0.adoc b/docs/documentation/release_notes/topics/26_0_0.adoc index 82b6009f2d25..74394cf29396 100644 --- a/docs/documentation/release_notes/topics/26_0_0.adoc +++ b/docs/documentation/release_notes/topics/26_0_0.adoc @@ -22,3 +22,10 @@ WARNING: JBoss Marshalling and Infinispan Protostream are not compatible with ea Consequently, all caches are cleared when upgrading to this version. Infinispan Protostream is based on https://protobuf.dev/programming-guides/proto3/[Protocol Buffers] (proto 3), which has the advantage of backwards/forwards compatibility. + += Keycloak CR supports standard scheduling options + +The Keycloak CR now exposes first class properties for controlling the scheduling of your Keycloak Pods. + +For more details, see the +https://www.keycloak.org/operator/advanced-configuration[Operator Advanced Configuration]. diff --git a/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc b/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc index 69603ac12dc0..fae9ada67fcd 100644 --- a/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc +++ b/docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc @@ -21,6 +21,10 @@ This is an optimization to reduce traffic and network related resources. In {project_name} 26, the new method has a default implementation to keep backward compatibility with custom implementation. The default implementation performs a single network call per an event, and it will be removed in a future version of {project_name}. += Operator scheduling defaults + +Keycloak Pods will now have default affinities to prevent multiple instances from the same CR from being deployed on the same node, and all Pods from the same CR will prefer to be in the same zone to prevent stretch cache clusters. + = Operator's default CPU and memory limits/requests In order to follow the best practices, the default CPU and memory limits/requests for the Operator were introduced. It affects both non-OLM and OLM installs. To override the default values for the OLM install, edit the `resources` section in the operator's https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/subscription-config.md#resources[subscription]. diff --git a/docs/guides/operator/advanced-configuration.adoc b/docs/guides/operator/advanced-configuration.adoc index aa6769eb0e9b..5376ab4f316c 100644 --- a/docs/guides/operator/advanced-configuration.adoc +++ b/docs/guides/operator/advanced-configuration.adoc @@ -204,6 +204,48 @@ It is achieved by providing certain JVM options. For more details, see <@links.server id="containers" />. +=== Scheduling + +You may control several aspects of the server Pod scheduling via the Keycloak CR. The scheduling stanza exposes optional standard Kubernetes affinity, tolerations, topology spread constraints, and the priority class name to fine tune the scheduling and placement of your server Pods. + +An example utilizing all scheduling fields: + +[source,yaml] +---- +apiVersion: k8s.keycloak.org/v2alpha1 +kind: Keycloak +metadata: + name: example-kc +spec: + scheduling: + priorityClassName: custom-high + affinity: + podAffinity: + preferredDuringSchedulingIgnoredDuringExecution: + - podAffinityTerm: + labelSelector: + matchLabels: + app: keycloak + app.kubernetes.io/managed-by: keycloak-operator + app.kubernetes.io/component: server + topologyKey: topology.kubernetes.io/zone + weight: 10 + tolerations: + - key: "some-taint" + operator: "Exists" + effect: "NoSchedule" + topologySpreadConstraints: + - maxSkew: 1 + topologyKey: kubernetes.io/hostname + whenUnsatisfiable: DoNotSchedule + ... + ... +---- + +Please see https://kubernetes.io/docs/concepts/scheduling-eviction[the kubernetes docs] for more on scheduling concepts. + +If you do not specify a custom affinity, your Pods will have an affinity for the same zone and an anti-affinity for the same node to improve availability. Scheduling to the same zone if possible helps prevent stretch clusters where cross zone cache cluster traffic may have too high of a latency. + === Management Interface To change the port of the management interface, use the first-class citizen field `httpManagement.port` in the Keycloak CR. 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 4b1c83790aab..ac65188d4b75 100644 --- a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java +++ b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeploymentDependentResource.java @@ -23,6 +23,7 @@ import io.fabric8.kubernetes.api.model.EnvVarSource; import io.fabric8.kubernetes.api.model.EnvVarSourceBuilder; import io.fabric8.kubernetes.api.model.PodSpec; +import io.fabric8.kubernetes.api.model.PodSpecFluent; import io.fabric8.kubernetes.api.model.PodTemplateSpec; import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.kubernetes.api.model.SecretKeySelector; @@ -45,6 +46,7 @@ import org.keycloak.operator.crds.v2alpha1.deployment.ValueOrSecret; import org.keycloak.operator.crds.v2alpha1.deployment.spec.CacheSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpManagementSpec; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.SchedulingSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.Truststore; import org.keycloak.operator.crds.v2alpha1.deployment.spec.TruststoreSource; import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpec; @@ -77,6 +79,8 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent private static final List COPY_ENV = Arrays.asList("HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"); + private static final String ZONE_KEY = "topology.kubernetes.io/zone"; + private static final String SERVICE_ACCOUNT_DIR = "/var/run/secrets/kubernetes.io/serviceaccount/"; private static final String SERVICE_CA_CRT = SERVICE_ACCOUNT_DIR + "service-ca.crt"; @@ -227,6 +231,8 @@ static Optional getPodTemplateSpec(Keycloak keycloakCR) { private StatefulSet createBaseDeployment(Keycloak keycloakCR, Context context) { Map labels = Utils.allInstanceLabels(keycloakCR); + labels.put("app.kubernetes.io/component", "server"); + Map schedulingLabels = new LinkedHashMap<>(labels); if (operatorConfig.keycloak().podLabels() != null) { labels.putAll(operatorConfig.keycloak().podLabels()); } @@ -264,6 +270,7 @@ private StatefulSet createBaseDeployment(Keycloak keycloakCR, Context if (!specBuilder.hasDnsPolicy()) { specBuilder.withDnsPolicy("ClusterFirst"); } + handleScheduling(keycloakCR, schedulingLabels, specBuilder); // there isn't currently an editOrNewFirstContainer, so we need to do this manually var containerBuilder = specBuilder.buildContainers().isEmpty() ? specBuilder.addNewContainer() : specBuilder.editFirstContainer(); @@ -353,6 +360,42 @@ private StatefulSet createBaseDeployment(Keycloak keycloakCR, Context return baseDeployment; } + private void handleScheduling(Keycloak keycloakCR, Map labels, PodSpecFluent specBuilder) { + SchedulingSpec schedulingSpec = keycloakCR.getSpec().getSchedulingSpec(); + if (schedulingSpec != null) { + if (!specBuilder.hasPriorityClassName()) { + specBuilder.withPriorityClassName(schedulingSpec.getPriorityClassName()); + } + if (!specBuilder.hasAffinity()) { + specBuilder.withAffinity(schedulingSpec.getAffinity()); + } + if (!specBuilder.hasTolerations()) { + specBuilder.withTolerations(schedulingSpec.getTolerations()); + } + if (!specBuilder.hasTopologySpreadConstraints()) { + specBuilder.withTopologySpreadConstraints(schedulingSpec.getTopologySpreadConstraints()); + } + } + + // set defaults if nothing was specified by the user + // - server pods will have an affinity for the same zone as to avoid stretch clusters + // - server pods will have a stronger anti-affinity for the same node + + if (!specBuilder.hasAffinity()) { + specBuilder.editOrNewAffinity().withNewPodAffinity().addNewPreferredDuringSchedulingIgnoredDuringExecution() + .withWeight(10).withNewPodAffinityTerm().withNewLabelSelector().withMatchLabels(labels) + .endLabelSelector().withTopologyKey(ZONE_KEY).endPodAffinityTerm() + .endPreferredDuringSchedulingIgnoredDuringExecution().endPodAffinity().endAffinity(); + + specBuilder.editOrNewAffinity().withNewPodAntiAffinity() + .addNewPreferredDuringSchedulingIgnoredDuringExecution().withWeight(50).withNewPodAffinityTerm() + .withNewLabelSelector().withMatchLabels(labels).endLabelSelector() + .withTopologyKey("kubernetes.io/hostname").endPodAffinityTerm() + .endPreferredDuringSchedulingIgnoredDuringExecution().endPodAntiAffinity().endAffinity(); + } + + } + private static String getJGroupsParameter(Keycloak keycloakCR) { return JGROUPS_DNS_QUERY_PARAM + KeycloakDiscoveryServiceDependentResource.getName(keycloakCR) +"." + keycloakCR.getMetadata().getNamespace(); } diff --git a/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakSpec.java b/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakSpec.java index 5b4711d15f63..79e2b73fd132 100644 --- a/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakSpec.java +++ b/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakSpec.java @@ -28,6 +28,7 @@ import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.ProxySpec; +import org.keycloak.operator.crds.v2alpha1.deployment.spec.SchedulingSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.Truststore; import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpec; @@ -110,6 +111,10 @@ public class KeycloakSpec { @JsonPropertyDescription("In this section you can configure Keycloak's management interface setting.") private HttpManagementSpec httpManagementSpec; + @JsonProperty("scheduling") + @JsonPropertyDescription("In this section you can configure Keycloak's scheduling") + private SchedulingSpec schedulingSpec; + public HttpSpec getHttpSpec() { return httpSpec; } @@ -251,4 +256,12 @@ public ProxySpec getProxySpec() { public void setProxySpec(ProxySpec proxySpec) { this.proxySpec = proxySpec; } + + public SchedulingSpec getSchedulingSpec() { + return schedulingSpec; + } + + public void setSchedulingSpec(SchedulingSpec schedulingSpec) { + this.schedulingSpec = schedulingSpec; + } } \ No newline at end of file diff --git a/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/spec/SchedulingSpec.java b/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/spec/SchedulingSpec.java new file mode 100644 index 000000000000..83e261cebd8c --- /dev/null +++ b/operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/spec/SchedulingSpec.java @@ -0,0 +1,56 @@ +package org.keycloak.operator.crds.v2alpha1.deployment.spec; + +import io.fabric8.kubernetes.api.model.Affinity; +import io.fabric8.kubernetes.api.model.Toleration; +import io.fabric8.kubernetes.api.model.TopologySpreadConstraint; +import io.sundr.builder.annotations.Buildable; + +import java.util.ArrayList; +import java.util.List; + +import com.fasterxml.jackson.annotation.JsonInclude; + +@JsonInclude(JsonInclude.Include.NON_NULL) +@Buildable(editableEnabled = false, builderPackage = "io.fabric8.kubernetes.api.builder") +public class SchedulingSpec { + + private Affinity affinity; + @JsonInclude(JsonInclude.Include.NON_EMPTY) + private List tolerations = new ArrayList(); + @JsonInclude(JsonInclude.Include.NON_EMPTY) + private List topologySpreadConstraints = new ArrayList(); + private String priorityClassName; + + public Affinity getAffinity() { + return affinity; + } + + public void setAffinity(Affinity affinity) { + this.affinity = affinity; + } + + public List getTolerations() { + return tolerations; + } + + public void setTolerations(List tolerations) { + this.tolerations = tolerations; + } + + public List getTopologySpreadConstraints() { + return topologySpreadConstraints; + } + + public void setTopologySpreadConstraints(List topologySpreadConstraints) { + this.topologySpreadConstraints = topologySpreadConstraints; + } + + public String getPriorityClassName() { + return priorityClassName; + } + + public void setPriorityClassName(String priorityClassName) { + this.priorityClassName = priorityClassName; + } + +} 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 2e2141d3b1bb..98e3794af3af 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 @@ -17,16 +17,22 @@ package org.keycloak.operator.testsuite.unit; +import io.fabric8.kubernetes.api.model.Affinity; +import io.fabric8.kubernetes.api.model.AffinityBuilder; import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.EnvVar; import io.fabric8.kubernetes.api.model.IntOrString; 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.Toleration; +import io.fabric8.kubernetes.api.model.TopologySpreadConstraint; +import io.fabric8.kubernetes.api.model.TopologySpreadConstraintBuilder; import io.fabric8.kubernetes.api.model.Volume; import io.fabric8.kubernetes.api.model.VolumeMount; import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder; +import io.fabric8.kubernetes.client.utils.Serialization; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.quarkus.test.InjectMock; import io.quarkus.test.junit.QuarkusTest; @@ -46,6 +52,7 @@ import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpec; import org.mockito.Mockito; +import java.util.Arrays; import java.util.Optional; import java.util.function.Consumer; import java.util.function.Function; @@ -398,6 +405,32 @@ public void testDefaults() { assertNotNull(startup); assertThat(startup.getPath()).isEqualTo("/health/started"); assertThat(startup.getPort().getIntVal()).isEqualTo(Constants.KEYCLOAK_MANAGEMENT_PORT); + + var affinity = podTemplate.getSpec().getAffinity(); + assertNotNull(affinity); + assertThat(Serialization.asYaml(affinity)).isEqualTo("---\n" + + "podAffinity:\n" + + " preferredDuringSchedulingIgnoredDuringExecution:\n" + + " - podAffinityTerm:\n" + + " labelSelector:\n" + + " matchLabels:\n" + + " app: \"keycloak\"\n" + + " app.kubernetes.io/managed-by: \"keycloak-operator\"\n" + + " app.kubernetes.io/instance: \"instance\"\n" + + " app.kubernetes.io/component: \"server\"\n" + + " topologyKey: \"topology.kubernetes.io/zone\"\n" + + " weight: 10\n" + + "podAntiAffinity:\n" + + " preferredDuringSchedulingIgnoredDuringExecution:\n" + + " - podAffinityTerm:\n" + + " labelSelector:\n" + + " matchLabels:\n" + + " app: \"keycloak\"\n" + + " app.kubernetes.io/managed-by: \"keycloak-operator\"\n" + + " app.kubernetes.io/instance: \"instance\"\n" + + " app.kubernetes.io/component: \"server\"\n" + + " topologyKey: \"kubernetes.io/hostname\"\n" + + " weight: 50\n"); } @Test @@ -484,4 +517,96 @@ public void testServiceCaCrt() { } } + @Test + public void testPriorityClass() { + // Arrange + PodTemplateSpec additionalPodTemplate = null; + + // Act + var podTemplate = getDeployment(additionalPodTemplate, null, + s -> s.withNewSchedulingSpec().withPriorityClassName("important").endSchedulingSpec()) + .getSpec().getTemplate(); + + // Assert + assertThat(podTemplate.getSpec().getPriorityClassName()).isEqualTo("important"); + + podTemplate = getDeployment(new PodTemplateSpecBuilder().withNewSpec().withPriorityClassName("existing").endSpec().build(), null, + s -> s.withNewSchedulingSpec().withPriorityClassName("important").endSchedulingSpec()) + .getSpec().getTemplate(); + + assertThat(podTemplate.getSpec().getPriorityClassName()).isEqualTo("existing"); + } + + @Test + public void testTolerations() { + // Arrange + PodTemplateSpec additionalPodTemplate = null; + + Toleration toleration = new Toleration("NoSchedule", "key", "=", null, "value"); + + // Act + var podTemplate = getDeployment(additionalPodTemplate, null, + s -> s.withNewSchedulingSpec().addToTolerations(toleration).endSchedulingSpec()) + .getSpec().getTemplate(); + + // Assert + assertThat(podTemplate.getSpec().getTolerations()).isEqualTo(Arrays.asList(toleration)); + + podTemplate = getDeployment(new PodTemplateSpecBuilder().withNewSpec().withTolerations(new Toleration()).endSpec().build(), null, + s -> s.withNewSchedulingSpec().addToTolerations(toleration).endSchedulingSpec()) + .getSpec().getTemplate(); + + // Assert + assertThat(podTemplate.getSpec().getTolerations()).isNotEqualTo(Arrays.asList(toleration)); + + } + + @Test + public void testTopologySpreadConstraints() { + // Arrange + PodTemplateSpec additionalPodTemplate = null; + + TopologySpreadConstraint tsc = new TopologySpreadConstraintBuilder().withTopologyKey("key").build(); + + // Act + var podTemplate = getDeployment(additionalPodTemplate, null, + s -> s.withNewSchedulingSpec().addToTopologySpreadConstraints(tsc).endSchedulingSpec()) + .getSpec().getTemplate(); + + // Assert + assertThat(podTemplate.getSpec().getTopologySpreadConstraints()).isEqualTo(Arrays.asList(tsc)); + + podTemplate = getDeployment(new PodTemplateSpecBuilder().withNewSpec().withTopologySpreadConstraints(new TopologySpreadConstraint()).endSpec().build(), null, + s -> s.withNewSchedulingSpec().addToTopologySpreadConstraints(tsc).endSchedulingSpec()) + .getSpec().getTemplate(); + + // Assert + assertThat(podTemplate.getSpec().getTopologySpreadConstraints()).isNotEqualTo(Arrays.asList(tsc)); + } + + @Test + public void testAffinity() { + // Arrange + PodTemplateSpec additionalPodTemplate = null; + + var affinity = new AffinityBuilder().withNewPodAffinity() + .addNewPreferredDuringSchedulingIgnoredDuringExecution().withNewPodAffinityTerm().withNamespaces("x") + .endPodAffinityTerm().endPreferredDuringSchedulingIgnoredDuringExecution().endPodAffinity().build(); + + // Act + var podTemplate = getDeployment(additionalPodTemplate, null, + s -> s.withNewSchedulingSpec().withAffinity(affinity).endSchedulingSpec()) + .getSpec().getTemplate(); + + // Assert + assertThat(podTemplate.getSpec().getAffinity()).isEqualTo(affinity); + + podTemplate = getDeployment(new PodTemplateSpecBuilder().withNewSpec().withAffinity(new Affinity()).endSpec().build(), null, + s -> s.withNewSchedulingSpec().withAffinity(affinity).endSchedulingSpec()) + .getSpec().getTemplate(); + + // Assert + assertThat(podTemplate.getSpec().getAffinity()).isNotEqualTo(affinity); + } + }