fix: runtime namespaces should always override buildtime values

[metacosm]

The previous behavior precluded operator-level defaulting at runtime
because setting the generate-with-watched-namespaces property was
marking the controller as having explicitly set namespaces. We now only
set that status if and only if namespaces were set using annotations.

Fixes #653

[metacosm]

/cc @Jamstah @shawkins

[shawkins]

Is it possible to have a variant of this that acts over all controllers?

[metacosm]

Is it possible to have a variant of this that acts over all controllers?

Not sure I understand the question

[shawkins]

Will this have to be specified for each controller?

[metacosm]

Will this have to be specified for each controller?

Using an operator-level default using quarkus.operator-sdk.namespaces at runtime should now work even if a controller uses the generate-with-watched-namespaces properties at build time. This is what https://github.com/quarkiverse/quarkus-operator-sdk/pull/654/files#diff-f84e1a6c265a5328f92e8c07e645a7c3b5685f6566c6ce9dc75a5927989d76c3R4-R5 and the associated test is checking.

Before this PR, https://github.com/quarkiverse/quarkus-operator-sdk/blob/main/integration-tests/src/test/java/io/quarkiverse/operatorsdk/it/OperatorSDKResourceTest.java#L117-L135 would fail because the namespaces would be the ones provided at build time. Now, it correctly uses the operator-level value specified at https://github.com/quarkiverse/quarkus-operator-sdk/blob/main/integration-tests/src/main/resources/application.properties#L1

[shawkins]

Sorry I mean could a variant of generate-with-watched-namespaces work over all controllers?

[metacosm]

Sorry I mean could a variant of generate-with-watched-namespaces work over all controllers?

Ah, that's a different issue, which we could indeed support. Can you open a ticket for it, please? Not sure if I'll get to it before my vacations, though.

[Jamstah]

This doesn't seem to have worked for me.

I built the keycloak operator with:

mvn clean package -Doperator -Dquarkus.container-image.build=true -Dquarkus.operator-sdk.controllers.keycloakrealmimportcontroller.generate-with-watched-namespaces=JOSDK_WATCH_CURRENT -Dquarkus.operator-sdk.controllers.keycloakcontroller.generate-with-watched-namespaces=JOSDK_WATCH_CURRENT

And run with the env var:

QUARKUS_OPERATOR_SDK_NAMESPACES=""

And I got:

2023-07-13 18:09:55,212 INFO [io.qua.ope.run.OperatorProducer] (main) Quarkus Java Operator SDK extension 6.2.2-SNAPSHOT (commit: a01a9fb on branch: fix-operator-level-namespaces) built on Thu Jul 13 17:20:47 GMT 2023
2023-07-13 18:09:55,368 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakrealmimportcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.realmimport.KeycloakRealmImport' for namespace(s): [openshift-operators]
2023-07-13 18:09:55,401 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.deployment.Keycloak' for namespace(s): [openshift-operators]
2023-

Does the runtime property have to be something other than the empty string perhaps?

[metacosm]

Does the runtime property have to be something other than the empty string perhaps?

Probably, yes, especially if it's set as an env variable because I think an empty value for an env var unsets it.

[Jamstah]

An empty envvar is different to an unset one:

jammy:~$ export ENVVAR=
jammy:~$ export | grep ENVVAR
declare -x ENVVAR=""
jammy:~$ unset ENVVAR
jammy:~$ export | grep ENVVAR
jammy:~$ 

[Jamstah]

Although I did a test setting:

QUARKUS_OPERATOR_SDK_NAMESPACES="jammy"

and it worked:

2023-07-13 18:50:54,259 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakrealmimportcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.realmimport.KeycloakRealmImport' for namespace(s): [jammy]
2023-07-13 18:50:54,282 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.deployment.Keycloak' for namespace(s): [jammy]

However, that doesn't work with OLM. The value OLM passes to tell the operator to watch all namespaces is the empty string, so while this PR helps, its not quite a complete solution to what we need.

[metacosm]

However, that doesn't work with OLM. The value OLM passes to tell the operator to watch all namespaces is the empty string, so while this PR helps, its not quite a complete solution to what we need.

Can you elaborate on this part, please? When you mean OLM, which part of OLM exactly? Can you open another issue with the details, please? I will merge this PR and we'll address other issues in subsequent PRs.

[Jamstah]

#656

[Jamstah]

And, thanks for this part :)