[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1779: fix: health check bug

["tao12345666333 (via GitHub)" <gi...@apache.org>]

tao12345666333 commented on code in PR #1779:
URL: https://github.com/apache/apisix-ingress-controller/pull/1779#discussion_r1164946215


##########
pkg/providers/controller.go:
##########
@@ -570,16 +570,20 @@ func (c *Controller) checkClusterHealth(ctx context.Context, cancelFunc context.
 
 		err := c.apisix.Cluster(c.cfg.APISIX.DefaultClusterName).HealthCheck(ctx)
 		if err != nil {
-			// Finally failed health check, then give up leader.
-			log.Warnf("failed to check health for default cluster: %s, give up leader", err)
 			c.apiServer.HealthState.Lock()
 			c.apiServer.HealthState.Err = err
 			c.apiServer.HealthState.Unlock()
-
-			return
+			// Finally failed health check, then give up leader.
+			log.Warnf("failed to check health for default cluster: %s, give up leader", err)
+		} else {
+			if c.apiServer.HealthState.Err != nil {
+				c.apiServer.HealthState.Lock()
+				c.apiServer.HealthState.Err = err
+				c.apiServer.HealthState.Unlock()
+			}
+			log.Debugf("success check health for default cluster")
+			c.MetricsCollector.IncrCheckClusterHealth(c.name)

Review Comment:
   The code here can be simplified. The state change for HealthState does not need to be placed in an if/else statement.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org