Skip to content

Fixes #318

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.

Sign up for GitHub

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 18 commits into from
Jan 26, 2021
Merged

Fixes #318

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e77f2dd
fix: use controller name instead of controller class name
metacosm Jan 22, 2021
4277b3d
refactor: merge register and registerController methods
metacosm Jan 22, 2021
174097f
refactor: generify CustomResourceFacade
metacosm Jan 22, 2021
8fa839e
feat: add version information to configuration
metacosm Jan 22, 2021
4eb057b
feat: add start method as entrypoint and output version information
metacosm Jan 22, 2021
f278c8f
feat: skip controller registration if CRD is not present on cluster
metacosm Jan 22, 2021
2db6b5f
fix: date format parser
metacosm Jan 22, 2021
042d1e4
fix: use finalizer and deletion marker methods from HasMetadata
metacosm Jan 22, 2021
b791c4e
fix: Controller annotation is not mandatory for default configuration
metacosm Jan 25, 2021
9edcde9
feat: use Operator.start()
metacosm Jan 25, 2021
538e468
fix: log warning if version.properties cannot be found
metacosm Jan 25, 2021
9c0ccbd
docs: add some javadocs
metacosm Jan 25, 2021
b9987a7
fix: controllers need to be annotated with @Controller
metacosm Jan 25, 2021
09ce889
fix: only keep **not** blank finalizer, not blank ones… :facepalm:
metacosm Jan 25, 2021
6cde0be
feat: throw an explicit exception if CRD is not found
metacosm Jan 25, 2021
ebb453e
fix: format
metacosm Jan 25, 2021
d4d48cd
fix: replace ellipsis by ... for greater compatibility
metacosm Jan 26, 2021
7608163
fix: rename getProject to getSdkVersion for greater clarity
metacosm Jan 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 87 additions & 62 deletions operator-framework-core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,72 +2,97 @@
<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">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>io.javaoperatorsdk</groupId>
<artifactId>java-operator-sdk</artifactId>
<version>1.7.1-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>io.javaoperatorsdk</groupId>
<artifactId>java-operator-sdk</artifactId>
<version>1.7.1-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

<artifactId>operator-framework-core</artifactId>
<name>Operator SDK - Framework - Core</name>
<description>Core framework for implementing Kubernetes operators</description>
<packaging>jar</packaging>
<artifactId>operator-framework-core</artifactId>
<name>Operator SDK - Framework - Core</name>
<description>Core framework for implementing Kubernetes operators</description>
<packaging>jar</packaging>

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

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${surefire.version}</version>
</plugin>
</plugins>
</build>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${surefire.version}</version>
</plugin>
<plugin>
<!-- Used to generate the version / commit information -->
<groupId>pl.project13.maven</groupId>
<artifactId>git-commit-id-plugin</artifactId>
<version>4.0.3</version>
<executions>
<execution>
<id>get-the-git-infos</id>
<goals>
<goal>revision</goal>
</goals>
<phase>initialize</phase>
</execution>
</executions>
<configuration>
<generateGitPropertiesFile>true</generateGitPropertiesFile>
<generateGitPropertiesFilename>${project.build.outputDirectory}/version.properties
</generateGitPropertiesFilename>
<includeOnlyProperties>
<includeOnlyProperty>^git.build.(time|version)$</includeOnlyProperty>
<includeOnlyProperty>^git.commit.id.(abbrev|full)$</includeOnlyProperty>
</includeOnlyProperties>
<commitIdGenerationMode>full</commitIdGenerationMode>
</configuration>
</plugin>
</plugins>
</build>


<dependencies>
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>openshift-client</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
<dependencies>
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>openshift-client</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-slf4j-impl</artifactId>
<version>2.13.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.18.0</version>
<scope>test</scope>
</dependency>
</dependencies>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-slf4j-impl</artifactId>
<version>2.13.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.18.0</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
6 changes: 0 additions & 6 deletions operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.javaoperatorsdk.operator;

import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.api.Controller;
import io.javaoperatorsdk.operator.api.ResourceController;
import java.util.Locale;
Expand All @@ -13,11 +12,6 @@ public static String getDefaultFinalizerName(String crdName) {
return crdName + FINALIZER_NAME_SUFFIX;
}

public static boolean hasGivenFinalizer(CustomResource resource, String finalizer) {
return resource.getMetadata().getFinalizers() != null
&& resource.getMetadata().getFinalizers().contains(finalizer);
}

public static String getNameFor(Class<? extends ResourceController> controllerClass) {
// if the controller annotation has a name attribute, use it
final var annotation = controllerClass.getAnnotation(Controller.class);
Expand Down
98 changes: 54 additions & 44 deletions operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager;
import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEventSource;
import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
import io.javaoperatorsdk.operator.processing.retry.Retry;
import java.util.Arrays;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -29,6 +28,15 @@ public Operator(KubernetesClient k8sClient, ConfigurationService configurationSe
this.configurationService = configurationService;
}

public void start() {
final var version = configurationService.getVersion();
log.info(
"Operator {} (commit: {}) built on {} starting…",
version.getProject(),
version.getCommit(),
version.getBuiltTime());
}

public <R extends CustomResource> void register(ResourceController<R> controller)
throws OperatorException {
register(controller, null);
Expand All @@ -49,55 +57,57 @@ public <R extends CustomResource> void register(
if (configuration == null) {
configuration = existing;
}

final var retry = GenericRetry.fromConfiguration(configuration.getRetryConfiguration());
final var targetNamespaces = configuration.getNamespaces().toArray(new String[] {});
registerController(controller, configuration.watchAllNamespaces(), retry, targetNamespaces);
}
}
Class<R> resClass = configuration.getCustomResourceClass();
String finalizer = configuration.getFinalizer();
final var client = k8sClient.customResources(resClass);
EventDispatcher dispatcher =
new EventDispatcher(
controller, finalizer, new EventDispatcher.CustomResourceFacade(client));

@SuppressWarnings("rawtypes")
private <R extends CustomResource> void registerController(
ResourceController<R> controller,
boolean watchAllNamespaces,
Retry retry,
String... targetNamespaces)
throws OperatorException {
final var configuration = configurationService.getConfigurationFor(controller);
Class<R> resClass = configuration.getCustomResourceClass();
String finalizer = configuration.getFinalizer();
MixedOperation client = k8sClient.customResources(resClass);
EventDispatcher eventDispatcher =
new EventDispatcher(
controller, finalizer, new EventDispatcher.CustomResourceFacade(client));
// check that the custom resource is known by the cluster
final var crdName = configuration.getCRDName();
final var crd =
k8sClient.apiextensions().v1().customResourceDefinitions().withName(crdName).get();
final var controllerName = configuration.getName();
if (crd == null) {
log.warn(
"'{}' CRD was not found on the {} cluster, skipping '{}' controller registration",
crdName,
configurationService.getClientConfiguration().getMasterUrl(),
controllerName);
return;
}

CustomResourceCache customResourceCache = new CustomResourceCache();
DefaultEventHandler defaultEventHandler =
new DefaultEventHandler(
customResourceCache, eventDispatcher, controller.getClass().getName(), retry);
DefaultEventSourceManager eventSourceManager =
new DefaultEventSourceManager(defaultEventHandler, retry != null);
defaultEventHandler.setEventSourceManager(eventSourceManager);
eventDispatcher.setEventSourceManager(eventSourceManager);
CustomResourceCache customResourceCache = new CustomResourceCache();
DefaultEventHandler defaultEventHandler =
new DefaultEventHandler(customResourceCache, dispatcher, controllerName, retry);
DefaultEventSourceManager eventSourceManager =
new DefaultEventSourceManager(defaultEventHandler, retry != null);
defaultEventHandler.setEventSourceManager(eventSourceManager);
dispatcher.setEventSourceManager(eventSourceManager);

controller.init(eventSourceManager);
CustomResourceEventSource customResourceEventSource =
createCustomResourceEventSource(
client,
customResourceCache,
watchAllNamespaces,
targetNamespaces,
defaultEventHandler,
configuration.isGenerationAware(),
finalizer);
eventSourceManager.registerCustomResourceEventSource(customResourceEventSource);
controller.init(eventSourceManager);
final boolean watchAllNamespaces = configuration.watchAllNamespaces();
CustomResourceEventSource customResourceEventSource =
createCustomResourceEventSource(
client,
customResourceCache,
watchAllNamespaces,
targetNamespaces,
defaultEventHandler,
configuration.isGenerationAware(),
finalizer);
eventSourceManager.registerCustomResourceEventSource(customResourceEventSource);

log.info(
"Registered Controller: '{}' for CRD: '{}' for namespaces: {}",
controller.getClass().getSimpleName(),
resClass,
targetNamespaces.length == 0
? "[all/client namespace]"
: Arrays.toString(targetNamespaces));
log.info(
"Registered Controller: '{}' for CRD: '{}' for namespaces: {}",
controller.getClass().getSimpleName(),
resClass,
watchAllNamespaces ? "[all namespaces]" : Arrays.toString(targetNamespaces));
}
}

private CustomResourceEventSource createCustomResourceEventSource(
Expand Down
10 changes: 10 additions & 0 deletions ...re/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
public abstract class AbstractConfigurationService implements ConfigurationService {

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

public AbstractConfigurationService(Version version) {
this.version = version;
}

protected <R extends CustomResource> void register(ControllerConfiguration<R> config) {
final var name = config.getName();
Expand Down Expand Up @@ -41,4 +46,9 @@ public <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor
public Set<String> getKnownControllerNames() {
return configurations.keySet();
}

@Override
public Version getVersion() {
return version;
}
}
2 changes: 2 additions & 0 deletions ...ework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ default Config getClientConfiguration() {
}

Set<String> getKnownControllerNames();

Version getVersion();
}
36 changes: 36 additions & 0 deletions operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package io.javaoperatorsdk.operator.api.config;

import java.io.IOException;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.time.Instant;
import java.util.Date;
import java.util.Properties;

public class Utils {

public static Version loadFromProperties() {
final var is =
Thread.currentThread().getContextClassLoader().getResourceAsStream("version.properties");
final var properties = new Properties();
try {
properties.load(is);
} catch (IOException e) {
e.printStackTrace();
}

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"));
} catch (ParseException e) {
builtTime = Date.from(Instant.EPOCH);
}
return new Version(
properties.getProperty("git.build.version", "unknown"),
properties.getProperty("git.commit.id.abbrev", "unknown"),
builtTime);
}
}
27 changes: 27 additions & 0 deletions operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Version.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.javaoperatorsdk.operator.api.config;

import java.util.Date;

public class Version {
Copy link
Collaborator

@adam-sandor adam-sandor Jan 25, 2021

Choose a reason for hiding this comment

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

immutable classes 🧡

private final String project;
private final String commit;
private final Date builtTime;

public Version(String project, String commit, Date builtTime) {
this.project = project;
Copy link
Collaborator

@csviri csviri Jan 25, 2021

Choose a reason for hiding this comment

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

@metacosm What is project stands for exactly? Isn't this information inherently redundant?

Copy link
Collaborator

@csviri csviri Jan 25, 2021

Choose a reason for hiding this comment

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

Or is this meant to be like the application name is spring boot?

Copy link
Collaborator

@csviri csviri Jan 26, 2021

Choose a reason for hiding this comment

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

@metacosm still not sure about why is this? this seems to me some opionated way. Especially with build time and project. Commit id is fine? Is there some deeper meening of this, or its required by quarkus?

Copy link
Collaborator Author

@metacosm metacosm Jan 26, 2021

Choose a reason for hiding this comment

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

@csviri Maybe a better name would be helpful, indeed. This corresponds to the Maven project version. Why would it be redundant? The goal is to make sure that the operator is running the SDK version the user thinks it's running which can be helpful to diagnose issues.

Copy link
Collaborator

@csviri csviri Jan 26, 2021

Choose a reason for hiding this comment

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

I see, makes sense, but isn't the commit id enough for that, like log "running sdk version: {{commit id}}" . Or rather even the release version, thus version of the dependency.

Copy link
Collaborator Author

@metacosm metacosm Jan 26, 2021

Choose a reason for hiding this comment

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

I think the project version is useful as a first check because it's not as easy to figure out which version of the SDK you're running solely from the commit id.

Copy link
Collaborator

@csviri csviri Jan 26, 2021

Choose a reason for hiding this comment

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

Absolutely, I agree, I just not see any field for real version (in terms of semantic versioning), just commit id and time stamp. Or am I missing something?

Copy link
Collaborator Author

@metacosm metacosm Jan 26, 2021

Choose a reason for hiding this comment

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

Why wouldn't the sdk version be useful? If you have a running operator on a cluster, it's easier to look at the logs than try to figure out which version of the code was used to build the image. Or make sure that the image is actually running the version of the code you think it is… 😄

Copy link
Collaborator

@csviri csviri Jan 26, 2021

Choose a reason for hiding this comment

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

I think its useful probably just the naming was confusing, so by "project" you meant the SDK Version right? I think that was the only issue, thus was not obviouse what it is.

Copy link
Collaborator Author

@metacosm metacosm Jan 26, 2021

Choose a reason for hiding this comment

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

Yeah, I renamed it to getSdkVersion

this.commit = commit;
this.builtTime = builtTime;
}

public String getProject() {
return project;
}

public String getCommit() {
return commit;
}

public Date getBuiltTime() {
return builtTime;
}
}
Loading