Skip to content

Commit

Permalink
Fix Redis container from aborting randomly (#405)
Browse files Browse the repository at this point in the history
* Removed "--maxclients 100000" from Redis containers default CMD

* Fix Redis memory overcommit warning by adding Redis configuration options

- Added --save "", --appendonly no, and --maxmemory-policy noeviction to Redis startup command
- Resolves the vm.overcommit_memory warning without using sysctl

* Fix Redis protected mode warning by disabling protected mode

Disabled Redis protected mode using the --protected-mode no option to
resolve the "Possible SECURITY ATTACK detected" warning during tests.
Protected mode is disabled because environments may trigger false positives
due to network interactions, causing Redis to exit unexpectedly.

This allows Redis to continue running without misinterpreting internal
connections as attacks.

* Revert "Added docker cleanup auto-fixture to improve tests stability (#396)"

This reverts commit acd8010.

* Reduced "xdist" and "parallel" tox env reruns from 5 to 3
  • Loading branch information
Nusnus authored Sep 13, 2024
1 parent b407343 commit 6fb93a0
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 62 deletions.
2 changes: 1 addition & 1 deletion src/pytest_celery/vendors/redis/backend/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def default_redis_backend_command(default_redis_backend_cls: type[RedisContainer
Returns:
list[str]: Docker CMD instruction.
"""
return default_redis_backend_cls.command("--maxclients", "100000")
return default_redis_backend_cls.command()


@pytest.fixture
Expand Down
2 changes: 1 addition & 1 deletion src/pytest_celery/vendors/redis/broker/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def default_redis_broker_command(default_redis_broker_cls: type[RedisContainer])
Returns:
list[str]: Docker CMD instruction.
"""
return default_redis_broker_cls.command("--maxclients", "100000")
return default_redis_broker_cls.command()


@pytest.fixture
Expand Down
13 changes: 12 additions & 1 deletion src/pytest_celery/vendors/redis/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,18 @@ def command(
wait_for_client: bool = True,
**kwargs: dict,
) -> list[str]:
return ["redis-server", *args]
return [
"redis-server",
"--save",
"",
"--appendonly",
"no",
"--maxmemory-policy",
"noeviction",
"--protected-mode",
"no",
*args,
]

@property
def url(self) -> str:
Expand Down
52 changes: 0 additions & 52 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import os

import docker
import pytest
from celery import Celery

Expand Down Expand Up @@ -46,54 +45,3 @@ def default_worker_app(default_worker_app: Celery) -> Celery:
if app.conf.broker_url and app.conf.broker_url.startswith("sqs"):
app.conf.broker_transport_options["region"] = LOCALSTACK_CREDS["AWS_DEFAULT_REGION"]
return app


@pytest.fixture(scope="module", autouse=True)
def auto_clean_docker_resources():
"""Clean up Docker resources after each test module."""
# Used for debugging
verbose = False

def log(message):
if verbose:
print(message)

def cleanup_docker_resources():
"""Function to clean up Docker containers, networks, and volumes based
on labels."""
docker_client = docker.from_env()

try:
# Clean up containers with the label 'creator=pytest-docker-tools'
containers = docker_client.containers.list(all=True, filters={"label": "creator=pytest-docker-tools"})
for con in containers:
con.reload() # Ensure we have the latest status
if con.status != "running": # Only remove non-running containers
log(f"Removing container {con.name}")
con.remove(force=True)
else:
log(f"Skipping running container {con.name}")

# Clean up networks with names starting with 'pytest-'
networks = docker_client.networks.list(names=["pytest-*"])
for network in networks:
if not network.containers: # Check if the network is in use
log(f"Removing network {network.name}")
network.remove()
else:
log(f"Skipping network {network.name}, still in use")

# Clean up volumes with names starting with 'pytest-*'
volumes = docker_client.volumes.list(filters={"name": "pytest-*"})
for volume in volumes:
if not volume.attrs.get("UsageData", {}).get("RefCount", 0): # Check if volume is not in use
log(f"Removing volume {volume.name}")
volume.remove()
else:
log(f"Skipping volume {volume.name}, still in use")

except Exception as e:
log(f"Error occurred while cleaning up Docker resources: {e}")

log("--- Running Docker resource cleanup ---")
cleanup_docker_resources()
5 changes: 0 additions & 5 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,3 @@ def default_redis_broker() -> RedisContainer:
@pytest.fixture
def default_worker_container() -> CeleryWorkerContainer:
return mocked_container(CeleryWorkerContainer)


@pytest.fixture(scope="module", autouse=True)
def auto_clean_docker_resources():
"""Skip cleanup in the unit tests."""
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ commands_pre =
commands =
poetry run pytest tests --exitfirst \
-n auto --dist=loadscope \
--reruns 5 --rerun-except AssertionError \
--reruns 3 --reruns-delay 5 --rerun-except AssertionError \
{posargs}

[testenv:parallel]
Expand All @@ -68,7 +68,7 @@ commands_pre =
commands =
tox -e py312-unit,py312-integration,py312-smoke -p auto -o -- --exitfirst \
-n auto --dist=loadscope \
--reruns 5 --reruns-delay 60 --rerun-except AssertionError \
--reruns 3 --reruns-delay 10 --rerun-except AssertionError \
{posargs}

[testenv:mypy]
Expand Down

0 comments on commit 6fb93a0

Please sign in to comment.