report if etcd peer is unreachable #20
Conversation
|
I'm going to change this so we also store last healthy time on the CR object |
I've done this in #22 |
controllers/etcdpeer_controller.go
Outdated
| func (r *EtcdPeerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { | ||
| ctx, cancel := context.WithTimeout(ctx, 10*time.Second) | ||
| defer cancel() | ||
| log := r.Log.WithValues("peer", req.NamespacedName) | ||
| if r.lastHealthy == nil { |
There was a problem hiding this comment.
That's not gonna race ? The calls to Reconcile will always be serialised?
There was a problem hiding this comment.
Yep, fixed in #22 I'll merge it as one commit
controllers/etcdpeer_controller.go
Outdated
| healthy = false | ||
| r.Recorder.Event(&peer, "Warning", "MemberConnectionFailure", "Cannot connect to etcd member") | ||
| if r.lastHealthy[peer.Name].IsZero() { | ||
| log.V(2).Info("Peer has never been healthy") |
There was a problem hiding this comment.
Why the (2) level, don't think we use this
There was a problem hiding this comment.
This is a very good point. #22 changes it so it's always logged
| } else { | ||
| defer c.Close() | ||
| if version, err := c.GetVersion(ctx); err != nil { | ||
| log.Error(err, "Unable to get Etcd version", "error", err) | ||
| } else { | ||
| serverVersion = version.Server | ||
| } | ||
| healthy = true |
There was a problem hiding this comment.
Very very nitpick but in case the c.GetVersion takes a long time for some reason it would be better to have recorded the lastHealthy just before that call
Adds a new status field that indicates whether the etcd peer is healthy.
Also adds some logging + an event for if the peer is unhealthy