fix: added some disable command additions to the rename-command (#2540)

* Added some disable command additions to the rename-command

* add rename replication test

---------

Co-authored-by: wuxianrong <wuxianrong@360.cn>

conf/pika.conf

@@ -513,18 +513,19 @@ cache-lfu-decay-time: 1
 #
 # aclfile : ../conf/users.acl
 
+# (experimental)
 # It is possible to change the name of dangerous commands in a shared environment.
 # For instance the CONFIG command may be renamed into something Warning: To prevent
 # data inconsistency caused by different configuration files, do not use the rename
 # command to modify write commands on the primary and secondary servers. If necessary,
 # ensure that the configuration files of the primary and secondary servers are consistent
 # In addition, when using the command rename, you must not use ""  to modify the command,
-# for example, rename-command: FLUSHALL "360flushall" is incorrect; instead, use
-# rename-command: FLUSHALL 360flushall is correct. After the rename command is executed,
+# for example, rename-command: FLUSHDB "360flushdb" is incorrect; instead, use
+# rename-command: FLUSHDB 360flushdb is correct. After the rename command is executed,
 # it is most appropriate to use a numeric string with uppercase or lowercase letters
-# for example: rename-command : FLUSHALL joYAPNXRPmcarcR4ZDgC81TbdkSmLAzRPmcarcR
+# for example: rename-command : FLUSHDB joYAPNXRPmcarcR4ZDgC81TbdkSmLAzRPmcarcR
+# Warning: Currently only applies to flushdb, slaveof, bgsave, shutdown, config command
+# Warning: Ensure that the Settings of rename-command on the master and slave servers are consistent
 #
 # Example:
-#
-# rename-command : FLUSHALL 360flushall
 # rename-command : FLUSHDB 360flushdb

tests/integration/options.go

@@ -7,13 +7,17 @@ import (
 )
 
 const (
-	LOCALHOST  = "127.0.0.1"
-	SLAVEPORT  = "9231"
-	MASTERPORT = "9241"
-	SINGLEADDR = "127.0.0.1:9221"
-	SLAVEADDR  = "127.0.0.1:9231"
-	MASTERADDR = "127.0.0.1:9241"
-	RenameADDR = "127.0.0.1:9251"
+	LOCALHOST        = "127.0.0.1"
+	SLAVEPORT        = "9231"
+	MASTERPORT       = "9241"
+	SLAVERENAMEPORT  = "9301"
+	MASTERRENAMEPORT = "9291"
+	SINGLEADDR       = "127.0.0.1:9221"
+	SLAVEADDR        = "127.0.0.1:9231"
+	MASTERADDR       = "127.0.0.1:9241"
+	SLAVERENAMEADDR  = "127.0.0.1:9301"
+	MASTERRENAMEADDR = "127.0.0.1:9291"
+	RenameADDR       = "127.0.0.1:9251"
 
 	CODISADDR = "127.0.0.1:19000"
 

tests/integration/renamecommand_test.go

@@ -40,23 +40,4 @@ var _ = Describe("Rename Command test", func() {
 		r = client.Do(ctx, "flushdb")
 		Expect(r.Val()).NotTo(Equal("OK"))
 	})
-
-	It("should 360FlushAll", func() {
-		set := client.Set(ctx, "key", "foobar", 0)
-		Expect(set.Err()).NotTo(HaveOccurred())
-		Expect(set.Val()).To(Equal("OK"))
-
-		bitCount := client.BitCount(ctx, "key", nil)
-		Expect(bitCount.Err()).NotTo(HaveOccurred())
-		Expect(bitCount.Val()).To(Equal(int64(26)))
-		_, err := client.Do(ctx, "360flushall").Result()
-		Expect(err).NotTo(HaveOccurred())
-		r := client.Do(ctx, "360flushall")
-		Expect(r.Val()).To(Equal("OK"))
-		n, err := client.Exists(ctx, "key").Result()
-		Expect(err).NotTo(HaveOccurred())
-		Expect(n).To(Equal(int64(0)))
-		r = client.Do(ctx, "flushall")
-		Expect(r.Val()).NotTo(Equal("OK"))
-	})
 })

tests/integration/replication_rename_test.go

@@ -0,0 +1,88 @@
+package pika_integration
+
+import (
+	"context"
+	"log"
+	"time"
+
+	. "github.com/bsm/ginkgo/v2"
+	. "github.com/bsm/gomega"
+	"github.com/redis/go-redis/v9"
+)
+
+var _ = Describe("should replication rename", func() {
+	Describe("all replication rename test", func() {
+		ctx := context.TODO()
+		var clientSlave *redis.Client
+		var clientMaster *redis.Client
+
+		BeforeEach(func() {
+			clientMaster = redis.NewClient(PikaOption(MASTERRENAMEADDR))
+			clientSlave = redis.NewClient(PikaOption(SLAVERENAMEADDR))
+			cleanEnv(ctx, clientMaster, clientSlave)
+			if GlobalBefore != nil {
+				GlobalBefore(ctx, clientMaster)
+				GlobalBefore(ctx, clientSlave)
+			}
+		})
+		AfterEach(func() {
+			cleanEnv(ctx, clientMaster, clientSlave)
+			Expect(clientSlave.Close()).NotTo(HaveOccurred())
+			Expect(clientMaster.Close()).NotTo(HaveOccurred())
+			log.Println("Replication test case done")
+		})
+
+		It("Let The slave become a replica of The master ", func() {
+			infoRes := clientSlave.Info(ctx, "replication")
+			Expect(infoRes.Err()).NotTo(HaveOccurred())
+			Expect(infoRes.Val()).To(ContainSubstring("role:master"))
+			infoRes = clientMaster.Info(ctx, "replication")
+			Expect(infoRes.Err()).NotTo(HaveOccurred())
+			Expect(infoRes.Val()).To(ContainSubstring("role:master"))
+			Expect(clientSlave.Do(ctx, "slaveof", LOCALHOST, SLAVERENAMEPORT).Err()).To(MatchError("ERR The master ip:port and the slave ip:port are the same"))
+
+			var count = 0
+			for {
+				res := trySlave(ctx, clientSlave, LOCALHOST, MASTERRENAMEPORT)
+				if res {
+					break
+				} else if count > 4 {
+					break
+				} else {
+					cleanEnv(ctx, clientMaster, clientSlave)
+					count++
+				}
+			}
+
+			infoRes = clientSlave.Info(ctx, "replication")
+			Expect(infoRes.Err()).NotTo(HaveOccurred())
+			Expect(infoRes.Val()).To(ContainSubstring("master_link_status:up"))
+
+			infoRes = clientMaster.Info(ctx, "replication")
+			Expect(infoRes.Err()).NotTo(HaveOccurred())
+			Expect(infoRes.Val()).To(ContainSubstring("connected_slaves:1"))
+
+			slaveWrite := clientSlave.Set(ctx, "foo", "bar", 0)
+			Expect(slaveWrite.Err()).To(MatchError("ERR Server in read-only"))
+			log.Println("Replication rename test 1 start")
+			set := clientMaster.Set(ctx, "x", "y", 0)
+			Expect(set.Err()).NotTo(HaveOccurred())
+			Expect(set.Val()).To(Equal("OK"))
+			set1 := clientMaster.Set(ctx, "a", "b", 0)
+			Expect(set1.Err()).NotTo(HaveOccurred())
+			Expect(set1.Val()).To(Equal("OK"))
+			r1 := clientMaster.Do(ctx, "flushdb")
+			Expect(r1.Val()).NotTo(Equal("OK"))
+			time.Sleep(3 * time.Second)
+			Expect(clientMaster.Do(ctx, "360flushdb").Err()).NotTo(HaveOccurred())
+			Eventually(func() error {
+				return clientMaster.Get(ctx, "x").Err()
+			}, "60s", "100ms").Should(Equal(redis.Nil))
+			Eventually(func() error {
+				return clientSlave.Get(ctx, "x").Err()
+			}, "60s", "100ms").Should(Equal(redis.Nil))
+			log.Println("Replication rename test 1 success")
+
+		})
+	})
+})

tests/integration/start_master_and_slave.sh

@@ -6,6 +6,8 @@ cp ../conf/pika.conf ./pika_single.conf
 cp ../conf/pika.conf ./pika_master.conf
 cp ../conf/pika.conf ./pika_slave.conf
 cp ../conf/pika.conf ./pika_rename.conf
+cp ../conf/pika.conf ./pika_master_rename.conf
+cp ../conf/pika.conf ./pika_slave_rename.conf
 cp ../conf/pika.conf ./pika_acl_both_password.conf
 cp ../conf/pika.conf ./pika_acl_only_admin_password.conf
 cp ../conf/pika.conf ./pika_has_other_acl_user.conf
@@ -38,7 +40,6 @@ sed -i ''  \
   -e 's|#daemonize : yes|daemonize : yes|' ./pika_slave.conf
 
 sed -i ''  \
-  -e 's|# rename-command : FLUSHALL 360flushall|rename-command : FLUSHALL 360flushall|'  \
   -e 's|# rename-command : FLUSHDB 360flushdb|rename-command : FLUSHDB 360flushdb|'  \
   -e 's|databases : 1|databases : 2|'  \
   -e 's|port : 9221|port : 9251|'  \
@@ -73,6 +74,7 @@ sed -i ''  \
   -e 's|pidfile : ./pika.pid|pidfile : ./acl2_data/pika.pid|'  \
   -e 's|db-sync-path : ./dbsync/|db-sync-path : ./acl2_data/dbsync/|'  \
   -e 's|#daemonize : yes|daemonize : yes|' ./pika_acl_only_admin_password.conf
+
 sed -i ''  \
   -e 's|requirepass :|requirepass : requirepass|'  \
   -e 's|masterauth :|masterauth : requirepass|'  \
@@ -87,6 +89,26 @@ sed -i ''  \
   -e 's|#daemonize : yes|daemonize : yes|' ./pika_has_other_acl_user.conf
 echo -e '\nuser : limit on >limitpass ~* +@all &*' >> ./pika_has_other_acl_user.conf
 
+sed -i ''  \
+  -e 's|# rename-command : FLUSHDB 360flushdb|rename-command : FLUSHDB 360flushdb|'  \
+  -e 's|port : 9221|port : 9291|'   \
+  -e 's|log-path : ./log/|log-path : ./master_rename_data/log/|'  \
+  -e 's|db-path : ./db/|db-path : ./master_rename_data/db/|'  \
+  -e 's|dump-path : ./dump/|dump-path : ./master_rename_data/dump/|'  \
+  -e 's|pidfile : ./pika.pid|pidfile : ./master_rename_data/pika.pid|'  \
+  -e 's|db-sync-path : ./dbsync/|db-sync-path : ./master_rename_data/dbsync/|'  \
+  -e 's|#daemonize : yes|daemonize : yes|' ./pika_master_rename.conf
+
+sed -i ''  \
+  -e 's|# rename-command : FLUSHDB 360flushdb|rename-command : FLUSHDB 360flushdb|'  \
+  -e 's|port : 9221|port : 9301|'   \
+  -e 's|log-path : ./log/|log-path : ./slave_rename_data/log/|'  \
+  -e 's|db-path : ./db/|db-path : ./slave_rename_data/db/|'  \
+  -e 's|dump-path : ./dump/|dump-path : ./slave_rename_data/dump/|'  \
+  -e 's|pidfile : ./pika.pid|pidfile : ./slave_rename_data/pika.pid|'  \
+  -e 's|db-sync-path : ./dbsync/|db-sync-path : ./slave_rename_data/dbsync/|'  \
+  -e 's|#daemonize : yes|daemonize : yes|' ./pika_slave_rename.conf
+
 # Start three nodes
 ./pika -c ./pika_single.conf
 ./pika -c ./pika_master.conf
@@ -95,5 +117,7 @@ echo -e '\nuser : limit on >limitpass ~* +@all &*' >> ./pika_has_other_acl_user.
 ./pika -c ./pika_acl_both_password.conf
 ./pika -c ./pika_acl_only_admin_password.conf
 ./pika -c ./pika_has_other_acl_user.conf
+./pika -c ./pika_master_rename.conf
+./pika -c ./pika_slave_rename.conf
 #ensure both master and slave are ready
 sleep 10