Custom resource controller with `withLabel` filter not filtering resources

[xguerin]

Hello there,

I am not sure if it's a client library issue or a controller issue, but when creating a controller for a custom resource, it looks like the ~.withFilter()~ .withLabel() constraint is not respected.

The following example would trigger the onAddition callback on every pod in namespace ns regardless of the labels:

controller = new Controller<>(
    client.customResources(this.crd, CRD.class, CRDList.class, DoneableCRD.class)
          .inNamespace(ns)
          .withLabel("label", "hello"),
    new GenericEventQueueConsumer<>(new HashMap<>(), this));

The namespace filter, on the other hand, is respected. Interestingly, when calling the client function outside the controller, the filtering seems to work:

client.customResources(this.crd, CRD.class, CRDList.class, DoneableCRD.class)
      .inNamespace(ns)
      .withLabel("label", "hello")
      .list()
      .getItems();
-> []

Any idea? If it's a client issue I'll be happy to follow up with them.

Thanks,

[ljnelson]

(There is no withFilter() method that I know of; I assume that's just a typo and you meant withLabel().)

All I do in the controller, really, is call list() and getItems() once you boil everything else away. If you're using the latest released version, and not the snapshot you were perhaps testing earlier, it is conceivable that some of the egregious hacks I put in to make CRDs work are failing in some way—but I'd expect an error in that case.

[xguerin]

Ah yes, sorry, I mean withLabel. That was indeed with the latest release, not the snapshot. I am trying now with the updated libs.

[xguerin]

Yeah, no dice with the 0.2.3-SNAPSHOT and version 4.1.1 of the client. I'll post an issue on the client side.

[ljnelson]

I'm guessing it's an issue here if it works outside of the controller framework. I'll classify this issue as a bug and look into it.

[xguerin]

I'm gonna try to reproduce the problem with a bare bone Watcher and I'll let you know.

[ljnelson]

I've had a quick look, but can't see where I'm doing anything obviously wrong. As I suspected, I'm basically taking the "operation" in and calling list() on it:

https://github.com/microbean/microbean-kubernetes-controller/blob/38a46e12da34cca012bc5fd9bb5b5dcc81da9a3f/src/main/java/org/microbean/kubernetes/controller/Reflector.java#L832

Now, I am using the withResourceVersion("0") "filter":

https://github.com/microbean/microbean-kubernetes-controller/blob/38a46e12da34cca012bc5fd9bb5b5dcc81da9a3f/src/main/java/org/microbean/kubernetes/controller/Reflector.java#L413

Then I store the last resourceVersion used so I can use it when I set up the watch:

https://github.com/microbean/microbean-kubernetes-controller/blob/38a46e12da34cca012bc5fd9bb5b5dcc81da9a3f/src/main/java/org/microbean/kubernetes/controller/Reflector.java#L906-L914

Maybe that has something to do with it?

[xguerin]

Intriguing. So with the bare bone watcher, it behaves as expected. I simply took the WatchExample from here. CRD that don't have the label don't get any events while CRD that have the labels do. ~Let me gist you the complete code and edit this comment.~ Here you are.

[xguerin]

Yeah, I just added withResourceVersion to my example, and it apparently cancels the withLabel call as I get events for all CRDs. Looking at the code, it looks like this operation is destructive and replaces whatever Watchable it is given with a brand new Watchable with the custom resource version.

[ljnelson]

Well that sucks. Unless I'm missing something awful, that sounds very much like a bad bug in the fabric8 library. I'll double-check the javadocs (ho ho) to make sure.

{time passes}

https://static.javadoc.io/io.fabric8/kubernetes-client/4.1.3/io/fabric8/kubernetes/client/dsl/Versionable.html

Yeah…not so much there. Off to the source code.

[ljnelson]

Hmm; I do wonder if perhaps since technically this is a modification of the original operation I need more of a "copy the whole thing as it exists and then bolt withResourceVersion() onto it" kind of workflow here.

But I have tests that show this working with built in objects, I think, so perhaps this is another CRD oddity? I'd be curious if your gist would exhibit the same behavior if it listed and watched, say, Pods or ConfigMaps instead of CRDs.

[xguerin]

But I have tests that show this working with built in objects, I think, so perhaps this is another CRD oddity?

Yeah it is. I have other examples of label filtering working perfectly well for pods, for instance.

[xguerin]

Also, re: the source for withResourceVersion, labels are passed down to the new object so my original theory does not hold. It might very well be a bug with the control path down the line.

Another odd thing is that is is called on a FilterWatchListDeletable and returns a Watchable. Since its using getClass to reconstruct the new object, I was kind of expecting another FilterWatchListDeletable ... or not, given the signature of the method 😅

[xguerin]

So it's definitely a bug in the client library: CustomResourceOperationsImpl override the withResourceVersion method and returns a new CustomResourceOperationsImpl without using a HasMetadataOperation construction that accepts labels, etc.

The fix should be pretty straightforward. I'm gonna take a shot at it and see if that helps. Thanks for the insight!

[ljnelson]

OK; do watch out as I think there's a pull request open in that project that would do away with any hand-tooled classes related to CRDs…https://github.com/fabric8io/kubernetes-client/pull/1346

[xguerin]

Thanks for letting me know, although I don't think there is any conflict. Here is the proposed patch: https://github.com/fabric8io/kubernetes-client/pull/1425.

[xguerin]

Merged.