Update health check for Fabric8 client 7.4.0

[metacosm]

The status is defined at the JOSDK level: https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java#L204

Definitely need to refine that for 7.4.0+

Can you elaborate, please? Refine how? Why?

Please see fabric8io/kubernetes-client#7309 (comment)

Originally posted by @shawkins in #1174

[metacosm]

Looking into this, I'm not sure there's anything that needs to be done at the JOSDK level. More recent versions of the Fabric8 client are more sensitive to watching failures of the informers so it is now more likely to see the informer reports that it's not watching.

We could implement some sort of threshold on the number of consecutive isWatching errors that the SDK sees before reporting an unhealthy status but I feel this would be redundant with the probe failureThreshold configuration so I'd be in favor of closing this issue.

[csviri]

Makes sense feel free to close it, thank you for looking into it.

[AdrianVasiliu]

@metacosm Our operator, using quarkus-operator-sdk-bom v7.6.0, also suffers from spurious, harmless but misleading SRHCK01001 / "health check down", logged at INFO level.

We could implement some sort of threshold on the number of consecutive isWatching errors that the SDK sees before reporting an unhealthy status but I feel this would be redundant with the probe failureThreshold configuration so I'd be in favor of closing this issue.

At least in our case, augmenting livenessProbe.failureThreshold from the default (3) up to 20 didn't change anything. My understanding is that it can’t impact the health checks logs, as it acts at kube level. And in our case the pod does NOT fail, already with the default failureThreshold - the only trouble is with the misleading health down logs.

Wondering if you could consider reopening this issue.

[csviri]

@AdrianVasiliu

note that in health info you get access to all the indeicators for informers, so you can fine tune this behavior, see:

https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/health/InformerHealthIndicator.java#L18

https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/health/ControllerHealthInfo.java#L71

[metacosm]

@csviri this won't help the issue: the informer will still be marked as unhealthy and logged as such as people don't control what's reported by RuntimeInfo.

[AdrianVasiliu]

@csviri It's like @metacosm wrote.

[csviri]

Yep, just added here for the record.

Some other remarks:

• Readyness check is easy win: informer is synced
• Liveness check is an issue. There are a couple of other alternatives (since informer api is not that useful):
  1. on events we could record a timestamp (also last synced resource version) when last event was received and check and users could check based on their use case if it is not too old.
  2. In Flink Operator we do something called canary resources, which are custom resources with special flags, that only serves this pupose to check if we receive the event, see it here.

This way it is quite reliable, @AdrianVasiliu I wonder you just having health issues or just health check issues in your controller?

Because in general informers should very good in self healing.

[metacosm]

This way it is quite reliable, @AdrianVasiliu I wonder you just having health issues or just health check issues in your controller?

Because in general informers should very good in self healing.

The issue is not that the operator is unhealthy. It's actually the reverse: in many instances, with the current implementation, JOSDK will report informers (and therefore the operator) as being unhealthy just because it happened that the underlying informer was disconnected at the time we probed but maybe is already reconnecting. We (and, most importantly, our users) have no way to know whether that information is significant when it happens. The only way to use that information is to keep track of how long that's been happening and if it's happening a lot, which we don't currently do.

[csviri]

Yeah maybe we should update the guidlines / implementation to consider informers healthy after synced. So use informers just for readyness check, not liveness.

[metacosm]

Yeah maybe we should update the guidlines / implementation to consider informers healthy after synced. So use informers just for readyness check, not liveness.

I don't think that's an appropriate solution because there are situations when informers will indeed become unhealthy and we should try to capture that because that's important for the operator to get restarted if that's the case.