Make it possible to run Strimzi across all namespaces

[scholzj]

Type of change

• Enhancement / new feature

Description

This PR adds the ability to run Strimzi across all namespaces by setting the STRIMZI_NAMESPACE option to *.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

• Update/write design documentation in ./design
• Write tests
• Make sure all tests pass
• Update documentation
• Check RBAC rights for Kubernetes / OpenShift roles
• Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
• Reference relevant issue(s) and close them after merging
• Update CHANGELOG.md

[scholzj]

@tombentley The code works, but I would appreciate if you could review the changes before I write the tests etc. as I'm not sure this is the ideal way how to implement this. Maybe you will have some improvement ideas.

[tombentley]

The way we currently handle multiple namespaces is to use a separate verticle for each namespace. That has an advantage that each verticle has an independent context thread, so each Kafka cluster is somewhat isolated from the others. "Somewhat" because each verticle is using the same "kubernetes-ops-pool" worker pool, so the reality is that while each has it's own context thread when monitoring a lot of namespaces there could be contention for threads from that worker pool (and right now we can't configure the size of that pool).

This change allows * to watch all namespaces, but they're all using the same context thread. I would expect that if the number of namespaces and Kafka clusters is very large there would be contention for handling events on the single context thread.

It would not be ideal to have a difference in how we handle * and how we handle foo,bar.baz.

To make the handling for all namespaces the same as the handling for multiple namespaces we would need to watch the creation and deletion of namespaces (and regularly reconcile that), so we could have a verticle per namespace. I don't know whether that's allowed using the default RBAC rules, or whether we'd need some new ClusterRole, which would make the whole thing more difficult to install. The alternative (which I don't think is such a great idea) would be to change our handling for multiple namespaces to use a single verticle.

Finally, I think we need some validation of the config to so that watched namespaces are either foo,bar,baz or *, but not foo,bar,*.

[scholzj]

This change allows * to watch all namespaces, but they're all using the same context thread. I would expect that if the number of namespaces and Kafka clusters is very large there would be contention for handling events on the single context thread.

Interesting ... I see this exactly the other way around. In a cluster with large number of namespaces the approach with separate verticles would run us soon out of resources. I also do not think that this necessarily means that the CO running with * would handle too many clusters. It might handle just few of them.

To make the handling for all namespaces the same as the handling for multiple namespaces we would need to watch the creation and deletion of namespaces (and regularly reconcile that), so we could have a verticle per namespace. I don't know whether that's allowed using the default RBAC rules, or whether we'd need some new ClusterRole, which would make the whole thing more difficult to install. The alternative (which I don't think is such a great idea) would be to change our handling for multiple namespaces to use a single verticle.

Watching namespaces just because of this is IMHO quite a lot of effort and can possibly generate quite a lot of watches (imaging cluster with 100 namespaces where is just one actual cluster - we would have to watch and react on the namespaces and at the same time have the watches for each namespace. 100 namespaces == 401 watches).

Watching multiple specific namespaces with single verticle seems just like a way to make our code more complex. There is IMHO no API call to watch just some namespaces. So there would be a lot of workarounds in the code just to handle this case where as for the * it is a simple if else which handles it.

Finally, I think we need some validation of the config to so that watched namespaces are either foo,bar,baz or , but not foo,bar,.

This makes definitely sense.

[tombentley]

I suspect that vertx would be fine with many verticles and would distribute them across the available cores. But needing lots of watches is a possible problem. But I'm not sure I understand where you get 401 from. Just one watch for namespaces being created and deleted, and one for Kafka resources in each namespace = 101.

But to be honest it's difficult to reason about the right way of doing this without knowing the sorts of scales people will be running at. My concern is more about problems arising from having two different ways of watching multiple namespaces.

[scholzj]

We have 4 watches per namespace. Plus one for the namespaces. So with 100 namespaces, you have 401 watches.

…

On Mon, Jan 28, 2019, 11:43 Tom Bentley ***@***.*** wrote: I suspect that vertx would be fine with many verticles and would distribute them across the available cores. But needing lots of watches is a possible problem. But I'm not sure I understand where you get 401 from. Just one watch for namespaces being created and deleted, and one for Kafka resources in each namespace = 101. But to be honest it's difficult to reason about the right way of doing this without knowing the sorts of scales people will be running at. My concern is more about problems arising from having two different ways of watching multiple namespaces. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1261 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFZXRz2MeODkwMy5_zNaR9a1pXzeeUqbks5vHtQ7gaJpZM4aUm02> .

[tombentley]

One for each of Kafka, KafkaConnect, KafkaConnectS2I and KafkaMirrorMaker, you mean? It makes we wonder why someone would want to watch every NS for a KafkaMirrorMaker. I would imagine a typical user would only create a KafkaMirrorMaker in a NS with a pre-existing Kafka. So I wonder whether we really need all those 400 in the typical ways the people would use the cluster operator.

But right now you're right, and that really wouldn't appear to scale well.

[scholzj]

Well, I guess it is only a question of time until you run into a user who wants to run mirror maker in separate namespace. Anyway - what would you do with it? Setup the MirrorMaker watch only into namespaces where Kafka runs? That would sound to me like a lot of effort with very little gain. And there is probably no reason to assume that the same would apply for Connect. So you end up with slightly less watches, but not significantly less.

While I think this discussion is useful for the long term, we also need to think about the short term. IMHO short-term there is nothing better than what I drafter in this PR. The only question IMHO is whether the handling of * should be in common operator classes or in cluster-operator classes.

[tombentley]

I guess I'm OK with using the single verticle for *. It's not like it ties us to doing it this way forever, or using some other wildcard operator to get different semantics.

[ppatierno]

A couple of questions ...

1. After starting the CO specifying to watch all namespaces with *, what happens if the cluster admin creates a new namespace and he wants the CO starting to watch it as well? As far as I understood, the CO needs to be restarted, right? (if we don't add a namespace watcher)
2. Talking about having one verticle per namespace, let me think that if you have all namespaces listed "foo,bar,biz, ...." or just "*" will drive to two different ways to run: first case multiple verticles, last one just one verticle. In terms of CPU cores allocation for verticles they are different so maybe the performance could be different. Should we try to reduce the number of verticles "grouping the namespace", so having one verticle not watching just one but a group of them so reducing the number of verticles?

[scholzj]

After starting the CO specifying to watch all namespaces with *, what happens if the cluster admin creates a new namespace and he wants the CO starting to watch it as well? As far as I understood, the CO needs to be restarted, right? (if we don't add a namespace watcher)

This PR uses a watcher which watches the whole cluster. It should not need to do anything when new namespace is added as there is always only one watch for all namespaces.

Talking about having one verticle per namespace, let me think that if you have all namespaces listed "foo,bar,biz, ...." or just "*" will drive to two different ways to run: first case multiple verticles, last one just one verticle. In terms of CPU cores allocation for verticles they are different so maybe the performance could be different. Should we try to reduce the number of verticles "grouping the namespace", so having one verticle not watching just one but a group of them so reducing the number of verticles?

This can be done, but it would be a bit complicated and is largely unrelated to how this PR solves it since it doesn't open any watches per namespaces but watches per cluster.

[ppatierno]

This PR uses a watcher which watches the whole cluster.

Should the CO has the rights for doing so? I mean in the installation file doesn't it need the right for watching namespaces in his cluster role?

... doesn't open any watches per namespaces but watches per cluster.

Which cluster? OpenShift? Kafka? We have 4 watches per namespaces as you mentioned before right? Maybe I didn't get what you meant.

[scholzj]

Should the CO has the rights for doing so? I mean in the installation file doesn't it need the right for watching namespaces in his cluster role?

It doesn't need any special rights for watching namespaces. It doesn't watch namespaces. It just tells Kubernetes Let me know if there is any change in Kafka resource in the whole cluster. Of course you need a general RBAC changes for that (such as basically changing all RoleBindings to ClusterRoleBindings). But you do not need to watch namespaces. The whole change is in using .inAnyNamespace()...watch(..) instead of inNamespace(namespace)...watch(...).

Which cluster? OpenShift? Kafka? We have 4 watches per namespaces as you mentioned before right? Maybe I didn't get what you meant.

In the whole OpenShift / Kubernetes cluster. Of course in total it is still 4 watches - one for each resource.

[scholzj]

I added some more tests. Could you please review @tombentley and @ppatierno? Thanks.

[tombentley]

nits

[ppatierno]

LGTM. Just a couple of nits.

[scholzj]

Anymore comments @tombentley?