Fix eclipse-jkube#672: Add missing fields in ProbeConfig for configur…

…ing Readiness/Liveness Probes Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia · Jun 23, 2021 · 02474bf · 02474bf
1 parent 0af0925
commit 02474bf
Show file tree

Hide file tree

Showing 14 changed files with 448 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@ Usage:
 * Port fabric8io/docker-maven-plugin#1318: Update ECR autorization token URL
 * Fix #710: Support DockerImage as output for Openshift builds
 * Fix #548: Define property for skipping cluster autodetect/offline mode
+* Fix #672: Add missing fields in ProbeConfig for configuring Readiness/Liveness Probes
 
 ### 1.3.0
 * Fix #497: Assembly descriptor removed but still in documentation

diff --git a/jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/KubernetesHelper.java b/jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/KubernetesHelper.java
@@ -40,6 +40,7 @@
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
+import io.fabric8.kubernetes.api.model.HTTPHeader;
 import org.eclipse.jkube.kit.common.GenericCustomResource;
 import org.eclipse.jkube.kit.common.KitLogger;
 import org.eclipse.jkube.kit.common.ResourceFileType;
@@ -957,6 +958,16 @@ public static CustomResourceDefinitionContext getCrdContext(CustomResourceDefini
             .orElse(null);
     }
 
+    public static List<HTTPHeader> convertMapToHTTPHeaderList(Map<String, String> headers) {
+        List<HTTPHeader> httpHeaders = new ArrayList<>();
+        if (headers != null) {
+            for (Map.Entry<String, String> header : headers.entrySet()) {
+                httpHeaders.add(new HTTPHeader(header.getKey(), header.getValue()));
+            }
+        }
+        return httpHeaders;
+    }
+
     private static Optional<CustomResourceDefinition> findCrdForCustomResource(CustomResourceDefinitionList crdList, GenericCustomResource gcr) {
         return crdList.getItems().stream()
             .filter(hasGroup(gcr))

diff --git a/jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/KubernetesHelperTest.java b/jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/KubernetesHelperTest.java
@@ -24,6 +24,8 @@
 import java.util.Map;
 import java.util.Set;
 
+import io.fabric8.kubernetes.api.model.HTTPHeader;
+import org.assertj.core.api.AssertionsForClassTypes;
 import org.eclipse.jkube.kit.common.GenericCustomResource;
 import org.eclipse.jkube.kit.common.KitLogger;
 
@@ -492,6 +494,26 @@ public void testGetCrdContextReturnsNullCrdContext() {
         assertThat(crdContext).isNull();
     }
 
+    @Test
+    public void testConvertMapToHTTPHeaderList() {
+        // Given
+        Map<String, String> headerAsMap = new HashMap<>();
+        headerAsMap.put("Accept", "application/json");
+        headerAsMap.put("User-Agent", "MyUserAgent");
+
+        // When
+        List<HTTPHeader> httpHeaders = KubernetesHelper.convertMapToHTTPHeaderList(headerAsMap);
+
+        // Then
+        assertThat(httpHeaders).isNotNull().hasSize(2)
+                .satisfies(h -> assertThat(h).element(0)
+                     .hasFieldOrPropertyWithValue("name", "Accept")
+                     .hasFieldOrPropertyWithValue("value", "application/json"))
+                .satisfies(h -> assertThat(h).element(1)
+                     .hasFieldOrPropertyWithValue("name", "User-Agent")
+                     .hasFieldOrPropertyWithValue("value", "MyUserAgent"));
+    }
+
     private void assertLocalFragments(File[] fragments, int expectedSize) {
         assertEquals(expectedSize, fragments.length);
         assertTrue(Arrays.stream(fragments).anyMatch( f -> f.getName().equals("deployment.yml")));

diff --git a/...-kit/config/resource/src/main/java/org/eclipse/jkube/kit/config/resource/ProbeConfig.java b/...-kit/config/resource/src/main/java/org/eclipse/jkube/kit/config/resource/ProbeConfig.java
@@ -19,6 +19,8 @@
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 
+import java.util.Map;
+
 /**
  * @author roland
  */
@@ -37,6 +39,10 @@ public class ProbeConfig {
      * Timeout in seconds how long the probe might take.
      */
     private Integer timeoutSeconds;
+    /**
+     * How often in seconds to perform the probe. Defaults to 10 seconds. Minimum value is 1.
+     */
+    private Integer periodSeconds;
     /**
      * Command to execute for probing.
      */
@@ -45,6 +51,11 @@ public class ProbeConfig {
      * Probe this URL.
      */
     private String getUrl;
+
+    /**
+     * Custom headers to set in the request.
+     */
+    private Map<String, String> httpHeaders;
     /**
      * TCP port to probe.
      */

diff --git a/...e-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/handler/ProbeHandler.java b/...e-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/handler/ProbeHandler.java
@@ -15,6 +15,7 @@
 
 import io.fabric8.kubernetes.api.model.ExecAction;
 import io.fabric8.kubernetes.api.model.HTTPGetAction;
+import io.fabric8.kubernetes.api.model.HTTPHeader;
 import io.fabric8.kubernetes.api.model.IntOrString;
 import io.fabric8.kubernetes.api.model.Probe;
 import io.fabric8.kubernetes.api.model.TCPSocketAction;
@@ -24,8 +25,12 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.List;
+import java.util.Map;
+import java.util.function.BiConsumer;
+import java.util.function.Supplier;
 
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
+import static org.eclipse.jkube.kit.common.util.KubernetesHelper.convertMapToHTTPHeaderList;
 
 
 /**
@@ -40,23 +45,12 @@ public Probe getProbe(ProbeConfig probeConfig)  {
         }
 
         Probe probe = new Probe();
-        Integer initialDelaySeconds = probeConfig.getInitialDelaySeconds();
-        if (initialDelaySeconds != null) {
-            probe.setInitialDelaySeconds(initialDelaySeconds);
-        }
-        Integer timeoutSeconds = probeConfig.getTimeoutSeconds();
-        if (timeoutSeconds != null) {
-            probe.setTimeoutSeconds(timeoutSeconds);
-        }
-        Integer failureThreshold = probeConfig.getFailureThreshold();
-        if(failureThreshold != null) {
-            probe.setFailureThreshold(failureThreshold);
-        }
-        Integer successThreshold = probeConfig.getSuccessThreshold();
-        if(successThreshold != null) {
-            probe.setSuccessThreshold(successThreshold);
-        }
-        HTTPGetAction getAction = getHTTPGetAction(probeConfig.getGetUrl());
+        setTimeoutInProbeIfNotNull(probe, probeConfig::getInitialDelaySeconds, (i, p) -> p.setInitialDelaySeconds(i));
+        setTimeoutInProbeIfNotNull(probe, probeConfig::getTimeoutSeconds, (i, p) -> p.setTimeoutSeconds(i));
+        setTimeoutInProbeIfNotNull(probe, probeConfig::getFailureThreshold, (i, p) -> p.setFailureThreshold(i));
+        setTimeoutInProbeIfNotNull(probe, probeConfig::getSuccessThreshold, (i, p) -> p.setSuccessThreshold(i));
+        setTimeoutInProbeIfNotNull(probe, probeConfig::getPeriodSeconds, (i, p) -> p.setPeriodSeconds(i));
+        HTTPGetAction getAction = getHTTPGetAction(probeConfig.getGetUrl(), probeConfig.getHttpHeaders());
         if (getAction != null) {
             probe.setHttpGet(getAction);
             return probe;
@@ -77,14 +71,16 @@ public Probe getProbe(ProbeConfig probeConfig)  {
 
     // ========================================================================================
 
-    private HTTPGetAction getHTTPGetAction(String getUrl) {
+    private HTTPGetAction getHTTPGetAction(String getUrl, Map<String, String> headers) {
         if (getUrl == null || !getUrl.subSequence(0,4).toString().equalsIgnoreCase("http")) {
             return null;
         }
         try {
             URL url = new URL(getUrl);
+            List<HTTPHeader> httpHeaders = convertMapToHTTPHeaderList(headers);
+
             return new HTTPGetAction(url.getHost(),
-                    null /* headers */,
+                    httpHeaders.isEmpty() ? null : httpHeaders,
                     url.getPath(),
                     new IntOrString(url.getPort()),
                     url.getProtocol().toUpperCase());
@@ -93,6 +89,14 @@ private HTTPGetAction getHTTPGetAction(String getUrl) {
         }
     }
 
+    @SuppressWarnings("java:S4276") // IntSupplier throws NullPointerException when unboxing null Integers
+    private void setTimeoutInProbeIfNotNull(Probe probe, Supplier<Integer> integerSupplier, BiConsumer<Integer, Probe> probeConsumer) {
+        Integer i = integerSupplier.get();
+        if (i != null) {
+            probeConsumer.accept(i, probe);
+        }
+    }
+
     private TCPSocketAction getTCPSocketAction(String getUrl, String port) {
         if (port != null) {
             IntOrString portObj = new IntOrString(port);

diff --git a/...t/enricher/api/src/test/java/org/eclipse/jkube/kit/enricher/handler/ProbeHandlerTest.java b/...t/enricher/api/src/test/java/org/eclipse/jkube/kit/enricher/handler/ProbeHandlerTest.java
@@ -13,10 +13,15 @@
  */
 package org.eclipse.jkube.kit.enricher.handler;
 
+import io.fabric8.kubernetes.api.model.IntOrString;
 import io.fabric8.kubernetes.api.model.Probe;
 import org.eclipse.jkube.kit.config.resource.ProbeConfig;
 import org.junit.Test;
 
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -297,4 +302,50 @@ public void getTCPProbeWithInvalidURLTest() {
 
         probe = probeHandler.getProbe(probeConfig);
     }
+
+    @Test
+    public void testHttpGetProbeWithLocalhostInUrl() {
+        // Given
+        probeConfig = ProbeConfig.builder()
+                .getUrl("http://:8080/healthz")
+                .build();
+
+        // When
+        probe = probeHandler.getProbe(probeConfig);
+
+        // Then
+        assertThat(probe)
+                .isNotNull()
+                .hasFieldOrPropertyWithValue("httpGet.host", "")
+                .hasFieldOrPropertyWithValue("httpGet.scheme", "HTTP")
+                .hasFieldOrPropertyWithValue("httpGet.port", new IntOrString(8080));
+    }
+
+    @Test
+    public void testHttpGetProbeWithCustomHeaders() {
+        // Given
+        Map<String, String> headers = new HashMap<>();
+        headers.put("Accept", "application/json");
+        headers.put("User-Agent", "MyUserAgent");
+        probeConfig = ProbeConfig.builder()
+                .getUrl("https://www.example.com:8080/healthz")
+                .httpHeaders(headers)
+                .build();
+
+        // When
+        probe = probeHandler.getProbe(probeConfig);
+
+        // Then
+        assertThat(probe)
+                .isNotNull()
+                .hasFieldOrPropertyWithValue("httpGet.host", "www.example.com")
+                .hasFieldOrPropertyWithValue("httpGet.port", new IntOrString(8080))
+                .hasFieldOrPropertyWithValue("httpGet.scheme", "HTTPS")
+                .satisfies(p -> assertThat(p).extracting("httpGet.httpHeaders").asList().element(0)
+                       .hasFieldOrPropertyWithValue("name", "Accept")
+                       .hasFieldOrPropertyWithValue("value", "application/json"))
+                .satisfies(p -> assertThat(p).extracting("httpGet.httpHeaders").asList().element(1)
+                       .hasFieldOrPropertyWithValue("name", "User-Agent")
+                       .hasFieldOrPropertyWithValue("value", "MyUserAgent"));
+    }
 }
diff --git a/kubernetes-maven-plugin/doc/src/main/asciidoc/inc/_introduction.adoc b/kubernetes-maven-plugin/doc/src/main/asciidoc/inc/_introduction.adoc
@@ -315,7 +315,7 @@ An extract of the plugin configuration is shown below:
     </configMap>
 
     <liveness> <!--13-->
-      <getUrl>http://localhost:8080/q/health</getUrl>
+      <getUrl>http://:8080/q/health</getUrl>
       <initialDelaySeconds>3</initialDelaySeconds>
       <timeoutSeconds>3</timeoutSeconds>
     </liveness>

diff --git a/kubernetes-maven-plugin/doc/src/main/asciidoc/inc/goals/build/_jkube-resource.adoc b/kubernetes-maven-plugin/doc/src/main/asciidoc/inc/goals/build/_jkube-resource.adoc
@@ -138,7 +138,7 @@ By default Deployment is generated in Kubernetes mode. You can easily configure
         <imagePullPolicy>Always</imagePullPolicy> <!--4-->
         <replicas>3</replicas> <!--5-->
         <liveness> <!--6-->
-            <getUrl>http://localhost:8080/q/health</getUrl>
+            <getUrl>http://:8080/q/health</getUrl>
             <tcpPort>8080</tcpPort>
             <initialDelaySeconds>3</initialDelaySeconds>
             <timeoutSeconds>3</timeoutSeconds>
@@ -219,7 +219,17 @@ Probe configuration is used for configuring https://kubernetes.io/docs/tasks/con
 | Command to execute for probing.
 
 | `getUrl`
-| Probe this URL.
+| Probe URL for HTTP Probe. Configures HTTP probe fields like `host`, `scheme`, `path` etc by parsing URL. For example, a `<getUrl>http://:8080/health</getUrl>` would result in probe generated with fields set like this:
+
+  host: ""
+
+  path: /actuator/health
+
+  port: 8080
+
+  scheme: HTTP
+
+Host name with empty value defaults to Pod IP. You probably want to set "Host" in httpHeaders instead.
 
 | `tcpPort`
 | TCP port to probe.
@@ -229,6 +239,12 @@ Probe configuration is used for configuring https://kubernetes.io/docs/tasks/con
 
 | `successThreshold`
 |  Minimum consecutive successes for the probe to be considered successful after having failed.
+
+|  `httpHeaders`
+| Custom headers to set in the request.
+
+| `periodSeconds`
+| How often in seconds to perform the probe. Defaults to 10 seconds. Minimum value is 1.
 |===
 
 [[volume-xml-configuration]]