Skip to content

Revert "fix: Prevent queue_test from overwriting completed test status" #980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
canihavesomecoffee merged 1 commit into from
Dec 28, 2025
Merged

Revert "fix: Prevent queue_test from overwriting completed test status" #980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 0 additions & 81 deletions celery_app.py
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions config_sample.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,3 @@
GCP_INSTANCE_MAX_RUNTIME = 120 # In minutes
GCS_BUCKET_NAME = 'spdev'
GCS_SIGNED_URL_EXPIRY_LIMIT = 720 # In minutes


# CELERY TASK QUEUE CONFIG
CELERY_BROKER_URL = 'redis://localhost:6379/0'
CELERY_RESULT_BACKEND = 'redis://localhost:6379/0'
CELERY_TASK_SERIALIZER = 'json'
CELERY_RESULT_SERIALIZER = 'json'
CELERY_ACCEPT_CONTENT = ['json']
CELERY_TIMEZONE = 'UTC'
CELERY_ENABLE_UTC = True
CELERY_TASK_ACKS_LATE = True # Task acknowledged after completion
CELERY_WORKER_PREFETCH_MULTIPLIER = 1 # One task at a time per worker
CELERY_TASK_REJECT_ON_WORKER_LOST = True # Requeue tasks if worker dies
CELERY_TASK_SOFT_TIME_LIMIT = 3600 # 1 hour soft limit
CELERY_TASK_TIME_LIMIT = 3900 # 1 hour 5 minutes hard limit

# Feature flag for gradual migration (set to True to enable Celery, False for cron fallback)
USE_CELERY_TASKS = False
23 changes: 0 additions & 23 deletions install/celery-beat.service
View file
Open in desktop

This file was deleted.

30 changes: 0 additions & 30 deletions install/celery-worker.service
View file
Open in desktop

This file was deleted.

101 changes: 0 additions & 101 deletions install/installation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,107 +217,6 @@ The file `mod_ci/cron.py` is to be run in periodic intervals. To setup a cron jo
```
Change the `/var/www/sample-plaform` directory, if you have installed the platform in a different directory.

## Optional: Setting up Celery Task Queue

As an alternative to cron-based polling, you can use Celery with Redis for event-driven test processing. This provides faster test execution, better retry handling, and parallel processing.

### Installing Redis

```bash
sudo apt update
sudo apt install redis-server

# Configure Redis
sudo nano /etc/redis/redis.conf
# Set: supervised systemd
# Set: bind 127.0.0.1 ::1

# Enable and start Redis
sudo systemctl enable redis-server
sudo systemctl start redis-server

# Verify Redis is running
redis-cli ping # Should return PONG
```

### Configuring Celery

Add the following to your `config.py`:

```python
# Celery Configuration
CELERY_BROKER_URL = 'redis://localhost:6379/0'
CELERY_RESULT_BACKEND = 'redis://localhost:6379/0'
USE_CELERY_TASKS = True # Set to False to use cron instead
```

### Installing Celery Services

```bash
# Create log directory
sudo mkdir -p /var/www/sample-platform/logs/celery
sudo chown -R www-data:www-data /var/www/sample-platform/logs/celery

# Create runtime directory
sudo mkdir -p /var/run/celery
sudo chown www-data:www-data /var/run/celery

# Install systemd services
sudo cp /var/www/sample-platform/install/celery-worker.service /etc/systemd/system/
sudo cp /var/www/sample-platform/install/celery-beat.service /etc/systemd/system/

# Reload systemd and enable services
sudo systemctl daemon-reload
sudo systemctl enable celery-worker celery-beat

# Start the services
sudo systemctl start celery-worker
sudo systemctl start celery-beat
```

### Monitoring Celery

```bash
# Check worker status
celery -A celery_app.celery inspect active

# Check queue depth
redis-cli LLEN celery

# View logs
tail -f /var/www/sample-platform/logs/celery/*.log

# Optional: Install Flower for web-based monitoring
pip install flower
celery -A celery_app.celery flower --port=5555
```

### Gradual Migration

For a safe transition from cron to Celery:

1. **Stage 1**: Set `USE_CELERY_TASKS = False` and keep cron running. Start Celery services and verify they work correctly in logs.

2. **Stage 2**: Set `USE_CELERY_TASKS = True`. Reduce cron frequency to every 30 minutes as a fallback.

3. **Stage 3**: Disable cron entirely once you're confident Celery is working correctly.

### Rollback to Cron

If you need to disable Celery:

```bash
# Stop Celery services
sudo systemctl stop celery-beat celery-worker

# Edit config.py and set USE_CELERY_TASKS = False

# Restart platform
sudo systemctl restart platform

# Ensure cron is running every 10 minutes
```

## GCS configuration to serve file downloads using Signed URLs

To serve file downloads directly from the private GCS bucket, Signed download URLs have been used.
Expand Down
69 changes: 6 additions & 63 deletions mod_ci/controllers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ def save_xml_to_file(xml_node, folder_name, file_name) -> None:
)


def add_test_entry(db, commit, test_type, branch="master", pr_nr=0) -> list:
def add_test_entry(db, commit, test_type, branch="master", pr_nr=0) -> None:
"""
Add test details entry into Test model for each platform.

Expand All @@ -1140,16 +1140,16 @@ def add_test_entry(db, commit, test_type, branch="master", pr_nr=0) -> list:
:type branch: str
:param pr_nr: Pull Request number, if applicable.
:type pr_nr: int
:return: List of created test IDs
:rtype: list
:return: Nothing
:rtype: None
"""
from run import log

# Validate commit hash before creating test entries
# Based on issue identified by NexionisJake in PR #937
if not is_valid_commit_hash(commit):
log.error(f"Invalid commit hash '{commit}' - skipping test entry creation")
return []
return

fork_url = f"%/{g.github['repository_owner']}/{g.github['repository']}.git"
fork = Fork.query.filter(Fork.github.like(fork_url)).first()
Expand All @@ -1158,59 +1158,12 @@ def add_test_entry(db, commit, test_type, branch="master", pr_nr=0) -> list:
log.debug('pull request test type detected')
branch = "pull_request"

test_ids = []
linux_test = Test(TestPlatform.linux, test_type, fork.id, branch, commit, pr_nr)
db.add(linux_test)
db.flush() # Get ID before commit
test_ids.append(linux_test.id)

windows_test = Test(TestPlatform.windows, test_type, fork.id, branch, commit, pr_nr)
db.add(windows_test)
db.flush() # Get ID before commit
test_ids.append(windows_test.id)

if not safe_db_commit(db, f"adding test entries for commit {commit[:7]}"):
log.error(f"Failed to add test entries for commit {commit}")
return []

return test_ids


def trigger_test_tasks(test_ids: list, bot_token: str) -> None:
"""
Optionally trigger Celery tasks for newly created tests.

Only triggers if USE_CELERY_TASKS is True in config.
Falls back to waiting for cron/periodic task otherwise.

:param test_ids: List of Test IDs to queue
:type test_ids: list
:param bot_token: GitHub bot token
:type bot_token: str
"""
from run import config, log

if not config.get('USE_CELERY_TASKS', False):
log.debug("Celery tasks disabled, tests will be picked up by cron/periodic task")
return

if not test_ids:
return

try:
from mod_ci.tasks import start_test_task

for test_id in test_ids:
start_test_task.apply_async(
args=[test_id, bot_token],
queue='test_execution',
countdown=30 # 30 second delay for artifact upload to complete
)
log.info(f"Queued test {test_id} via Celery")
except ImportError:
log.warning("Celery tasks module not available, falling back to cron")
except Exception as e:
log.error(f"Failed to queue Celery tasks: {e}, tests will be picked up by cron")


def schedule_test(gh_commit: Commit.Commit) -> None:
Expand Down Expand Up @@ -1352,14 +1305,6 @@ def queue_test(gh_commit: Commit.Commit, commit, test_type, platform, branch="ma
add_customized_regression_tests(platform_test.id)

if gh_commit is not None:
# Check if test already has progress (started or completed)
# If so, don't overwrite the GitHub status with "Tests queued"
# This prevents the bug where a completed test gets its status overwritten
# when a later webhook triggers queue_test for the same commit
if len(platform_test.progress) > 0:
log.info(f"Test {platform_test.id} already has progress, not posting 'Tests queued' status")
return

target_url = url_for('test.by_id', test_id=platform_test.id, _external=True)
status_context = f"CI - {platform_test.platform.value}"
update_status_on_github(gh_commit, Status.PENDING, "Tests queued", status_context, target_url)
Expand Down Expand Up @@ -1484,8 +1429,7 @@ def start_ci():
last_commit.value = ref.object.sha
if not safe_db_commit(g.db, "updating last commit"):
return 'ERROR'
test_ids = add_test_entry(g.db, commit_hash, TestType.commit)
trigger_test_tasks(test_ids, g.github['bot_token'])
add_test_entry(g.db, commit_hash, TestType.commit)
else:
g.log.warning('Unknown push type! Dumping payload for analysis')
g.log.warning(payload)
Expand Down Expand Up @@ -1515,8 +1459,7 @@ def start_ci():
try:
pr = retry_with_backoff(lambda: repository.get_pull(number=pr_nr))
if pr.mergeable is not False:
test_ids = add_test_entry(g.db, commit_hash, TestType.pull_request, pr_nr=pr_nr)
trigger_test_tasks(test_ids, g.github['bot_token'])
add_test_entry(g.db, commit_hash, TestType.pull_request, pr_nr=pr_nr)
except GithubException as e:
g.log.error(f"Failed to get PR {pr_nr} after retries: {e}")

Expand Down
Loading