chore: fix logging and tests

- Replace `.Info` calls with `.Warn`
- Log closing/write errors in tests.
- TestLoadBalancer now starts with negative score - that ensure that we also test backend tiers.

Signed-off-by: Dmitriy Matrenichev <[email protected]>

Author: DmitriyMV

controlplane/controlplane.go

@@ -118,7 +118,7 @@ func (lb *LoadBalancer) Start(upstreamCh <-chan []string) error {
 			select {
 			case upstreams := <-upstreamCh:
 				if err := lb.lb.ReconcileRoute(lb.endpoint, upstreams); err != nil {
-					lb.lb.Logger.Info("failed reconciling list of upstreams",
+					lb.lb.Logger.Warn("failed reconciling list of upstreams",
 						zap.Strings("upstreams", upstreams),
 						zap.Error(err),
 					)

controlplane/controlplane_test.go

@@ -12,20 +12,24 @@ import (
 	"testing"
 	"time"
 
+	"github.com/siderolabs/gen/slices"
 	"github.com/siderolabs/go-retry/retry"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/goleak"
 	"go.uber.org/zap/zaptest"
 
 	"github.com/siderolabs/go-loadbalancer/controlplane"
+	"github.com/siderolabs/go-loadbalancer/upstream"
 )
 
+//nolint:govet
 type mockUpstream struct {
+	T        testing.TB
+	Identity string
+
 	addr string
 	l    net.Listener
-
-	identity string
 }
 
 func (u *mockUpstream) Start() error {
@@ -50,13 +54,14 @@ func (u *mockUpstream) serve() {
 			return
 		}
 
-		c.Write([]byte(u.identity)) //nolint: errcheck
-		c.Close()                   //nolint: errcheck
+		_, err = c.Write([]byte(u.Identity))
+		require.NoError(u.T, err)
+		require.NoError(u.T, c.Close())
 	}
 }
 
 func (u *mockUpstream) Close() {
-	u.l.Close() //nolint: errcheck
+	require.NoError(u.T, u.l.Close())
 }
 
 func TestLoadBalancer(t *testing.T) {
@@ -69,16 +74,24 @@ func TestLoadBalancer(t *testing.T) {
 
 	upstreams := make([]mockUpstream, upstreamCount)
 	for i := range upstreams {
-		upstreams[i].identity = strconv.Itoa(i)
+		upstreams[i].T = t
+		upstreams[i].Identity = strconv.Itoa(i)
 		require.NoError(t, upstreams[i].Start())
 	}
 
-	upstreamAddrs := make([]string, len(upstreams))
-	for i := range upstreamAddrs {
-		upstreamAddrs[i] = upstreams[i].addr
-	}
-
-	lb, err := controlplane.NewLoadBalancer("localhost", 0, zaptest.NewLogger(t))
+	upstreamAddrs := slices.Map(upstreams, func(u mockUpstream) string { return u.addr })
+
+	lb, err := controlplane.NewLoadBalancer(
+		"localhost",
+		0,
+		zaptest.NewLogger(t),
+		controlplane.WithHealthCheckOptions(
+			// start with negative initlal score so that every healthcheck will be performed
+			// at least once. It will also make upstream tiers.
+			upstream.WithInitialScore(-1),
+			upstream.WithHealthcheckInterval(10*time.Millisecond),
+		),
+	)
 	require.NoError(t, err)
 
 	upstreamCh := make(chan []string)
@@ -93,17 +106,19 @@ func TestLoadBalancer(t *testing.T) {
 			return 0, retry.ExpectedError(err)
 		}
 
-		defer c.Close() //nolint:errcheck
+		defer ensure(t, c.Close)
 
 		id, err := io.ReadAll(c)
 		if err != nil {
 			return 0, retry.ExpectedError(err)
+		} else if len(id) == 0 {
+			return 0, retry.ExpectedErrorf("zero length response")
 		}
 
 		return strconv.Atoi(string(id))
 	}
 
-	assert.NoError(t, retry.Constant(10*time.Second, retry.WithUnits(time.Second)).Retry(func() error {
+	assert.NoError(t, retry.Constant(10*time.Second, retry.WithUnits(30*time.Millisecond)).Retry(func() error {
 		identity, err := readIdentity()
 		if err != nil {
 			return err
@@ -170,3 +185,7 @@ func TestLoadBalancer(t *testing.T) {
 
 	assert.NoError(t, lb.Shutdown())
 }
+
+func ensure(t *testing.T, closer func() error) {
+	require.NoError(t, closer())
+}

loadbalancer/node.go

@@ -31,7 +31,7 @@ func (upstream node) healthCheck(ctx context.Context) error {
 
 	c, err := d.DialContext(ctx, "tcp", upstream.address)
 	if err != nil {
-		upstream.logger.Info("healthcheck failed", zap.String("address", upstream.address), zap.Error(err))
+		upstream.logger.Warn("healthcheck failed", zap.String("address", upstream.address), zap.Error(err))
 
 		return err
 	}

loadbalancer/target.go

@@ -25,11 +25,18 @@ type lbTarget struct {
 func (target *lbTarget) HandleConn(conn net.Conn) {
 	upstreamBackend, err := target.list.Pick()
 	if err != nil {
-		target.logger.Info(
+		target.logger.Warn(
 			"no upstreams available, closing connection",
 			zap.String("remote_addr", conn.RemoteAddr().String()),
 		)
-		conn.Close() //nolint: errcheck
+
+		if closeErr := conn.Close(); closeErr != nil {
+			target.logger.Warn(
+				"error closing connection",
+				zap.String("remote_addr", conn.RemoteAddr().String()),
+				zap.Error(closeErr),
+			)
+		}
 
 		return
 	}
@@ -45,9 +52,15 @@ func (target *lbTarget) HandleConn(conn net.Conn) {
 	upstreamTarget.KeepAlivePeriod = target.keepAlivePeriod
 	upstreamTarget.TCPUserTimeout = target.tcpUserTimeout
 	upstreamTarget.OnDialError = func(src net.Conn, dstDialErr error) {
-		src.Close() //nolint: errcheck
+		if err := src.Close(); err != nil {
+			target.logger.Warn(
+				"error closing connection",
+				zap.String("remote_addr", src.RemoteAddr().String()),
+				zap.Error(err),
+			)
+		}
 
-		target.logger.Info(
+		target.logger.Warn(
 			"error dialing upstream",
 			zap.String("upstream_addr", upstreamBackend.address),
 			zap.Error(dstDialErr),