feat: Separating the RBAC and Remote related integration tests. (#4905)

lokeshrangineni · web-flow · commit 76e1e2178c28 · 2025-01-08T09:44:26.000-05:00
* Separating the tests related to remote and rbac functionality.
* Added a new test marker to separate the tests related to rbac and remote functionality.
* Added a new github action to perform tests related to rbac and remote functionality.
* Filtered the rbac integration tests in the current github job. and added new make target to run the new tests.

Signed-off-by: lrangine &lt;19699092+lokeshrangineni@users.noreply.github.com&gt;
diff --git a/.github/workflows/operator-e2e-integration-tests.yml b/.github/workflows/operator-e2e-integration-tests.yml
@@ -10,6 +10,10 @@ on:
       - opened
       - synchronize
       - labeled
+    paths-ignore:
+      - 'community/**'
+      - 'docs/**'
+      - 'examples/**'
 
 jobs:
   operator-e2e-tests:
diff --git a/.github/workflows/pr_remote_rbac_integration_tests.yml b/.github/workflows/pr_remote_rbac_integration_tests.yml
@@ -0,0 +1,58 @@
+name: pr-remote-rbac-integration-tests
+# This runs the integration tests related to rbac functionality and remote registry and online features.
+
+on:
+  pull_request:
+    types:
+      - opened
+      - synchronize
+      - labeled
+    paths-ignore:
+      - 'community/**'
+      - 'docs/**'
+      - 'examples/**'
+
+jobs:
+  remote-rbac-integration-tests-python:
+    if:
+      ((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
+      (github.event.action != 'labeled' && (contains(github.event.pull_request.labels.*.name, 'ok-to-test') || contains(github.event.pull_request.labels.*.name, 'approved') || contains(github.event.pull_request.labels.*.name, 'lgtm')))) &&
+      github.event.pull_request.base.repo.full_name == 'feast-dev/feast'
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version: [ "3.11" ]
+        os: [ ubuntu-latest ]
+    env:
+      OS: ${{ matrix.os }}
+      PYTHON: ${{ matrix.python-version }}
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          repository: ${{ github.event.repository.full_name }}  # Uses the full repository name
+          ref: ${{ github.ref }}                                  # Uses the ref from the event
+          token: ${{ secrets.GITHUB_TOKEN }}                     # Automatically provided token
+          submodules: recursive
+      - name: Setup Python
+        uses: actions/setup-python@v5
+        id: setup-python
+        with:
+          python-version: ${{ matrix.python-version }}
+          architecture: x64
+      - name: Install uv
+        run: curl -LsSf https://astral.sh/uv/install.sh | sh
+      - name: Get uv cache dir
+        id: uv-cache
+        run: |
+          echo "dir=$(uv cache dir)" >> $GITHUB_OUTPUT
+      - name: uv cache
+        uses: actions/cache@v4
+        with:
+          path: ${{ steps.uv-cache.outputs.dir }}
+          key: ${{ runner.os }}-${{ matrix.python-version }}-uv-${{ hashFiles(format('**/py{0}-ci-requirements.txt', matrix.python-version)) }}
+      - name: Install dependencies
+        run: make install-python-dependencies-ci
+      - name: Test rbac and remote feature integration tests
+        if: ${{ always() }}  # this will guarantee that step won't be canceled and resources won't leak
+        run: make test-python-integration-rbac-remote
diff --git a/Makefile b/Makefile
@@ -107,13 +107,26 @@ test-python-unit:
 test-python-integration:
 	python -m pytest --tb=short -v -n 8 --integration --color=yes --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
 		-k "(not snowflake or not test_historical_features_main)" \
+		-m "not rbac_remote_integration_test" \
+		--log-cli-level=INFO -s \
 		sdk/python/tests
 
 test-python-integration-local:
 	FEAST_IS_LOCAL_TEST=True \
 	FEAST_LOCAL_ONLINE_CONTAINER=True \
 	python -m pytest --tb=short -v -n 8 --color=yes --integration --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
 		-k "not test_lambda_materialization and not test_snowflake_materialization" \
+		-m "not rbac_remote_integration_test" \
+		--log-cli-level=INFO -s \
+		sdk/python/tests
+
+test-python-integration-rbac-remote:
+	FEAST_IS_LOCAL_TEST=True \
+	FEAST_LOCAL_ONLINE_CONTAINER=True \
+	python -m pytest --tb=short -v -n 8 --color=yes --integration --durations=10 --timeout=1200 --timeout_method=thread --dist loadgroup \
+		-k "not test_lambda_materialization and not test_snowflake_materialization" \
+		-m "rbac_remote_integration_test" \
+		--log-cli-level=INFO -s \
 		sdk/python/tests
 
 test-python-integration-container:
diff --git a/sdk/python/pytest.ini b/sdk/python/pytest.ini
@@ -4,6 +4,7 @@ asyncio_mode = auto
 markers =
     universal_offline_stores: mark a test as using all offline stores.
     universal_online_stores: mark a test as using all online stores.
+    rbac_remote_integration_test: mark a integration test related to rbac and remote functionality.
 
 env =
     IS_TEST=True
diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py
@@ -310,6 +310,10 @@ def pytest_generate_tests(metafunc: pytest.Metafunc):
                                 pytest.mark.xdist_group(name=m)
                                 for m in c.offline_store_creator.xdist_groups()
                             ]
+                            # Check if there are any test markers associated with the creator and add them.
+                            if c.offline_store_creator.test_markers():
+                                marks.extend(c.offline_store_creator.test_markers())
+
                             _config_cache[c] = pytest.param(c, marks=marks)
 
                         configs.append(_config_cache[c])
diff --git a/sdk/python/tests/integration/conftest.py b/sdk/python/tests/integration/conftest.py
@@ -1,4 +1,7 @@
 import logging
+import random
+import time
+from multiprocessing import Manager
 
 import pytest
 from testcontainers.keycloak import KeycloakContainer
@@ -9,14 +12,30 @@
 from tests.utils.auth_permissions_util import setup_permissions_on_keycloak
 
 logger = logging.getLogger(__name__)
+logger.setLevel(logging.INFO)
+
+shared_state = Manager().dict()
 
 
 @pytest.fixture(scope="session")
 def start_keycloak_server():
+    # Add random sleep between 0 and 2 before checking the state to avoid concurrency issues.
+    random_sleep_time = random.uniform(0, 2)
+    time.sleep(random_sleep_time)
+
+    # If the Keycloak instance is already started (in any worker), reuse it
+    if shared_state.get("keycloak_started", False):
+        return shared_state["keycloak_url"]
     logger.info("Starting keycloak instance")
     with KeycloakContainer("quay.io/keycloak/keycloak:24.0.1") as keycloak_container:
         setup_permissions_on_keycloak(keycloak_container.get_client())
-        yield keycloak_container.get_url()
+        shared_state["keycloak_started"] = True
+        shared_state["keycloak_url"] = keycloak_container.get_url()
+        yield shared_state["keycloak_url"]
+
+    # After the fixture is done, cleanup the shared state
+    del shared_state["keycloak_started"]
+    del shared_state["keycloak_url"]
 
 
 @pytest.fixture(scope="session")
diff --git a/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py b/sdk/python/tests/integration/feature_repos/universal/data_source_creator.py
@@ -2,6 +2,7 @@
 from typing import Dict, Optional
 
 import pandas as pd
+from _pytest.mark import MarkDecorator
 
 from feast.data_source import DataSource
 from feast.feature_logging import LoggingDestination
@@ -64,3 +65,11 @@ def teardown(self):
     @staticmethod
     def xdist_groups() -> list[str]:
         return []
+
+    @staticmethod
+    def test_markers() -> list[MarkDecorator]:
+        """
+        return the array of test markers to add dynamically to the tests created by this creator method. override this method in your implementations. By default, it will not add any markers.
+        :return:
+        """
+        return []
diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py
@@ -11,7 +11,9 @@
 import pandas as pd
 import pyarrow as pa
 import pyarrow.parquet as pq
+import pytest
 import yaml
+from _pytest.mark import MarkDecorator
 from minio import Minio
 from testcontainers.core.generic import DockerContainer
 from testcontainers.core.waiting_utils import wait_for_logs
@@ -372,6 +374,10 @@ def __init__(self, project_name: str, *args, **kwargs):
         self.server_port: int = 0
         self.proc: Optional[Popen[bytes]] = None
 
+    @staticmethod
+    def test_markers() -> list[MarkDecorator]:
+        return [pytest.mark.rbac_remote_integration_test]
+
     def setup(self, registry: RegistryConfig):
         parent_offline_config = super().create_offline_store_config()
         config = RepoConfig(
@@ -418,6 +424,10 @@ def __init__(self, project_name: str, *args, **kwargs):
         self.server_port: int = 0
         self.proc: Optional[Popen[bytes]] = None
 
+    @staticmethod
+    def test_markers() -> list[MarkDecorator]:
+        return [pytest.mark.rbac_remote_integration_test]
+
     def setup(self, registry: RegistryConfig):
         parent_offline_config = super().create_offline_store_config()
         config = RepoConfig(
@@ -515,6 +525,10 @@ def __init__(self, project_name: str, *args, **kwargs):
     def xdist_groups() -> list[str]:
         return ["keycloak"]
 
+    @staticmethod
+    def test_markers() -> list[MarkDecorator]:
+        return [pytest.mark.rbac_remote_integration_test]
+
     def setup(self, registry: RegistryConfig):
         parent_offline_config = super().create_offline_store_config()
         config = RepoConfig(
diff --git a/sdk/python/tests/integration/online_store/test_remote_online_store.py b/sdk/python/tests/integration/online_store/test_remote_online_store.py
@@ -22,6 +22,7 @@
 
 
 @pytest.mark.integration
+@pytest.mark.rbac_remote_integration_test
 @pytest.mark.parametrize(
     "tls_mode", [("True", "True"), ("True", "False"), ("False", "")], indirect=True
 )
diff --git a/sdk/python/tests/integration/registration/test_universal_registry.py b/sdk/python/tests/integration/registration/test_universal_registry.py
@@ -344,7 +344,10 @@ def mock_remote_registry():
             marks=pytest.mark.xdist_group(name="mysql_registry"),
         ),
         lazy_fixture("sqlite_registry"),
-        lazy_fixture("mock_remote_registry"),
+        pytest.param(
+            lazy_fixture("mock_remote_registry"),
+            marks=pytest.mark.rbac_remote_integration_test,
+        ),
     ]
 
 sql_fixtures = [

Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,7 @@`
`22`	`22`
`23`	`23`
`24`	`24`	`@pytest.mark.integration`
	`25`	`+@pytest.mark.rbac_remote_integration_test`
`25`	`26`	`@pytest.mark.parametrize(`
`26`	`27`	`"tls_mode", [("True", "True"), ("True", "False"), ("False", "")], indirect=True`
`27`	`28`	`)`