Merge branch 'master' into implement-tls-url-parameters-pr2076

Author: ndyakov

CONTRIBUTING.md

@@ -37,7 +37,7 @@ Here's how to get started with your code contribution:
 > Note: this clones and builds the docker containers specified in `docker-compose.yml`, to understand more about
 > the infrastructure that will be started you can check the `docker-compose.yml`. You also have the possiblity
 > to specify the redis image that will be pulled with the env variable `CLIENT_LIBS_TEST_IMAGE`.
-> By default the docker image that will be pulled and started is `redislabs/client-libs-test:rs-7.4.0-v2`.
+> By default the docker image that will be pulled and started is `redislabs/client-libs-test:8.2.1-pre`.
 > If you want to test with newer Redis version, using a newer version of `redislabs/client-libs-test` should work out of the box.
 
 4.  While developing, make sure the tests pass by running `make test` (if you have the docker containers running, `make test.ci` may be sufficient).

Makefile

@@ -1,6 +1,14 @@
 GO_MOD_DIRS := $(shell find . -type f -name 'go.mod' -exec dirname {} \; | sort)
+REDIS_VERSION ?= 8.2
+RE_CLUSTER ?= false
+RCE_DOCKER ?= true
+CLIENT_LIBS_TEST_IMAGE ?= redislabs/client-libs-test:8.2.1-pre
 
 docker.start:
+	export RE_CLUSTER=$(RE_CLUSTER) && \
+	export RCE_DOCKER=$(RCE_DOCKER) && \
+	export REDIS_VERSION=$(REDIS_VERSION) && \
+	export CLIENT_LIBS_TEST_IMAGE=$(CLIENT_LIBS_TEST_IMAGE) && \
 	docker compose --profile all up -d --quiet-pull
 
 docker.stop:
@@ -27,6 +35,9 @@ test.ci:
 	set -e; for dir in $(GO_MOD_DIRS); do \
 	  echo "go test in $${dir}"; \
 	  (cd "$${dir}" && \
+	    export RE_CLUSTER=$(RE_CLUSTER) && \
+	    export RCE_DOCKER=$(RCE_DOCKER) && \
+	    export REDIS_VERSION=$(REDIS_VERSION) && \
 	    go mod tidy -compat=1.18 && \
 	    go vet && \
 	    go test -v -coverprofile=coverage.txt -covermode=atomic ./... -race -skip Example); \
@@ -38,6 +49,9 @@ test.ci.skip-vectorsets:
 	set -e; for dir in $(GO_MOD_DIRS); do \
 	  echo "go test in $${dir} (skipping vector sets)"; \
 	  (cd "$${dir}" && \
+	    export RE_CLUSTER=$(RE_CLUSTER) && \
+	    export RCE_DOCKER=$(RCE_DOCKER) && \
+	    export REDIS_VERSION=$(REDIS_VERSION) && \
 	    go mod tidy -compat=1.18 && \
 	    go vet && \
 	    go test -v -coverprofile=coverage.txt -covermode=atomic ./... -race \
@@ -47,11 +61,17 @@ test.ci.skip-vectorsets:
 	go vet -vettool ./internal/customvet/customvet
 
 bench:
+	export RE_CLUSTER=$(RE_CLUSTER) && \
+	export RCE_DOCKER=$(RCE_DOCKER) && \
+	export REDIS_VERSION=$(REDIS_VERSION) && \
 	go test ./... -test.run=NONE -test.bench=. -test.benchmem -skip Example
 
 .PHONY: all test test.ci test.ci.skip-vectorsets bench fmt
 
 build:
+	export RE_CLUSTER=$(RE_CLUSTER) && \
+	export RCE_DOCKER=$(RCE_DOCKER) && \
+	export REDIS_VERSION=$(REDIS_VERSION) && \
 	go build .
 
 fmt:

README.md

@@ -432,36 +432,9 @@ res, err := rdb.Do(ctx, "set", "key", "value").Result()
 
 ## Run the test
 
-go-redis will start a redis-server and run the test cases.
-
-The paths of redis-server bin file and redis config file are defined in `main_test.go`:
-
-```go
-var (
-	redisServerBin, _  = filepath.Abs(filepath.Join("testdata", "redis", "src", "redis-server"))
-	redisServerConf, _ = filepath.Abs(filepath.Join("testdata", "redis", "redis.conf"))
-)
-```
-
-For local testing, you can change the variables to refer to your local files, or create a soft link
-to the corresponding folder for redis-server and copy the config file to `testdata/redis/`:
-
-```shell
-ln -s /usr/bin/redis-server ./go-redis/testdata/redis/src
-cp ./go-redis/testdata/redis.conf ./go-redis/testdata/redis/
-```
-
-Lastly, run:
-
-```shell
-go test
-```
-
-Another option is to run your specific tests with an already running redis. The example below, tests
-against a redis running on port 9999.:
-
+Recommended to use Docker, just need to run:
 ```shell
-REDIS_PORT=9999 go test <your options>
+make test
 ```
 
 ## See also

commands.go

@@ -253,6 +253,7 @@ var (
 	_ Cmdable = (*Tx)(nil)
 	_ Cmdable = (*Ring)(nil)
 	_ Cmdable = (*ClusterClient)(nil)
+	_ Cmdable = (*Pipeline)(nil)
 )
 
 type cmdable func(ctx context.Context, cmd Cmder) error

docker-compose.yml

@@ -2,7 +2,7 @@
 
 services:
   redis:
-    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:rs-7.4.0-v2}
+    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:8.2.1-pre}
     platform: linux/amd64
     container_name: redis-standalone
     environment:
@@ -23,7 +23,7 @@ services:
       - all
 
   osscluster:
-    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:rs-7.4.0-v2}
+    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:8.2.1-pre}
     platform: linux/amd64
     container_name: redis-osscluster
     environment:
@@ -40,7 +40,7 @@ services:
       - all
 
   sentinel-cluster:
-    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:rs-7.4.0-v2}
+    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:8.2.1-pre}
     platform: linux/amd64
     container_name: redis-sentinel-cluster
     network_mode: "host"
@@ -60,7 +60,7 @@ services:
       - all
 
   sentinel:
-    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:rs-7.4.0-v2}
+    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:8.2.1-pre}
     platform: linux/amd64
     container_name: redis-sentinel
     depends_on:
@@ -84,7 +84,7 @@ services:
       - all
 
   ring-cluster:
-    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:rs-7.4.0-v2}
+    image: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:8.2.1-pre}
     platform: linux/amd64
     container_name: redis-ring-cluster
     environment:

extra/redisotel/metrics.go

@@ -155,6 +155,23 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro
 		return nil, err
 	}
 
+	waits, err := conf.meter.Int64ObservableUpDownCounter(
+		"db.client.connections.waits",
+		metric.WithDescription("The number of times a connection was waited for"),
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	waitsDuration, err := conf.meter.Int64ObservableUpDownCounter(
+		"db.client.connections.waits_duration",
+		metric.WithDescription("The total time spent for waiting a connection in nanoseconds"),
+		metric.WithUnit("ns"),
+	)
+	if err != nil {
+		return nil, err
+	}
+
 	timeouts, err := conf.meter.Int64ObservableUpDownCounter(
 		"db.client.connections.timeouts",
 		metric.WithDescription("The number of connection timeouts that have occurred trying to obtain a connection from the pool"),
@@ -191,6 +208,9 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro
 			o.ObserveInt64(usage, int64(stats.IdleConns), metric.WithAttributeSet(idleAttrs))
 			o.ObserveInt64(usage, int64(stats.TotalConns-stats.IdleConns), metric.WithAttributeSet(usedAttrs))
 
+			o.ObserveInt64(waits, int64(stats.WaitCount), metric.WithAttributeSet(poolAttrs))
+			o.ObserveInt64(waitsDuration, stats.WaitDurationNs, metric.WithAttributeSet(poolAttrs))
+
 			o.ObserveInt64(timeouts, int64(stats.Timeouts), metric.WithAttributeSet(poolAttrs))
 			o.ObserveInt64(hits, int64(stats.Hits), metric.WithAttributeSet(poolAttrs))
 			o.ObserveInt64(misses, int64(stats.Misses), metric.WithAttributeSet(poolAttrs))

internal_test.go

@@ -383,3 +383,38 @@ var _ = Describe("ClusterClient", func() {
 		})
 	})
 })
+
+var _ = Describe("isLoopback", func() {
+	DescribeTable("should correctly identify loopback addresses",
+		func(host string, expected bool) {
+			result := isLoopback(host)
+			Expect(result).To(Equal(expected))
+		},
+		// IP addresses
+		Entry("IPv4 loopback", "127.0.0.1", true),
+		Entry("IPv6 loopback", "::1", true),
+		Entry("IPv4 non-loopback", "192.168.1.1", false),
+		Entry("IPv6 non-loopback", "2001:db8::1", false),
+
+		// Well-known loopback hostnames
+		Entry("localhost lowercase", "localhost", true),
+		Entry("localhost uppercase", "LOCALHOST", true),
+		Entry("localhost mixed case", "LocalHost", true),
+
+		// Docker-specific loopbacks
+		Entry("host.docker.internal", "host.docker.internal", true),
+		Entry("HOST.DOCKER.INTERNAL", "HOST.DOCKER.INTERNAL", true),
+		Entry("custom.docker.internal", "custom.docker.internal", true),
+		Entry("app.docker.internal", "app.docker.internal", true),
+
+		// Non-loopback hostnames
+		Entry("redis hostname", "redis-cluster", false),
+		Entry("FQDN", "redis.example.com", false),
+		Entry("docker but not internal", "redis.docker.com", false),
+
+		// Edge cases
+		Entry("empty string", "", false),
+		Entry("invalid IP", "256.256.256.256", false),
+		Entry("partial docker internal", "docker.internal", false),
+	)
+})

main_test.go

@@ -68,7 +68,7 @@ var RCEDocker = false
 // Notes version of redis we are executing tests against.
 // This can be used before we change the bsm fork of ginkgo for one,
 // which have support for label sets, so we can filter tests per redis version.
-var RedisVersion float64 = 7.2
+var RedisVersion float64 = 8.2
 
 func SkipBeforeRedisVersion(version float64, msg string) {
 	if RedisVersion < version {
@@ -95,7 +95,7 @@ var _ = BeforeSuite(func() {
 	RedisVersion, _ = strconv.ParseFloat(strings.Trim(os.Getenv("REDIS_VERSION"), "\""), 64)
 
 	if RedisVersion == 0 {
-		RedisVersion = 7.2
+		RedisVersion = 8.2
 	}
 
 	fmt.Printf("RECluster: %v\n", RECluster)

osscluster.go

@@ -847,12 +847,25 @@ func replaceLoopbackHost(nodeAddr, originHost string) string {
 	return net.JoinHostPort(originHost, nodePort)
 }
 
+// isLoopback returns true if the host is a loopback address.
+// For IP addresses, it uses net.IP.IsLoopback().
+// For hostnames, it recognizes well-known loopback hostnames like "localhost"
+// and Docker-specific loopback patterns like "*.docker.internal".
 func isLoopback(host string) bool {
 	ip := net.ParseIP(host)
-	if ip == nil {
+	if ip != nil {
+		return ip.IsLoopback()
+	}
+
+	if strings.ToLower(host) == "localhost" {
 		return true
 	}
-	return ip.IsLoopback()
+
+	if strings.HasSuffix(strings.ToLower(host), ".docker.internal") {
+		return true
+	}
+
+	return false
 }
 
 func (c *clusterState) slotMasterNode(slot int) (*clusterNode, error) {
@@ -1887,14 +1900,33 @@ func (c *ClusterClient) pubSub() *PubSub {
 			}
 
 			var err error
+
 			if len(channels) > 0 {
 				slot := hashtag.Slot(channels[0])
-				node, err = c.slotMasterNode(ctx, slot)
+
+				// newConn in PubSub is only used for subscription connections, so it is safe to
+				// assume that a slave node can always be used when client options specify ReadOnly.
+				if c.opt.ReadOnly {
+					state, err := c.state.Get(ctx)
+					if err != nil {
+						return nil, err
+					}
+
+					node, err = c.slotReadOnlyNode(state, slot)
+					if err != nil {
+						return nil, err
+					}
+				} else {
+					node, err = c.slotMasterNode(ctx, slot)
+					if err != nil {
+						return nil, err
+					}
+				}
 			} else {
 				node, err = c.nodes.Random()
-			}
-			if err != nil {
-				return nil, err
+				if err != nil {
+					return nil, err
+				}
 			}
 
 			cn, err := node.Client.newConn(context.TODO())