From 4e10ff91c5bb2d4ad87bb00ab281cc3ee95a258b Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Sun, 23 Jul 2023 15:44:55 +0530 Subject: [PATCH] pkg/proxy: move proxier health eventing to cmd/kube-proxy Signed-off-by: Daman Arora --- cmd/kube-proxy/app/server.go | 19 +++++++++++++------ pkg/proxy/healthcheck/healthcheck_test.go | 4 ++-- pkg/proxy/healthcheck/proxier_health.go | 19 ++++--------------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index d019212db28..987395ee4d8 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -574,7 +574,7 @@ func newProxyServer(config *kubeproxyconfig.KubeProxyConfiguration, master strin } if len(config.HealthzBindAddress) > 0 { - s.HealthzServer = healthcheck.NewProxierHealthServer(config.HealthzBindAddress, 2*config.IPTables.SyncPeriod.Duration, s.Recorder, s.NodeRef) + s.HealthzServer = healthcheck.NewProxierHealthServer(config.HealthzBindAddress, 2*config.IPTables.SyncPeriod.Duration) } err = s.platformSetup() @@ -822,16 +822,17 @@ func (s *ProxyServer) Run() error { // TODO(thockin): make it possible for healthz and metrics to be on the same port. - var errCh chan error + var healthzErrCh, metricsErrCh chan error if s.Config.BindAddressHardFail { - errCh = make(chan error) + healthzErrCh = make(chan error) + metricsErrCh = make(chan error) } // Start up a healthz server if requested - serveHealthz(s.HealthzServer, errCh) + serveHealthz(s.HealthzServer, healthzErrCh) // Start up a metrics server if requested - serveMetrics(s.Config.MetricsBindAddress, s.Config.Mode, s.Config.EnableProfiling, errCh) + serveMetrics(s.Config.MetricsBindAddress, s.Config.Mode, s.Config.EnableProfiling, metricsErrCh) noProxyName, err := labels.NewRequirement(apis.LabelServiceProxyName, selection.DoesNotExist, nil) if err != nil { @@ -896,7 +897,13 @@ func (s *ProxyServer) Run() error { go s.Proxier.SyncLoop() - return <-errCh + select { + case err = <-healthzErrCh: + s.Recorder.Eventf(s.NodeRef, nil, api.EventTypeWarning, "FailedToStartProxierHealthcheck", "StartKubeProxy", err.Error()) + case err = <-metricsErrCh: + s.Recorder.Eventf(s.NodeRef, nil, api.EventTypeWarning, "FailedToStartMetricServer", "StartKubeProxy", err.Error()) + } + return err } func (s *ProxyServer) birthCry() { diff --git a/pkg/proxy/healthcheck/healthcheck_test.go b/pkg/proxy/healthcheck/healthcheck_test.go index d77a561c870..3be8feac77e 100644 --- a/pkg/proxy/healthcheck/healthcheck_test.go +++ b/pkg/proxy/healthcheck/healthcheck_test.go @@ -470,7 +470,7 @@ func TestHealthzServer(t *testing.T) { httpFactory := newFakeHTTPServerFactory() fakeClock := testingclock.NewFakeClock(time.Now()) - hs := newProxierHealthServer(listener, httpFactory, fakeClock, "127.0.0.1:10256", 10*time.Second, nil, nil) + hs := newProxierHealthServer(listener, httpFactory, fakeClock, "127.0.0.1:10256", 10*time.Second) server := hs.httpFactory.New(hs.addr, healthzHandler{hs: hs}) hsTest := &serverTest{ @@ -524,7 +524,7 @@ func TestLivezServer(t *testing.T) { httpFactory := newFakeHTTPServerFactory() fakeClock := testingclock.NewFakeClock(time.Now()) - hs := newProxierHealthServer(listener, httpFactory, fakeClock, "127.0.0.1:10256", 10*time.Second, nil, nil) + hs := newProxierHealthServer(listener, httpFactory, fakeClock, "127.0.0.1:10256", 10*time.Second) server := hs.httpFactory.New(hs.addr, livezHandler{hs: hs}) hsTest := &serverTest{ diff --git a/pkg/proxy/healthcheck/proxier_health.go b/pkg/proxy/healthcheck/proxier_health.go index 2ffdcc2b9d2..6f5e90f5be4 100644 --- a/pkg/proxy/healthcheck/proxier_health.go +++ b/pkg/proxy/healthcheck/proxier_health.go @@ -23,9 +23,7 @@ import ( "time" v1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/events" "k8s.io/klog/v2" - api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/proxy/metrics" "k8s.io/utils/clock" ) @@ -68,8 +66,6 @@ type proxierHealthServer struct { addr string healthTimeout time.Duration - recorder events.EventRecorder - nodeRef *v1.ObjectReference lastUpdated atomic.Value oldestPendingQueued atomic.Value @@ -77,19 +73,17 @@ type proxierHealthServer struct { } // NewProxierHealthServer returns a proxier health http server. -func NewProxierHealthServer(addr string, healthTimeout time.Duration, recorder events.EventRecorder, nodeRef *v1.ObjectReference) ProxierHealthUpdater { - return newProxierHealthServer(stdNetListener{}, stdHTTPServerFactory{}, clock.RealClock{}, addr, healthTimeout, recorder, nodeRef) +func NewProxierHealthServer(addr string, healthTimeout time.Duration) ProxierHealthUpdater { + return newProxierHealthServer(stdNetListener{}, stdHTTPServerFactory{}, clock.RealClock{}, addr, healthTimeout) } -func newProxierHealthServer(listener listener, httpServerFactory httpServerFactory, c clock.Clock, addr string, healthTimeout time.Duration, recorder events.EventRecorder, nodeRef *v1.ObjectReference) *proxierHealthServer { +func newProxierHealthServer(listener listener, httpServerFactory httpServerFactory, c clock.Clock, addr string, healthTimeout time.Duration) *proxierHealthServer { hs := &proxierHealthServer{ listener: listener, httpFactory: httpServerFactory, clock: c, addr: addr, healthTimeout: healthTimeout, - recorder: recorder, - nodeRef: nodeRef, } // The node is eligible (and thus the proxy healthy) while it's starting up // and until we've processed the first node event that indicates the @@ -166,12 +160,7 @@ func (hs *proxierHealthServer) Run() error { listener, err := hs.listener.Listen(hs.addr) if err != nil { - msg := fmt.Sprintf("failed to start proxier healthz on %s: %v", hs.addr, err) - // TODO(thockin): move eventing back to caller - if hs.recorder != nil { - hs.recorder.Eventf(hs.nodeRef, nil, api.EventTypeWarning, "FailedToStartProxierHealthcheck", "StartKubeProxy", msg) - } - return fmt.Errorf("%v", msg) + return fmt.Errorf("failed to start proxier healthz on %s: %v", hs.addr, err) } klog.V(3).InfoS("Starting healthz HTTP server", "address", hs.addr)