From a5790d5caa276a2b7aa02786a9bfcfb82e672e21 Mon Sep 17 00:00:00 2001 From: Quan Tian Date: Wed, 28 Dec 2022 19:57:53 +0800 Subject: [PATCH] Do not log errors when ServiceHealthServer is closed normally Server.Serve() always returns a non-nil error. If it exits because the http server is closed, it will return ErrServerClosed. To differentiate the error, this patch changes to close the http server instead of the listener when the Service is deleted. Closing http server automatically closes the listener, there is no need to cache the listeners any more. Signed-off-by: Quan Tian --- pkg/proxy/healthcheck/common.go | 1 + pkg/proxy/healthcheck/healthcheck_test.go | 4 ++++ pkg/proxy/healthcheck/service_health.go | 13 +++++-------- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/proxy/healthcheck/common.go b/pkg/proxy/healthcheck/common.go index f65cf1666e8..2013508c1c4 100644 --- a/pkg/proxy/healthcheck/common.go +++ b/pkg/proxy/healthcheck/common.go @@ -39,6 +39,7 @@ type httpServerFactory interface { // It is designed so that http.Server satisfies this interface, type httpServer interface { Serve(listener net.Listener) error + Close() error } // Implement listener in terms of net.Listen. diff --git a/pkg/proxy/healthcheck/healthcheck_test.go b/pkg/proxy/healthcheck/healthcheck_test.go index 7f52c0c3eff..a82418ed5ca 100644 --- a/pkg/proxy/healthcheck/healthcheck_test.go +++ b/pkg/proxy/healthcheck/healthcheck_test.go @@ -104,6 +104,10 @@ func (fake *fakeHTTPServer) Serve(listener net.Listener) error { return nil // Cause the goroutine to return } +func (fake *fakeHTTPServer) Close() error { + return nil +} + func mknsn(ns, name string) types.NamespacedName { return types.NamespacedName{ Namespace: ns, diff --git a/pkg/proxy/healthcheck/service_health.go b/pkg/proxy/healthcheck/service_health.go index 8f97a219057..9a71ae7a09d 100644 --- a/pkg/proxy/healthcheck/service_health.go +++ b/pkg/proxy/healthcheck/service_health.go @@ -150,7 +150,6 @@ type hcInstance struct { nsn types.NamespacedName port uint16 - listeners []net.Listener httpServers []httpServer endpoints int // number of local endpoints for a service @@ -162,7 +161,6 @@ func (hcI *hcInstance) listenAndServeAll(hcs *server) error { var listener net.Listener addresses := hcs.nodeAddresses.List() - hcI.listeners = make([]net.Listener, 0, len(addresses)) hcI.httpServers = make([]httpServer, 0, len(addresses)) // for each of the node addresses start listening and serving @@ -181,16 +179,15 @@ func (hcI *hcInstance) listenAndServeAll(hcs *server) error { // start serving go func(hcI *hcInstance, listener net.Listener, httpSrv httpServer) { - // Serve() will exit when the listener is closed. + // Serve() will exit and return ErrServerClosed when the http server is closed. klog.V(3).InfoS("Starting goroutine for healthcheck", "service", hcI.nsn, "address", listener.Addr()) - if err := httpSrv.Serve(listener); err != nil { + if err := httpSrv.Serve(listener); err != nil && err != http.ErrServerClosed { klog.ErrorS(err, "Healthcheck closed", "service", hcI.nsn) return } klog.V(3).InfoS("Healthcheck closed", "service", hcI.nsn, "address", listener.Addr()) }(hcI, listener, httpSrv) - hcI.listeners = append(hcI.listeners, listener) hcI.httpServers = append(hcI.httpServers, httpSrv) } @@ -199,9 +196,9 @@ func (hcI *hcInstance) listenAndServeAll(hcs *server) error { func (hcI *hcInstance) closeAll() error { errors := []error{} - for _, listener := range hcI.listeners { - if err := listener.Close(); err != nil { - klog.ErrorS(err, "Error closing listener for health check service", "service", hcI.nsn, "address", listener.Addr()) + for _, server := range hcI.httpServers { + if err := server.Close(); err != nil { + klog.ErrorS(err, "Error closing server for health check service", "service", hcI.nsn) errors = append(errors, err) } }