Use port name instead of port number for the ingress (#30925)
Also adding a retry if server-side-apply fails Closes #30924 Signed-off-by: Alexander Schwartz <aschwart@redhat.com> Co-authored-by: Steven Hawkins <shawkins@redhat.com>
This commit is contained in:
parent
a0c99a7ae0
commit
bebb314a16
2 changed files with 38 additions and 4 deletions
|
@ -18,6 +18,7 @@ package org.keycloak.operator.controllers;
|
|||
|
||||
import io.fabric8.kubernetes.api.model.networking.v1.Ingress;
|
||||
import io.fabric8.kubernetes.api.model.networking.v1.IngressBuilder;
|
||||
import io.fabric8.kubernetes.client.KubernetesClientException;
|
||||
import io.javaoperatorsdk.operator.api.reconciler.Context;
|
||||
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
|
||||
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
|
||||
|
@ -25,6 +26,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDep
|
|||
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
|
||||
|
||||
import io.quarkus.logging.Log;
|
||||
import org.jboss.logging.Logger;
|
||||
import org.keycloak.operator.Constants;
|
||||
import org.keycloak.operator.Utils;
|
||||
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
|
||||
|
@ -40,6 +42,8 @@ import static org.keycloak.operator.crds.v2alpha1.CRDUtils.isTlsConfigured;
|
|||
@KubernetesDependent(labelSelector = Constants.DEFAULT_LABELS_AS_STRING)
|
||||
public class KeycloakIngressDependentResource extends CRUDKubernetesDependentResource<Ingress, Keycloak> {
|
||||
|
||||
private static final Logger LOG = Logger.getLogger(KeycloakIngressDependentResource.class.getName());
|
||||
|
||||
public static class EnabledCondition implements Condition<Ingress, Keycloak> {
|
||||
@Override
|
||||
public boolean isMet(DependentResource<Ingress, Keycloak> dependentResource, Keycloak primary,
|
||||
|
@ -48,6 +52,8 @@ public class KeycloakIngressDependentResource extends CRUDKubernetesDependentRes
|
|||
}
|
||||
}
|
||||
|
||||
private static final ThreadLocal<Boolean> USE_SSA = new ThreadLocal<>();
|
||||
|
||||
public KeycloakIngressDependentResource() {
|
||||
super(Ingress.class);
|
||||
}
|
||||
|
@ -56,11 +62,39 @@ public class KeycloakIngressDependentResource extends CRUDKubernetesDependentRes
|
|||
return Optional.ofNullable(keycloak.getSpec().getIngressSpec()).map(IngressSpec::isIngressEnabled).orElse(true);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Ingress update(Ingress actual, Ingress desired, Keycloak primary, Context<Keycloak> context) {
|
||||
try {
|
||||
return super.update(actual, desired, primary, context);
|
||||
} catch (KubernetesClientException e) {
|
||||
if (e.getCode() == 422) {
|
||||
// This attempts to recover from errors like "Failure executing: PATCH" / "Invalid value: cannot set both port name & port number."
|
||||
// when the server side apply fails.
|
||||
try {
|
||||
LOG.info("Failed to update Ingress with server-side apply, retrying with client-side", e);
|
||||
USE_SSA.set(false);
|
||||
return super.update(actual, desired, primary, context);
|
||||
} finally {
|
||||
USE_SSA.remove();
|
||||
}
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected boolean useSSA(Context<Keycloak> context) {
|
||||
if (Boolean.FALSE.equals(USE_SSA.get())) {
|
||||
return false;
|
||||
}
|
||||
return super.useSSA(context);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Ingress desired(Keycloak keycloak, Context<Keycloak> context) {
|
||||
var annotations = new HashMap<String, String>();
|
||||
boolean tlsConfigured = isTlsConfigured(keycloak);
|
||||
var port = KeycloakServiceDependentResource.getServicePort(tlsConfigured, keycloak);
|
||||
var portName = tlsConfigured ? Constants.KEYCLOAK_HTTPS_PORT_NAME : Constants.KEYCLOAK_HTTP_PORT_NAME;
|
||||
|
||||
if (tlsConfigured) {
|
||||
annotations.put("nginx.ingress.kubernetes.io/backend-protocol", "HTTPS");
|
||||
|
@ -86,7 +120,7 @@ public class KeycloakIngressDependentResource extends CRUDKubernetesDependentRes
|
|||
.withNewService()
|
||||
.withName(KeycloakServiceDependentResource.getServiceName(keycloak))
|
||||
.withNewPort()
|
||||
.withNumber(port)
|
||||
.withName(portName)
|
||||
.endPort()
|
||||
.endService()
|
||||
.endDefaultBackend()
|
||||
|
@ -98,7 +132,7 @@ public class KeycloakIngressDependentResource extends CRUDKubernetesDependentRes
|
|||
.withNewService()
|
||||
.withName(KeycloakServiceDependentResource.getServiceName(keycloak))
|
||||
.withNewPort()
|
||||
.withNumber(port)
|
||||
.withName(portName)
|
||||
.endPort()
|
||||
.endService()
|
||||
.endBackend()
|
||||
|
|
|
@ -238,7 +238,7 @@ public class KeycloakIngressTest extends BaseOperatorTest {
|
|||
assertEquals("HTTPS", i.getMetadata().getAnnotations().get("nginx.ingress.kubernetes.io/backend-protocol"));
|
||||
assertEquals("passthrough", i.getMetadata().getAnnotations().get("route.openshift.io/termination"));
|
||||
assertEquals("true", i.getMetadata().getAnnotations().get("haproxy.router.openshift.io/disable_cookies"));
|
||||
assertEquals(Constants.KEYCLOAK_HTTPS_PORT, i.getSpec().getDefaultBackend().getService().getPort().getNumber());
|
||||
assertEquals(Constants.KEYCLOAK_HTTPS_PORT_NAME, i.getSpec().getDefaultBackend().getService().getPort().getName());
|
||||
});
|
||||
|
||||
// Delete the ingress
|
||||
|
|
Loading…
Reference in a new issue