Fixes #318

metacosm · 2021-01-22T22:26:30Z

Fixes #313
Fixes #314
Fixes #315
Fixes #317

Fixes #313 Fixes #314

Fixes #315

Fixes #317

adam-sandor · 2021-01-25T11:47:04Z

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Version.java

+
+import java.util.Date;
+
+public class Version {


immutable classes 🧡

adam-sandor · 2021-01-25T12:09:16Z

I think the Quarkus sample should be updated with the new start() method. Also it would be good to make it minimal.. only inject the necessary stuff, as it's a bit weird now, makes me think every time why do I need all those injected things.

adam-sandor · 2021-01-25T12:13:58Z

TomcatOperator fails to start:

Exception in thread "main" java.lang.ExceptionInInitializerError
	at io.javaoperatorsdk.operator.sample.TomcatOperator.main(TomcatOperator.java:25)
Caused by: java.lang.NullPointerException: inStream parameter is null
	at java.base/java.util.Objects.requireNonNull(Objects.java:233)
	at java.base/java.util.Properties.load(Properties.java:407)
	at io.javaoperatorsdk.operator.api.config.Utils.loadFromProperties(Utils.java:17)
	at io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService.<init>(DefaultConfigurationService.java:15)
	at io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService.<clinit>(DefaultConfigurationService.java:12)
	... 1 more

adam-sandor · 2021-01-25T12:20:32Z

Running the Quarkus sample I don't see any new output with git versions and stuff:

C:\Users\adams\.jdks\adopt-openjdk-15.0.1\bin\java.exe -Dmaven.multiModuleProjectDirectory=C:\Users\adams\IdeaProjects\java-operator-sdk\samples\quarkus "-Dmaven.home=C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.2\plugins\maven\lib\maven3" "-Dclassworlds.conf=C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.2\plugins\maven\lib\maven3\bin\m2.conf" "-Dmaven.ext.class.path=C:\Program Files\JetBrains\IntelliJ IDEA 2020.2.1\plugins\maven\lib\maven-event-listener.jar" "-javaagent:C:\Program Files\JetBrains\IntelliJ IDEA 2020.2.1\lib\idea_rt.jar=55507:C:\Program Files\JetBrains\IntelliJ IDEA 2020.2.1\bin" -Dfile.encoding=UTF-8 -classpath "C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.2\plugins\maven\lib\maven3\boot\plexus-classworlds-2.6.0.jar;C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.2\plugins\maven\lib\maven3\boot\plexus-classworlds.license" org.codehaus.classworlds.Launcher -Didea.version=2020.3.1 -DskipTests=true io.quarkus:quarkus-maven-plugin:1.11.0.Final:dev
[INFO] Scanning for projects...
[INFO] 
[INFO] -------< io.javaoperatorsdk:operator-framework-samples-quarkus >--------
[INFO] Building Operator SDK - Samples - Quarkus 1.7.1-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- quarkus-maven-plugin:1.11.0.Final:dev (default-cli) @ operator-framework-samples-quarkus ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\Users\adams\IdeaProjects\java-operator-sdk\samples\quarkus\src\main\resources
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 1 source file to C:\Users\adams\IdeaProjects\java-operator-sdk\samples\quarkus\target\classes
[WARNING] [io.quarkus.bootstrap.devmode.DependenciesFilter] Local Quarkus extension dependency io.javaoperatorsdk:operator-framework-quarkus-extension will not be hot-reloadable
[WARNING] [io.quarkus.bootstrap.devmode.DependenciesFilter] Local Quarkus extension dependency io.javaoperatorsdk:operator-framework-core will not be hot-reloadable
Listening for transport dt_socket at address: 5005
[INFO] Checking for existing resources in: C:\Users\adams\IdeaProjects\java-operator-sdk\samples\quarkus\src\main\kubernetes.
__  ____  __  _____   ___  __ ____  ______ 
 --/ __ \/ / / / _ | / _ \/ //_/ / / / __/ 
 -/ /_/ / /_/ / __ |/ , _/ ,< / /_/ /\ \   
--\___\_\____/_/ |_/_/|_/_/|_|\____/___/   
2021-01-25 13:19:31,219 INFO  [io.jav.qua.ext.dep.QuarkusExtensionProcessor] (build-13) Processed 'io.javaoperatorsdk.operator.sample.CustomServiceController' controller named 'customservicecontroller' for 'customservices.sample.javaoperatorsdk' CR (version 'sample.javaoperatorsdk/v1')
2021-01-25 13:19:32,462 INFO  [io.quarkus] (Quarkus Main Thread) operator-framework-samples-quarkus 1.7.1-SNAPSHOT on JVM (powered by Quarkus 1.11.0.Final) started in 2.376s. Listening on: http://localhost:8080
2021-01-25 13:19:32,463 INFO  [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
2021-01-25 13:19:32,463 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, kubernetes, kubernetes-client, operator-sdk, smallrye-health]
2021-01-25 13:19:33,128 INFO  [io.jav.ope.Operator] (Quarkus Main Thread) Registered Controller: 'CustomServiceController_ClientProxy' for CRD: 'class io.javaoperatorsdk.operator.sample.CustomService' for namespaces: [all namespaces]
CR class: class io.javaoperatorsdk.operator.sample.CustomService

adam-sandor · 2021-01-25T12:22:30Z

About #315 - What's the new functionality there? Until now the Operator crashed if the CRD wasn't present. Does it just quietly not register the controller now?

metacosm · 2021-01-25T14:58:56Z

About #315 - What's the new functionality there? Until now the Operator crashed if the CRD wasn't present. Does it just quietly not register the controller now?

It logs a warning and skips the controller registration so that other controllers can still be registered and the operator proceeds.

metacosm · 2021-01-25T15:00:12Z

Running the Quarkus sample I don't see any new output with git versions and stuff:

C:\Users\adams\.jdks\adopt-openjdk-15.0.1\bin\java.exe -Dmaven.multiModuleProjectDirectory=C:\Users\adams\IdeaProjects\java-operator-sdk\samples\quarkus "-Dmaven.home=C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.2\plugins\maven\lib\maven3" "-Dclassworlds.conf=C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.2\plugins\maven\lib\maven3\bin\m2.conf" "-Dmaven.ext.class.path=C:\Program Files\JetBrains\IntelliJ IDEA 2020.2.1\plugins\maven\lib\maven-event-listener.jar" "-javaagent:C:\Program Files\JetBrains\IntelliJ IDEA 2020.2.1\lib\idea_rt.jar=55507:C:\Program Files\JetBrains\IntelliJ IDEA 2020.2.1\bin" -Dfile.encoding=UTF-8 -classpath "C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.2\plugins\maven\lib\maven3\boot\plexus-classworlds-2.6.0.jar;C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.2\plugins\maven\lib\maven3\boot\plexus-classworlds.license" org.codehaus.classworlds.Launcher -Didea.version=2020.3.1 -DskipTests=true io.quarkus:quarkus-maven-plugin:1.11.0.Final:dev
[INFO] Scanning for projects...
[INFO] 
[INFO] -------< io.javaoperatorsdk:operator-framework-samples-quarkus >--------
[INFO] Building Operator SDK - Samples - Quarkus 1.7.1-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- quarkus-maven-plugin:1.11.0.Final:dev (default-cli) @ operator-framework-samples-quarkus ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\Users\adams\IdeaProjects\java-operator-sdk\samples\quarkus\src\main\resources
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 1 source file to C:\Users\adams\IdeaProjects\java-operator-sdk\samples\quarkus\target\classes
[WARNING] [io.quarkus.bootstrap.devmode.DependenciesFilter] Local Quarkus extension dependency io.javaoperatorsdk:operator-framework-quarkus-extension will not be hot-reloadable
[WARNING] [io.quarkus.bootstrap.devmode.DependenciesFilter] Local Quarkus extension dependency io.javaoperatorsdk:operator-framework-core will not be hot-reloadable
Listening for transport dt_socket at address: 5005
[INFO] Checking for existing resources in: C:\Users\adams\IdeaProjects\java-operator-sdk\samples\quarkus\src\main\kubernetes.
__  ____  __  _____   ___  __ ____  ______ 
 --/ __ \/ / / / _ | / _ \/ //_/ / / / __/ 
 -/ /_/ / /_/ / __ |/ , _/ ,< / /_/ /\ \   
--\___\_\____/_/ |_/_/|_/_/|_|\____/___/   
2021-01-25 13:19:31,219 INFO  [io.jav.qua.ext.dep.QuarkusExtensionProcessor] (build-13) Processed 'io.javaoperatorsdk.operator.sample.CustomServiceController' controller named 'customservicecontroller' for 'customservices.sample.javaoperatorsdk' CR (version 'sample.javaoperatorsdk/v1')
2021-01-25 13:19:32,462 INFO  [io.quarkus] (Quarkus Main Thread) operator-framework-samples-quarkus 1.7.1-SNAPSHOT on JVM (powered by Quarkus 1.11.0.Final) started in 2.376s. Listening on: http://localhost:8080
2021-01-25 13:19:32,463 INFO  [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
2021-01-25 13:19:32,463 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, kubernetes, kubernetes-client, operator-sdk, smallrye-health]
2021-01-25 13:19:33,128 INFO  [io.jav.ope.Operator] (Quarkus Main Thread) Registered Controller: 'CustomServiceController_ClientProxy' for CRD: 'class io.javaoperatorsdk.operator.sample.CustomService' for namespaces: [all namespaces]
CR class: class io.javaoperatorsdk.operator.sample.CustomService

That's weird. Did you clean and recompile the whole project?

csviri · 2021-01-25T16:10:33Z

About #315 - What's the new functionality there? Until now the Operator crashed if the CRD wasn't present. Does it just quietly not register the controller now?

It logs a warning and skips the controller registration so that other controllers can still be registered and the operator proceeds.

Not sure that is a good thing, won't be better just fail? Follow the fail fast principle. If something in the code that intends to register a controller but it is not working, is a big issue, this means the operator is de facto not working as intended.

adam-sandor · 2021-01-25T16:30:24Z

About #315 - What's the new functionality there? Until now the Operator crashed if the CRD wasn't present. Does it just quietly not register the controller now?

It logs a warning and skips the controller registration so that other controllers can still be registered and the operator proceeds.

Not sure that is a good thing, won't be better just fail? Follow the fail fast principle. If something in the code that intends to register a controller but it is not working, is a big issue, this means the operator is de facto not working as intended.

I agree that fail fast is better then not registering a controller. Kubernetes is already so async it's hard to find what's causing an issue. This could lead to a situation where the admin says the operator is running while users are complaining that their CRs aren't working.

The reason for this is that the default configuration service relies on these annotations to be present to do its work. Note: Quarkus applications don't need to be annotated if no changes to the default configuration is needed.

metacosm · 2021-01-25T16:52:57Z

About #315 - What's the new functionality there? Until now the Operator crashed if the CRD wasn't present. Does it just quietly not register the controller now?

It logs a warning and skips the controller registration so that other controllers can still be registered and the operator proceeds.

Not sure that is a good thing, won't be better just fail? Follow the fail fast principle. If something in the code that intends to register a controller but it is not working, is a big issue, this means the operator is de facto not working as intended.

I agree that fail fast is better then not registering a controller. Kubernetes is already so async it's hard to find what's causing an issue. This could lead to a situation where the admin says the operator is running while users are complaining that their CRs aren't working.

That's a fair point, however in a Kubernetes environment, it might be tricky to diagnose the issue if you just crash the operator as it will cause the pod to restart all the time. Not sure what's the best solution here from an ops' perspective… 🤔

csviri · 2021-01-25T17:01:58Z

About #315 - What's the new functionality there? Until now the Operator crashed if the CRD wasn't present. Does it just quietly not register the controller now?

It logs a warning and skips the controller registration so that other controllers can still be registered and the operator proceeds.

Not sure that is a good thing, won't be better just fail? Follow the fail fast principle. If something in the code that intends to register a controller but it is not working, is a big issue, this means the operator is de facto not working as intended.

I agree that fail fast is better then not registering a controller. Kubernetes is already so async it's hard to find what's causing an issue. This could lead to a situation where the admin says the operator is running while users are complaining that their CRs aren't working.

That's a fair point, however in a Kubernetes environment, it might be tricky to diagnose the issue if you just crash the operator as it will cause the pod to restart all the time. Not sure what's the best solution here from an ops' perspective… 🤔

You can still check the logs of the pod if it started up and process exited, so that would not be a problem. (The problem is when the container cannot be started, in that case you don't see anything, but this is not this case.)

Fail fast when the CRD is not found instead of letting watchers throw a cryptic exception based on PR discussion. Fixes #315

metacosm · 2021-01-26T09:02:42Z

Can we merge this, @adam-sandor @csviri ?

csviri · 2021-01-26T09:30:09Z

@metacosm LGTM

csviri · 2021-01-25T18:59:11Z

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Version.java

+  private final Date builtTime;
+
+  public Version(String project, String commit, Date builtTime) {
+    this.project = project;


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

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

@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?

@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.

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.

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.

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?

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… 😄

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.

Yeah, I renamed it to getSdkVersion

adam-sandor · 2021-01-26T10:58:20Z

LGTM too

Note that the rename trickles down to several spots because Quarkus' @RecordableConstructor is sensitive to parameter names: the parameter names used in the constructor must match an existing getter.

metacosm self-assigned this Jan 22, 2021

metacosm requested review from csviri, s-soroosh and adam-sandor and removed request for csviri January 22, 2021 23:16

metacosm added 8 commits January 23, 2021 00:31

fix: use controller name instead of controller class name

e77f2dd

refactor: merge register and registerController methods

4277b3d

refactor: generify CustomResourceFacade

174097f

feat: add version information to configuration

8fa839e

feat: add start method as entrypoint and output version information

4eb057b

Fixes #313 Fixes #314

feat: skip controller registration if CRD is not present on cluster

f278c8f

Fixes #315

fix: date format parser

2db6b5f

fix: use finalizer and deletion marker methods from HasMetadata

042d1e4

Fixes #317

metacosm force-pushed the fixes branch from 24f20d5 to 042d1e4 Compare January 22, 2021 23:34

adam-sandor reviewed Jan 25, 2021

View reviewed changes

metacosm added 2 commits January 25, 2021 15:52

fix: Controller annotation is not mandatory for default configuration

b791c4e

feat: use Operator.start()

9edcde9

metacosm added 2 commits January 25, 2021 16:26

fix: log warning if version.properties cannot be found

538e468

docs: add some javadocs

9c0ccbd

fix: controllers need to be annotated with @controller

b9987a7

The reason for this is that the default configuration service relies on these annotations to be present to do its work. Note: Quarkus applications don't need to be annotated if no changes to the default configuration is needed.

fix: only keep **not** blank finalizer, not blank ones… 🤦

09ce889

metacosm added 2 commits January 25, 2021 19:26

feat: throw an explicit exception if CRD is not found

6cde0be

Fail fast when the CRD is not found instead of letting watchers throw a cryptic exception based on PR discussion. Fixes #315

fix: format

ebb453e

metacosm requested a review from adam-sandor January 26, 2021 08:29

csviri approved these changes Jan 26, 2021

View reviewed changes

adam-sandor approved these changes Jan 26, 2021

View reviewed changes

metacosm added 2 commits January 26, 2021 12:52

fix: replace ellipsis by ... for greater compatibility

d4d48cd

fix: rename getProject to getSdkVersion for greater clarity

7608163

Note that the rename trickles down to several spots because Quarkus' @RecordableConstructor is sensitive to parameter names: the parameter names used in the constructor must match an existing getter.

metacosm merged commit cb585d7 into master Jan 26, 2021

metacosm deleted the fixes branch January 26, 2021 13:30

Fixes #318

Fixes #318

Uh oh!

Conversation

metacosm commented Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adam-sandor commented Jan 25, 2021

Uh oh!

adam-sandor commented Jan 25, 2021

Uh oh!

adam-sandor commented Jan 25, 2021

Uh oh!

adam-sandor commented Jan 25, 2021

Uh oh!

metacosm commented Jan 25, 2021

Uh oh!

metacosm commented Jan 25, 2021

Uh oh!

csviri commented Jan 25, 2021

Uh oh!

adam-sandor commented Jan 25, 2021

Uh oh!

metacosm commented Jan 25, 2021

Uh oh!

csviri commented Jan 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metacosm commented Jan 26, 2021

Uh oh!

csviri commented Jan 26, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adam-sandor commented Jan 26, 2021

Uh oh!

Uh oh!

metacosm commented Jan 22, 2021 •

edited

Loading

csviri commented Jan 25, 2021 •

edited

Loading