Skip to content

Commit

Permalink
Fix eclipse-jkube#672: Add missing fields in ProbeConfig for configur…
Browse files Browse the repository at this point in the history
…ing Readiness/Liveness Probes

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
  • Loading branch information
rohanKanojia committed Jun 11, 2021
1 parent c01c5c3 commit d956e7f
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Usage:
### 1.4.0-SNAPSHOT
* Fix #705: JIB assembly works on Windows
* Fix #714: feat: Helm support for Golang expressions
* Fix #672: Add missing fields in ProbeConfig for configuring Readiness/Liveness Probes

### 1.3.0
* Fix #497: Assembly descriptor removed but still in documentation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -492,6 +494,24 @@ 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);
assertThat(httpHeaders.get(0).getName()).isEqualTo("Accept");
assertThat(httpHeaders.get(0).getValue()).isEqualTo("application/json");
assertThat(httpHeaders.get(1).getName()).isEqualTo("User-Agent");
assertThat(httpHeaders.get(1).getValue()).isEqualTo("MyUserAgent");
}

private void assertLocalFragments(File[] fragments, int expectedSize) {
assertEquals(expectedSize, fragments.length);
assertTrue(Arrays.stream(fragments).anyMatch( f -> f.getName().equals("deployment.yml")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import lombok.Getter;
import lombok.NoArgsConstructor;

import java.util.Map;

/**
* @author roland
*/
Expand All @@ -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.
*/
Expand All @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;


/**
Expand All @@ -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;
Expand All @@ -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);
return new HTTPGetAction(url.getHost(),
null /* headers */,
List<HTTPHeader> httpHeaders = convertMapToHTTPHeaderList(headers);

return new HTTPGetAction(resolveHost(url.getHost()),
httpHeaders.isEmpty() ? null : httpHeaders,
url.getPath(),
new IntOrString(url.getPort()),
url.getProtocol().toUpperCase());
Expand All @@ -93,6 +89,21 @@ private HTTPGetAction getHTTPGetAction(String getUrl) {
}
}

private String resolveHost(String host) {
if (host.equalsIgnoreCase("localhost")) {
// No need to set host in case of localhost HTTP request, will default to Pod IP
return null;
}
return host;
}

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
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;
Expand Down Expand Up @@ -297,4 +301,48 @@ public void getTCPProbeWithInvalidURLTest() {

probe = probeHandler.getProbe(probeConfig);
}

@Test
public void testHttpGetProbeWithLocalhostInUrl() {
// Given
probeConfig = ProbeConfig.builder()
.getUrl("http://localhost:8080/healthz")
.build();

// When
probe = probeHandler.getProbe(probeConfig);

// Then
assertThat(probe).isNotNull();
assertThat(probe.getHttpGet()).isNotNull();
assertThat(probe.getHttpGet().getScheme()).isEqualTo("HTTP");
assertThat(probe.getHttpGet().getHost()).isNull();
assertThat(probe.getHttpGet().getPort().getIntVal()).isEqualTo(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();
assertThat(probe.getHttpGet()).isNotNull();
assertThat(probe.getHttpGet().getHost()).isEqualTo("www.example.com");
assertThat(probe.getHttpGet().getPort().getIntVal()).isEqualTo(8080);
assertThat(probe.getHttpGet().getScheme()).isEqualTo("HTTPS");
assertThat(probe.getHttpGet().getHttpHeaders().get(0).getName()).isEqualTo("Accept");
assertThat(probe.getHttpGet().getHttpHeaders().get(0).getValue()).isEqualTo("application/json");
assertThat(probe.getHttpGet().getHttpHeaders().get(1).getName()).isEqualTo("User-Agent");
assertThat(probe.getHttpGet().getHttpHeaders().get(1).getValue()).isEqualTo("MyUserAgent");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,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]]
Expand Down

0 comments on commit d956e7f

Please sign in to comment.