Skip to content

feat: introduce JUnit5 extension #545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Sep 21, 2021
10 changes: 0 additions & 10 deletions operator-framework-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,6 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-slf4j-impl</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>kubernetes-server-mock</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.Closeable;
import java.io.IOException;
import java.net.ConnectException;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.locks.ReentrantLock;
Expand Down Expand Up @@ -57,6 +58,10 @@ public ConfigurationService getConfigurationService() {
return configurationService;
}

public List<ConfiguredController> getControllers() {
return Collections.unmodifiableList(controllers.controllers);
}

/**
* Finishes the operator startup process. This is mostly used in injection-aware applications
* where there is no obvious entrypoint to the application which can trigger the injection process
Expand Down Expand Up @@ -99,6 +104,8 @@ public void close() {
"Operator SDK {} is shutting down...", configurationService.getVersion().getSdkVersion());

controllers.close();

k8sClient.close();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,8 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Stream;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public abstract class AbstractConfigurationService implements ConfigurationService {

public static final String LOGGER_NAME = "Default ConfigurationService implementation";
protected static final Logger log = LoggerFactory.getLogger(LOGGER_NAME);

private final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
private final Version version;

Expand Down Expand Up @@ -60,12 +54,14 @@ public <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor
final var key = keyFor(controller);
final var configuration = configurations.get(key);
if (configuration == null) {
log.warn(
"Configuration for controller '{}' was not found. {}", key, getControllersNameMessage());
logMissingControllerWarning(key, getControllersNameMessage());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to my understanding, why is this better?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benefit is minimal, indeed, just to be able to not pull SLF4J when using AbstractConfigurationService.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, isn't this a little bit over-engineering? We use SLF4J everywhere, and it's quite an industry standard

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other issue is also that the previous version hardcoded the name of the logger, which might not have been appropriate depending on the actual implementation being executed (for example, even the Quarkus implementation would have logged something with Default ConfigurationService implementation logger name).

}
return configuration;
}

protected abstract void logMissingControllerWarning(String controllerKey,
String controllersNameMessage);

private String getControllersNameMessage() {
return "Known controllers: "
+ getKnownControllerNames().stream().reduce((s, s2) -> s + ", " + s2).orElse("None")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.javaoperatorsdk.operator.api.config;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class BaseConfigurationService extends AbstractConfigurationService {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get this class. Why are we creating loggers this way?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is just used to be able to have a non-abstract implementation that can be used in the junit extension. I'm debating whether to just put the DefaultConfigurationService into core itself and get rid of this class completely but I'm not too sure of the impact on the quarkus extension…

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oki,
(will take a look on the quarkus side to understand these aspects better)


private static final String LOGGER_NAME = "Default ConfigurationService implementation";
private static final Logger logger = LoggerFactory.getLogger(LOGGER_NAME);

public BaseConfigurationService(Version version) {
super(version);
}

@Override
protected void logMissingControllerWarning(String controllerKey, String controllersNameMessage) {
logger.warn("Configuration for controller '{}' was not found. {}", controllerKey,
controllersNameMessage);
}

public String getLoggerName() {
return LOGGER_NAME;
}

protected Logger getLogger() {
return logger;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ public static Version loadFromProperties() {

Date builtTime;
try {
builtTime =
// RFC 822 date is the default format used by git-commit-id-plugin
new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ")
.parse(properties.getProperty("git.build.time"));
String time = properties.getProperty("git.build.time");
if (time != null) {
builtTime =
// RFC 822 date is the default format used by git-commit-id-plugin
new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ").parse(time);
} else {
builtTime = Date.from(Instant.EPOCH);
}
} catch (Exception e) {
log.debug("Couldn't parse git.build.time property", e);
builtTime = Date.from(Instant.EPOCH);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package io.javaoperatorsdk.operator.api.config;

import java.time.Instant;
import java.util.Date;

/** A class encapsulating the version information associated with this SDK instance. */
public class Version {

public static final Version UNKNOWN = new Version("unknown", "unknown", Date.from(Instant.EPOCH));

private final String sdk;
private final String commit;
private final Date builtTime;
Expand Down
46 changes: 46 additions & 0 deletions operator-framework-junit5/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>java-operator-sdk</artifactId>
<groupId>io.javaoperatorsdk</groupId>
<version>1.9.7-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>operator-framework-junit-5</artifactId>
<name>Operator SDK - Framework - JUnit 5 extension</name>

<properties>
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>
</properties>

<dependencies>
<dependency>
<groupId>io.javaoperatorsdk</groupId>
<artifactId>operator-framework-core</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.20.2</version>
</dependency>
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
<version>4.1.0</version>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package io.javaoperatorsdk.operator.junit;

import io.fabric8.kubernetes.client.KubernetesClient;

public interface HasKubernetesClient {
KubernetesClient getKubernetesClient();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package io.javaoperatorsdk.operator.junit;

import io.fabric8.kubernetes.client.KubernetesClient;

public interface KubernetesClientAware extends HasKubernetesClient {
void setKubernetesClient(KubernetesClient kubernetesClient);
}
Loading