Make unit tests use redis (#6245)

Author: cognifloyd

.github/workflows/ci.yaml

@@ -133,6 +133,18 @@ jobs:
             - 5672:5672/tcp   # AMQP standard port
             - 15672:15672/tcp # Management: HTTP, CLI
 
+      redis:
+        # Docker Hub image
+        image: redis
+        # Set health checks to wait until redis has started
+        options: >-
+          --name "redis"
+          --health-cmd "redis-cli ping"
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 6379:6379/tcp
     env:
       # CI st2.conf (with ST2_CI_USER user instead of stanley)
       ST2_CONF: 'conf/st2.ci.conf'
@@ -163,11 +175,6 @@ jobs:
       - name: Install requirements
         run: |
           ./scripts/ci/install-requirements.sh
-      - name: Run Redis Service Container
-        timeout-minutes: 2
-        run: |
-          docker run --rm --detach -p 127.0.0.1:6379:6379/tcp --name redis redis:latest
-          until [ "$(docker inspect -f {{.State.Running}} redis)" == "true" ]; do sleep 0.1; done
       - name: Setup Tests
         run: |
           # prep a ci-specific dev conf file that uses runner instead of stanley
@@ -234,9 +241,6 @@ jobs:
           name: logs-py${{ matrix.python-version }}
           path: logs.tar.gz
           retention-days: 7
-      - name: Stop Redis Service Container
-        if: "${{ always() }}"
-        run: docker rm --force redis || true
 
   unit-tests:
     needs: pre_job
@@ -287,6 +291,19 @@ jobs:
         image: mongo:4.4
         ports:
           - 27017:27017
+      redis:
+        # Docker Hub image
+        image: redis
+        # Set health checks to wait until redis has started
+        options: >-
+          --name "redis"
+          --health-cmd "redis-cli ping"
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 6379:6379/tcp
+
 
       rabbitmq:
         image: rabbitmq:3.8-management
@@ -471,21 +488,18 @@ jobs:
           #- 4369:4369/tcp   # epmd
           #
 
-      # Used for the coordination backend for integration tests
-      # NOTE: To speed things up, we only start redis for integration tests
-      # where it's needed
-      # redis:
-      #   # Docker Hub image
-      #   image: redis
-      #   # Set health checks to wait until redis has started
-      #   options: >-
-      #     --name "redis"
-      #     --health-cmd "redis-cli ping"
-      #     --health-interval 10s
-      #     --health-timeout 5s
-      #     --health-retries 5
-      #   ports:
-      #     - 6379:6379/tcp
+      redis:
+        # Docker Hub image
+        image: redis
+        # Set health checks to wait until redis has started
+        options: >-
+          --name "redis"
+          --health-cmd "redis-cli ping"
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 6379:6379/tcp
 
     env:
       TASK: '${{ matrix.task }}'
@@ -538,11 +552,6 @@ jobs:
           cp conf/st2.dev.conf "${ST2_CONF}" ; sed -i -e "s/stanley/${ST2_CI_USER}/" "${ST2_CONF}"
 
           sudo -E ./scripts/ci/add-itest-user-key.sh
-      - name: Run Redis Service Container
-        timeout-minutes: 2
-        run: |
-          docker run --rm --detach -p 127.0.0.1:6379:6379/tcp --name redis redis:latest
-          until [ "$(docker inspect -f {{.State.Running}} redis)" == "true" ]; do sleep 0.1; done
       - name: Permissions Workaround
         run: |
           echo "$ST2_CI_REPO_PATH"
@@ -585,9 +594,6 @@ jobs:
           name: logs-py${{ matrix.python-version }}-nose-${{ matrix.nosetests_node_index }}
           path: logs.tar.gz
           retention-days: 7
-      - name: Stop Redis Service Container
-        if: "${{ always() }}"
-        run: docker rm --force redis || true
 
   slack-notification:
     name: Slack notification for failed master builds

CHANGELOG.rst

@@ -28,6 +28,9 @@ Changed
 * Update st2client deps: editor and prompt-toolkit. #6189 (by @nzlosh)
 * Updated dependency oslo.config to prepare for python 3.10 support. #6193 (by @nzlosh)
 
+* Updated unit tests to use redis for coordination instead of the NoOp driver. This will hopefully make CI more stable. #6245
+  Contributed by @FileMagic, @guzzijones, and @cognifloyd
+
 Added
 ~~~~~
 * Continue introducing `pants <https://www.pantsbuild.org/docs>`_ to improve DX (Developer Experience)

Makefile

@@ -53,6 +53,10 @@ COVERAGE_GLOBS_QUOTED := $(foreach glob,$(COVERAGE_GLOBS),'$(glob)')
 
 REQUIREMENTS := test-requirements.txt requirements.txt
 
+# Redis config for testing
+ST2TESTS_REDIS_HOST := 127.0.0.1
+ST2TESTS_REDIS_PORT := 6379
+
 # Pin common pip version here across all the targets
 # Note! Periodic maintenance pip upgrades are required to be up-to-date with the latest pip security fixes and updates
 PIP_VERSION ?= 24.2
@@ -824,6 +828,8 @@ unit-tests: requirements .unit-tests
 		echo "Running tests in" $$component; \
 		echo "-----------------------------------------------------------"; \
 		. $(VIRTUALENV_DIR)/bin/activate; \
+		 ST2TESTS_REDIS_HOST=$(ST2TESTS_REDIS_HOST) \
+		 ST2TESTS_REDIS_PORT=$(ST2TESTS_REDIS_PORT) \
 		    nosetests $(NOSE_OPTS) -s -v \
 		    $$component/tests/unit || ((failed+=1)); \
 		echo "-----------------------------------------------------------"; \
@@ -848,6 +854,8 @@ endif
 		echo "Running tests in" $$component; \
 		echo "-----------------------------------------------------------"; \
 		. $(VIRTUALENV_DIR)/bin/activate; \
+		 ST2TESTS_REDIS_HOST=$(ST2TESTS_REDIS_HOST) \
+		 ST2TESTS_REDIS_PORT=$(ST2TESTS_REDIS_PORT) \
 		    COVERAGE_FILE=.coverage.unit.$$(echo $$component | tr '/' '.') \
 		    nosetests $(NOSE_OPTS) -s -v $(NOSE_COVERAGE_FLAGS) \
 		    $(NOSE_COVERAGE_PACKAGES) \

st2actions/tests/unit/test_policies.py

@@ -28,6 +28,7 @@
 from st2common.transport.publishers import CUDPublisher
 from st2common.bootstrap import runnersregistrar as runners_registrar
 from st2tests import ExecutionDbTestCase
+from st2tests import config as tests_config
 from st2tests.fixtures.generic.fixture import PACK_NAME as PACK
 from st2tests.fixturesloader import FixturesLoader
 from st2tests.mocks.runners import runner
@@ -36,6 +37,8 @@
 from st2tests.policies.concurrency import FakeConcurrencyApplicator
 from st2tests.policies.mock_exception import RaiseExceptionApplicator
 
+# This needs to run before creating FakeConcurrencyApplicator below.
+tests_config.parse_args()
 
 TEST_FIXTURES = {
     "actions": ["action1.yaml"],

st2actions/tests/unit/test_worker.py

@@ -20,12 +20,14 @@
 import mock
 import os
 from oslo_config import cfg
+from tooz.drivers.redis import RedisDriver
 import tempfile
 
 # This import must be early for import-time side-effects.
 from st2tests.base import DbTestCase
 
 import st2actions.worker as actions_worker
+import st2tests.config as tests_config
 from st2common.constants import action as action_constants
 from st2common.models.db.liveaction import LiveActionDB
 from st2common.models.system.common import ResourceReference
@@ -66,6 +68,35 @@ def setUpClass(cls):
         )
         WorkerTestCase.local_action_db = models["actions"]["local.yaml"]
 
+    @staticmethod
+    def reset_config(
+        graceful_shutdown=True,  # default is True (st2common.config)
+        exit_still_active_check=None,  # default is 300 (st2common.config)
+        still_active_check_interval=None,  # default is 2 (st2common.config)
+        service_registry=None,  # default is False (st2common.config)
+    ):
+        tests_config.reset()
+        tests_config.parse_args()
+        cfg.CONF.set_override(
+            name="graceful_shutdown", override=graceful_shutdown, group="actionrunner"
+        )
+        if exit_still_active_check is not None:
+            cfg.CONF.set_override(
+                name="exit_still_active_check",
+                override=exit_still_active_check,
+                group="actionrunner",
+            )
+        if still_active_check_interval is not None:
+            cfg.CONF.set_override(
+                name="still_active_check_interval",
+                override=still_active_check_interval,
+                group="actionrunner",
+            )
+        if service_registry is not None:
+            cfg.CONF.set_override(
+                name="service_registry", override=service_registry, group="coordination"
+            )
+
     def _get_liveaction_model(self, action_db, params):
         status = action_constants.LIVEACTION_STATUS_REQUESTED
         start_timestamp = date_utils.get_datetime_utc_now()
@@ -116,9 +147,8 @@ def test_non_utf8_action_result_string(self):
             )
 
     def test_worker_shutdown(self):
-        cfg.CONF.set_override(
-            name="graceful_shutdown", override=False, group="actionrunner"
-        )
+        self.reset_config(graceful_shutdown=False)
+
         action_worker = actions_worker.get_worker()
         temp_file = None
 
@@ -169,14 +199,19 @@ def test_worker_shutdown(self):
         runner_thread.wait()
 
     @mock.patch.object(
-        coordination.NoOpDriver,
+        RedisDriver,
         "get_members",
-        mock.MagicMock(return_value=coordination.NoOpAsyncResult("member-1")),
+        mock.MagicMock(
+            return_value=coordination.NoOpAsyncResult(("member-1", "member-2"))
+        ),
     )
     def test_worker_graceful_shutdown_with_multiple_runners(self):
-        cfg.CONF.set_override(
-            name="graceful_shutdown", override=True, group="actionrunner"
+        self.reset_config(
+            exit_still_active_check=10,
+            still_active_check_interval=1,
+            service_registry=True,
         )
+
         action_worker = actions_worker.get_worker()
         temp_file = None
 
@@ -234,9 +269,12 @@ def test_worker_graceful_shutdown_with_multiple_runners(self):
         shutdown_thread.kill()
 
     def test_worker_graceful_shutdown_with_single_runner(self):
-        cfg.CONF.set_override(
-            name="graceful_shutdown", override=True, group="actionrunner"
+        self.reset_config(
+            exit_still_active_check=10,
+            still_active_check_interval=1,
+            service_registry=True,
         )
+
         action_worker = actions_worker.get_worker()
         temp_file = None
 
@@ -296,17 +334,13 @@ def test_worker_graceful_shutdown_with_single_runner(self):
         shutdown_thread.kill()
 
     @mock.patch.object(
-        coordination.NoOpDriver,
+        RedisDriver,
         "get_members",
-        mock.MagicMock(return_value=coordination.NoOpAsyncResult("member-1")),
+        mock.MagicMock(return_value=coordination.NoOpAsyncResult(("member-1",))),
     )
     def test_worker_graceful_shutdown_exit_timeout(self):
-        cfg.CONF.set_override(
-            name="graceful_shutdown", override=True, group="actionrunner"
-        )
-        cfg.CONF.set_override(
-            name="exit_still_active_check", override=5, group="actionrunner"
-        )
+        self.reset_config(exit_still_active_check=5)
+
         action_worker = actions_worker.get_worker()
         temp_file = None