Skip to content

Commit aabfae0

Browse files
committed
CFbot integration
This adds our own custom replication between the CFbot and the commitfest app. We currently keep the last CFbot runs in our database, in an attempt to keep the queries easy and efficient. The CFbot results are now shown on the overview page of each commitfest, as well as on the page for each specific patch.
1 parent 63faa7d commit aabfae0

14 files changed

+299
-4
lines changed

Diff for: media/commitfest/needs_rebase_success.svg

+4
Loading

Diff for: media/commitfest/new_failure.svg

+5
Loading

Diff for: media/commitfest/new_success.svg

+4
Loading

Diff for: media/commitfest/old_failure.svg

+5
Loading

Diff for: media/commitfest/old_success.svg

+4
Loading

Diff for: media/commitfest/running.svg

+4
Loading

Diff for: media/commitfest/waiting_to_start.svg

+3
Loading
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Generated by Django 3.2.25 on 2024-10-31 22:53
2+
3+
from django.db import migrations, models
4+
import django.db.models.deletion
5+
6+
7+
class Migration(migrations.Migration):
8+
9+
dependencies = [
10+
('commitfest', '0005_history_dateindex'),
11+
]
12+
13+
operations = [
14+
migrations.CreateModel(
15+
name='CfbotBranch',
16+
fields=[
17+
('patch', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, related_name='cfbot_branch', serialize=False, to='commitfest.patch')),
18+
('branch_id', models.IntegerField()),
19+
('branch_name', models.TextField()),
20+
('commit_id', models.TextField(blank=True, null=True)),
21+
('apply_url', models.TextField()),
22+
('status', models.TextField(choices=[('testing', 'Testing'), ('finished', 'Finished'), ('failed', 'Failed'), ('timeout', 'Timeout')])),
23+
('created', models.DateTimeField(auto_now_add=True)),
24+
('modified', models.DateTimeField(auto_now=True)),
25+
],
26+
),
27+
migrations.CreateModel(
28+
name='CfbotTask',
29+
fields=[
30+
('id', models.TextField(primary_key=True, serialize=False)),
31+
('task_name', models.TextField()),
32+
('branch_id', models.IntegerField()),
33+
('position', models.IntegerField()),
34+
('status', models.TextField(choices=[('CREATED', 'Created'), ('NEEDS_APPROVAL', 'Needs Approval'), ('TRIGGERED', 'Triggered'), ('EXECUTING', 'Executing'), ('FAILED', 'Failed'), ('COMPLETED', 'Completed'), ('ABORTED', 'Aborted'), ('ERRORED', 'Errored')])),
35+
('created', models.DateTimeField(auto_now_add=True)),
36+
('modified', models.DateTimeField(auto_now=True)),
37+
('patch', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='cfbot_tasks', to='commitfest.patch')),
38+
],
39+
),
40+
]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# Generated by Django 3.2.25 on 2024-12-06 08:38
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
('commitfest', '0006_cfbot_integration'),
10+
]
11+
12+
operations = [
13+
migrations.AlterField(
14+
model_name='cfbottask',
15+
name='status',
16+
field=models.TextField(choices=[('CREATED', 'Created'), ('NEEDS_APPROVAL', 'Needs Approval'), ('TRIGGERED', 'Triggered'), ('EXECUTING', 'Executing'), ('FAILED', 'Failed'), ('COMPLETED', 'Completed'), ('SCHEDULED', 'Scheduled'), ('ABORTED', 'Aborted'), ('ERRORED', 'Errored')]),
17+
),
18+
]

Diff for: pgcommitfest/commitfest/models.py

+40
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,43 @@ class PatchStatus(models.Model):
341341
class PendingNotification(models.Model):
342342
history = models.ForeignKey(PatchHistory, blank=False, null=False, on_delete=models.CASCADE)
343343
user = models.ForeignKey(User, blank=False, null=False, on_delete=models.CASCADE)
344+
345+
346+
class CfbotBranch(models.Model):
347+
STATUS_CHOICES = [
348+
('testing', 'Testing'),
349+
('finished', 'Finished'),
350+
('failed', 'Failed'),
351+
('timeout', 'Timeout'),
352+
]
353+
354+
patch = models.OneToOneField(Patch, on_delete=models.CASCADE, related_name="cfbot_branch", primary_key=True)
355+
branch_id = models.IntegerField(null=False)
356+
branch_name = models.TextField(null=False)
357+
commit_id = models.TextField(null=True, blank=True)
358+
apply_url = models.TextField(null=False)
359+
status = models.TextField(choices=STATUS_CHOICES, null=False)
360+
created = models.DateTimeField(auto_now_add=True)
361+
modified = models.DateTimeField(auto_now=True)
362+
363+
class CfbotTask(models.Model):
364+
STATUS_CHOICES = [
365+
('CREATED', 'Created'),
366+
('NEEDS_APPROVAL', 'Needs Approval'),
367+
('TRIGGERED', 'Triggered'),
368+
('EXECUTING', 'Executing'),
369+
('FAILED', 'Failed'),
370+
('COMPLETED', 'Completed'),
371+
('SCHEDULED', 'Scheduled'),
372+
('ABORTED', 'Aborted'),
373+
('ERRORED', 'Errored'),
374+
]
375+
376+
id = models.TextField(primary_key=True)
377+
task_name = models.TextField(null=False)
378+
patch = models.ForeignKey(Patch, on_delete=models.CASCADE, related_name="cfbot_tasks")
379+
branch_id = models.IntegerField(null=False)
380+
position = models.IntegerField(null=False)
381+
status = models.TextField(choices=STATUS_CHOICES, null=False)
382+
created = models.DateTimeField(auto_now_add=True)
383+
modified = models.DateTimeField(auto_now=True)

Diff for: pgcommitfest/commitfest/templates/commitfest.html

+26
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ <h3>{{p.is_open|yesno:"Active patches,Closed patches"}}</h3>
6363
<th>{%if p.is_open%}<a href="#" style="color:#333333;" onclick="return sortpatches(0);">Patch</a>{%if sortkey == 0%}<div style="float:right;"><i class="glyphicon glyphicon-arrow-down"></i></div>{%endif%}{%endif%}</th>
6464
<th>Status</th>
6565
<th>Ver</th>
66+
<th>CI status</th>
6667
<th>Author</th>
6768
<th>Reviewers</th>
6869
<th>Committer</th>
@@ -86,6 +87,31 @@ <h3>{{p.is_open|yesno:"Active patches,Closed patches"}}</h3>
8687
<td><a href="{{p.id}}/">{{p.name}}</a></td>
8788
<td><span class="label label-{{p.status|patchstatuslabel}}">{{p.status|patchstatusstring}}</span></td>
8889
<td>{%if p.targetversion%}<span class="label label-default">{{p.targetversion}}</span>{%endif%}</td>
90+
<td>
91+
{%if p.cfbot_results %}
92+
<a href="https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F{{p.id}}"
93+
{%if p.cfbot_results.failed_task_names %}
94+
title="Failed: {{p.cfbot_results.failed_task_names}}"
95+
{%endif%}
96+
>
97+
{%if p.cfbot_results.needs_rebase %}
98+
<span class="label label-warning">Needs rebase!</span>
99+
{%else%}
100+
{%if p.cfbot_results.failed > 0 %}
101+
<span class="label label-danger">{{p.cfbot_results.failed}} failed</span>
102+
{%endif%}
103+
{%if p.cfbot_results.completed > 0 %}
104+
<span class="label label-success">{{p.cfbot_results.completed}} succeeded</span>
105+
{%endif%}
106+
{%if p.cfbot_results.running > 0 %}
107+
<span class="label label-info">{{p.cfbot_results.running}} running</span>
108+
{%endif%}
109+
{%endif%}
110+
</a>
111+
{% else %}
112+
?????
113+
{%endif%}
114+
</td>
89115
<td>{{p.author_names|default:''}}</td>
90116
<td>{{p.reviewer_names|default:''}}</td>
91117
<td>{{p.committer|default:''}}</td>

Diff for: pgcommitfest/commitfest/templates/patch.html

+26
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,32 @@
1212
<th>Title</th>
1313
<td>{{patch.name}}</td>
1414
</tr>
15+
<tr>
16+
<th>CI (CFBot)</th>
17+
<td>
18+
{%if not cfbot_branch %}
19+
?????
20+
{%elif not cfbot_branch.commit_id %}
21+
<a href="https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F{{patch.id}}">
22+
<span class="label label-warning">Needs rebase!</span></a>
23+
{%else%}
24+
<a href="https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F{{patch.id}}">
25+
<span class="label label-default">Summary</span></a>
26+
{%for c in cfbot_tasks %}
27+
{%if c.status == 'COMPLETED'%}
28+
<a href="https://cirrusci.com/task/{{c.id}}" title="{{c.task_name}}: {{c.status}}"><img src="/media/commitfest/new_success.svg"/></a>
29+
{%elif c.status == 'CREATED' %}
30+
<a href="https://cirrusci.com/task/{{c.id}}" title="{{c.task_name}}: {{c.status}}"><img src="/media/commitfest/waiting_to_start.svg"/></a>
31+
{%elif c.status == 'EXECUTING' %}
32+
<a href="https://cirrusci.com/task/{{c.id}}" title="{{c.task_name}}: {{c.status}}"><img src="/media/commitfest/running.svg"/></a>
33+
{%else %}
34+
<a href="https://cirrusci.com/task/{{c.id}}" title="{{c.task_name}}: {{c.status}}"><img src="/media/commitfest/new_failure.svg"/></a>
35+
{%endif%}
36+
{%endfor%}
37+
{%endif%}
38+
</a>
39+
</td>
40+
</tr>
1541
<tr>
1642
<th>Topic</th>
1743
<td>{{patch.topic}}</td>

Diff for: pgcommitfest/commitfest/views.py

+119-4
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from django.conf import settings
1212

13-
from datetime import datetime
13+
from datetime import datetime, timezone
1414
from email.mime.text import MIMEText
1515
from email.utils import formatdate, make_msgid
1616
import json
@@ -19,7 +19,7 @@
1919
from pgcommitfest.mailqueue.util import send_mail, send_simple_mail
2020
from pgcommitfest.userprofile.util import UserWrapper
2121

22-
from .models import CommitFest, Patch, PatchOnCommitFest, PatchHistory, Committer
22+
from .models import CommitFest, Patch, PatchOnCommitFest, PatchHistory, Committer, CfbotBranch, CfbotTask
2323
from .models import MailThread
2424
from .forms import PatchForm, NewPatchForm, CommentForm, CommitFestFilterForm
2525
from .forms import BulkEmailForm
@@ -209,11 +209,27 @@ def commitfest(request, cfid):
209209

210210
# Let's not overload the poor django ORM
211211
curs = connection.cursor()
212-
curs.execute("""SELECT p.id, p.name, poc.status, v.version AS targetversion, p.created, p.modified, p.lastmail, committer.username AS committer, t.topic,
212+
curs.execute("""
213+
SELECT p.id, p.name, poc.status, v.version AS targetversion, p.created, p.modified, p.lastmail, committer.username AS committer, t.topic,
213214
(poc.status=ANY(%(openstatuses)s)) AS is_open,
214215
(SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_authors cpa ON cpa.user_id=auth_user.id WHERE cpa.patch_id=p.id) AS author_names,
215216
(SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_reviewers cpr ON cpr.user_id=auth_user.id WHERE cpr.patch_id=p.id) AS reviewer_names,
216-
(SELECT count(1) FROM commitfest_patchoncommitfest pcf WHERE pcf.patch_id=p.id) AS num_cfs
217+
(SELECT count(1) FROM commitfest_patchoncommitfest pcf WHERE pcf.patch_id=p.id) AS num_cfs,
218+
(
219+
SELECT row_to_json(t) as cfbot_results
220+
from (
221+
SELECT
222+
sum((task.status = 'COMPLETED')::int) as completed,
223+
sum((task.status in ('CREATED', 'SCHEDULED', 'EXECUTING'))::int) running,
224+
sum((task.status in ('ABORTED', 'ERRORED', 'FAILED'))::int) failed,
225+
string_agg(task.task_name, ', ') FILTER (WHERE task.status in ('ABORTED', 'ERRORED', 'FAILED')) as failed_task_names,
226+
branch.commit_id IS NULL as needs_rebase
227+
FROM commitfest_cfbotbranch branch
228+
LEFT JOIN commitfest_cfbottask task ON task.branch_id = branch.branch_id
229+
WHERE branch.patch_id=p.id
230+
GROUP BY branch.commit_id
231+
) t
232+
)
217233
FROM commitfest_patch p
218234
INNER JOIN commitfest_patchoncommitfest poc ON poc.patch_id=p.id
219235
INNER JOIN commitfest_topic t ON t.id=p.topic_id
@@ -311,6 +327,12 @@ def patch(request, cfid, patchid):
311327
patch_commitfests = PatchOnCommitFest.objects.select_related('commitfest').filter(patch=patch).order_by('-commitfest__startdate')
312328
committers = Committer.objects.filter(active=True).order_by('user__last_name', 'user__first_name')
313329

330+
try:
331+
cfbot_branch = patch.cfbot_branch
332+
except CfbotBranch.DoesNotExist:
333+
cfbot_branch = None
334+
cfbot_tasks = patch.cfbot_tasks.order_by('position') if cfbot_branch else []
335+
314336
# XXX: this creates a session, so find a smarter way. Probably handle
315337
# it in the callback and just ask the user then?
316338
if request.user.is_authenticated:
@@ -333,6 +355,8 @@ def patch(request, cfid, patchid):
333355
'cf': cf,
334356
'patch': patch,
335357
'patch_commitfests': patch_commitfests,
358+
'cfbot_branch': cfbot_branch,
359+
'cfbot_tasks': cfbot_tasks,
336360
'is_committer': is_committer,
337361
'is_this_committer': is_this_committer,
338362
'is_reviewer': is_reviewer,
@@ -788,6 +812,97 @@ def _user_and_mail(u):
788812
})
789813

790814

815+
@transaction.atomic
816+
def cfbot_ingest(message):
817+
"""Ingest a single message status update message receive from cfbot. It
818+
should be a Python dictionary, decoded from JSON already."""
819+
820+
cursor = connection.cursor()
821+
822+
# Every message should have a shared_secret, and it should match.
823+
if message["shared_secret"] != settings.CFBOT_SECRET:
824+
raise Exception("Invalid shared_secret from CFbot")
825+
826+
branch_status = message["branch_status"]
827+
patch_id = branch_status["submission_id"]
828+
branch_id = branch_status["branch_id"]
829+
created = datetime.fromisoformat(branch_status["created"]).replace(tzinfo=timezone.utc)
830+
831+
try:
832+
Patch.objects.get(pk=patch_id)
833+
except Patch.DoesNotExist:
834+
# If the patch doesn't exist, there's nothing to do. This should never
835+
# happen in production, but on the test system it's possible because
836+
# not it doesn't contain the newest patches that the CFBot knows about.
837+
return
838+
839+
old_branch = CfbotBranch.objects.select_for_update().filter(patch_id=patch_id).first()
840+
if old_branch and old_branch.branch_id != branch_id and old_branch.created.replace(tzinfo=timezone.utc) > created:
841+
# This is a message for an old branch, ignore it.
842+
return
843+
844+
# Every message should have a branch_status, which we will INSERT
845+
# or UPDATE. We do this first, because cfbot_task refers to it.
846+
cursor.execute("""INSERT INTO commitfest_cfbotbranch (patch_id, branch_id,
847+
branch_name, commit_id,
848+
apply_url, status,
849+
created, modified)
850+
VALUES (%s, %s, %s, %s, %s, %s, %s, %s)
851+
ON CONFLICT (patch_id) DO UPDATE
852+
SET status = EXCLUDED.status,
853+
modified = EXCLUDED.modified,
854+
branch_id = EXCLUDED.branch_id,
855+
branch_name = EXCLUDED.branch_name,
856+
commit_id = EXCLUDED.commit_id,
857+
apply_url = EXCLUDED.apply_url,
858+
created = EXCLUDED.created
859+
WHERE commitfest_cfbotbranch.modified < EXCLUDED.modified
860+
""",
861+
(patch_id,
862+
branch_id,
863+
branch_status["branch_name"],
864+
branch_status["commit_id"],
865+
branch_status["apply_url"],
866+
branch_status["status"],
867+
branch_status["created"],
868+
branch_status["modified"]))
869+
870+
# Most messages have a task_status. It might be missing in rare cases, like
871+
# when cfbot decides that a whole branch has timed out. We INSERT or
872+
# UPDATE.
873+
if "task_status" in message:
874+
task_status = message["task_status"]
875+
cursor.execute("""INSERT INTO commitfest_cfbottask (id, task_name, patch_id, branch_id,
876+
position, status,
877+
created, modified)
878+
VALUES (%s, %s, %s, %s, %s, %s, %s, %s)
879+
ON CONFLICT (id) DO UPDATE
880+
SET status = EXCLUDED.status,
881+
modified = EXCLUDED.modified
882+
WHERE commitfest_cfbottask.modified < EXCLUDED.modified""",
883+
(task_status["task_id"],
884+
task_status["task_name"],
885+
patch_id,
886+
branch_id,
887+
task_status["position"],
888+
task_status["status"],
889+
task_status["created"],
890+
task_status["modified"]))
891+
892+
cursor.execute("DELETE FROM commitfest_cfbottask WHERE patch_id=%s AND branch_id != %s", (patch_id, branch_id))
893+
894+
@csrf_exempt
895+
def cfbot_notify(request):
896+
if request.method != 'POST':
897+
return HttpResponseForbidden("Invalid method")
898+
899+
j = json.loads(request.body)
900+
if j['shared_secret'] != settings.CFBOT_SECRET:
901+
return HttpResponseForbidden("Invalid API key")
902+
903+
cfbot_ingest(j)
904+
return HttpResponse(status=200)
905+
791906
@csrf_exempt
792907
def thread_notify(request):
793908
if request.method != 'POST':

Diff for: pgcommitfest/urls.py

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
re_path(r'^ajax/(\w+)/$', ajax.main),
3737
re_path(r'^lookups/user/$', lookups.userlookup),
3838
re_path(r'^thread_notify/$', views.thread_notify),
39+
re_path(r'^cfbot_notify/$', views.cfbot_notify),
3940

4041
# Auth system integration
4142
re_path(r'^(?:account/)?login/?$', pgcommitfest.auth.login),

0 commit comments

Comments
 (0)