simplifying status updates to a single method for each controller (#22081)

also removing the getValueFromSubSpec method

Closes #22182

operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java

@@ -39,7 +39,6 @@ import org.keycloak.operator.Constants;
 import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatus;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
-import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition;
 
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -122,14 +121,11 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit
         kcDeployment.updateStatus(statusAggregator);
 
         var kcService = new KeycloakService(client, kc);
-        kcService.updateStatus(statusAggregator);
         kcService.createOrUpdateReconciled();
         var kcDiscoveryService = new KeycloakDiscoveryService(client, kc);
-        kcDiscoveryService.updateStatus(statusAggregator);
         kcDiscoveryService.createOrUpdateReconciled();
 
         var kcIngress = new KeycloakIngress(client, kc);
-        kcIngress.updateStatus(statusAggregator);
         kcIngress.createOrUpdateReconciled();
 
         var status = statusAggregator.build();
@@ -145,8 +141,7 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit
             updateControl = UpdateControl.updateStatus(kc);
         }
 
-        if (status.findCondition(KeycloakStatusCondition.READY)
+        if (!status.isReady()) {
-                .filter(c -> !Boolean.TRUE.equals(c.getStatus())).isPresent()) {
             updateControl.rescheduleAfter(10, TimeUnit.SECONDS);
         }
 

operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeployment.java

@@ -60,7 +60,7 @@ import java.util.stream.Stream;
 
 import static org.keycloak.operator.crds.v2alpha1.CRDUtils.isTlsConfigured;
 
-public class KeycloakDeployment extends OperatorManagedResource<StatefulSet> implements StatusUpdater<KeycloakStatusAggregator> {
+public class KeycloakDeployment extends OperatorManagedResource<StatefulSet> {
 
     private final Config operatorConfig;
     private final KeycloakDistConfigurator distConfigurator;
@@ -364,7 +364,6 @@ public class KeycloakDeployment extends OperatorManagedResource<StatefulSet> imp
         return envVars;
     }
 
-    @Override
     public void updateStatus(KeycloakStatusAggregator status) {
         status.apply(b -> b.withSelector(Utils.toSelectorString(getInstanceLabels())));
         validatePodTemplate(status);

operator/src/main/java/org/keycloak/operator/controllers/KeycloakDiscoveryService.java

@@ -22,19 +22,16 @@ import io.fabric8.kubernetes.api.model.ServiceBuilder;
 import io.fabric8.kubernetes.api.model.ServiceSpec;
 import io.fabric8.kubernetes.api.model.ServiceSpecBuilder;
 import io.fabric8.kubernetes.client.KubernetesClient;
+
 import org.keycloak.operator.Constants;
 import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
-import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
 
 import java.util.Optional;
 
-public class KeycloakDiscoveryService extends OperatorManagedResource implements StatusUpdater<KeycloakStatusAggregator> {
+public class KeycloakDiscoveryService extends OperatorManagedResource {
-
-    private Service existingService;
 
     public KeycloakDiscoveryService(KubernetesClient client, Keycloak keycloakCR) {
         super(client, keycloakCR);
-        this.existingService = fetchExistingService();
     }
 
     private ServiceSpec getServiceSpec() {
@@ -65,22 +62,6 @@ public class KeycloakDiscoveryService extends OperatorManagedResource implements
         return service;
     }
 
-    private Service fetchExistingService() {
-        return client
-                .services()
-                .inNamespace(getNamespace())
-                .withName(getName())
-                .get();
-    }
-
-    @Override
-    public void updateStatus(KeycloakStatusAggregator status) {
-        if (existingService == null) {
-            status.addNotReadyMessage("No existing Discovery Service found, waiting for creating a new one");
-            return;
-        }
-    }
-
     @Override
     public String getName() {
         return cr.getMetadata().getName() + Constants.KEYCLOAK_DISCOVERY_SERVICE_SUFFIX;

operator/src/main/java/org/keycloak/operator/controllers/KeycloakIngress.java

@@ -23,7 +23,6 @@ import io.fabric8.kubernetes.client.KubernetesClient;
 
 import org.keycloak.operator.Constants;
 import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
-import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpec;
 
 import java.util.HashMap;
@@ -31,7 +30,7 @@ import java.util.Optional;
 
 import static org.keycloak.operator.crds.v2alpha1.CRDUtils.isTlsConfigured;
 
-public class KeycloakIngress extends OperatorManagedResource implements StatusUpdater<KeycloakStatusAggregator> {
+public class KeycloakIngress extends OperatorManagedResource {
 
     private final Ingress existingIngress;
     private final Keycloak keycloak;
@@ -130,18 +129,6 @@ public class KeycloakIngress extends OperatorManagedResource implements StatusUp
                 .get();
     }
 
-    @Override
-    public void updateStatus(KeycloakStatusAggregator status) {
-        IngressSpec ingressSpec = keycloak.getSpec().getIngressSpec();
-        if (ingressSpec == null) {
-            ingressSpec = new IngressSpec();
-            ingressSpec.setIngressEnabled(true);
-        }
-        if (ingressSpec.isIngressEnabled() && existingIngress == null) {
-            status.addNotReadyMessage("No existing Keycloak Ingress found, waiting for creating a new one");
-        }
-    }
-
     @Override
     public String getName() {
         return cr.getMetadata().getName() + Constants.KEYCLOAK_INGRESS_SUFFIX;

operator/src/main/java/org/keycloak/operator/controllers/KeycloakService.java

@@ -22,25 +22,22 @@ import io.fabric8.kubernetes.api.model.ServiceBuilder;
 import io.fabric8.kubernetes.api.model.ServiceSpec;
 import io.fabric8.kubernetes.api.model.ServiceSpecBuilder;
 import io.fabric8.kubernetes.client.KubernetesClient;
+
 import org.keycloak.operator.Constants;
 import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
-import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec;
 
 import java.util.Optional;
 
-import static org.keycloak.operator.crds.v2alpha1.CRDUtils.getValueFromSubSpec;
 import static org.keycloak.operator.crds.v2alpha1.CRDUtils.isTlsConfigured;
 
-public class KeycloakService extends OperatorManagedResource implements StatusUpdater<KeycloakStatusAggregator> {
+public class KeycloakService extends OperatorManagedResource {
 
-    private Service existingService;
     private final Keycloak keycloak;
 
     public KeycloakService(KubernetesClient client, Keycloak keycloakCR) {
         super(client, keycloakCR);
         this.keycloak = keycloakCR;
-        this.existingService = fetchExistingService();
     }
 
     private ServiceSpec getServiceSpec() {
@@ -71,22 +68,6 @@ public class KeycloakService extends OperatorManagedResource implements StatusUp
         return service;
     }
 
-    private Service fetchExistingService() {
-        return client
-                .services()
-                .inNamespace(getNamespace())
-                .withName(getName())
-                .get();
-    }
-
-    @Override
-    public void updateStatus(KeycloakStatusAggregator status) {
-        if (existingService == null) {
-            status.addNotReadyMessage("No existing Keycloak Service found, waiting for creating a new one");
-            return;
-        }
-    }
-
     @Override
     public String getName() {
         return getServiceName(cr);
@@ -99,9 +80,9 @@ public class KeycloakService extends OperatorManagedResource implements StatusUp
     public static int getServicePort(Keycloak keycloak) {
         // we assume HTTP when TLS is not configured
         if (!isTlsConfigured(keycloak)) {
-            return getValueFromSubSpec(keycloak.getSpec().getHttpSpec(), HttpSpec::getHttpPort).orElse(Constants.KEYCLOAK_HTTP_PORT);
+            return Optional.ofNullable(keycloak.getSpec().getHttpSpec()).map(HttpSpec::getHttpPort).orElse(Constants.KEYCLOAK_HTTP_PORT);
         } else {
-            return getValueFromSubSpec(keycloak.getSpec().getHttpSpec(), HttpSpec::getHttpsPort).orElse(Constants.KEYCLOAK_HTTPS_PORT);
+            return Optional.ofNullable(keycloak.getSpec().getHttpSpec()).map(HttpSpec::getHttpsPort).orElse(Constants.KEYCLOAK_HTTPS_PORT);
         }
     }
 }

operator/src/main/java/org/keycloak/operator/controllers/StatusUpdater.java

@@ -1,6 +0,0 @@
-package org.keycloak.operator.controllers;
-
-public interface StatusUpdater<T> {
-
-    void updateStatus(T status);
-}

operator/src/main/java/org/keycloak/operator/crds/v2alpha1/CRDUtils.java

@@ -21,22 +21,14 @@ import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec;
 
 import java.util.Optional;
-import java.util.function.Function;
 
 /**
  * @author Vaclav Muzikar <vmuzikar@redhat.com>
  */
 public final class CRDUtils {
     public static boolean isTlsConfigured(Keycloak keycloakCR) {
-        var tlsSecret = getValueFromSubSpec(keycloakCR.getSpec().getHttpSpec(), HttpSpec::getTlsSecret);
+        var tlsSecret = Optional.ofNullable(keycloakCR.getSpec().getHttpSpec()).map(HttpSpec::getTlsSecret);
         return tlsSecret.isPresent() && !tlsSecret.get().trim().isEmpty();
     }
 
-    public static <T, R> Optional<R> getValueFromSubSpec(T subSpec, Function<T, R> valueSupplier) {
-        if (subSpec != null) {
-            return Optional.ofNullable(valueSupplier.apply(subSpec));
-        } else {
-            return Optional.empty();
-        }
-    }
 }

operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakStatus.java

@@ -20,6 +20,8 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
+
 import io.fabric8.kubernetes.model.annotation.LabelSelector;
 import io.fabric8.kubernetes.model.annotation.StatusReplicas;
 import io.javaoperatorsdk.operator.api.ObservedGenerationAware;
@@ -70,6 +72,11 @@ public class KeycloakStatus implements ObservedGenerationAware {
         return conditions.stream().filter(c -> type.equals(c.getType())).findFirst();
     }
 
+    @JsonIgnore
+    public boolean isReady() {
+        return findCondition(KeycloakStatusCondition.READY).map(KeycloakStatusCondition::getStatus).orElse(false);
+    }
+
     @Override
     public Long getObservedGeneration() {
         return observedGeneration;

operator/src/test/java/org/keycloak/operator/testsuite/integration/BaseOperatorTest.java

@@ -49,9 +49,10 @@ import org.keycloak.operator.controllers.KeycloakDeployment;
 import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpecBuilder;
 import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpecFluent.UnsupportedNested;
-import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition;
+import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatus;
 import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpecFluent.PodTemplateNested;
 import org.keycloak.operator.testsuite.utils.K8sUtils;
+import org.opentest4j.TestAbortedException;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -264,15 +265,14 @@ public class BaseOperatorTest implements QuarkusTestAfterEachCallback {
           return;
       }
       try {
-          if (!context.getTestStatus().isTestFailed()) {
+          if (!context.getTestStatus().isTestFailed() && !(context.getTestStatus().getTestErrorCause() instanceof TestAbortedException)) {
               return;
           }
+          Log.warnf("Test failed with %s: %s", context.getTestStatus().getTestErrorCause().getMessage(), context.getTestStatus().getTestErrorCause().getClass().getName());
           savePodLogs();
           // provide some helpful entries in the main log as well
           k8sclient.resources(Keycloak.class).list().getItems().stream()
-                  .filter(kc -> !Boolean.TRUE.equals(Optional.ofNullable(kc.getStatus())
+                  .filter(kc -> !Optional.ofNullable(kc.getStatus()).map(KeycloakStatus::isReady).orElse(false))
-                          .flatMap(keycloak -> keycloak.findCondition(KeycloakStatusCondition.READY))
-                          .map(KeycloakStatusCondition::getStatus).orElse(null)))
                   .forEach(kc -> {
                       Log.warnf("Keycloak failed to become ready \"%s\" %s", kc.getMetadata().getName(), Serialization.asYaml(kc.getStatus()));
                       var statefulSet = k8sclient.resources(StatefulSet.class).withName(KeycloakDeployment.getName(kc)).get();
@@ -283,7 +283,7 @@ public class BaseOperatorTest implements QuarkusTestAfterEachCallback {
                                       try {
                                           String log = k8sclient.pods().resource(pod).getLog();
                                           if (log.length() > 5000) {
-                                              log = log.substring(log.length() - 5000, log.length());
+                                              log = log.substring(log.length() - 5000);
                                           }
                                           Log.warnf("Not ready pod log \"%s\": %s", pod.getMetadata().getName(), log);
                                       } catch (KubernetesClientException e) {