Informer creation behavior for custom resource with CRDContext in SharedInformerFactory

[LukaTchumburidze]

The factory has a method sharedIndexInformerForCustomResource to create an informer with a CRDContext for CRs. It's is using sharedInformerFor to register the informer inside the factory object. This causes a problem when registering new informers using several different CRDContexts, but using same set of classes (extending CustomResource and CustomResourceList). Case will result in creation of several informers, but registering only the one which is has been created the latest (since factory's internal map uses only classname as key).
Example:

        SharedInformerFactory factory = defaultKubernetesClient.informers();

        CustomResourceDefinitionContext firstCrdContext = getFirstCrdContext();
        CustomResourceDefinitionContext secondCrdContext = getSecondCrdContext();

        SharedIndexInformer<MyCustomResourceClass> lostIndexInformer = factory.sharedIndexInformerForCustomResource(
                firstCrdContext,
                MyCustomResourceClass.class,
                MyCustomResourceClassList.class,
                0);

        SharedIndexInformer<MyCustomResourceClass> savedIndexInformer = factory.sharedIndexInformerForCustomResource(
                secondCrdContext,
                MyCustomResourceClass.class,
                MyCustomResourceClassList.class,
                0);

        factory.startAllRegisteredInformers();

As I have said lostIndexInformer will be rewritten by the time we reach to startAllRegisteredInformers. I don't think this is the right behavior, since idea of passing the different CRDContexts to factory methods means that several informers should be registered and that they should operate on different CRs which has been described in CRDContext.

[rohanKanojia]

This issue also seems related to #2738

We're using type as the key for informer

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/SharedInformerFactory.java

Line 48 in 00d23e8

private final Map<Type, SharedIndexInformer> informers = new HashMap<>();

[rohanKanojia]

@LukaTchumburidze: Hi, Would it be possible for you to share contents for firstCrdContext and secondCrdContext?

[rohanKanojia]

Bdw, these methods accepting CustomResourceDefinitionContext in SharedInformerFactory were removed in #2667 and are not part of API since v5.1.0

But I still see a way to provide different versions by providing an OperationContext .However this doesn't work due to apiversion always getting picked from annotation:

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/SharedInformerFactory.java

Line 309 in 24a5fae

KubernetesDeserializer.registerCustomKind(HasMetadata.getApiVersion(apiTypeClass), apiTypeClass.getSimpleName(), apiTypeClass);

SharedIndexInformer<Bicycle> bicycleV1Informer = informerFactory.sharedIndexInformerForCustomResource(Bicycle.class, resyncPeriodMillis);
SharedIndexInformer<Bicycle> bicycleV1beta1Informer = informerFactory.sharedIndexInformerForCustomResource(Bicycle.class, new OperationContext().withApiGroupVersion("v1beta1"), resyncPeriodMillis);

bicycleV1beta1Informer.addEventHandler(new GenericResourceHandler<>("v1beta1-bicycle"));
bicycleV1Informer.addEventHandler(new GenericResourceHandler<>("v1-bicycle"));

Moreover having Type as the key doesn't only affect CustomResources. This informer lost behavior is also visible in standard Kubernetes resources:

SharedIndexInformer<Pod> podInDefaultNamespaceInformer = informerFactory.inNamespace("default")
  .sharedIndexInformerFor(Pod.class, resyncPeriodMillis);
SharedIndexInformer<Pod> podInKubePublicNamespaceInformer = informerFactory.inNamespace("kube-system")
  .sharedIndexInformerFor(Pod.class, resyncPeriodMillis); // Overwrites previous entry

I admit key should not be Type and should be a composite key depending upon resource's ApiGroup, ApiVersion, Namespace and Name

[LukaTchumburidze]

@rohanKanojia Since kubernetes-client's approach with CRDs has changed much during these months you're right that my example is obsolete.
However, you have also correctly remarked, that core issue still stays the same, hashmap's keys are being populated only according to Types.

I admit key should not be Type and should be a composite key depending upon resource's ApiGroup, ApiVersion, Namespace and Name

Currently factory does not support several OperatorContexts for one class as well. The key should contain OperationContext, since OperationContext might contain any queries needed for two informers to be distinguished from each other. I might be wrong on this one, but it can be checked.

[manusa]

Relates to: #2010