Repository: kroxylicious/kroxylicious (java)

Issue:
Kubernetes healthchecks for operator
The operator should have its own healthchecks configured for its Deployment.

(See also #1573 for the healthchecks for the proxy itself).

Pull request:
Add health probe for operator
### Type of change

- Enhancement / new feature

### Description

* Add a liveness endpoint to the operator wired up to the health of the informers.
* Declare it in the operator Deployment

### Additional Context

Fixes #1574.



Patch:
diff --git a/kroxylicious-operator/install/03.Deployment.kroxylicious-operator.yaml b/kroxylicious-operator/install/03.Deployment.kroxylicious-operator.yaml
index 8ba8ae5c..0f6e01fe 100644
--- a/kroxylicious-operator/install/03.Deployment.kroxylicious-operator.yaml
+++ b/kroxylicious-operator/install/03.Deployment.kroxylicious-operator.yaml
@@ -33,7 +33,18 @@ spec:
           resources:
             limits:
               memory: 300M
-              cpu: 1
+              cpu: "1"
             requests:
               memory: 300M
-              cpu: 1
+              cpu: "1"
+          ports:
+            - containerPort: 8080
+              name: http
+          livenessProbe:
+            initialDelaySeconds: 10
+            periodSeconds: 10
+            failureThreshold: 3
+            timeoutSeconds: 5
+            httpGet:
+              port: http
+              path: /livez
diff --git a/kroxylicious-operator/src/main/java/io/kroxylicious/kubernetes/operator/OperatorMain.java b/kroxylicious-operator/src/main/java/io/kroxylicious/kubernetes/operator/OperatorMain.java
index 28a7fb6d..158a249a 100644
--- a/kroxylicious-operator/src/main/java/io/kroxylicious/kubernetes/operator/OperatorMain.java
+++ b/kroxylicious-operator/src/main/java/io/kroxylicious/kubernetes/operator/OperatorMain.java
@@ -11,6 +11,7 @@ import java.time.Duration;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.function.IntSupplier;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -43,6 +44,8 @@ public class OperatorMain {
     private static final Logger LOGGER = LoggerFactory.getLogger(OperatorMain.class);
     private static final String BIND_ADDRESS_VAR_NAME = "BIND_ADDRESS";
     private static final int DEFAULT_MANAGEMENT_PORT = 8080;
+    static final String HTTP_PATH_LIVEZ = "/livez";
+    static final String HTTP_PATH_METRICS = "/metrics";
     private final Operator operator;
     private final HttpServer managementServer;
 
@@ -78,18 +81,37 @@ public class OperatorMain {
      */
     void start() {
         operator.installShutdownHook(Duration.ofSeconds(10));
-        var registeredController = operator.register(new ProxyReconciler(runtimeDecl()));
-        // TODO couple the health of the registeredController to the operator's HTTP healthchecks
-        managementServer.createContext("/", exchange -> {
+        operator.register(new ProxyReconciler(runtimeDecl()));
+        addHttpGetHandler("/", () -> 404);
+        managementServer.start();
+        operator.start();
+        addHttpGetHandler(HTTP_PATH_LIVEZ, this::livezStatusCode);
+        LOGGER.info("Operator started");
+    }
+
+    private void addHttpGetHandler(
+                                   String path,
+                                   IntSupplier statusCodeSupplier) {
+        managementServer.createContext(path, exchange -> {
             try (exchange) {
                 // note while the JDK docs advise exchange.getRequestBody().transferTo(OutputStream.nullOutputStream()); we explicitly don't do that!
                 // As a denial-of-service protection we don't expect anything other than GET requests so there should be no input to read.
-                exchange.sendResponseHeaders(404, -1);
+                exchange.sendResponseHeaders(statusCodeSupplier.getAsInt(), -1);
             }
-        }).getFilters().add(UnsupportedHttpMethodFilter.INSTANCE); // This works as a request to `/metrics` has a more explicit match and thus gets served.
-        managementServer.start();
-        operator.start();
-        LOGGER.info("Operator started.");
+        }).getFilters().add(UnsupportedHttpMethodFilter.INSTANCE);
+    }
+
+    private int livezStatusCode() {
+        int sc;
+        try {
+            sc = operator.getRuntimeInfo().allEventSourcesAreHealthy() ? 200 : 400;
+        }
+        catch (Exception e) {
+            sc = 400;
+            LOGGER.error("Ignoring exception caught while getting operator health info", e);
+        }
+        LOGGER.trace("Responding {} to GET {}", sc, HTTP_PATH_LIVEZ);
+        return sc;
     }
 
     void stop() {
@@ -113,7 +135,7 @@ public class OperatorMain {
 
     private void configurePrometheusMetrics(HttpServer managementServer) {
         final PrometheusMeterRegistry prometheusMeterRegistry = new PrometheusMeterRegistry(PrometheusConfig.DEFAULT);
-        final HttpContext metricsContext = managementServer.createContext("/metrics",
+        final HttpContext metricsContext = managementServer.createContext(HTTP_PATH_METRICS,
                 new MetricsHandler(prometheusMeterRegistry.getPrometheusRegistry()));
         metricsContext.getFilters().add(UnsupportedHttpMethodFilter.INSTANCE);
         Metrics.globalRegistry.add(prometheusMeterRegistry);


Test patch:
diff --git a/kroxylicious-operator/src/test/java/io/kroxylicious/kubernetes/operator/OperatorMainTest.java b/kroxylicious-operator/src/test/java/io/kroxylicious/kubernetes/operator/OperatorMainTest.java
index 88b6de71..742918a2 100644
--- a/kroxylicious-operator/src/test/java/io/kroxylicious/kubernetes/operator/OperatorMainTest.java
+++ b/kroxylicious-operator/src/test/java/io/kroxylicious/kubernetes/operator/OperatorMainTest.java
@@ -44,7 +44,6 @@ import io.kroxylicious.kubernetes.operator.management.UnsupportedHttpMethodFilte
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -63,12 +62,22 @@ class OperatorMainTest {
     HttpServer managementServer;
 
     @Mock
-    HttpContext httpContext;
+    HttpContext rootHttpContext;
+    ArgumentCaptor<HttpHandler> rootCaptor = ArgumentCaptor.forClass(HttpHandler.class);
+
+    @Mock
+    HttpContext metricsHttpContext;
+
+    @Mock
+    HttpContext livezHttpContext;
+    ArgumentCaptor<HttpHandler> livezCaptor = ArgumentCaptor.forClass(HttpHandler.class);
 
     @BeforeEach
     void setUp() {
         expectApiResources();
-        when(managementServer.createContext(anyString(), any(HttpHandler.class))).thenReturn(httpContext);
+        when(managementServer.createContext(eq("/"), rootCaptor.capture())).thenReturn(rootHttpContext);
+        when(managementServer.createContext(eq(OperatorMain.HTTP_PATH_METRICS), any(HttpHandler.class))).thenReturn(metricsHttpContext);
+        when(managementServer.createContext(eq(OperatorMain.HTTP_PATH_LIVEZ), livezCaptor.capture())).thenReturn(livezHttpContext);
         operatorMain = new OperatorMain(kubeClient, managementServer);
     }
 
@@ -138,30 +147,52 @@ class OperatorMainTest {
         operatorMain.start();
 
         // Then
-        verify(managementServer).createContext(eq("/metrics"), any(HttpHandler.class));
+        verify(managementServer).createContext(eq(OperatorMain.HTTP_PATH_METRICS), any(HttpHandler.class));
+    }
+
+    @Test
+    void shouldRegisterLivezWithManagementServer() {
+        // Given
+
+        // When
+        operatorMain.start();
+
+        // Then
+        verify(managementServer).createContext(eq(OperatorMain.HTTP_PATH_LIVEZ), any(HttpHandler.class));
     }
 
     @Test
     void shouldRegisterUnsupportedMethodsHandlerWithManagementServer() {
         // Given
         final ArrayList<Filter> filters = new ArrayList<>();
-        when(httpContext.getFilters()).thenReturn(filters);
+        when(rootHttpContext.getFilters()).thenReturn(filters);
 
         // When
         operatorMain.start();
 
         // Then
         verify(managementServer).createContext(eq("/"), any(HttpHandler.class));
-        assertThat(filters).isNotEmpty()
+        assertThat(filters)
                 .singleElement()
                 .isInstanceOf(UnsupportedHttpMethodFilter.class);
+
     }
 
     @Test
     void shouldRespondWith404ForRequestsManagementServer() throws IOException {
+        shouldRespondWithStatusCode(rootCaptor, 404);
+    }
+
+    @Test
+    void shouldRespondWith200ForRequestsLivez() throws IOException {
+        // Given
+        shouldRespondWithStatusCode(livezCaptor, 200);
+    }
+
+    private void shouldRespondWithStatusCode(ArgumentCaptor<HttpHandler> captor,
+                                             int statusCode)
+            throws IOException {
         // Given
-        final ArgumentCaptor<HttpHandler> captor = ArgumentCaptor.forClass(HttpHandler.class);
-        when(managementServer.createContext(anyString(), captor.capture())).thenReturn(httpContext);
         operatorMain.start();
         final HttpExchange httpExchange = mock(HttpExchange.class);
 
@@ -169,7 +200,7 @@ class OperatorMainTest {
         captor.getValue().handle(httpExchange);
 
         // Then
-        verify(httpExchange).sendResponseHeaders(404, -1);
+        verify(httpExchange).sendResponseHeaders(statusCode, -1);
     }
 
     private void expectApiResources() {