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

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

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


##########
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 in if block because it will only be changed when current health state error is nil and last health state error is not nil. Otherwise，it will be changed every health check time.



-- 
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