chore(hybrid-cloud): Stable silo tests run by default (#39903)

Run tests in silo modes when marked stable, add utility scripts to add in hybrid cloud test iteration.

.github/workflows/backend.yml

@@ -107,52 +107,6 @@ jobs:
       - name: Handle artifacts
         uses: ./.github/actions/artifacts
 
-  backend-test-in-silo-modes:
-    needs: files-changed
-    if: needs.files-changed.outputs.backend == 'true'
-      && (
-            contains(github.head_ref, '/hc-')
-         || contains(github.head_ref, 'hybrid-cloud')
-         )
-    timeout-minutes: 60
-    name: backend tests, hybrid cloud
-    runs-on: ubuntu-20.04
-    strategy:
-      fail-fast: false
-      matrix:
-        # XXX: When updating this, make sure you also update MATRIX_INSTANCE_TOTAL.
-        instance: [0, 1]
-        pg-version: ['9.6']
-        silo-mode:
-          - CONTROL
-          - REGION
-    env:
-      MATRIX_INSTANCE_TOTAL: 2
-      MIGRATIONS_TEST_MIGRATE: 1
-      SENTRY_SILO_MODE: ${{ matrix.silo-mode }}
-
-    steps:
-      - uses: actions/checkout@7884fcad6b5d53d10323aee724dc68d8b9096a2e  # v2
-        with:
-          # Avoid codecov error message related to SHA resolution:
-          # https://github.com/codecov/codecov-bash/blob/7100762afbc822b91806a6574658129fe0d23a7d/codecov#L891
-          fetch-depth: '2'
-
-      - name: Setup sentry env
-        uses: ./.github/actions/setup-sentry
-        id: setup
-        with:
-          snuba: true
-          # Right now, we run so few bigtable related tests that the
-          # overhead of running bigtable in all backend tests
-          # is way smaller than the time it would take to run in its own job.
-          bigtable: true
-          pg-version: ${{ matrix.pg-version }}
-
-      - name: Run backend test (${{ steps.setup.outputs.matrix-instance-number }} of ${{ steps.setup.outputs.matrix-instance-total }})
-        run: |
-          make test-python-ci
-
   backend-test-snuba-contains-metrics-tag-values:
     if: needs.files-changed.outputs.backend == 'true'
     needs: files-changed

bin/decorate-silo-mode-tests

@@ -0,0 +1,169 @@
+#!/usr/bin/env python
+import ast
+import os
+import os.path
+import shutil
+import tempfile
+from typing import Union
+
+import click
+
+from sentry.utils import json
+from sentry.utils.types import Any
+
+
+def find_test_cases_matching(model_name: str):
+    manifest = json.loads(open(os.environ["SENTRY_MODEL_MANIFEST_FILE_PATH"]).read())
+    for test_node_id, hits in manifest.items():
+        if any(hit["model"] == model_name for hit in hits):
+            parts = test_node_id.split("::")
+            yield parts[0], parts[1]
+
+
+@click.command()
+@click.option(
+    "silo_mode",
+    "--silo-mode",
+    required=True,
+    help="Which mode to apply to tests",
+    type=click.Choice(
+        [
+            "control",
+            "region",
+        ]
+    ),
+)
+@click.option("set_stable", "--set-stable", default=False, is_flag=True, help="Set tests as stable")
+@click.argument("target_model", required=True)
+def main(target_model: str, silo_mode: str, set_stable: bool):
+    """
+    Script to decorate the given target test for silo tests, making it easier to deploy changes to given tests.
+    """
+
+    for file_name, test_case_name in find_test_cases_matching(target_model):
+        print(f"Trying {test_case_name} in {file_name}")  # noqa
+        test_case_name = test_case_name.split("[")[
+            0
+        ]  # remove any parameterization off the test case
+        file_path = os.path.abspath(file_name)
+        file_ast = ast.parse(open(file_path).read())
+        test_visitor = TestVisitor(test_case_name, f"{silo_mode}_silo_test", set_stable)
+        test_visitor.visit(file_ast)
+
+        test_visitor.rewrite(file_path)
+
+
+class TestVisitor(ast.NodeVisitor):
+    def __init__(self, target_symbol_path: str, target_test_silo_mode: str, set_stable: bool):
+        self.set_stable = set_stable
+        self.target_test_silo_mode = target_test_silo_mode
+        self.target_symbol_parts = target_symbol_path.split(".")
+        self.import_match_line = False
+        self.decorator_match_line = None
+        self.func_match_line = None
+        self.class_node = None
+
+    def visit_ImportFrom(self, node: ast.ImportFrom) -> Any:
+        if node.module == "sentry.testutils.silo":
+            for name in node.names:
+                if isinstance(name, ast.alias):
+                    if name.name.endswith("_silo_test"):
+                        self.import_match_line = (node.lineno, node.col_offset)
+
+    def visit_ClassDef(self, node: ast.ClassDef) -> Any:
+        if len(self.target_symbol_parts) == 2 and self.target_test_silo_mode[0] == node.name:
+            self.class_node = node
+            self.generic_visit(node)
+            self.class_node = None
+        elif len(self.target_symbol_parts) == 1:
+            if self.target_symbol_parts[-1] == node.name or self.target_symbol_parts[-1] in {
+                e.id for e in node.bases if isinstance(e, ast.Name)
+            }:
+                self.mark_target(node)
+
+    def visit_FunctionDef(self, node: Union[ast.FunctionDef, ast.ClassDef]) -> Any:
+        if self.target_symbol_parts[-1] == node.name:
+            if self.class_node:
+                node = self.class_node
+            elif len(self.target_symbol_parts) != 1:
+                return
+
+            self.mark_target(node)
+            return
+
+        return self.generic_visit(node)
+
+    def mark_target(self, node: Union[ast.FunctionDef, ast.ClassDef]):
+        self.func_match_line = (node.lineno, node.col_offset)
+        for expr in node.decorator_list:
+            decorator_visitor = DecoratorVisitor(self.target_test_silo_mode)
+            decorator_visitor.visit(expr)
+            if decorator_visitor.match_line:
+                self.decorator_match_line = decorator_visitor.match_line
+                break
+
+    def _decorate(self, lineno, match_line):
+        if not match_line:
+            return False
+
+        if not match_line[0] == lineno:
+            return False
+
+        ws = b" " * match_line[1]
+        if self.set_stable:
+            return ws + f"@{self.target_test_silo_mode}(stable=True)\n".encode()
+        else:
+            return ws + f"@{self.target_test_silo_mode}\n".encode()
+
+    def rewrite(self, path):
+        import_line = f"from sentry.testutils.silo import {self.target_test_silo_mode}\n".encode()
+        if not self.decorator_match_line and not self.func_match_line:
+            raise Exception(f"Could not find test case {self.target_symbol_parts}!")
+
+        with tempfile.NamedTemporaryFile(delete=False) as tf:
+            with open(path) as f:
+                if not self.import_match_line:
+                    tf.write(import_line)
+
+                for i, line in enumerate(f.readlines()):
+                    i += 1
+
+                    if self.import_match_line and self.import_match_line[0] == i:
+                        tf.write(import_line)
+                        continue
+
+                    if newline := self._decorate(i, self.decorator_match_line):
+                        # If the decorator type is not changing, keep the original line.
+                        if self.target_test_silo_mode in line:
+                            tf.write(line.encode("utf8"))
+                        else:
+                            tf.write(newline)
+                        continue
+
+                    if not self.decorator_match_line and (
+                        newline := self._decorate(i, self.func_match_line)
+                    ):
+                        tf.write(newline)
+
+                    tf.write(line.encode("utf8"))
+
+            tf.close()
+            shutil.move(tf.name, path)
+
+
+class DecoratorVisitor(ast.NodeVisitor):
+    def __init__(self, target_test_silo_mode: str):
+        self.target_test_silo_mode = target_test_silo_mode
+        self.match_line = None
+
+    def visit_Name(self, node: ast.Name) -> Any:
+        if node.id.endswith("_silo_test"):
+            self.match_line = (node.lineno, node.col_offset - 1)
+        return ast.NodeVisitor.generic_visit(self, node)
+
+    def visit_Attribute(self, node: ast.Attribute) -> Any:
+        pass
+
+
+if __name__ == "__main__":
+    main()

bin/run-model-tests

@@ -0,0 +1,33 @@
+#!/usr/bin/env python
+import os
+import os.path
+
+import click
+
+from sentry.utils import json
+
+
+def find_test_cases_matching(model_name: str):
+    manifest = json.loads(open(os.environ["SENTRY_MODEL_MANIFEST_FILE_PATH"]).read())
+    for test_node_id, hits in manifest.items():
+        if any(hit["model"] == model_name for hit in hits):
+            yield test_node_id.split("::")[1]
+
+
+@click.command()
+@click.argument("target_model", required=True)
+@click.argument("pytest_options", nargs=-1)
+def main(target_model: str, pytest_options):
+    """
+    Script that uses the SENTRY_MODEL_MANIFEST_FILE_PATH path to execute tests affected by a specific model.
+    """
+
+    os.execvp(
+        "pytest",
+        ["pytest", "-k", " or ".join(find_test_cases_matching(target_model))]
+        + list(pytest_options),
+    )
+
+
+if __name__ == "__main__":
+    main()

src/sentry/conf/server.py

@@ -2849,7 +2849,7 @@ SENTRY_FUNCTIONS_REGION = "us-central1"
 # Settings related to SiloMode
 SILO_MODE = os.environ.get("SENTRY_SILO_MODE", None)
 FAIL_ON_UNAVAILABLE_API_CALL = False
-SILO_MODE_SPLICE_TESTS = bool(os.environ.get("SENTRY_SILO_MODE_SPLICE_TESTS", False))
+SILO_MODE_UNSTABLE_TESTS = bool(os.environ.get("SENTRY_SILO_MODE_UNSTABLE_TESTS", False))
 
 DISALLOWED_CUSTOMER_DOMAINS = []
 

src/sentry/models/useremail.py

@@ -10,7 +10,7 @@ from django.db.models import QuerySet
 from django.utils import timezone
 from django.utils.translation import ugettext_lazy as _
 
-from sentry.db.models import BaseManager, FlexibleForeignKey, Model, region_silo_model, sane_repr
+from sentry.db.models import BaseManager, FlexibleForeignKey, Model, control_silo_model, sane_repr
 from sentry.db.models.query import in_iexact
 from sentry.utils.security import get_secure_token
 
@@ -50,7 +50,7 @@ class UserEmailManager(BaseManager):
         }
 
 
-@region_silo_model
+@control_silo_model
 class UserEmail(Model):
     __include_in_export__ = True
 

src/sentry/models/userip.py

@@ -3,13 +3,13 @@ from django.core.cache import cache
 from django.db import models
 from django.utils import timezone
 
-from sentry.db.models import FlexibleForeignKey, Model, all_silo_model, sane_repr
+from sentry.db.models import FlexibleForeignKey, Model, control_silo_model, sane_repr
 from sentry.models import User
 from sentry.region_to_control.messages import UserIpEvent
 from sentry.utils.geo import geo_by_addr
 
 
-@all_silo_model
+@control_silo_model
 class UserIP(Model):
     __include_in_export__ = True
 

src/sentry/testutils/silo.py

@@ -1,8 +1,9 @@
 from __future__ import annotations
 
 import functools
+import inspect
 from contextlib import contextmanager
-from typing import Any, Callable, Generator, Iterable, Tuple
+from typing import Any, Callable, Generator, Iterable, Tuple, cast
 from unittest import TestCase
 
 import pytest
@@ -17,34 +18,26 @@ TestMethod = Callable[..., None]
 class SiloModeTest:
     """Decorate a test case that is expected to work in a given silo mode.
 
-    By default, the test is executed if the environment is in that silo mode or
-    in monolith mode. The test is skipped in an incompatible mode.
-
-    If the SILO_MODE_SPLICE_TESTS environment flag is set, any decorated test
-    class will be modified by having new test methods inserted. These new
-    methods run in the given modes and have generated names (such as
-    "test_response__in_region_silo"). This can be used in a dev environment to
-    test in multiple modes conveniently during a single test run. Individually
-    decorated methods and stand-alone functions are treated as normal.
+    Tests marked to work in monolith mode are always executed.
+    Tests marked additionally to work in silo or control mode only do so when either
+    1. the test is marked as stable=True
+    2. the test is being run with SILO_MODE_UNSTABLE_TESTS=1
     """
 
     def __init__(self, *silo_modes: SiloMode) -> None:
         self.silo_modes = frozenset(silo_modes)
-        self.splice = bool(settings.SILO_MODE_SPLICE_TESTS)
+        self.run_unstable_tests = bool(settings.SILO_MODE_UNSTABLE_TESTS)
 
     @staticmethod
     def _find_all_test_methods(test_class: type) -> Iterable[Tuple[str, TestMethod]]:
         for attr_name in dir(test_class):
-            if attr_name.startswith("test_"):
+            if attr_name.startswith("test_") or attr_name == "test":
                 attr = getattr(test_class, attr_name)
                 if callable(attr):
                     yield attr_name, attr
 
-    def _create_mode_methods_to_splice(
-        self, test_method: TestMethod
-    ) -> Iterable[Tuple[str, TestMethod]]:
-        for mode in self.silo_modes:
-
+    def _create_mode_methods(self, test_method: TestMethod) -> Iterable[Tuple[str, TestMethod]]:
+        def method_for_mode(mode: SiloMode) -> Iterable[Tuple[str, TestMethod]]:
             def replacement_test_method(*args: Any, **kwargs: Any) -> None:
                 with override_settings(SILO_MODE=mode):
                     test_method(*args, **kwargs)
@@ -54,31 +47,63 @@ class SiloModeTest:
             replacement_test_method.__name__ = modified_name
             yield modified_name, replacement_test_method
 
-    def _splice_mode_methods(self, test_class: type) -> type:
+        for mode in self.silo_modes:
+            yield from method_for_mode(mode)
+
+    def _add_silo_modes_to_methods(self, test_class: type) -> type:
         for (method_name, test_method) in self._find_all_test_methods(test_class):
-            for (new_name, new_method) in self._create_mode_methods_to_splice(test_method):
-                setattr(test_class, new_name, new_method)
+            for (new_method_name, new_test_method) in self._create_mode_methods(test_method):
+                setattr(test_class, new_method_name, new_test_method)
         return test_class
 
-    def __call__(self, decorated_obj: Any) -> Any:
+    def __call__(self, decorated_obj: Any = None, stable: bool = False) -> Any:
+        if decorated_obj:
+            return self._call(decorated_obj, stable)
+
+        def receive_decorated_obj(f: Any) -> Any:
+            return self._call(f, stable)
+
+        return receive_decorated_obj
+
+    def _mark_parameterized_by_silo_mode(self, test_method: TestMethod) -> TestMethod:
+        def replacement_test_method(*args: Any, **kwargs: Any) -> None:
+            with override_settings(SILO_MODE=kwargs.pop("silo_mode")):
+                return test_method(*args, **kwargs)
+
+        orig_sig = inspect.signature(test_method)
+        new_test_method = functools.update_wrapper(replacement_test_method, test_method)
+        if "silo_mode" not in orig_sig.parameters:
+            new_params = tuple(orig_sig.parameters.values()) + (
+                inspect.Parameter("silo_mode", inspect.Parameter.KEYWORD_ONLY),
+            )
+            new_sig = orig_sig.replace(parameters=new_params)
+            new_test_method.__setattr__("__signature__", new_sig)
+        return cast(
+            TestMethod,
+            pytest.mark.parametrize("silo_mode", [mode for mode in self.silo_modes])(
+                new_test_method
+            ),
+        )
+
+    def _call(self, decorated_obj: Any, stable: bool) -> Any:
         is_test_case_class = isinstance(decorated_obj, type) and issubclass(decorated_obj, TestCase)
         is_function = callable(decorated_obj)
         if not (is_test_case_class or is_function):
             raise ValueError("@SiloModeTest must decorate a function or TestCase class")
 
-        if self.splice and is_test_case_class:
-            return self._splice_mode_methods(decorated_obj)
+        # Only run non monolith tests when they are marked stable or we are explicitly running for that mode.
+        if not stable and not self.run_unstable_tests:
+            # In this case, simply force the current silo mode (monolith)
+            return decorated_obj
 
-        current_silo_mode = SiloMode.get_current_mode()
-        is_skipped = (
-            current_silo_mode != SiloMode.MONOLITH and current_silo_mode not in self.silo_modes
-        )
-        reason = f"Test case is not part of {current_silo_mode} mode"
-        return pytest.mark.skipif(is_skipped, reason=reason)(decorated_obj)
+        if is_test_case_class:
+            return self._add_silo_modes_to_methods(decorated_obj)
+
+        return self._mark_parameterized_by_silo_mode(decorated_obj)
 
 
-control_silo_test = SiloModeTest(SiloMode.CONTROL)
-region_silo_test = SiloModeTest(SiloMode.REGION)
+control_silo_test = SiloModeTest(SiloMode.CONTROL, SiloMode.MONOLITH)
+region_silo_test = SiloModeTest(SiloMode.REGION, SiloMode.MONOLITH)
 
 
 @contextmanager