Umbrella issue for handling Informer Permission Errors and Probes

[csviri]

This issue is to cover more related issues and come up with a design how to handle them, see:

• [bug] Operator becoming non-functional after transient RBAC changes #1419
• Allow operator to start even if not all watched namespaces are accessible #1405
• Support for K8S Probes #1412

Summary

Informers are by definition not working if related RBAC is not configured or miss configured.

1. Either when an operator starts up and there are no proper permissions or
2. when running and some permissions are revoked.

This is true both for informers for primary and secondary resources.

When a permission is revoked what happens is that Informer (from fabri8 client) will try retry few times but eventually just stops. (TODO verify this behavior, also configurability)

Note that framework however supports dynamically changing watched namespaces, so it can again happen a new namespace added but there are no permissions. It might be desirable to have the operator functioning on the former namespaces.

There are multiple ways to solve this:

1. Just stop/restart the operator when an informer is not started properly
2. Try to reconnect the informer (infinitely)
3. Expose metrics regarding the health of informers so readyness probes can be added that eventually kill the operator.

@andreaTP propsed that having a callback function might be useful too.

[csviri]

After thinking more of this issue, there is a serious problem when an informer is not working. The caches are not populated with the fresh resources, so until the RBAC issues is not solved it is basically could be inconsistent. For example there is a new primary resource that on reconciliation tries to create a secondary resource, if the informer of that secondary resource is not working the cache will not know about the resource so the reconciler will try to recreate it over and over again.

Therefore would lean in the first phase just to restart the operator if there is such an error with the Informer. Maybe allowing to users to configure some retry options for the informer before a stop.

There could be a feature flag to not automatically shut down the operator, but delegate this for probes and expose the health metrics of informers as mentioned in point 3.

[csviri]

For the case when dynamically adding watched namespaces, there could be some optimizations, like reconcile a resource only when all the informers watching resources from a namespaces are healthy. But these will be very specific cases, its questionable such features is worth the effort.

[gyfora]

I think when the informer for a specific namespace cannot be created, we need to remove that namespace from the config and clear the caches.

The general problem with the restarting/current behaviour on errors is that malicious users (or simple user error) can cripple the operator for everyone in the cluster. If the user deletes rolebinding in their own namespaces the operator will go in a restart loop and there is nothing anyone else can do to fix it.

The whole cluster and resources are affected this way and restart won't do any good.

[csviri]

@gyfora
So you proposing something like: if a new namespace is added dynamically in runtime, and there is an informer (among possible more new informers), that cannot be start because of some permission issues, basically this operation should reject the new namespace (throw an exception - we could check with k8s api is there such permission upfront) ?

How should we behave on startup? Do the checks also I guess on watched namespace.
So if I understand correctly you are saying that we should be kinda validate and/or pre-process configuration?

This basically could be done just for informers which are following the general namespace configuration. I guess for the others if failing (informers not following the namespace in primary controller config) we should basically stop the operator too.

Do you agree with the case when a permission revoked, and basically from that point one of the informers won't work, therefore the caches won't function properly, that we should restart the operator?

[csviri]

However validating the namespace list and related permissions, and filtering the namespace list, could be something outside of the operator.

[gyfora]

No I am not really proposing anything about newly added namespaces.

What I am proposing if at any time any watched namespaces becomes unaccessible we should be able to remove that from the list and clean up caches and continue with the rest.

I think the best would be to have this at least a configurable option (default could be fail-operator/restart as you proposed).

Basically we need to address the following situation robustly:

1. watchedNamespaces: userNs1, userNs2
2. User 1 decides to revoke permissions for userNs1
3. User 2 using userNs2 should not be affected by the action of user1 as they are not aware of each other

[gyfora]

Unless this problem is addressed there is a big risk for any operator deployment that watches multiple independent user namespaces. One user basically can send the whole operator in a restart loop, this is not something that many production platforms can afford