Skip to content

Commit

Permalink
refactor: replace glog in healthcheck.go & remove glog dependency (#6628
Browse files Browse the repository at this point in the history
)
  • Loading branch information
AlexFenlon authored Oct 10, 2024
1 parent 992abd2 commit ec22bcf
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 17 deletions.
3 changes: 0 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ require (
github.com/go-chi/chi/v5 v5.1.0
github.com/go-kit/log v0.2.1
github.com/golang-jwt/jwt/v4 v4.5.0
github.com/golang/glog v1.2.2
github.com/google/go-cmp v0.6.0
github.com/jinzhu/copier v0.4.0
github.com/kr/pretty v0.3.1
Expand Down Expand Up @@ -151,8 +150,6 @@ require (
sigs.k8s.io/yaml v1.4.0 // indirect
)

replace github.com/golang/glog => github.com/nginxinc/glog v1.1.2

replace google.golang.org/protobuf v1.26.0 => google.golang.org/protobuf v1.33.0

replace google.golang.org/protobuf v1.26.0-rc.1 => google.golang.org/protobuf v1.33.0
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,6 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/nginxinc/glog v1.1.2 h1:zyoZXhCoYvNMJq4qMsKislUCGyJ4eU2gNl3Nt7cjESg=
github.com/nginxinc/glog v1.1.2/go.mod h1:Q2FpGp/qFhJEVnuC88BVfbLDPmio9aHYUj4al6w0138=
github.com/nginxinc/nginx-plus-go-client v1.3.0 h1:q/aeT4B5k0KLwWlefoBzfLfraBBvIKLuDg+lLFWAo4I=
github.com/nginxinc/nginx-plus-go-client v1.3.0/go.mod h1:n8OFLzrJulJ2fur28Cwa1Qp5DZNS2VicLV+Adt30LQ4=
github.com/nginxinc/nginx-prometheus-exporter v1.3.0 h1:1JtdxsZH0Uwhu1nL/j/QyOXytP5V5j68AEo2X+DFWb0=
Expand Down
29 changes: 17 additions & 12 deletions internal/healthcheck/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,32 @@ import (
"encoding/json"
"errors"
"fmt"
"log/slog"
"net/http"
"strconv"
"strings"
"time"

nl "github.com/nginxinc/kubernetes-ingress/internal/logger"

v1 "k8s.io/api/core/v1"

"github.com/go-chi/chi/v5"
"github.com/golang/glog"
"github.com/nginxinc/kubernetes-ingress/internal/configs"
"github.com/nginxinc/nginx-plus-go-client/client"
"k8s.io/utils/strings/slices"
)

// RunHealthCheck starts the deep healthcheck service.
func RunHealthCheck(port int, plusClient *client.NginxClient, cnf *configs.Configurator, healthProbeTLSSecret *v1.Secret) {
l := nl.LoggerFromContext(cnf.CfgParams.Context)
addr := fmt.Sprintf(":%s", strconv.Itoa(port))
hs, err := NewHealthServer(addr, plusClient, cnf, healthProbeTLSSecret)
if err != nil {
glog.Fatal(err)
nl.Fatal(l, err)
}
glog.Infof("Starting Service Insight listener on: %v%v", addr, "/probe")
glog.Fatal(hs.ListenAndServe())
nl.Infof(l, "Starting Service Insight listener on: %v%v", addr, "/probe")
nl.Fatal(l, hs.ListenAndServe())
}

// HealthServer holds data required for running
Expand All @@ -41,6 +44,7 @@ type HealthServer struct {
NginxUpstreams func() (*client.Upstreams, error)
StreamUpstreamsForName func(host string) []string
NginxStreamUpstreams func() (*client.StreamUpstreams, error)
Logger *slog.Logger
}

// NewHealthServer creates Health Server. If secret is provided,
Expand All @@ -57,6 +61,7 @@ func NewHealthServer(addr string, nc *client.NginxClient, cnf *configs.Configura
NginxUpstreams: nc.GetUpstreams,
StreamUpstreamsForName: cnf.StreamUpstreamsForName,
NginxStreamUpstreams: nc.GetStreamUpstreams,
Logger: nl.LoggerFromContext(cnf.CfgParams.Context),
}

if secret != nil {
Expand Down Expand Up @@ -97,22 +102,22 @@ func (hs *HealthServer) UpstreamStats(w http.ResponseWriter, r *http.Request) {

upstreamNames := hs.UpstreamsForHost(host)
if len(upstreamNames) == 0 {
glog.Errorf("no upstreams for requested hostname %s or hostname does not exist", host)
nl.Errorf(hs.Logger, "no upstreams for requested hostname %s or hostname does not exist", host)
w.WriteHeader(http.StatusNotFound)
return
}

upstreams, err := hs.NginxUpstreams()
if err != nil {
glog.Errorf("error retrieving upstreams for requested hostname: %s", host)
nl.Errorf(hs.Logger, "error retrieving upstreams for requested hostname: %s", host)
w.WriteHeader(http.StatusInternalServerError)
return
}

stats := countStats(upstreams, upstreamNames)
data, err := json.Marshal(stats)
if err != nil {
glog.Error("error marshaling result", err)
nl.Error(hs.Logger, "error marshaling result", err)
w.WriteHeader(http.StatusInternalServerError)
return
}
Expand All @@ -124,7 +129,7 @@ func (hs *HealthServer) UpstreamStats(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}
if _, err = w.Write(data); err != nil {
glog.Error("error writing result", err)
nl.Error(hs.Logger, "error writing result", err)
http.Error(w, "internal error", http.StatusInternalServerError)
}
}
Expand All @@ -136,20 +141,20 @@ func (hs *HealthServer) StreamStats(w http.ResponseWriter, r *http.Request) {
n := sanitize(name)
streamUpstreamNames := hs.StreamUpstreamsForName(n)
if len(streamUpstreamNames) == 0 {
glog.Errorf("no stream upstreams for requested name '%s' or name does not exist", n)
nl.Errorf(hs.Logger, "no stream upstreams for requested name '%s' or name does not exist", n)
w.WriteHeader(http.StatusNotFound)
return
}
streams, err := hs.NginxStreamUpstreams()
if err != nil {
glog.Errorf("error retrieving stream upstreams for requested name: %s", n)
nl.Errorf(hs.Logger, "error retrieving stream upstreams for requested name: %s", n)
w.WriteHeader(http.StatusInternalServerError)
return
}
stats := countStreamStats(streams, streamUpstreamNames)
data, err := json.Marshal(stats)
if err != nil {
glog.Error("error marshaling result", err)
nl.Error(hs.Logger, "error marshaling result", err)
w.WriteHeader(http.StatusInternalServerError)
return
}
Expand All @@ -161,7 +166,7 @@ func (hs *HealthServer) StreamStats(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}
if _, err := w.Write(data); err != nil {
glog.Error("error writing result", err)
nl.Error(hs.Logger, "error writing result", err)
http.Error(w, "internal error", http.StatusInternalServerError)
}
}
Expand Down
16 changes: 16 additions & 0 deletions internal/healthcheck/healthcheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@ package healthcheck_test
import (
"encoding/json"
"errors"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"testing"

nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
"github.com/nginxinc/kubernetes-ingress/internal/logger/levels"

"github.com/go-chi/chi/v5"
"github.com/google/go-cmp/cmp"
"github.com/nginxinc/kubernetes-ingress/internal/healthcheck"
Expand All @@ -25,6 +30,7 @@ func TestHealthCheckServer_Returns404OnMissingHostname(t *testing.T) {
hs := healthcheck.HealthServer{
UpstreamsForHost: getUpstreamsForHost,
NginxUpstreams: getUpstreamsFromNGINXAllUp,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand All @@ -45,6 +51,7 @@ func TestHealthCheckServer_ReturnsCorrectStatsForHostnameOnAllPeersUp(t *testing
hs := healthcheck.HealthServer{
UpstreamsForHost: getUpstreamsForHost,
NginxUpstreams: getUpstreamsFromNGINXAllUp,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand Down Expand Up @@ -78,6 +85,7 @@ func TestHealthCheckServer_ReturnsCorrectStatsForHostnameOnAllPeersDown(t *testi
hs := healthcheck.HealthServer{
UpstreamsForHost: getUpstreamsForHost,
NginxUpstreams: getUpstreamsFromNGINXAllUnhealthy,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand Down Expand Up @@ -112,6 +120,7 @@ func TestHealthCheckServer_ReturnsCorrectStatsForValidHostnameOnPartOfPeersDown(
hs := healthcheck.HealthServer{
UpstreamsForHost: getUpstreamsForHost,
NginxUpstreams: getUpstreamsFromNGINXPartiallyUp,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand Down Expand Up @@ -146,6 +155,7 @@ func TestHealthCheckServer_RespondsWith404OnNotExistingHostname(t *testing.T) {
hs := healthcheck.HealthServer{
UpstreamsForHost: getUpstreamsForHost,
NginxUpstreams: getUpstreamsFromNGINXNotExistingHost,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand All @@ -166,6 +176,7 @@ func TestHealthCheckServer_RespondsWith500OnErrorFromNGINXAPI(t *testing.T) {
hs := healthcheck.HealthServer{
UpstreamsForHost: getUpstreamsForHost,
NginxUpstreams: getUpstreamsFromNGINXErrorFromAPI,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand All @@ -186,6 +197,7 @@ func TestHealthCheckServer_Returns404OnMissingTransportServerActionName(t *testi
hs := healthcheck.HealthServer{
StreamUpstreamsForName: streamUpstreamsForName,
NginxStreamUpstreams: streamUpstreamsFromNGINXAllUp,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand All @@ -206,6 +218,7 @@ func TestHealthCheckServer_Returns404OnBogusTransportServerActionName(t *testing
hs := healthcheck.HealthServer{
StreamUpstreamsForName: streamUpstreamsForName,
NginxStreamUpstreams: streamUpstreamsFromNGINXAllUp,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand All @@ -226,6 +239,7 @@ func TestHealthCheckServer_ReturnsCorrectTransportServerStatsForNameOnAllPeersUp
hs := healthcheck.HealthServer{
StreamUpstreamsForName: streamUpstreamsForName,
NginxStreamUpstreams: streamUpstreamsFromNGINXAllUp,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand Down Expand Up @@ -259,6 +273,7 @@ func TestHealthCheckServer_ReturnsCorrectTransportServerStatsForNameOnSomePeersU
hs := healthcheck.HealthServer{
StreamUpstreamsForName: streamUpstreamsForName,
NginxStreamUpstreams: streamUpstreamsFromNGINXPartiallyUp,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand Down Expand Up @@ -292,6 +307,7 @@ func TestHealthCheckServer_ReturnsCorrectTransportServerStatsForNameOnAllPeersDo
hs := healthcheck.HealthServer{
StreamUpstreamsForName: streamUpstreamsForName,
NginxStreamUpstreams: streamUpstreamsFromNGINXAllPeersDown,
Logger: slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
}

ts := httptest.NewServer(testHandler(&hs))
Expand Down

0 comments on commit ec22bcf

Please sign in to comment.