From ca945ed65bc0d976062ea40e38bca0f5b03bc0a4 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 16 Jan 2025 12:30:41 +0000 Subject: [PATCH 1/6] test: Reorganize backup/restore tests for speed and reliability We should do the backup/restore tests _after_ we do the basic tests. This is both more efficient as we avoid an extra up/down cycle and more meaningful as we will back up and restore an actually used system. --- _integration-test/conftest.py | 5 ++++ .../{test_run.py => test_01_basics.py} | 0 .../{test_backup.py => test_02_backup.py} | 27 +++++-------------- action.yaml | 3 +-- 4 files changed, 12 insertions(+), 23 deletions(-) rename _integration-test/{test_run.py => test_01_basics.py} (100%) rename _integration-test/{test_backup.py => test_02_backup.py} (84%) diff --git a/_integration-test/conftest.py b/_integration-test/conftest.py index e80cef95e6..e73a7f286c 100644 --- a/_integration-test/conftest.py +++ b/_integration-test/conftest.py @@ -12,6 +12,11 @@ @pytest.fixture(scope="session", autouse=True) def configure_self_hosted_environment(request): + subprocess.run( + ["docker", "compose", "--ansi", "never", "up", "--wait"], + check=True, + capture_output=True, + ) # Create test user subprocess.run( [ diff --git a/_integration-test/test_run.py b/_integration-test/test_01_basics.py similarity index 100% rename from _integration-test/test_run.py rename to _integration-test/test_01_basics.py diff --git a/_integration-test/test_backup.py b/_integration-test/test_02_backup.py similarity index 84% rename from _integration-test/test_backup.py rename to _integration-test/test_02_backup.py index b73e0cfb16..c61345e7ca 100644 --- a/_integration-test/test_backup.py +++ b/_integration-test/test_02_backup.py @@ -20,7 +20,7 @@ def test_sentry_admin(setup_backup_restore_env_variables): assert "Usage: ./sentry-admin.sh permissions" in output -def test_backup(setup_backup_restore_env_variables): +def test_01_backup(setup_backup_restore_env_variables): # Docker was giving me permission issues when trying to create this file and write to it even after giving read + write access # to group and owner. Instead, try creating the empty file and then give everyone write access to the backup file file_path = os.path.join(os.getcwd(), "sentry", "backup.json") @@ -41,19 +41,20 @@ def test_backup(setup_backup_restore_env_variables): assert os.path.getsize(file_path) > 0 -def test_import(setup_backup_restore_env_variables): +def test_02_import(setup_backup_restore_env_variables): # Bring postgres down and recreate the docker volume subprocess.run(["docker", "compose", "--ansi", "never", "down"], check=True) # We reset all DB-related volumes here and not just Postgres although the backups # are only for Postgres. The reason is to get a "clean slate" as we need the Kafka - # and Clickhouse volumes to be back to their initial state as well ( without any events) - # We cannot just rm and create them as they still need migrations. + # and Clickhouse volumes to be back to their initial state as well (without any events) + # We cannot just rm and create them as they still need the migrations. for name in ("postgres", "clickhouse", "kafka"): subprocess.run(["docker", "volume", "rm", f"sentry-{name}"], check=True) subprocess.run( [ "rsync", "-aW", + "--numeric-ids", "--no-compress", "--mkpath", join(os.environ["RUNNER_TEMP"], "volumes", f"sentry-{name}", ""), @@ -64,23 +65,6 @@ def test_import(setup_backup_restore_env_variables): ) subprocess.run(["docker", "volume", "create", f"sentry-{name}"], check=True) - subprocess.run( - [ - "docker", - "run", - "--rm", - "-v", - "sentry-kafka:/data", - "busybox", - "chown", - "-R", - "1000:1000", - "/data", - ], - check=True, - capture_output=True, - ) - subprocess.run( ["docker", "compose", "--ansi", "never", "up", "--wait"], check=True, @@ -97,3 +81,4 @@ def test_import(setup_backup_restore_env_variables): ], check=True, ) + # TODO: Check something actually restored here like the test user we have from earlier diff --git a/action.yaml b/action.yaml index ec5897f0a2..14b90c80e9 100644 --- a/action.yaml +++ b/action.yaml @@ -111,13 +111,12 @@ runs: shell: bash run: | sudo chown root /usr/bin/rsync && sudo chmod u+s /usr/bin/rsync - rsync -aW --no-compress --mkpath \ + rsync -aW --numeric-ids --no-compress --mkpath \ /var/lib/docker/volumes/sentry-postgres \ /var/lib/docker/volumes/sentry-clickhouse \ /var/lib/docker/volumes/sentry-kafka \ "$RUNNER_TEMP/volumes/" cd ${{ github.action_path }} - docker compose up --wait pytest -x --cov --junitxml=junit.xml _integration-test/ - name: Upload coverage to Codecov From e712e4b442e15325432ed94e9f628a574647ee3c Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 16 Jan 2025 13:16:54 +0000 Subject: [PATCH 2/6] try --fake-super for uid/guid issue --- _integration-test/test_02_backup.py | 1 + action.yaml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/_integration-test/test_02_backup.py b/_integration-test/test_02_backup.py index c61345e7ca..4337928ecf 100644 --- a/_integration-test/test_02_backup.py +++ b/_integration-test/test_02_backup.py @@ -54,6 +54,7 @@ def test_02_import(setup_backup_restore_env_variables): [ "rsync", "-aW", + "--fake-super", "--numeric-ids", "--no-compress", "--mkpath", diff --git a/action.yaml b/action.yaml index 14b90c80e9..a56b92fd7b 100644 --- a/action.yaml +++ b/action.yaml @@ -111,7 +111,7 @@ runs: shell: bash run: | sudo chown root /usr/bin/rsync && sudo chmod u+s /usr/bin/rsync - rsync -aW --numeric-ids --no-compress --mkpath \ + rsync -aW --fake-super --numeric-ids --no-compress --mkpath \ /var/lib/docker/volumes/sentry-postgres \ /var/lib/docker/volumes/sentry-clickhouse \ /var/lib/docker/volumes/sentry-kafka \ From 890a146fac881596eb6b0cf89c09d448adcb190d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 16 Jan 2025 13:18:34 +0000 Subject: [PATCH 3/6] try upping just postgres for backup/restore --- sentry-admin.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry-admin.sh b/sentry-admin.sh index 3db5f2ed04..ef0e75e98a 100755 --- a/sentry-admin.sh +++ b/sentry-admin.sh @@ -22,7 +22,8 @@ on the host filesystem. Commands that write files should write them to the '/sen # Actual invocation that runs the command in the container. invocation() { - $dcr --quiet-pull -v "$VOLUME_MAPPING" -T -e SENTRY_LOG_LEVEL=CRITICAL web "$@" 2>&1 + $dc up postgres --wait + $dcr --no-deps --quiet-pull -v "$VOLUME_MAPPING" -T -e SENTRY_LOG_LEVEL=CRITICAL web "$@" 2>&1 } # Function to modify lines starting with `Usage: sentry` to say `Usage: ./sentry-admin.sh` instead. From 6610b863bc9c6d3c655cbdafcb8cbf33f1339347 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 16 Jan 2025 13:35:43 +0000 Subject: [PATCH 4/6] super it is --- _integration-test/test_02_backup.py | 2 +- action.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/_integration-test/test_02_backup.py b/_integration-test/test_02_backup.py index 4337928ecf..07375143de 100644 --- a/_integration-test/test_02_backup.py +++ b/_integration-test/test_02_backup.py @@ -54,7 +54,7 @@ def test_02_import(setup_backup_restore_env_variables): [ "rsync", "-aW", - "--fake-super", + "--super", "--numeric-ids", "--no-compress", "--mkpath", diff --git a/action.yaml b/action.yaml index a56b92fd7b..9f87b93d4f 100644 --- a/action.yaml +++ b/action.yaml @@ -111,7 +111,7 @@ runs: shell: bash run: | sudo chown root /usr/bin/rsync && sudo chmod u+s /usr/bin/rsync - rsync -aW --fake-super --numeric-ids --no-compress --mkpath \ + rsync -aW --super --numeric-ids --no-compress --mkpath \ /var/lib/docker/volumes/sentry-postgres \ /var/lib/docker/volumes/sentry-clickhouse \ /var/lib/docker/volumes/sentry-kafka \ From 513c7bd1fbcf2d561b67d45bffc8ef13542fdf56 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 16 Jan 2025 13:46:08 +0000 Subject: [PATCH 5/6] going sudo --- _integration-test/test_02_backup.py | 1 + action.yaml | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/_integration-test/test_02_backup.py b/_integration-test/test_02_backup.py index 07375143de..9e0ec0f73e 100644 --- a/_integration-test/test_02_backup.py +++ b/_integration-test/test_02_backup.py @@ -52,6 +52,7 @@ def test_02_import(setup_backup_restore_env_variables): subprocess.run(["docker", "volume", "rm", f"sentry-{name}"], check=True) subprocess.run( [ + "sudo", "rsync", "-aW", "--super", diff --git a/action.yaml b/action.yaml index 9f87b93d4f..5ad1110082 100644 --- a/action.yaml +++ b/action.yaml @@ -110,8 +110,7 @@ runs: - name: Integration Test shell: bash run: | - sudo chown root /usr/bin/rsync && sudo chmod u+s /usr/bin/rsync - rsync -aW --super --numeric-ids --no-compress --mkpath \ + sudo rsync -aW --super --numeric-ids --no-compress --mkpath \ /var/lib/docker/volumes/sentry-postgres \ /var/lib/docker/volumes/sentry-clickhouse \ /var/lib/docker/volumes/sentry-kafka \ From e4040368bc05d2a3c8b67bd93d97f0487ce552d4 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 16 Jan 2025 13:58:19 +0000 Subject: [PATCH 6/6] :facepalm: --- _integration-test/test_02_backup.py | 3 +-- action.yaml | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/_integration-test/test_02_backup.py b/_integration-test/test_02_backup.py index 9e0ec0f73e..0898f9a951 100644 --- a/_integration-test/test_02_backup.py +++ b/_integration-test/test_02_backup.py @@ -50,9 +50,9 @@ def test_02_import(setup_backup_restore_env_variables): # We cannot just rm and create them as they still need the migrations. for name in ("postgres", "clickhouse", "kafka"): subprocess.run(["docker", "volume", "rm", f"sentry-{name}"], check=True) + subprocess.run(["docker", "volume", "create", f"sentry-{name}"], check=True) subprocess.run( [ - "sudo", "rsync", "-aW", "--super", @@ -65,7 +65,6 @@ def test_02_import(setup_backup_restore_env_variables): check=True, capture_output=True, ) - subprocess.run(["docker", "volume", "create", f"sentry-{name}"], check=True) subprocess.run( ["docker", "compose", "--ansi", "never", "up", "--wait"], diff --git a/action.yaml b/action.yaml index 5ad1110082..9f87b93d4f 100644 --- a/action.yaml +++ b/action.yaml @@ -110,7 +110,8 @@ runs: - name: Integration Test shell: bash run: | - sudo rsync -aW --super --numeric-ids --no-compress --mkpath \ + sudo chown root /usr/bin/rsync && sudo chmod u+s /usr/bin/rsync + rsync -aW --super --numeric-ids --no-compress --mkpath \ /var/lib/docker/volumes/sentry-postgres \ /var/lib/docker/volumes/sentry-clickhouse \ /var/lib/docker/volumes/sentry-kafka \