Error when using multiple dependent resources of the same class

[derlin]

Bug Report

What did you do?

I am using the JOSDK through quarkus-operator-sdk. I have a specific use case that I explained in #1437. The code is available at https://github.com/derlin/josdk-operator-dependent-crs/blob/main/src/main/java/ch/derlin/configreconciler/DbReconciler.java .

Now, I am trying to migrate to quarkus-operator-sdk 5.X, which updates JOSDK from 3.X to 4.X. The difficulty comes from the fact that I have two different event sources for the same class Secret.

My event sources are prepared like this. The createInformer method creates its own event source, while the dbSecret is an instance of CRUDKubernetesDependentResource<Secret, Db>:

  @Override
  public Map<String, EventSource> prepareEventSources(EventSourceContext<Db> context) {
    Map<String, EventSource> eventSources = Map.of(
        CONFIG_INFORMER, createInformer(context, Config.class, CONFIG_INFORMER),
        CREDENTIALS_SECRET_INFORMER, createInformer(context, Secret.class, CREDENTIALS_SECRET_INFORMER, CREDENTIALS_SECRET_LABEL),
        DB_SECRET_INFORMER, dbSecret.initEventSource(context)
    );
    dbSecret.useEventSourceWithName(DB_SECRET_INFORMER); // <- added for 4.X migration
    return eventSources;
  }

In my reconciliation, I have the following:

  @Override
  public UpdateControl<Db> reconcile(Db resource, Context<Db> context) {
    // ...

    // ⮕ .getSecondaryResource deprecated, how to do the same in 4.X ?
    var config = context.getSecondaryResource(Config.class).orElseThrow(() ->
        new ValidationException("Missing config"));
    var credentialsSecret = context.getSecondaryResource(Secret.class, CREDENTIALS_SECRET_INFORMER).orElseThrow(() ->
        new ValidationException("Missing credentials secret"));

    dbSecret.reconcile(resource, context);  //  ⮕ THROWING EXCEPTION
    var secret = context.getSecondaryResource(Secret.class, DB_SECRET_INFORMER).orElseThrow();
    
    // ...
  }

This was working fine with the old version but with 4.X I get the following exception when calling dbSecret.reconcile(resource, context):

Error during event processing ExecutionScope{ resource id: ResourceID{name='example-db', namespace='upgrade'}, version: 340777} failed.: io.javaoperatorsdk.operator.OperatorException: java.lang.IllegalArgumentException: There are multiple EventSources registered for type io.fabric8.kubernetes.api.model.Secret, you need to provide a name to specify which EventSource you want to query. Known names: dependent_db_secret,dependent_credentials_secret
	at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.lambda$timeControllerExecution$0(MicrometerMetrics.java:58)
	at io.micrometer.core.instrument.composite.CompositeTimer.record(CompositeTimer.java:65)
	at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.timeControllerExecution(MicrometerMetrics.java:54)
	at io.javaoperatorsdk.operator.processing.Controller.reconcile(Controller.java:93)
	at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.reconcileExecution(ReconciliationDispatcher.java:130)
	at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleReconcile(ReconciliationDispatcher.java:110)
	at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleDispatch(ReconciliationDispatcher.java:81)
	at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleExecution(ReconciliationDispatcher.java:54)
	at io.javaoperatorsdk.operator.processing.event.EventProcessor$ReconcilerExecutor.run(EventProcessor.java:406)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.lang.IllegalArgumentException: There are multiple EventSources registered for type io.fabric8.kubernetes.api.model.Secret, you need to provide a name to specify which EventSource you want to query. Known names: dependent_db_secret,dependent_credentials_secret
	at io.javaoperatorsdk.operator.processing.event.EventSources.get(EventSources.java:118)
	at io.javaoperatorsdk.operator.processing.event.EventSourceManager.getResourceEventSourceFor(EventSourceManager.java:199)
	at io.javaoperatorsdk.operator.api.reconciler.DefaultContext.getSecondaryResource(DefaultContext.java:47)
	at io.javaoperatorsdk.operator.api.reconciler.Context.getSecondaryResource(Context.java:16)
	at io.javaoperatorsdk.operator.processing.dependent.AbstractDependentResource.getSecondaryResource(AbstractDependentResource.java:92)
	at io.javaoperatorsdk.operator.processing.dependent.SingleDependentResourceReconciler.reconcile(SingleDependentResourceReconciler.java:18)
	at io.javaoperatorsdk.operator.processing.dependent.AbstractDependentResource.reconcile(AbstractDependentResource.java:50)
	at ch.derlin.configreconciler.DbReconciler.reconcile(DbReconciler.java:77)
	at ch.derlin.configreconciler.DbReconciler.reconcile(DbReconciler.java:34)
	at ch.derlin.configreconciler.DbReconciler_ClientProxy.reconcile(Unknown Source)
	at io.javaoperatorsdk.operator.processing.Controller$1.execute(Controller.java:136)
	at io.javaoperatorsdk.operator.processing.Controller$1.execute(Controller.java:94)
	at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.lambda$timeControllerExecution$0(MicrometerMetrics.java:56)
	... 11 more

I added the line dbSecret.useEventSourceWithName() (see first code snippet) to try to avoid this error, to no avail.

SIDE NOTE: I also have no idea how to do the getSecondaryResource with the new implementation, any hint ?

What did you expect to see?

I would expect the code to keep working, meaning I am able to reconcile my dbSecret (and also fetch my secondary resources) as before.

What did you see instead? Under which circumstances?

I see the exception (see above):

java.lang.IllegalArgumentException: There are multiple EventSources registered for type io.fabric8.kubernetes.api.model.Secret, you need to provide a name to specify which EventSource you want to query. Known names: custom_secret,dependent_secret

Environment

I am using K3D for testing (vanilla kubernetes).

$ Mention java-operator-sdk version from pom.xml file

    <dependency>
      <groupId>io.quarkiverse.operatorsdk</groupId>
      <artifactId>quarkus-operator-sdk</artifactId>
     <version>5.0.0.Beta2</version>
    </dependency>

    <dependency>
      <groupId>io.javaoperatorsdk</groupId>
      <artifactId>operator-framework-core</artifactId>
      <version>4.1.1</version>
   </dependency>

$ java -version

java -version
openjdk version "11.0.16.1" 2022-08-12
OpenJDK Runtime Environment Homebrew (build 11.0.16.1+0)
OpenJDK 64-Bit Server VM Homebrew (build 11.0.16.1+0, mixed mode)

$ kubectl version

kubectl version --short
Flag --short has been deprecated, and will be removed in the future. The --short output will become the default.
Client Version: v1.25.4
Kustomize Version: v4.5.7
Server Version: v1.24.4+k3s1

Possible Solution

I think the problem comes from https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/SingleDependentResourceReconciler.java#L18 where the .getSecondaryResource() doesn't use any discriminator / doesn't look for the event source name.

Additional context

I already shared my use case in this issue #1437. A minimal reproducible example is available at https://github.com/derlin/josdk-operator-dependent-crs/tree/quarkus-operator-5.X :

• branch main ⮕ working version with 3.X
• branch quarkus-operator-5.X ⮕ failing version with 4.X

[derlin]

any news on this?

[csviri]

Hi @derlin ,

this seems to be suspicious, you are both initializing and using the event source of dbSecret, that is at least not intended.

  @Override
  public Map<String, EventSource> prepareEventSources(EventSourceContext<Db> context) {
    Map<String, EventSource> eventSources = Map.of(
        CONFIG_INFORMER, createInformer(context, Config.class, CONFIG_INFORMER),
        CREDENTIALS_SECRET_INFORMER, createInformer(context, Secret.class, CREDENTIALS_SECRET_INFORMER, CREDENTIALS_SECRET_LABEL),
        DB_SECRET_INFORMER, dbSecret.initEventSource(context)
    );
    dbSecret.useEventSourceWithName(DB_SECRET_INFORMER); // <- added for 4.X migration
    return eventSources;
  }

What you should do is create an informer and use that in all the dependent resource of same type.

So maybe just remoce DB_SECRET_INFORMER, dbSecret.initEventSource(context); and
dbSecret.useEventSourceWithName(CREDENTIALS_SECRET_INFORMER);

Maybe rename CREDENTIALS_SECRET_INFORMER to something more generic.

[csviri]

I created an issue: #1691

Will take a look on this soon, remember something that it was an issue in the past to properly support multiple informers in same types in all cases; But I don't see now why it would be a problem.

Is there a particular reason why you cant use just one informer for both the Secret? @derlin

[derlin]

@csviri What do you mean ? Both sources are for secrets, but with completely different configuration.

The first one, credentials secret, looks for a specific label and a namespace+name matching the namespace+name in the CR spec spec.configRef. The second one, db secret, matches the namespace+name of the cr, but the name is suffixed with -db. There is one db secret per cr, but the same cr may reference the same credentials secrets (one-to-one vs many-to-one).

How else would you configure this if not using separate informers ?

Also remember that this configuration worked fine prior to the major update, so I don't think it was a problem before.

[csviri]

There could be just one Informer for all secretes a proper label selector.

Also remember that this configuration worked fine prior to the major update

there is a lot going in the background, but will take a look and try to make it as generic as possible, ideally covering this case

[csviri]

What you can do is to set a resourceDiscriminator that will select the target resource atm, see the releated implementation when things go bad for you:

https://github.com/java-operator-sdk/java-operator-sdk/blob/256fcc50bb62b404d784dda09afbe05bd633e9d8/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java#L93-L96

Just, to give a little insight, discriminators were designed for these purposes. But mainly for multiple resources with same type, should work also here. (Hopefully will have time to take a look in tomorrow into it deeper.)

[csviri]

@derlin
pls check PR: #1698
This shows how to do this.

Note that before it was no possible to manage this if there were multiple resources with same type in the informer. That is why the discriminator.

Pls let us know if this helps.

[derlin]

@csviri happy new year !

I had a look at the PR, thank you for the example on how to use discriminators. I tried using them and it works: see this commit. However, I wasn't able to get rid of the two separate informers for Secrets as you suggested. Do you have any tips on this ?

I also failed to use read-only dependent resources. It says in the docs:

if your implementation doesn’t implement Creator, for example, AbstractDependentResource will never try to create the associated secondary resource, even if it doesn’t exist. It is possible to not implement any of these traits and therefore create read-only dependent resources that will trigger your reconciler whenever a user interacts with them but that are never modified by your reconciler itself.

When subclassing AbstractDependentResource, there is no initEventSource or anything related to event source, so how do I create the informer ? Am I supposed to do it manually ? In this case, what is the purpose of the abstract class?

The docs also mentions AbstractSimpleDependentResource, but this is a dead link / it doesn't exist.

[csviri]

Hi @derlin , happy new year!

I wasn't able to get rid of the two separate informers for Secrets as you suggested. Do you have any tips on this ?

Usually if you just watch one resource from one namespace and an other from another namespace is fine also. Alternatively you can have a cluster scoped InformerEventSource, or with a selected namespaces - that is also supported. Ideally resources are filtered by label. There is not much can be done more then these.

Will update also the docs but:

1. We had few discussions around this, but in general you don't need a read only dependent resource. It's enough to have an event source, that will make sure that resource is cached. There might be some situations that it would "look nice", like when you want to add a pre-condition on a read only resource, but that can be also just done using an event source.

2. AbstractSimpleDependentResource is removed, will update the docs.

Note that you usually don't extend the AbstractDependentResource, there is the AbstractEventSourceHolderDependentResource. This ha the required method.

Note that the highest level abstraction we are using in the workflows is DependentResource

You can even implement a custom hierarchy for this interface.

But right this needs to be better documented, will create an issue, and update the docs also regarding to this.

[derlin]

Thank you for the explanations and the (soon-to-be) updated docs!

One more question (sorry for the spam ^^'). In my case, the config and credentials secrets are both resources that are used by many dbs (akka primary resources). That is, multiple db/primary may refer to the same secondary resource(s).

I am currently creating an InformerEventSource manually and use the context.getPrimaryCache. Something like this:

context.getPrimaryCache().addIndexer(indexName, db -> List.of(indexKey(db)));

var informerConfiguration = InformerConfiguration.from(cls)
    .withPrimaryToSecondaryMapper((Db db) -> Set.of(new ResourceID(
        db.getSpec().getConfigRef().getName(),
        db.getSpec().getConfigRef().getNamespace())))
    .withSecondaryToPrimaryMapper(
        secondary -> context
            .getPrimaryCache()
            .byIndex(indexName, indexKey(secondary))
            .stream()
            .map(ResourceID::fromResource)
            .collect(Collectors.toSet()))
    .build();

I tried to move this into a dependent resource in multiple ways, but here is one of the latest iterations::

@Slf4j
public class DependentConfig extends KubernetesDependentResource<Config, Db>
 implements SecondaryToPrimaryMapper<Config>,   PrimaryToSecondaryMapper<Db> {

  public static final String INFORMER_NAME = "dependent_config";
  private IndexerResourceCache<Db> cache; // ⬅ don't know how to access it otherwise

  public DependentConfig(KubernetesClient kubernetesClient) {
    super(Config.class);
    setKubernetesClient(kubernetesClient);
  }

  @Override
  protected InformerEventSource<Config, Db> createEventSource(EventSourceContext<Db> context) {
    this.cache = context.getPrimaryCache(); // ⬅ get a reference for later
    cache.addIndexer(INFORMER_NAME,
        db -> List.of(db.getSpec().getConfigRef().getNamespace() + "#" + db.getSpec().getConfigRef().getName()));
    return super.createEventSource(context);
  }

  @Override
  public Set<ResourceID> toSecondaryResourceIDs(Db db) {
    return Set.of(new ResourceID(
        db.getSpec().getConfigRef().getName(),
        db.getSpec().getConfigRef().getNamespace()));
  }

  @Override
  public Set<ResourceID> toPrimaryResourceIDs(Config config) {
    return cache
        .byIndex(INFORMER_NAME, config.getMetadata().getNamespace() + "#" + config.getMetadata().getName())
        .stream().map(ResourceID::fromResource) .collect(Collectors.toSet());
  }
}

Using a dependent resource for the config like above, if I start the operator and there is already an existing config + db, all is fine (the reconcile method is able to find the config). If I create a db after the operator has started, then it cannot find the secondary resource / the config anymore. If I restart, it is fine again.

I guess this means the informer cache is correctly initialized, but not updated. But why ? I don't see exactly the difference between the dependent implementation and the one where I create the informer event source manually. Did I miss something ?

[csviri]

@derlin yes, this is not supported for reason explained here:
#1703

Basically the only reason you might want to have a dependent resources for such is to have some reconcile precondition conditions - thus have it more nicely expressed.

So if you have such a use case, we can support this, but again, its just to have a condition on a read only resource usually. Will comment this on that issue, and reopen it for further discussion.

[derlin]

Ok thanks. The "manual" way seems to still work on 4.X, so as long as it doesn't break, I am fine with this way of doing. We can close this issue.

Thanks again for your time!