RuntimeInfo reports started even if leader elector cannot access lease

[gyfora]

Bug Report

If the operator service account doesn't have access to leases the leaderelection logic cannot work correctly and the operator simply won't start but keeps throwing errors.

At the same time the RuntimeInfo object reports isStarted() == true, so health probes set up for this do not catch this problem.

What did you do?

Start operator with leader election enabled without lease access.

What did you expect to see?

Startup probe errors.

What did you see instead? Under which circumstances?

Healthy startup probes and multiple replicas doing nothing, throwing lease access errors

Possible Solution

Only set isStarted to true when the lease was successfully accessed.

[csviri]

thx @gyfora this is totally valid issue, leader election is not in the scope of RuntimeInfo now and it should be.

[csviri]

Actually not sure if we should start the operator in this case. So simply it just should throw on exception for start(). So would start with that if no other opinions. @gyfora, @metacosm ?

Maybe bound it to the stopOnInformerErrorDuringStartup variable. (And rename that to something more describing eventually), So automatically stop if this flag is true on startup.

https://github.com/java-operator-sdk/java-operator-sdk/blob/92f2f281e9f714f33d01e8af310512c0bf04181c/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java#L205-L205

[metacosm]

I agree that the operator shouldn't start in this situation, but I think it shouldn't depend on anything else: if leader election is configured but no access to the lease is possible, we should never start. We need to fix RuntimeInfo.isStarted in this situation, though, so that health/ready checks are correct.

[csviri]

The started() currently basically only tells if start() is called on Operator. It is only for a purpose to check in a health check implementation if make sense to do additional checks. So not sura about that parts, so if start is not called will return true, but in case somebody calls start() it will fail (to should stop the pod at the end) anyways.

[gyfora]

I think we should not make this dependent on stopOnInformerErrorDuringStartup as @metacosm said this should always fail because the operator simply cannot work. Informer errors in general are completely diferent problem and if only some informers are failing the operator may work "correctly"

[csviri]

I think we should not make this dependent on stopOnInformerErrorDuringStartup as @metacosm said this should always fail because the operator simply cannot work.

yep, I agree, just was an idea, definitelly not going to implement something like this in the first iteration (just something to think about, not necessarirly for this flag but maybe an other).

Note that we might one to emit some events for better observability. But would cover that rather independently from this issue.