The Store API is not writable

[hibnico]

Describe the bug

I am updating the kubernetes client in my application from 5.4.1 to the latest 5.12.1. And the API changed, I cannot write in the store of the informer anymore. The workaround I have is to explicietly cast the Store instance into a Cache, to be able to use the add function.
My use case is that we have an external database where is stored all the history of the statuses of the pods launched by Argo. To be sure that we don't miss any status update, we use the informer pattern. And in case of error where our application is not up, but pods are deleted during that time, before starting the informer we prepopulate the cache with the last known list of running pods. This way, at startup we get notified of any missed deletion. This pattern was found described in this exemple in the go client:
https://github.com/kubernetes/client-go/blob/2f52a105e63e9ac7affb1a0f533c62883611ba81/examples/workqueue/main.go#L202

Looking at the history of the fabric8 client, I see that the APIs have been removed by #3091

Fabric8 Kubernetes Client version

other (please specify in additional context)

Steps to reproduce

Use the Store API.

Expected behavior

A writable Store API, like the go client has.

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

1.21.6

Environment

other (please specify in additional context)

Fabric8 Kubernetes Client Logs

No response

Additional context

No response

[shawkins]

Looking at the history of the fabric8 client, I see that the APIs have been removed by #3091

Yes that was intentional to avoid ad hoc modifications to the store state.

A writable Store API, like the go client has.

Something like Informable.withLastKnownState(Iterator items)? Or are you looking to make other modifications?

This is also somewhat related to #3943 which internally creates a customizable ItemStore. In your case it should like you would potentially want to plug-in a database backed store.

[hibnico]

Our current code use the KubernetesClient.informers() API. From what I just learned from the Informable API, probably a withLastKnownState function could fit part of our need, handle the restart of our application.

We also manipulate the cache in the event handler hooked to the informer. If there is an error in the side effect of the event (basically an error writing in the database), we push back in the cache the original value. It reverts the update of the cache and we have a chance to reapply the update later, with a later resync. If we don't do that, we won't recover the error until the informer is restarted, so not until our application restart.

[shawkins]

we push back in the cache the original value

This changes the notion of the informer store from being representative of the api server state, to instead being the desired state of your database. An update (or resync) to that pushed back resource emit an update event that will only completely make sense to your database event handler. Any cache modifications mean that all handlers added to the informer must understand the semantics of those modifications in terms of the events they are receiving. I was hoping to avoid these kind of situations because of the coupling between the store and event handlers.

What you are describing could be done without modifying the store:

    SharedIndexInformer<HasMetadata> informer = ... // some informer
    Set<String> pending = Collections.newSetFromMap(Collections.synchronizedMap(new LinkedHashMap<>()));

    final ResourceEventHandler<HasMetadata> handler = new ResourceEventHandler<HasMetadata>() {

      @Override
      public void onAdd(HasMetadata obj) {
        updatePending(obj);
      }

      private void updatePending(HasMetadata obj) {
        pending.add(Cache.metaNamespaceKeyFunc(obj));
      }

      @Override
      public void onDelete(HasMetadata obj, boolean deletedFinalStateUnknown) {
        updatePending(obj);
      }

      @Override
      public void onUpdate(HasMetadata oldObj, HasMetadata newObj) {
        updatePending(newObj);
      }

    };

    informer.addEventHandler(handler);

    // in another thread consume the pending events - this could be scheduled or notified on 
    // eventhandler modifications to pending
    // the events could be consumed all at once as this shows, which could even involve sql batch updates
    // or it could be a single event at a time 
    List<String> toUpdate = null;
    synchronized (pending) {
      toUpdate = new ArrayList<>(pending);
      pending.clear();
    }
    for (String key : toUpdate) {
      HasMetadata value = informer.getStore().getByKey(key);
      boolean success = false;
      try {
        // database logic - if current == null, it's a delete, otherwise it's an upsert
        success = true;
      } finally {
        if (!success) {
          pending.add(key); // retry
        }
      }
    }

At worst that may process the same update twice, but it won't miss any events.

[hibnico]

In our code the informer and its handler are tighly coupled. But I understand the rationale of restricting the Store API.
Thank you for the code sample. After our discussion I was also starting to think of this pattern with another queue.

[shawkins]

Adding to the 6.x milestone for the creation of a withLastKnownState method.

[shawkins]

A possible change for this is on #3981 - that adds back the store write methods, but they are implemented to ensure consistency. For your usage you'd obtain the informer from the factory or with Informable.runnableInformer and add the state before calling run.

[hibnico]

Awesome, thanks.