Skip to content
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.

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

Option to limit CRs the operator watches #453

Closed
lburgazzoli opened this issue Jul 12, 2021 · 10 comments · Fixed by #499
Closed

Option to limit CRs the operator watches #453

lburgazzoli opened this issue Jul 12, 2021 · 10 comments · Fixed by #499
Assignees

Comments

@lburgazzoli
Copy link
Collaborator

We should be able to restrict what CRs an operator handles, as example, by using a selector:

public @interface Controller(
    LabelSelector[] selectors() default {}
)

This could be required when a canary deployment of operators is required so at certain point in time, there will be more than one operator running in a cluster each one is taking into account a subset of the CRs.

@lburgazzoli
Copy link
Collaborator Author

@metacosm I have some spare cycles this week so if you need any help on this feature , I'd be happy to take a stab

@csviri
Copy link
Collaborator

csviri commented Aug 18, 2021

@lburgazzoli This sounds awsome!

@metacosm
Copy link
Collaborator

Ideally, however we implement this, it should boil down to generating a Predicate. Other aspects to keep in mind as well:

  • I don't think it will be possible (nor desirable, for that matter) to use LabelSelector on the Controller annotation
  • Whatever configuration we add to the annotation, it should be possible to have similar configuration in application properties as well

@lburgazzoli
Copy link
Collaborator Author

lburgazzoli commented Aug 18, 2021

I don't know if a predicate would be the best approach as then it would filter events after we receive them, instead a label selector (or anything similar) would configure the watcher to only watch some resources

I guess on quarkus a possible representation using properties could be:

quarkus.operator-sdk.controllers."foo".selectors[0] = "environment=production"
quarkus.operator-sdk.controllers."foo".selectors[1] = "tier!=frontend"

or

quarkus.operator-sdk.controllers."foo".selectors = "environment=production,tier!=frontend"

@metacosm
Copy link
Collaborator

I don't know if a predicate would be the best approach as then it would filter events after we receive them, instead a label selector (or anything similar) would configure the watcher to only watch some resources

Ah, right. Should we also filter on annotations?

I guess on quarkus a possible representation using properties could be:

quarkus.operator-sdk.controllers."foo".selectors[0] = "environment=production"
quarkus.operator-sdk.controllers."foo".selectors[1] = "tier!=frontend"

or

quarkus.operator-sdk.controllers."foo".selectors = "environment=production,tier!=frontend"

I'd prefer the second option (which I think Quarkus supports out of the box, iirc).

@lburgazzoli
Copy link
Collaborator Author

I don't know if a predicate would be the best approach as then it would filter events after we receive them, instead a label selector (or anything similar) would configure the watcher to only watch some resources

Ah, right. Should we also filter on annotations?

As far as I know, you can't use annotation to select objects, only labels can be used.

@csviri
Copy link
Collaborator

csviri commented Aug 19, 2021

Yep annotations are not indexed in etcd AFK, so we cannot filtern on them (at least efficiently).

I particulary like the idea to the the filtering on server side (K8S API) if possible based on labels, this is much more efficient, then doing it with the predicate in the operator. Although it's limited to labels, so we might need both if we want to be generic and cover every use case. But maybe just start with labels?

An interesting thing is that admission controllers can do this based on namespaces so namespace selector:
https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector
What we support to some extent, just like naming a list of namespaces, not based on labels. For canary deployments and testing purposes I think could be quite nice too, much more elegant then using static lists.

Agree with @metacosm that this should be dynamically configurable.

@lburgazzoli
Copy link
Collaborator Author

@metacosm @csviri I'm working on a POC for this issue so we can discuss about code and wonder if we could add some tests based on the KubernetesServer in addition to the mocked tests as it simplifies things a lot.

@metacosm
Copy link
Collaborator

I agree that we need to beef up our testing story and adding tests based on KubernetesServer would make sense to explore that space and see what we're missing in terms of testing support.

@csviri
Copy link
Collaborator

csviri commented Aug 19, 2021

Don't know much about that mock server, but worked with the one from kube builder - that was awsome (but its a real api server not really stubbing). For development purposes I'm happy personally with minukube / kind. Very happy to discuss this, maybe on weekly, improving tests is always a good thing :)

lburgazzoli added a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 19, 2021
lburgazzoli added a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 19, 2021
lburgazzoli added a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 19, 2021
lburgazzoli added a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 19, 2021
lburgazzoli added a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 19, 2021
lburgazzoli added a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 19, 2021
lburgazzoli added a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 20, 2021
lburgazzoli added a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 30, 2021
metacosm pushed a commit to lburgazzoli/java-operator-sdk that referenced this issue Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants