From b51e91cb8b509a6af280d3a4c55b69ccb14d4afe Mon Sep 17 00:00:00 2001 From: meir Date: Thu, 3 Feb 2022 16:42:10 +0200 Subject: [PATCH 01/11] Moved to redispy 4.2 and add tls-passphrase option --- .github/workflows/ci.yml | 4 +-- RLTest/__main__.py | 4 +++ RLTest/env.py | 9 +++-- RLTest/redis_cluster.py | 57 ++++++++++-------------------- RLTest/redis_enterprise_cluster.py | 1 - RLTest/redis_std.py | 6 +++- pyproject.toml | 2 +- 7 files changed, 36 insertions(+), 47 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 180c842..2b310d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,4 @@ -name: CI +name: CI` on: push: @@ -16,7 +16,7 @@ jobs: strategy: matrix: platform: ['ubuntu-20.04', 'ubuntu-18.04', 'ubuntu-16.04'] - python: ['2.7', '3.6', '3.7', '3.8', '3.9'] + python: ['3.6', '3.7', '3.8', '3.9'] steps: - uses: actions/checkout@v2 diff --git a/RLTest/__main__.py b/RLTest/__main__.py index 45b0521..342b780 100644 --- a/RLTest/__main__.py +++ b/RLTest/__main__.py @@ -283,6 +283,9 @@ def do_normal_conn(self, line): parser.add_argument( '--tls-ca-cert-file', default=None, help='/path/to/ca.crt') +parser.add_argument( + '--tls-passphrase', default=None, help='passphrase to use on decript key file') + class EnvScopeGuard: def __init__(self, runner): self.runner = runner @@ -402,6 +405,7 @@ def __init__(self): Defaults.tls_cert_file = self.args.tls_cert_file Defaults.tls_key_file = self.args.tls_key_file Defaults.tls_ca_cert_file = self.args.tls_ca_cert_file + Defaults.tls_passphrase = self.args.tls_passphrase Defaults.oss_password = self.args.oss_password Defaults.cluster_node_timeout = self.args.cluster_node_timeout if Defaults.use_unix and Defaults.use_slaves: diff --git a/RLTest/env.py b/RLTest/env.py index 580931e..090e16b 100644 --- a/RLTest/env.py +++ b/RLTest/env.py @@ -8,7 +8,6 @@ import unittest import warnings -from .Enterprise import EnterpriseClusterEnv from .exists_redis import ExistsRedisEnv from .redis_cluster import ClusterEnv from .redis_enterprise_cluster import EnterpriseRedisClusterEnv @@ -115,6 +114,7 @@ class Defaults: tls_cert_file = None tls_key_file = None tls_ca_cert_file = None + tls_passphrase = None debugger = None debug_print = False debug_pause = False @@ -146,6 +146,7 @@ def getKwargs(self): 'tlsCertFile': self.tls_cert_file, 'tlsKeyFile': self.tls_key_file, 'tlsCaCertFile': self.tls_ca_cert_file, + 'tlsPassphrase': self.tls_passphrase, 'password': self.oss_password } return kwargs @@ -167,7 +168,7 @@ def compareEnvs(self, env): def __init__(self, testName=None, testDescription=None, module=None, moduleArgs=None, env=None, useSlaves=None, shardsCount=None, decodeResponses=None, useAof=None, useRdbPreamble=None, forceTcp=False, useTLS=False, tlsCertFile=None, tlsKeyFile=None, - tlsCaCertFile=None, logDir=None, redisBinaryPath=None, dmcBinaryPath=None, + tlsCaCertFile=None, tlsPassphrase=None, logDir=None, redisBinaryPath=None, dmcBinaryPath=None, redisEnterpriseBinaryPath=None, noDefaultModuleArgs=False, clusterNodeTimeout = None, freshEnv=False): @@ -198,6 +199,7 @@ def __init__(self, testName=None, testDescription=None, module=None, self.tlsCertFile = tlsCertFile if tlsCertFile else Defaults.tls_cert_file self.tlsKeyFile = tlsKeyFile if tlsKeyFile else Defaults.tls_key_file self.tlsCaCertFile = tlsCaCertFile if tlsCaCertFile else Defaults.tls_ca_cert_file + self.tlsPassphrase = tlsPassphrase if tlsPassphrase else Defaults.tls_passphrase self.redisBinaryPath = expandBinary(redisBinaryPath) if redisBinaryPath else Defaults.binary self.dmcBinaryPath = expandBinary(dmcBinaryPath) if dmcBinaryPath else Defaults.proxy_binary @@ -298,7 +300,8 @@ def getEnvKwargs(self): 'tlsCertFile': self.tlsCertFile, 'tlsKeyFile': self.tlsKeyFile, 'tlsCaCertFile': self.tlsCaCertFile, - 'clusterNodeTimeout': self.clusterNodeTimeout + 'clusterNodeTimeout': self.clusterNodeTimeout, + 'tlsPassphrase': self.tlsPassphrase } return kwargs diff --git a/RLTest/redis_cluster.py b/RLTest/redis_cluster.py index 34452e8..a98234d 100644 --- a/RLTest/redis_cluster.py +++ b/RLTest/redis_cluster.py @@ -1,9 +1,7 @@ from __future__ import print_function -from rediscluster.connection import SSLClusterConnection, ClusterConnectionPool - from .redis_std import StandardEnv -import rediscluster +from redis.cluster import ClusterNode import redis import time from RLTest.utils import Colors @@ -21,6 +19,7 @@ def __init__(self, **kwargs): useSlaves = kwargs.get('useSlaves', False) self.useTLS = kwargs['useTLS'] self.decodeResponses = kwargs.get('decodeResponses', False) + self.tlsPassphrase = kwargs.get('tlsPassphrase', None) startPort = 20000 totalRedises = self.shardsCount * (2 if useSlaves else 1) randomizePorts = kwargs.pop('randomizePorts', False) @@ -51,7 +50,11 @@ def waitCluster(self, timeout_sec=40): ok = 0 for shard in self.shards: con = shard.getConnection() - status = con.execute_command('CLUSTER', 'INFO') + try: + status = con.execute_command('CLUSTER', 'INFO') + except Exception as e: + print('got error on cluster info, will try again, %s' % str(e)) + continue if 'cluster_state:ok' in str(status): ok += 1 if ok == len(self.shards): @@ -115,28 +118,21 @@ def getConnection(self, shardId=1): return self.shards[shardId - 1].getConnection() def getClusterConnection(self): + statupNode = [ClusterNode(a['host'], a['port']) for a in self.getMasterNodesList()] if self.useTLS: - # workaround for error on - # got an unexpected keyword argument 'ssl' - # we enforce the connection_class instead of setting ssl=True - pool = ClusterConnectionPool( - startup_nodes=self.getMasterNodesList(), - connection_class=SSLClusterConnection, - ssl_cert_reqs=None, + return redis.RedisCluster( + ssl=True, ssl_keyfile=self.shards[0].getTLSKeyFile(), ssl_certfile=self.shards[0].getTLSCertFile(), + ssl_cert_reqs=None, ssl_ca_certs=self.shards[0].getTLSCACertFile(), - ) - if pool.connection_kwargs: - pool.connection_kwargs.pop('ssl', None) - return rediscluster.RedisCluster( - startup_nodes=self.getMasterNodesList(), - connection_pool=pool, + ssl_password=self.tlsPassphrase, + startup_nodes=statupNode, decode_responses=self.decodeResponses ) else: - return rediscluster.RedisCluster( - startup_nodes=self.getMasterNodesList(), + return redis.RedisCluster( + startup_nodes=statupNode, decode_responses=self.decodeResponses, password=self.password) def getSlaveConnection(self): @@ -159,26 +155,9 @@ def getOSSMasterNodesConnectionList(self): # Gets a cluster connection by key. On std redis the default connection is returned. def getConnectionByKey(self, key, command): - if self.useTLS: - # workaround for error on - # got an unexpected keyword argument 'ssl' - # we enforce the connection_class instead of setting ssl=True - pool = ClusterConnectionPool( - startup_nodes=self.getMasterNodesList(), - connection_class=SSLClusterConnection, - ssl_cert_reqs=None, - ssl_keyfile=self.shards[0].getTLSKeyFile(), - ssl_certfile=self.shards[0].getTLSCertFile(), - ssl_ca_certs=self.shards[0].getTLSCACertFile(), - ) - if pool.connection_kwargs: - pool.connection_kwargs.pop('ssl', None) - else: - pool = ClusterConnectionPool( - startup_nodes=self.getMasterNodesList() - ) - con = pool.get_connection_by_key(key, command) - return redis.StrictRedis(host=con.host, port=con.port, decode_responses=self.decodeResponses, password=self.password) + clusterConn = self.getClusterConnection() + target_node = clusterConn._determine_nodes(command) + return clusterConn.get_redis_connection(target_node) def flush(self): self.getClusterConnection().flushall() diff --git a/RLTest/redis_enterprise_cluster.py b/RLTest/redis_enterprise_cluster.py index 34935db..7edaa5d 100644 --- a/RLTest/redis_enterprise_cluster.py +++ b/RLTest/redis_enterprise_cluster.py @@ -1,4 +1,3 @@ -import rediscluster from redis import StrictRedis from .exists_redis import ExistsRedisEnv diff --git a/RLTest/redis_std.py b/RLTest/redis_std.py index 5d73500..848d9fc 100644 --- a/RLTest/redis_std.py +++ b/RLTest/redis_std.py @@ -21,7 +21,7 @@ class StandardEnv(object): def __init__(self, redisBinaryPath, port=6379, modulePath=None, moduleArgs=None, outputFilesFormat=None, dbDirPath=None, useSlaves=False, serverId=1, password=None, libPath=None, clusterEnabled=False, decodeResponses=False, useAof=False, useRdbPreamble=True, debugger=None, noCatch=False, unix=False, verbose=False, useTLS=False, tlsCertFile=None, - tlsKeyFile=None, tlsCaCertFile=None, clusterNodeTimeout = None): + tlsKeyFile=None, tlsCaCertFile=None, clusterNodeTimeout = None, tlsPassphrase = None): self.uuid = uuid.uuid4().hex self.redisBinaryPath = os.path.expanduser(redisBinaryPath) if redisBinaryPath.startswith( '~/') else redisBinaryPath @@ -53,6 +53,7 @@ def __init__(self, redisBinaryPath, port=6379, modulePath=None, moduleArgs=None, self.tlsKeyFile = tlsKeyFile self.tlsCaCertFile = tlsCaCertFile self.clusterNodeTimeout = clusterNodeTimeout + self.tlsPassphrase = tlsPassphrase if port > 0: self.port = port @@ -197,6 +198,8 @@ def createCmdArgs(self, role): cmdArgs += ['--tls-cert-file', self.getTLSCertFile()] cmdArgs += ['--tls-key-file', self.getTLSKeyFile()] cmdArgs += ['--tls-ca-cert-file', self.getTLSCACertFile()] + if self.tlsPassphrase: + cmdArgs += ['--tls-key-file-pass', self.tlsPassphrase] return cmdArgs @@ -362,6 +365,7 @@ def _getConnection(self, role): return redis.StrictRedis('localhost', self.getPort(role), password=self.password, ssl=True, + ssl_password=self.tlsPassphrase, ssl_keyfile=self.getTLSKeyFile(), ssl_certfile=self.getTLSCertFile(), ssl_cert_reqs=None, diff --git a/pyproject.toml b/pyproject.toml index 0ae1151..61de94b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ classifiers = [ [tool.poetry.dependencies] python = "^2.7,<2.8 || >= 3.5.0" distro = "^1.5.0" -redis = "^3.5.3" +redis = "^4.1.2" redis-py-cluster = "*" psutil = "5.8.0" # 5.9.0 currently broken on macOS pytest-cov = "2.5" From 88d07483b538bc07113c8bed70fcdbf3c12f5928 Mon Sep 17 00:00:00 2001 From: meir Date: Thu, 3 Feb 2022 16:43:18 +0200 Subject: [PATCH 02/11] remove extra ' --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b310d4..cec635b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,4 @@ -name: CI` +name: CI on: push: From b86d0fbfcb6e6e7bed445f1bc5ea28c6eb1219f3 Mon Sep 17 00:00:00 2001 From: meir Date: Thu, 3 Feb 2022 16:47:00 +0200 Subject: [PATCH 03/11] min python support 3.6 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 61de94b..49b92d2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ classifiers = [ ] [tool.poetry.dependencies] -python = "^2.7,<2.8 || >= 3.5.0" +python = ">= 3.6.0" distro = "^1.5.0" redis = "^4.1.2" redis-py-cluster = "*" From 21c9f8a1a448238dea970f854995061bbad967de Mon Sep 17 00:00:00 2001 From: meir Date: Thu, 3 Feb 2022 17:09:18 +0200 Subject: [PATCH 04/11] fix tests --- RLTest/redis_cluster.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/RLTest/redis_cluster.py b/RLTest/redis_cluster.py index a98234d..aab391a 100644 --- a/RLTest/redis_cluster.py +++ b/RLTest/redis_cluster.py @@ -156,8 +156,8 @@ def getOSSMasterNodesConnectionList(self): # Gets a cluster connection by key. On std redis the default connection is returned. def getConnectionByKey(self, key, command): clusterConn = self.getClusterConnection() - target_node = clusterConn._determine_nodes(command) - return clusterConn.get_redis_connection(target_node) + target_node = clusterConn._determine_nodes(command, key) # we will always which will give us the node responsible for the key + return clusterConn.get_redis_connection(target_node[0]) def flush(self): self.getClusterConnection().flushall() From 84cbfec23524b1ce78b3eb2ab1531f3e63b987f8 Mon Sep 17 00:00:00 2001 From: meir Date: Sun, 6 Feb 2022 13:15:20 +0200 Subject: [PATCH 05/11] review fixes --- pyproject.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 49b92d2..d06d12a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,10 +24,9 @@ classifiers = [ ] [tool.poetry.dependencies] -python = ">= 3.6.0" +python = ">= 3.6.0 || < 4.0.0" distro = "^1.5.0" redis = "^4.1.2" -redis-py-cluster = "*" psutil = "5.8.0" # 5.9.0 currently broken on macOS pytest-cov = "2.5" From b288e37e4e27f74dcfc4f466b24349ffd3099524 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Tue, 8 Feb 2022 09:52:24 +0200 Subject: [PATCH 06/11] poetry dependencies --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d06d12a..86f6c9a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ classifiers = [ ] [tool.poetry.dependencies] -python = ">= 3.6.0 || < 4.0.0" +python = ">= 3.6.0" distro = "^1.5.0" redis = "^4.1.2" psutil = "5.8.0" # 5.9.0 currently broken on macOS @@ -42,7 +42,7 @@ flake8 = "*" rmtest = "^0.7.0" nose = "^1.3.7" ml2rt = "^0.2.0" -pytest = "4.6" +pytest = "^6.0" [build-system] requires = ["poetry-core>=1.0.0"] From 3f33dc4da637c15a1cdf40782d0a62aeb1c166f5 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Tue, 8 Feb 2022 10:03:07 +0200 Subject: [PATCH 07/11] fixing SyntaxError --- RLTest/env.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RLTest/env.py b/RLTest/env.py index f32d5ed..1861498 100644 --- a/RLTest/env.py +++ b/RLTest/env.py @@ -303,7 +303,7 @@ def getEnvKwargs(self): 'tlsKeyFile': self.tlsKeyFile, 'tlsCaCertFile': self.tlsCaCertFile, 'clusterNodeTimeout': self.clusterNodeTimeout, - 'tlsPassphrase': self.tlsPassphrase + 'tlsPassphrase': self.tlsPassphrase, 'port': self.port } return kwargs From fb1e7f2cac1d9e48a35df10c6f0ad1465402d0f6 Mon Sep 17 00:00:00 2001 From: "Chayim I. Kirshen" Date: Tue, 8 Feb 2022 10:53:14 +0200 Subject: [PATCH 08/11] bumping up the timeout --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cec635b..f370637 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: timeout-minutes: 40 strategy: matrix: - platform: ['ubuntu-20.04', 'ubuntu-18.04', 'ubuntu-16.04'] + platform: ['ubuntu-20.04'] python: ['3.6', '3.7', '3.8', '3.9'] steps: @@ -75,7 +75,7 @@ jobs: ./utils/gen-test-certs.sh - name: Unit Test with pytest - timeout-minutes: 10 + timeout-minutes: 20 run: | TLS_CERT=./redis/tests/tls/redis.crt \ TLS_KEY=./redis/tests/tls/redis.key \ @@ -187,7 +187,7 @@ jobs: cd ../.. - name: Generate coverage report - if: matrix.platform == 'ubuntu-18.04' && matrix.python == '3.6' + if: matrix.python == '3.6' run: | TLS_CERT=./redis/tests/tls/redis.crt \ TLS_KEY=./redis/tests/tls/redis.key \ @@ -197,7 +197,7 @@ jobs: - name: Upload coverage to Codecov uses: codecov/codecov-action@v1 - if: matrix.platform == 'ubuntu-18.04' && matrix.python == '3.6' + if: matrix.python == '3.6' with: token: ${{ secrets.CODECOV_TOKEN }} file: ./coverage.xml From 801d80500db294320f4ae0aafc002cf984371cb3 Mon Sep 17 00:00:00 2001 From: Guy Korland Date: Tue, 8 Feb 2022 15:58:07 +0200 Subject: [PATCH 09/11] increase timeout to 30m --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f370637..187d9d3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: ./utils/gen-test-certs.sh - name: Unit Test with pytest - timeout-minutes: 20 + timeout-minutes: 30 run: | TLS_CERT=./redis/tests/tls/redis.crt \ TLS_KEY=./redis/tests/tls/redis.key \ From 0ce559a882bf9b7f02346faa600f9dc6ccf8ac41 Mon Sep 17 00:00:00 2001 From: meir Date: Sun, 13 Feb 2022 11:02:09 +0200 Subject: [PATCH 10/11] run with -v so we will see the problematic test --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 187d9d3..2ee51ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,7 +81,7 @@ jobs: TLS_KEY=./redis/tests/tls/redis.key \ TLS_CACERT=./redis/tests/tls/ca.crt \ REDIS_BINARY=./redis/src/redis-server \ - pytest --ignore=tests/flow --ignore=test_example.py + pytest --ignore=tests/flow --ignore=test_example.py -v - name: Install RLTest run: | From 85a371c8ea275e0bf81225c0b2e1d958c88fcc80 Mon Sep 17 00:00:00 2001 From: meir Date: Sun, 13 Feb 2022 11:29:28 +0200 Subject: [PATCH 11/11] Use redis 6.2.6 --- .github/workflows/ci.yml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ee51ca..be9bf4a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,19 +51,19 @@ jobs: poetry export --dev --without-hashes -o requirements-${{matrix.platform}}-${{matrix.python}}.txt pip install -r requirements-${{matrix.platform}}-${{matrix.python}}.txt - - name: Cache Redis - id: cache-redis - uses: actions/cache@v1 - with: - path: redis - key: ${{ matrix.platform }}-${{ matrix.python }}-redis - restore-keys: | - ${{ matrix.platform }}-${{ matrix.python }}-redis + # - name: Cache Redis + # id: cache-redis + # uses: actions/cache@v1 + # with: + # path: redis + # key: ${{ matrix.platform }}-${{ matrix.python }}-redis + # restore-keys: | + # ${{ matrix.platform }}-${{ matrix.python }}-redis - name: Install Redis Server test dependencies - if: steps.cache-redis.outputs.cache-hit != 'true' + # if: steps.cache-redis.outputs.cache-hit != 'true' run: | - git clone https://github.com/redis/redis.git --branch unstable --depth 1 + git clone https://github.com/redis/redis.git --branch 6.2.6 --depth 1 cd redis make BUILD_TLS=yes -j ./src/redis-server --version