Skip to content

Commit

Permalink
Fix property "http-action-port-name" is ignored by Kubernetes
Browse files Browse the repository at this point in the history
When binding a new port:

```
quarkus.kubernetes.ports.custom.container-port=8888
```

The new port name is `custom` which port `8888`. 

Then, if we configure any probe to use the port name `custom`:

```
quarkus.kubernetes.readiness-probe.http-action-port-name=custom
```

The generated probe wrongly kept using the port name `http`.
  • Loading branch information
Sgitario committed Jun 12, 2023
1 parent 197c921 commit 9387120
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -1,23 +1,34 @@
package io.quarkus.kubernetes.deployment;

import static io.dekorate.ConfigReference.joinProperties;
import static io.dekorate.utils.Metadata.getMetadata;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import io.dekorate.ConfigReference;
import io.dekorate.WithConfigReferences;
import io.dekorate.kubernetes.decorator.AbstractAddProbeDecorator;
import io.dekorate.kubernetes.decorator.AddSidecarDecorator;
import io.dekorate.kubernetes.decorator.Decorator;
import io.dekorate.kubernetes.decorator.ResourceProvidingDecorator;
import io.dekorate.utils.Strings;
import io.fabric8.kubernetes.api.builder.Builder;
import io.fabric8.kubernetes.api.model.ContainerBuilder;
import io.fabric8.kubernetes.api.model.HTTPGetActionFluent;

public class ApplyHttpGetActionPortDecorator extends Decorator<HTTPGetActionFluent<?>> {
public class ApplyHttpGetActionPortDecorator extends Decorator<HTTPGetActionFluent<?>> implements WithConfigReferences {

private static final String PATH_ALL_EXPRESSION = "*.spec.containers.";
private static final String PATH_DEPLOYMENT_CONTAINER_EXPRESSION = "(metadata.name == %s).spec.template.spec.containers.(name == %s).";
private static final String PATH_DEPLOYMENT_EXPRESSION = "(metadata.name == %s).spec.template.spec.containers.";
private static final String PATH_CONTAINER_EXPRESSION = "*.spec.containers.(name == %s).";

private final String deployment;
private final String container;
private final String portName;
private final Integer port;
private final String scheme;
private final String probeKind;
Expand All @@ -43,12 +54,14 @@ public ApplyHttpGetActionPortDecorator(String deployment, String container, Inte
}

public ApplyHttpGetActionPortDecorator(String deployment, String container, Integer port, String probeKind) {
this(deployment, container, port, probeKind, port != null && (port == 443 || port == 8443) ? "HTTPS" : "HTTP"); // this is the original convention coming from dekorate
this(deployment, container, null, port, probeKind, port != null && (port == 443 || port == 8443) ? "HTTPS" : "HTTP"); // this is the original convention coming from dekorate
}

public ApplyHttpGetActionPortDecorator(String deployment, String container, Integer port, String probeKind, String scheme) {
public ApplyHttpGetActionPortDecorator(String deployment, String container, String portName, Integer port, String probeKind,
String scheme) {
this.deployment = deployment;
this.container = container;
this.portName = portName;
this.port = port;
this.probeKind = probeKind;
this.scheme = scheme;
Expand Down Expand Up @@ -107,4 +120,28 @@ public void visit(HTTPGetActionFluent<?> action) {
public Class<? extends Decorator>[] after() {
return new Class[] { ResourceProvidingDecorator.class, AddSidecarDecorator.class, AbstractAddProbeDecorator.class };
}

@Override
public List<ConfigReference> getConfigReferences() {
if (portName != null && probeKind != null) {
return List.of(buildConfigReference(joinProperties("ports." + portName),
"httpGet.port", port, "The http port to use for the probe."));
}

return Collections.emptyList();
}

private ConfigReference buildConfigReference(String property, String probeField, Object value, String description) {
String expression = PATH_ALL_EXPRESSION;
if (Strings.isNotNullOrEmpty(deployment) && Strings.isNotNullOrEmpty(container)) {
expression = String.format(PATH_DEPLOYMENT_CONTAINER_EXPRESSION, deployment, container);
} else if (Strings.isNotNullOrEmpty(deployment)) {
expression = String.format(PATH_DEPLOYMENT_EXPRESSION, deployment);
} else if (Strings.isNotNullOrEmpty(container)) {
expression = String.format(PATH_CONTAINER_EXPRESSION, container);
}

String yamlPath = expression + probeKind + "." + probeField;
return new ConfigReference.Builder(property, yamlPath).withDescription(description).withValue(value).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -971,16 +971,23 @@ public static DecoratorBuildItem createProbeHttpPortDecorator(String name, Strin
.or(() -> portName.map(KubernetesProbePortNameBuildItem::getName))
.orElse(HTTP_PORT);

Integer port = probeConfig.httpActionPort
.orElse(ports.stream().filter(p -> httpPortName.equals(p.getName()))
.map(KubernetesPortBuildItem::getPort).findFirst().orElse(DEFAULT_HTTP_PORT));
Integer port;
PortConfig portFromConfig = portsFromConfig.get(httpPortName);
if (probeConfig.httpActionPort.isPresent()) {
port = probeConfig.httpActionPort.get();
} else if (portFromConfig != null && portFromConfig.containerPort.isPresent()) {
port = portFromConfig.containerPort.getAsInt();
} else {
port = ports.stream().filter(p -> httpPortName.equals(p.getName()))
.map(KubernetesPortBuildItem::getPort).findFirst().orElse(DEFAULT_HTTP_PORT);
}

// Resolve scheme property from:
String scheme;
if (probeConfig.httpActionScheme.isPresent()) {
// 1. User in Probe config
scheme = probeConfig.httpActionScheme.get();
} else if (portsFromConfig.containsKey(httpPortName) && portsFromConfig.get(httpPortName).tls) {
} else if (portFromConfig != null && portFromConfig.tls) {
// 2. User in Ports config
scheme = SCHEME_HTTPS;
} else if (portName.isPresent()
Expand All @@ -993,7 +1000,8 @@ public static DecoratorBuildItem createProbeHttpPortDecorator(String name, Strin
scheme = port != null && (port == 443 || port == 8443) ? SCHEME_HTTPS : SCHEME_HTTP;
}

return new DecoratorBuildItem(target, new ApplyHttpGetActionPortDecorator(name, name, port, probeKind, scheme));
return new DecoratorBuildItem(target,
new ApplyHttpGetActionPortDecorator(name, name, httpPortName, port, probeKind, scheme));
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package io.quarkus.it.kubernetes;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.io.IOException;
import java.nio.file.Path;
import java.util.List;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.quarkus.builder.Version;
import io.quarkus.maven.dependency.Dependency;
import io.quarkus.test.ProdBuildResults;
import io.quarkus.test.ProdModeTestResults;
import io.quarkus.test.QuarkusProdModeTest;

public class KubernetesWithProbePortByNameTest {

private static final String NAME = "kubernetes-with-probe-port-by-name";

@RegisterExtension
static final QuarkusProdModeTest config = new QuarkusProdModeTest()
.withApplicationRoot((jar) -> jar.addClasses(GreetingResource.class))
.setApplicationName(NAME)
.setApplicationVersion("0.1-SNAPSHOT")
.overrideConfigKey("quarkus.kubernetes.ports.custom.container-port", "8888")
.overrideConfigKey("quarkus.kubernetes.readiness-probe.http-action-port-name", "custom")
.setLogFileName("k8s.log")
.setForcedDependencies(List.of(
Dependency.of("io.quarkus", "quarkus-kubernetes", Version.getVersion()),
Dependency.of("io.quarkus", "quarkus-smallrye-health", Version.getVersion())));

@ProdBuildResults
private ProdModeTestResults prodModeTestResults;

@Test
public void assertGeneratedResources() throws IOException {

final Path kubernetesDir = prodModeTestResults.getBuildDir().resolve("kubernetes");
assertThat(kubernetesDir)
.isDirectoryContaining(p -> p.getFileName().endsWith("kubernetes.json"))
.isDirectoryContaining(p -> p.getFileName().endsWith("kubernetes.yml"));
List<HasMetadata> kubernetesList = DeserializationUtil
.deserializeAsList(kubernetesDir.resolve("kubernetes.yml"));
assertThat(kubernetesList.get(0)).isInstanceOfSatisfying(Deployment.class, d -> {
assertThat(d.getMetadata()).satisfies(m -> {
assertThat(m.getName()).isEqualTo(NAME);
});

assertThat(d.getSpec()).satisfies(deploymentSpec -> {
assertThat(deploymentSpec.getTemplate()).satisfies(t -> {
assertThat(t.getSpec()).satisfies(podSpec -> {
assertThat(podSpec.getContainers()).singleElement()
.satisfies(container -> {
assertThat(container.getReadinessProbe()).isNotNull().satisfies(p -> {
assertEquals(p.getHttpGet().getPort().getIntVal(), 8888);
assertEquals(p.getHttpGet().getScheme(), "HTTP");
});
});
});
});
});
});
}
}

0 comments on commit 9387120

Please sign in to comment.