Status field should tell me about reconciliation failure

[st3v]

Is your feature request related to a problem? Please describe.
In #171080093, we added a couple of conditions. As a result, we now have the infrastructure to implement all the other conditions.
At the moment our CR status doesn't report on reconciliation progress and potential failures during reconciliation.

Describe the solution you'd like
In this story, let's implement the conditions related to reconciliation as we discussed previously.

Conditions:
Reconcilable:

• Bubble up errors during reconcile. Irreconcilable CR, Validation failures
• True state is the happy state.

InProgress (represents if the cluster is being reconciled) as discussed, InProgress will be addressed in a separate issue

Add these in the status field of the Cluster CRD. Set them during reconciliation.

For each condition above:

Scenario: Reconcilable Happy path 😀
Given that I deploy a RMQ cluster
And there are no reconciliation failures for any k8s child resource
When I describe the RabbitMQCluster CR
Then I see the condition `Reconcilable` in the status field set to `True`

Scenario: Reconcilable Unhappy path 🙁
Given that I deploy a RMQ cluster
And a k8s child resource fails to reconcile
When I describe the RabbitMQCluster CR
Then I see the `Reconcilable` condition in the status field set to `False`
And the condition has a reason and a message field explaining the conditition status

Describe alternatives you've considered
See design document.

Additional context

The conditions should follow the Kubernetes API conventions.

This document has the conclusions of our team meeting and how we want the conditions to look like.

Edit: Reconcilable condition happy state is True. It was False before.

[j4mcs]

I'm unsure about the inProgress condition. In a naive example, we'd end up setting the condition to true every time we entered the reconcile function and then setting it to false every time we exited.

I'm not sure whether we have a marker which would let us say that at the end of a given reconcile loop that there is still more reconciliation to do (unless it's a requeue). Likewise I can't think of a way to say for sure that reconciliation is 'done'.

If we can't find a way to flag that there is still more reconciliation to do I would be opposed to adding this condition. I don't think we gain anything either by setting the status so many times or by having a condition which effectively says the reconciliation loop is firing (at the arbitrary moment in the past when the status was observed).

Will investigate further.

[j4mcs]

Likewise, I'm not sure how we could properly implement a ReconcileFailure condition. All of the failures I can think about that would fall under this (e.g. an imagePullBackoff) are flagged via events. I could see us setting this on, for instance, any error response from the reconcile function, but it's not clear to me how we would resolve any of those in a targeted way. Other than to say if we get to the end of the function then set this back to false which I'm not sure is effective. Another way of looking at this is I'm not sure these are 'states' but rather issues in state transition.

I think the goals of this issues will be better covered by building them into events with #40

@st3v thoughts?

[st3v]

Maybe it helps to take a step back and look at the main outcomes we were hoping for when talking about these two conditions:

1. Indicate to users that a particular RMQ cluster is undergoing what could be a rather long reconciliation, e.g. an upgrade under load which can involve long periods of draining for each node. Without dedicated condition(s) on the CR, the user would have to look at the StatefulSet or RMQ-specific metrics. Just looking at the CR, they would see that not all nodes are available but they would have no idea that this is to be expected as we're currently rolling the cluster.

2. Indicate to users that they configured the RabbitmqCluster CR in a way that cannot be reconciled. This is different from the types of errors we already indicate through the existing conditions. This type of error happens when we apply a particular child resource (most likely during validation or mutation) and requires the user to actively reconfigure the RabbitmqCluster CR as the problem won't resolve by itself. I believe this becomes particularly important once we are working on implementing Refactor RabbitmqCluster CRD #47. Maybe a bit far-fetched, but in a way, this is a bit like compile vs runtime errors for the child resources. The latter should be addressed by the conditions we already have. But how do we communicate the former?

I'm not saying that the conditions as laid out in the description of this issue are the best approach to achieve those outcomes. Can you think of other ways or can you contradict my thinking and reason why the two points I mentioned above don't apply?

[j4mcs]

As agreed in the sync-up, we will modify ReconcileFailure to be Reconcilable and constrain this specifically to errors generated by CRUD requests to the kube-api (i.e. errors from the CreateOrUpdate function).

We will pause work on the InProgress condition and consider writing a new issue to cover monitoring the replica status of the StatefulSet to determine if an upgrade is in progress