quarkiverse / quarkus-operator-sdk

Quarkus Extension to create Kubernetes Operators in Java using the Java Operator SDK (https://github.com/java-operator-sdk/java-operator-sdk) project
Apache License 2.0
120 stars 50 forks source link

Injected operator in tests ignores kubernetes client configuration from properties #612

Closed jcechace closed 1 year ago

jcechace commented 1 year ago

Kubernetes client of operator instance injected in tests is not properly configured as config properties are completely ignored.

Steps to reproduce

1) Locally set a namespace for current k8 context E.g.kubectl config set-context --current --namespace=debeziu 2) Run tests for pingpong example with debugger attached and inspect the values

image

As you can see the the injected client points to the default namespace whereas the client inside the inject operator points to the debezium namespace which comes from current k8 context. Additionally the client in injected operator will ignore config properties such as

quarkus.operator-sdk.namespaces=do-test-namespace
quarkus.kubernetes-client.namespace=do-test-namespace
jcechace commented 1 year ago

This is due to to the fact that the KubernetesClient instance obtained from QuarkusConfigurationService at OperatorProducer.java#L59 is null.

This in turn leads to Operator constructor creating a client via KubernetesClientBuilder which ignores all configuration properties here.

Viable fix might be adding a null check to the OperatorProducer

final var client = configuration.getClient() != null 
  ? configuration.getClient()
  : KubernetesClientUtils.createClient();
Operator operator = new Operator(configuration.getClient(), configuration);
metacosm commented 1 year ago

Is this happening in your custom code or is this only something you observed in the QOSDK samples? The reason I'm asking is that the samples are specifically configured to ignore the local kubernetes configuration for testing purposes using the Quarkus kubernetes dev services (this is controlled by https://github.com/quarkiverse/quarkus-operator-sdk/blob/237a1b68632f4b0c613be21e52a64ea3de789e4f/samples/pingpong/src/main/resources/application.properties#L5 in the pingpong sample). More information can be found at https://quarkus.io/guides/kubernetes-dev-services.

jcechace commented 1 year ago

It's something I've observed in my code too. Nevertheless if OperatorProducer is the source of the operator instance, then it should be quite clear that the k8 client created inside the operator constructor will ignore every and all configurations

jcechace commented 1 year ago

@metacosm moreover you are proving my point :). The injected client is not honoring the k8 context namespace, rather it uses default... However the client in the operator is still using the namespace from k8 context. This means it ignores that particular property too :).

metacosm commented 1 year ago

@metacosm moreover you are proving my point :). The injected client is not honoring the k8 context namespace, rather it uses default... However the client in the operator is still using the namespace from k8 context. This means it ignores that particular property too :).

What do you mean exactly by "the client in the operator"?

jcechace commented 1 year ago

Have a look at the attach screenshot... Operator.kubernetesClient. Or using different description. The client inside the injected Operator instance

jcechace commented 1 year ago

@metacosm so I did some additional research and dev services will have something to do with this. Few (to me) weird discoveries

1) When quarkus.kubernetes-client.devservices.override-kubeconfig=true then the client namespaces are configured as expected 2) When quarkus.kubernetes-client.devservices.override-kubeconfig=true then a breakpoint placed in OperatorProducer.operator method is intercepted (thus the Injected operator is created via the 2 argument constructor). What I find weird is that if quarkus.kubernetes-client.devservices.override-kubeconfig=false and local kubeconfig is present then this breakpoint is never intercepted and based on callstask it looks like the Operator instance is created via non-param constructor call.

metacosm commented 1 year ago

Using dev services, you do not need to set a client configuration to access the cluster, it should be automatically configured for you, i.e. a new cluster will be created and your code will get a client configured to access that cluster.

Now, the question is whether or not you're experiencing issues trying to access your own cluster during tests and if yes, using which set up. I'm trying to figure out what you're trying to achieve and what issue you're experiencing as a user. Please note under which circumstances the dev services will kick in or not based on the documentation I linked to in an earlier comment. Pointing to an example that is configured to use dev services that, as far as I'm aware, behaves as expected, doesn't help me help you :)

jcechace commented 1 year ago

Ok.. let's try it from the other side... What I want to achieve is the following in a test

1) Use a real cluster with existing kubeconfig (assume current namespace of this kube config is set to e.g. "debezium") 2) I want to explicitly set the default namespace for the KubernetesClient created via KubernetesClientUtils.create() 3) I want to explicitly set the namespace where the injected operator instance operates

Initially I thought that setting the following ought to be enough

quarkus.operator-sdk.namespaces=do-test-namespace
quarkus.kubernetes-client.namespace=do-test-namespace

However it clearly isn't as the injected operator instance internally holds a kubernetes client which will still have its namespace set to "debezium" instead of "do-test-namespace"

csviri commented 1 year ago

@jcechace pls ping me on discord we can take a look together. I create a sample, where injected client and operator. It seems that client default namespace is properly set using:

quarkus.kubernetes-client.namespace=do-test-namespace

this will make the default namespace set correclty in the client.

This won't set the default namespace in the client of the operator: quarkus.operator-sdk.namespaces=do-test-namespace (It is a different client instance naturally) However, this is not relevant for an operator, since the namespace in this case is explicitly used inside the controller implementations.

csviri commented 1 year ago

So in other words the client default namespace in operator and the defined namespace in config quarkus.operator-sdk.namespaces=do-test-namespace are not related in case you set a specific NS. The client's default only matters if you watch the current namespace - when you set JOSDK_WATCH_CURRENT for quarkus.operator-sdk.namespaces - this does not seems to be well document @metacosm

metacosm commented 1 year ago

quarkus.operator-sdk.namespaces=do-test-namespace means that you request all your reconcilers to watch this particular namespace by default, i.e. they will react to changes made to their target resource only when these changes happen to resources in this particular namespace. This happens regardless of how the kubernetes client injected in the operator is configured, assuming that the client is at least configured to access the same cluster, which is all that matters here. If you want to have finer grained control over which namespace(s) a given reconciler watches, please either use the @ControllerConfiguration annotation (namespaces field) or the quarkus.operator-sdk.controllers.<reconciler name>.namespaces property. That should address your third point.

The other points seem unrelated to the Java Operator SDK. In your operator code, the only thing that matters is that you get a client instance that can connect to your cluster. You can then request via the API of the client to operate on resources in any namespace you'd like so the original namespace with which the client is configured is of little concern in a typical operator.

I still struggle to understand what you're trying to achieve with these particular low-level steps, i.e. which user-level problem you're trying to address. Is your operator somehow not able to interact with your resources because the client is improperly configured? Could you explain more concretely what your setup is, i.e. how is your operator deployed (is it running locally or on a cluster? which namespace is it running on, if deployed on a cluster?) and how your reconciler(s) are configured and/or where do you expect the resources your operator operates on to live?

jcechace commented 1 year ago

@csviri apology for not getting back to you and postponing -- we are head over heels in the release process this week. I will definitively circle back to it.

However I think you've managed to answer my question. The reason why the operator in tests seems to actually watch just the current namespace is that my controller is annotated with

namespaces = Constants.WATCH_CURRENT_NAMESPACE

My mistake likely was thinking that setting either quarkus.operator-sdk.namespaces or (which would make more sense) quarkus.kubernetes-client.namespace in tests would also define what a current namespace is. The point is that in test environment you have no means of actually configuring this.

csviri commented 1 year ago

I think actually that should be the case, that quarkus.operator-sdk.namespaces should take a precedence over what is configured in the annotation.

jcechace commented 1 year ago

Closing as this was explained and doesn't seem to be a bug. Apology.