[Bug]: Fault quarantine should not process old events when circuit breaker is closed

[lalitadithya]

Prerequisites

• I searched existing issues
• I can reproduce this issue

Bug Description

We had a cluster that had the circuit breaker in a tripped state for many days and when we closed the braker, the FQ module started to process very old events. This resulted in nodes being cordoned and uncondoned for older issues. Ideally, the FQ module should only cordon nodes that have fault at the current time

Component

Fault Management

Steps to Reproduce

1. Trip the circuit breaker
2. Send many unhealthy events for X hours/days
3. Send many healthy events for X hours/days
4. Close the circuit breaker

Environment

• NVSentinel version: 0.2.0
• Kubernetes version: all
• Deployment method: ArgoCD

Logs/Output

N/A

[jyotimahapatra]

should reseting circuit breaker reset the cursor automatically?

[lalitadithya]

should reseting circuit breaker reset the cursor automatically?

Possibly, but we may need to think through it. I guess at the end of the day it will depend on the guidance that we are proving to the operators. If the guidance is that the operators are going to manually uncordon the nodes, then it makes sense to auto reset. But if we expect the operators to not manually uncordon the nodes, they only need to investigate and reset the breaker and NVSentinel will take care of the uncordon, then we can't reset the cursor.

Then again, we also have the additional complexity of what caused the breaker to trip as well - if the breaker was tripped because every node in the cluster ran into a faulty health check then without resetting the cursor, we may cause a cycle of trips in the breaker.

We also have the complexity of how the breaker was closed -- the user can change the state in the configmap or delete the configmap. If the user changes the state without deleting it, then it is more complex to determine if it needs to be reset.

TBH, I don't have a good answer for you at the moment. We should think though the possibilities and what we want to and can handle without adding too much additional complexity and we should go with that.

[jyotimahapatra]

f the breaker was tripped because every node in the cluster ran into a faulty health check then without resetting the cursor, we may cause a cycle of trips in the breaker.

can you explain more about the cycle of trips in light of the previous stanza. What could be the possibilities of the user action when all nodes run into unhealthy signal.

If the user changes the state without deleting it, then it is more complex to determine if it needs to be reset.

What is the content of the configmap that causes the ambiguity? Is it a json and hence we cannot determine a state change from the content of configmap? I think we can form an opinion for consistency about how to perform reset.

I'm asking the questions to create breadcrumbs for our choice in posterity. Thank you for explaining the nuances.

[lalitadithya]

can you explain more about the cycle of trips in light of the previous stanza. What could be the possibilities of the user action when all nodes run into unhealthy signal.

I have seen this happen in two cases:

• Something caused an XID error on every single node: this has happened with some XIDs in some clusters and in such cases the user will likely have to go through the dmesg along with the application logs to determine what could be causing this error.
• An upgrade that went wrong: In this case as well the user will have to check the changes to the cluster to determine what went wrong and how the cluster can be recovered.

In the above two cases what will happen is that 100% of the nodes will report an error, but the breaker will trip at 50% (in the default configuration) and if the user simply resets the breaker, it will then process the rest of the 50% of the nodes and trip again. In the first case it will depends on if the user is continuously re-running the workload and it will cause a cycle. In the second case it is very likely that a DCGM health check is failing (which is run on poll cycle) so what would happen is that we would have so many events that have built up in the database due to frequency of the health checks, so what will happen is that the user will uncordon, reset the breaker and NVSentinel will cordon it back again.

What is the content of the configmap that causes the ambiguity? Is it a json and hence we cannot determine a state change from the content of configmap?

The confimap currently looks like so:

apiVersion: v1
data:
  status: CLOSED    # Can be either CLOSED or TRIPPED
kind: ConfigMap
metadata:
  name: circuit-breaker
  namespace: nvsentinel

to close the breaker the user has 2 options:

1. they can either change the status from TRIPPED to CLOSED
2. delete the configmap

Now if the user deletes the confimap it is an easier problem because on application start, we can say that we have lost state and hence we will move the cursor to the head of the events. But if the user updates the values, then we will have a hard time to determine if the user closed the breaker or if the pod was restarted. For this case we can always have another state somewhere else to determine what happened which is added complexity. Maybe it is fine for us to say that if the configmap is not present then we will reset the cursor to the head of the queue. This also does make the configmap load bearing in the sense that it is now doing more than what it was originally intended for.

Maybe the simplest solution is that we make the configmap:

apiVersion: v1
data:
  status: CLOSED    # Can be either CLOSED or TRIPPED
  cursor: RESUME   # Can be either RESUME or FRESH
kind: ConfigMap
metadata:
  name: circuit-breaker
  namespace: nvsentinel

and the user can determine what needs to be done based on what is happening in the cluster.