Repository navigation
Implement Ready status condition #24
Description
In general
We need to implement checking quorum status of an initialized cluster and update Ready status condition in accordance. After initializing cluster and making sure pods found each other and formed a cluster, controller must update cluster state configmap to set cluster state existing (from new)
Design proposal
When cluster is initialized, we should check if:
- StatefulSet is
Ready - Connect to any etcd and check if quorum is fine
- if everything is fine, set status condition Ready and update value in cluster state configmap to
existing. - If some pods in statefulset aren't ready but quorum is fine, set Ready, but mark cluster as degraded in Reason field.
- If pods in sts aren't ready and quorum is lost, mark cluster as not Ready
If configmap already has existing state, do not change it anymore, as cluster should be already initialized
Reacted by Andrei Kvapil
Activity
aobort commentedon Mar 15, 2024
Any option like "pods & sts are ready but quorum is lost"? Shouldn't we check the health of the cluster throughout its entire lifecycle? I mean, that etcd cluster controller will react the changes of sts state and act accordingly since controller reference is set, but in case loss of quorum or anything else happen to etcd app which will not affect pod/sts state operator will not be informed. Thinking out loud.
sircthulhu commentedon Mar 15, 2024
Artem Bortnikov (@aobort)
Thanks for your opinion, I really appreciate it :)
If I got it right, the case you mentioned is the first if. We should check that sts is ready AND check if cluster has quorum at all and there're desired number of members. If not, update cluster state accordingly.
In future, the controller will perform actions if possible
Kirill-Garbar commentedon Mar 15, 2024
Kirill Ilin (@sircthulhu) I didn't get from the description and from the answer should this status be continiously updated or just once? I see the etcd-operator (controller) as a "watchdog" who updates the status by some events/regularly. I will try to dive deeper into the docs and code to get such answers but currently it is not clear should this logic be implemented while working on this issue or not.
aobort commentedon Mar 15, 2024
Kirill-Garbar if I got things right, the goal at the moment is to implement such a check on etcd cluster creation:
Therefore, from my perspective, the workflow should be like the following:
Next reconciliation run schould be triggered by the update of sts state - NotReady -> Ready:
Continuous state tracking is definitely should be implemented. But, from my perspective it should be implemented separately from the controller's code. May be as another service which implements
Runnableinterface, so it could be added to manager withmgr.Add()call. It could just check etcd cluster's health and in case something happen, update sts annotation which should trigger the reconciliation of etcd cluster object.sircthulhu commentedon Mar 15, 2024
Artem Bortnikov (@aobort)
You're definetly right about the workflow.
I'd like to make it more precise
We do not rely on what's written in status to get the current state,
Reconcilefunction should be idempotentI agree with the point that continuous tracking should be implemented as
Runnableinterface, as it better for architecture. For cluster bootstrap it is just enough to get update when StatefulSet is changed somehowaobort commentedon Mar 15, 2024
Kirill Ilin (@sircthulhu)
Absolutely. I meant, that the update of sts object (it does not matter whether it will be regular status update or manual or whatnot) will trigger etcd cluster object reconciliation. 'Cuz there is the controller reference applied to sts.
kvaps commentedon Mar 15, 2024
This requires additional condition, eg:
Quorum: True|Falsewith reasonClusterHasQuorumkvaps commentedon Mar 15, 2024
Internaly we agreed to implement just single condition for now -
Ready: True|FalseEtcdClustershould be consideredReadywhen StatefulSet isReadyEtcdClustershould be consideredNotReadywhen StatefulSet isNotReadyEvery pod in StatefulSet should have healthchecks defined:
We can borrow the logic from Kubeadm, for example:
https://github.com/kubernetes/kubernetes/blob/89b1db79d75e367f567ea06c0528ef4b8f3fecb4/cmd/kubeadm/app/phases/etcd/local.go#L227-L228
Here is latest issue where they were updated:
kubernetes/kubeadm#2567
Hovewer Grigory Rochev (@Uburro) noted that:
gaalw commentedon Mar 15, 2024
operator is not monitoring solution. Also if you would implement it, the data in CR will be always stale.
aobort commentedon Mar 15, 2024
György Gaál (@gaalw)
Agree. However according to plans to implement maintenance of the cluster monitoring of its state becomes mandatory capability.
It depends. In case cluster health was not changed, there would be no reason for watchdog to update CR.
Uburro commentedon Mar 15, 2024
According to this KEP, the ETCD introduced full-fledged checks of readiness and liveness probes. /readyz for us mention that no raft leader, raft loop deadlock, data coraption. In this case, it will disappear from the endpoints and for our controller this will be a signal that everything is ok or not ok with the cluster. We also subscribe to events from the shared informer so there is no need for the controller to go through the clusters and poll their status. This will be done by Kubernetes native way itself using kubelet. /livez will be a signal about a problem with the database (apps) itself and a trigger for its restart. This way we avoid unnecessary restarting of the etcd. This is useful in cases where the etcd will be used outside the kubernetes.
in this case we have to start to support since 3.5.12 and 3.4.29 vertion of etcd
Uburro commentedon Mar 17, 2024
etcd-io/etcd#16666 we can think about --experimental-wait-cluster-ready-timeout with starupprobe in a feature
Implementation of Ready status condition (#26)
feat: spec.resources, /scale subresource, always-on metrics port (coz…