fixed lint issue with caching

.github/workflows/pull_request.yml

@@ -25,10 +25,9 @@ jobs:
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@v8
         with:
-          args: --timeout=3m --enable copyloopvar --out-format colored-line-number
-          skip-pkg-cache: true
-          skip-build-cache: true
-
+          version: latest
+          args: --timeout=3m --enable copyloopvar
+
       - name: Run integration tests
         run: make test
 

pkg/redis/close_helpers.go

@@ -0,0 +1,58 @@
+package redis
+
+import (
+	"io"
+	"testing"
+
+	"github.com/golang/glog"
+)
+
+// CloseWithLog safely closes a Closer and logs any error that occurs.
+// This is a helper to reduce boilerplate error handling for Close() calls.
+func CloseWithLog(closer io.Closer, context string) {
+	if err := closer.Close(); err != nil {
+		glog.Errorf("Error closing %s: %v", context, err)
+	}
+}
+
+// CloseWithLogf safely closes a Closer and logs any error using formatted context.
+// This is a helper to reduce boilerplate error handling for Close() calls.
+func CloseWithLogf(closer io.Closer, format string, args ...interface{}) {
+	if err := closer.Close(); err != nil {
+		glog.Errorf("Error closing "+format+": %v", append(args, err)...)
+	}
+}
+
+// DeferCloseWithLog returns a function that safely closes a Closer and logs any error.
+// This is designed to be used with defer statements.
+func DeferCloseWithLog(closer io.Closer, context string) func() {
+	return func() {
+		CloseWithLog(closer, context)
+	}
+}
+
+// DeferCloseWithLogf returns a function that safely closes a Closer and logs any error with formatted context.
+// This is designed to be used with defer statements.
+func DeferCloseWithLogf(closer io.Closer, format string, args ...interface{}) func() {
+	return func() {
+		CloseWithLogf(closer, format, args...)
+	}
+}
+
+// CloseWithTestLog safely closes a Closer and logs any error using testing.T.
+// This is specifically for test contexts where glog might not be appropriate.
+func CloseWithTestLog(t *testing.T, closer io.Closer, context string) {
+	if err := closer.Close(); err != nil {
+		if t != nil {
+			t.Logf("Error closing %s: %v", context, err)
+		}
+	}
+}
+
+// DeferCloseWithTestLog returns a function that safely closes a Closer and logs any error using testing.T.
+// This is designed to be used with defer statements in tests.
+func DeferCloseWithTestLog(t *testing.T, closer io.Closer, context string) func() {
+	return func() {
+		CloseWithTestLog(t, closer, context)
+	}
+}

pkg/redis/connections.go

@@ -84,7 +84,7 @@ func NewAdminConnections(ctx context.Context, addrs []string, options *AdminOpti
 // Close used to close all possible resources instantiated by the Connections
 func (cnx *AdminConnections) Close() {
 	for _, c := range cnx.clients {
-		c.Close()
+		CloseWithLog(c, "client connection")
 	}
 }
 
@@ -98,7 +98,7 @@ func (cnx *AdminConnections) Add(ctx context.Context, addr string) error {
 // Remove disconnect and remove the client connection from the map
 func (cnx *AdminConnections) Remove(addr string) {
 	if c, ok := cnx.clients[addr]; ok {
-		c.Close()
+		CloseWithLogf(c, "connection to %s", addr)
 		delete(cnx.clients, addr)
 	}
 }
@@ -108,7 +108,7 @@ func (cnx *AdminConnections) Remove(addr string) {
 func (cnx *AdminConnections) Update(ctx context.Context, addr string) (ClientInterface, error) {
 	// if already exist close the current connection
 	if c, ok := cnx.clients[addr]; ok {
-		c.Close()
+		CloseWithLogf(c, "existing connection to %s", addr)
 	}
 
 	c, err := cnx.connect(ctx, addr)
@@ -206,7 +206,7 @@ func (cnx *AdminConnections) ReplaceAll(ctx context.Context, addrs []string) {
 // Reset close all connections and clear the connection map
 func (cnx *AdminConnections) Reset() {
 	for _, c := range cnx.clients {
-		c.Close()
+		CloseWithLog(c, "client connection")
 	}
 	cnx.clients = map[string]ClientInterface{}
 }
@@ -288,7 +288,7 @@ func buildCommandReplaceMapping(filePath string) map[string]string {
 		glog.Errorf("Cannot open %s: %v", filePath, err)
 		return mapping
 	}
-	defer file.Close()
+	defer DeferCloseWithLogf(file, "file %s", filePath)()
 
 	scanner := bufio.NewScanner(file)
 	for scanner.Scan() {

pkg/redis/const.go

@@ -24,5 +24,5 @@ const (
 // Redis error constants
 const (
 	// ErrNotFound cannot find a node to connect to
-	ErrNotFound = "Unable to find a node to connect"
+	ErrNotFound = "unable to find a node to connect"
 )

pkg/redis/fake/redis.go

@@ -11,6 +11,8 @@ import (
 	"strings"
 	"sync"
 	"testing"
+
+	"github.com/IBM/operator-for-redis-cluster/pkg/redis"
 )
 
 // RedisServer Fake Redis Server struct
@@ -43,7 +45,7 @@ func NewRedisServer(t *testing.T) *RedisServer {
 
 // Close possible resources
 func (r *RedisServer) Close() {
-	r.Ln.Close()
+	redis.CloseWithTestLog(r.test, r.Ln, "fake redis server listener")
 }
 
 // GetHostPort return the host port of redis server
@@ -87,7 +89,7 @@ func (r *RedisServer) handleConnection() {
 				break
 			}
 		}
-		defer conn.Close()
+		defer redis.DeferCloseWithTestLog(r.test, conn, "connection")()
 		var wait sync.WaitGroup
 		wait.Add(1)
 		go func(conn net.Conn, wait *sync.WaitGroup) {

pkg/redis/fake/redis_test.go

@@ -114,7 +114,11 @@ func TestNewRedisServer(t *testing.T) {
 	if err != nil {
 		t.Errorf("Cannot connec to fake redis server: %v", err)
 	}
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Logf("Error closing connection: %v", err)
+		}
+	}()
 
 	testCases := []struct {
 		input  string
@@ -127,7 +131,10 @@ func TestNewRedisServer(t *testing.T) {
 
 	for i, tt := range testCases {
 		// write to fake redis
-		fmt.Fprint(conn, tt.input)
+		if _, err := fmt.Fprint(conn, tt.input); err != nil {
+			t.Errorf("[test %d] Error writing to connection: %v", i, err)
+			continue
+		}
 
 		//read from fake redis
 		var message []string

pkg/redisnode/node.go

@@ -128,7 +128,11 @@ func (n *Node) addSettingInConfigFile(line string) error {
 		return fmt.Errorf("unable to set '%s' in config file, openfile error %s err:%v", line, n.config.Redis.ConfigFileName, err)
 	}
 
-	defer f.Close()
+	defer func() {
+		if err := f.Close(); err != nil {
+			glog.Errorf("Error closing config file %s: %v", n.config.Redis.ConfigFileName, err)
+		}
+	}()
 
 	_, err = f.WriteString(line + "\n")
 	if err != nil {
@@ -150,7 +154,11 @@ func (n *Node) getConfig(name string) (string, error) {
 			return "", fmt.Errorf("unable to read a config from file, openfile error %s err:%v", configFile, err)
 		}
 
-		defer f.Close()
+		defer func() {
+			if err := f.Close(); err != nil {
+				glog.Errorf("Error closing config file %s: %v", configFile, err)
+			}
+		}()
 		scanner := bufio.NewScanner(f)
 		for scanner.Scan() {
 			line := strings.TrimSpace(scanner.Text())
@@ -216,7 +224,11 @@ func clearFolder(folder string) error {
 		glog.Infof("Cannot access folder %s: %v", folder, err)
 		return err
 	}
-	defer d.Close()
+	defer func() {
+		if err := d.Close(); err != nil {
+			glog.Errorf("Error closing directory %s: %v", folder, err)
+		}
+	}()
 	names, err := d.Readdirnames(-1)
 	if err != nil {
 		glog.Infof("Cannot read files in %s: %v", folder, err)
@@ -243,7 +255,11 @@ func getPodMemoryLimit(memFilePath string) (uint64, error) {
 		}
 		return 0, err
 	}
-	defer f.Close()
+	defer func() {
+		if err := f.Close(); err != nil {
+			glog.Errorf("Error closing memory limit file %s: %v", memFilePath, err)
+		}
+	}()
 
 	memLimitStr, err := io.ReadAll(f)
 	if err != nil {

pkg/redisnode/node_test.go

@@ -72,20 +72,32 @@ cluster-node-timeout 321`,
 			if createerr != nil {
 				t.Errorf("Couldn' t create temporary config file: %v", createerr)
 			}
-			defer os.RemoveAll(redisConfDir)
-			redisConfFile.Close()
+			defer func() {
+				if err := os.RemoveAll(redisConfDir); err != nil {
+					t.Logf("Error removing temp dir %s: %v", redisConfDir, err)
+				}
+			}()
+			if err := redisConfFile.Close(); err != nil {
+				t.Errorf("Error closing redis config file: %v", err)
+			}
 
 			podInfoTempDir, _ := os.MkdirTemp("", "pod-info-test")
 			memLimitFile, err := os.Create(filepath.Join(podInfoTempDir, "mem-limit"))
 			if err != nil {
 				t.Errorf("Couldn' t create temporary config file: %v", err)
 			}
-			defer os.RemoveAll(podInfoTempDir)
+			defer func() {
+				if err := os.RemoveAll(podInfoTempDir); err != nil {
+					t.Logf("Error removing temp dir %s: %v", podInfoTempDir, err)
+				}
+			}()
 			_, err = memLimitFile.Write([]byte(tc.podRequestLimit))
 			if err != nil {
 				t.Errorf("Couldn't write to temporary config file: %v", err)
 			}
-			memLimitFile.Close()
+			if err := memLimitFile.Close(); err != nil {
+				t.Errorf("Error closing memory limit file: %v", err)
+			}
 
 			var additionalConfigFileNames []string
 			additionalConfDir, _ := os.MkdirTemp("", "additional-redisconf")
@@ -99,8 +111,14 @@ cluster-node-timeout 321`,
 				if err != nil {
 					t.Errorf("Couldn't write to temporary config file: %v", err)
 				}
-				defer os.RemoveAll(redisConfDir)
-				configFile.Close()
+				defer func() {
+					if err := os.RemoveAll(redisConfDir); err != nil {
+						t.Logf("Error removing temp dir %s: %v", redisConfDir, err)
+					}
+				}()
+				if err := configFile.Close(); err != nil {
+					t.Errorf("Error closing config file: %v", err)
+				}
 			}
 
 			a := admin.NewFakeAdmin()

pkg/redisnode/redisnode.go

@@ -269,29 +269,37 @@ func (r *RedisNode) configureHealth(ctx context.Context) error {
 func readinessCheck(ctx context.Context, addr string) error {
 	client, rediserr := redis.NewClient(ctx, addr, time.Second, map[string]string{}) // will fail if node not accessible or slot range not set
 	if rediserr != nil {
-		return fmt.Errorf("Readiness failed, err: %v", rediserr)
+		return fmt.Errorf("readiness failed, err: %v", rediserr)
 	}
-	defer client.Close()
+	defer func() {
+		if err := client.Close(); err != nil {
+			glog.Errorf("Error closing redis client in readiness check: %v", err)
+		}
+	}()
 
 	var resp radix.ClusterTopo
 	err := client.DoCmd(ctx, &resp, "CLUSTER", "SLOTS")
 	if err != nil {
-		return fmt.Errorf("Readiness failed, cluster slots response err: %v", err)
+		return fmt.Errorf("readiness failed, cluster slots response err: %v", err)
 	}
 	if len(resp) == 0 {
-		return fmt.Errorf("Readiness failed, cluster slots response empty")
+		return fmt.Errorf("readiness failed, cluster slots response empty")
 	}
-	glog.V(6).Info("Readiness probe ok")
+	glog.V(6).Info("readiness probe ok")
 	return nil
 }
 
 func livenessCheck(ctx context.Context, addr string) error {
 	client, rediserr := redis.NewClient(ctx, addr, time.Second, map[string]string{}) // will fail if node not accessible or slot range not set
 	if rediserr != nil {
-		return fmt.Errorf("Liveness failed, err: %v", rediserr)
+		return fmt.Errorf("liveness failed, err: %v", rediserr)
 	}
-	defer client.Close()
-	glog.V(6).Info("Liveness probe ok")
+	defer func() {
+		if err := client.Close(); err != nil {
+			glog.Errorf("Error closing redis client in liveness check: %v", err)
+		}
+	}()
+	glog.V(6).Info("liveness probe ok")
 	return nil
 }
 
@@ -349,14 +357,22 @@ func testAndWaitConnection(ctx context.Context, addr string, maxWait time.Durati
 			time.Sleep(100 * time.Millisecond)
 			continue
 		}
-		defer client.Close()
+		defer func() {
+			if err := client.Close(); err != nil {
+				glog.Errorf("Error closing test connection: %v", err)
+			}
+		}()
 		var resp string
 		if err := client.Do(ctx, radix.Cmd(&resp, "PING")); err != nil {
-			client.Close()
+			if err := client.Close(); err != nil {
+				glog.Errorf("Error closing client after PING error: %v", err)
+			}
 			time.Sleep(100 * time.Millisecond)
 			continue
 		} else if resp != "PONG" {
-			client.Close()
+			if err := client.Close(); err != nil {
+				glog.Errorf("Error closing client after wrong PING response: %v", err)
+			}
 			time.Sleep(100 * time.Millisecond)
 			continue
 		}

pkg/redisnode/redisnode_test.go

@@ -118,7 +118,11 @@ func TestRedisInitializationAttach(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer os.Remove(tmpfile.Name()) // clean up
+	defer func() {
+		if err := os.Remove(tmpfile.Name()); err != nil {
+			t.Logf("Error removing temp file %s: %v", tmpfile.Name(), err)
+		}
+	}() // clean up
 
 	c := &Config{
 		Redis:   config.Redis{ServerPort: "1234", ConfigFileName: tmpfile.Name()},

test/e2e/framework/logger.go

@@ -12,7 +12,9 @@ func nowStamp() string {
 }
 
 func log(level string, format string, args ...interface{}) {
-	fmt.Fprintf(ginkgo.GinkgoWriter, nowStamp()+": "+level+": "+format+"\n", args...)
+	if _, err := fmt.Fprintf(ginkgo.GinkgoWriter, nowStamp()+": "+level+": "+format+"\n", args...); err != nil {
+		ginkgo.Fail(fmt.Sprintf("failed to log: %v", err), 1)
+	}
 }
 
 // Logf logs in e2e framework