See #4577. Backports some hyperkitty fixes.

README.md

@@ -54,7 +54,7 @@ the listener does not become available after starting or restarting Mailman.
 
 ## Legacy HyperKitty compatibility fixes
 
-The optional compatibility patcher addresses three failures observed with the
+The optional compatibility patcher addresses failures observed with the
 legacy Python 3.6 web stack: malformed `From`/`Subject` headers, nested
 `message/rfc822` parts without `Content-Transfer-Encoding`, and unreadable
 gettext catalogs. It also normalizes non-ASCII sender addresses and backports
@@ -64,6 +64,9 @@ Named MIME parts whose content type is missing or empty are extracted as binary
 attachments instead of being appended to the plain-text message body. As an
 additional safeguard, the NUL filtering introduced by django-mailman3 1.3.6 is
 backported for scrubbed text fields only; binary attachments remain unchanged.
+It also backports the HyperKitty 1.3.5 guards for all four asynchronous cache
+tasks that may still reference a mailing list after its archive has been
+deleted. Those obsolete tasks are logged and skipped instead of being retried.
 Enable it only for the known package versions:
 
 ```yaml
@@ -78,10 +81,47 @@ and installed atomically. The task fails without changing any source file if
 the installed versions or expected source fragments do not match. Invalid
 `django_extensions` `.mo` files are preserved beside the original name with a
 `.disabled-by-ansible` suffix, allowing Django to fall back to another locale.
-The uWSGI and Mailman services are restarted only when a source file or catalog
-actually changes. After restarting Mailman, the handler requires the configured
-LMTP listener to become available.
+The standalone HyperKitty qcluster, uWSGI and Mailman services are restarted
+only when a source file or catalog actually changes. Restarting qcluster is
+required because its workers otherwise retain the old Python module in memory.
+After restarting Mailman, the handler requires the configured LMTP listener to
+become available.
 
 Disable this option before upgrading HyperKitty, django-mailman3, or
 mailman-hyperkitty, then review whether the compatibility patches are still
 needed with the new versions.
+
+## HyperKitty archive cleanup
+
+Mailman Core does not delete HyperKitty data when a list is removed, and an
+`archive_policy` of `never` only prevents future archiving. The optional
+cleanup timer removes the corresponding HyperKitty `MailingList`, threads and
+messages in both cases:
+
+```yaml
+mailman_enable_hyperkitty_archive_cleanup: true
+mailman_hyperkitty_archive_cleanup_on_calendar: 'Sun *-*-* 03:00:00'
+mailman_hyperkitty_archive_cleanup_deleted_lists: true
+mailman_hyperkitty_archive_cleanup_disabled_lists: true
+mailman_hyperkitty_archive_cleanup_max_deletions: 50
+```
+
+Deletion is irreversible. Before changing the database, the cleanup script
+retrieves a complete list from Mailman Core and rechecks each candidate. It
+aborts if Core is unavailable, if Core returns no lists, or if the configured
+maximum number of deletions would be exceeded. HyperKitty deletion signals
+that would enqueue cache rebuilds for the objects being removed are disabled
+only inside the cleanup process. This avoids the known failure in legacy
+HyperKitty releases where those asynchronous jobs try to load a list after it
+has been deleted.
+
+The timer is not persistent, so a missed run is not started after boot. A
+read-only preview can be run before the first scheduled execution:
+
+```bash
+sudo -u mailman /usr/local/sbin/hyperkitty-archive-cleanup --dry-run
+```
+
+The monthly HyperKitty job already configured by this role removes stale
+entries from the full-text search index. Therefore the weekly cleanup does not
+run an additional, expensive full index scan.

defaults/main.yml

@@ -52,7 +52,18 @@ mailman_legacy_django_mailman3_version: '1.3.4'
 mailman_legacy_mailman_hyperkitty_version: '1.1.0'
 mailman_legacy_hyperkitty_patch_script: '/usr/local/sbin/mailman-legacy-hyperkitty-compatibility'
 
-  # Documentation that must be followed to configure the social auth providers
+# Remove HyperKitty archives whose list no longer exists in Mailman Core, or
+# whose archive policy has been changed to "never". This is deliberately
+# opt-in because archive deletion is irreversible.
+mailman_enable_hyperkitty_archive_cleanup: false
+mailman_hyperkitty_archive_cleanup_on_calendar: 'Sun *-*-* 03:00:00'
+mailman_hyperkitty_archive_cleanup_deleted_lists: true
+mailman_hyperkitty_archive_cleanup_disabled_lists: true
+mailman_hyperkitty_archive_cleanup_max_deletions: 50
+mailman_hyperkitty_archive_cleanup_script: '/usr/local/sbin/hyperkitty-archive-cleanup'
+mailman_hyperkitty_archive_cleanup_lock: '{{ mailman_lock_dir }}/hyperkitty-archive-cleanup.lock'
+
+# Documentation that must be followed to configure the social auth providers
 # https://django-allauth.readthedocs.io/en/latest/installation.html
 mailman_use_social_account_providers: False
 mailman_postorius_social_auth_providers:

handlers/main.yml

@@ -1,4 +1,8 @@
 ---
+- name: Restart hyperkitty qcluster
+  service: name=hyperkitty-qcluster state=restarted
+  when: mailman_hyperkitty_qcluster_standalone_service | bool
+
 - name: Restart mailmansuite
   service: name={{ mailman_postorius_uwsgi_servicename }} state=restarted
 

tasks/mailman.yml

@@ -73,6 +73,7 @@
         - mailman_enable_legacy_hyperkitty_compatibility_patches | bool
         - not ansible_check_mode
       notify:
+        - Restart hyperkitty qcluster
         - Restart mailmansuite
         - Restart mailman
       tags: [ 'mailman', 'mailman_conf', 'mailman_hyperkitty_compatibility' ]

tasks/postorius-hyperkitty.yml

@@ -98,6 +98,49 @@
   when: not mailman_hyperkitty_qcluster_standalone_service
   tags: [ 'mailman', 'postorius', 'hyperkitty', 'mailman_conf', 'hyperkitty_async' ]
 
+- name: Configure automatic cleanup of obsolete HyperKitty archives
+  block:
+    - name: Install the HyperKitty archive cleanup script
+      ansible.builtin.template:
+        src: hyperkitty-archive-cleanup.py.j2
+        dest: '{{ mailman_hyperkitty_archive_cleanup_script }}'
+        owner: root
+        group: '{{ mailman_user }}'
+        mode: '0750'
+
+    - name: Install the HyperKitty archive cleanup service
+      ansible.builtin.template:
+        src: hyperkitty-archive-cleanup.service.systemd.j2
+        dest: /etc/systemd/system/hyperkitty-archive-cleanup.service
+        owner: root
+        group: root
+        mode: '0644'
+      register: mailman_hyperkitty_archive_cleanup_service_install
+
+    - name: Install the HyperKitty archive cleanup timer
+      ansible.builtin.template:
+        src: hyperkitty-archive-cleanup.timer.systemd.j2
+        dest: /etc/systemd/system/hyperkitty-archive-cleanup.timer
+        owner: root
+        group: root
+        mode: '0644'
+      register: mailman_hyperkitty_archive_cleanup_timer_install
+
+    - name: Reload systemd after installing the HyperKitty cleanup units
+      ansible.builtin.systemd:
+        daemon_reload: true
+      when: >-
+        mailman_hyperkitty_archive_cleanup_service_install is changed or
+        mailman_hyperkitty_archive_cleanup_timer_install is changed
+
+    - name: Set the HyperKitty archive cleanup timer state
+      ansible.builtin.systemd:
+        name: hyperkitty-archive-cleanup.timer
+        state: "{{ mailman_enable_hyperkitty_archive_cleanup | bool | ternary('started', 'stopped') }}"
+        enabled: '{{ mailman_enable_hyperkitty_archive_cleanup | bool }}'
+
+  tags: [ 'mailman', 'postorius', 'hyperkitty', 'mailman_conf', 'hyperkitty_archive_cleanup' ]
+
 - name: Setup the postorius cron jobs
   block:
     - name: add a cron job that syncs the mailman core and postorius settings
@@ -138,4 +181,3 @@
   - name: Install a script that should be run after an update of the mailman suite
     template: src=mailman-postupdate.sh.j2 dest={{ mailman_bindir }}/mailman-postupdate owner={{ mailman_user }} group={{ mailman_user }} mode='0750'
   tags: [ 'mailman', 'postorius', 'hyperkitty', 'mailman_upgrade', 'postorius_upgrade' ]
-

templates/hyperkitty-archive-cleanup.py.j2

@@ -0,0 +1,233 @@
+#!{{ mailman_bindir }}/python3
+
+"""Delete HyperKitty archives that Mailman Core no longer archives."""
+
+import argparse
+import fcntl
+import logging
+import os
+import sys
+from urllib.error import HTTPError
+
+
+PROJECT_DIR = {{ mailman_postorius_dir | to_json }}
+LOCK_FILE = {{ mailman_hyperkitty_archive_cleanup_lock | to_json }}
+DELETE_DELETED_LISTS = {{ mailman_hyperkitty_archive_cleanup_deleted_lists | bool | ternary('True', 'False') }}
+DELETE_DISABLED_LISTS = {{ mailman_hyperkitty_archive_cleanup_disabled_lists | bool | ternary('True', 'False') }}
+MAX_DELETIONS = {{ mailman_hyperkitty_archive_cleanup_max_deletions | int }}
+# HyperKitty 1.3.3 itself uses pages of 10 with the legacy Core client.
+PAGE_SIZE = 10
+
+
+logging.basicConfig(
+    level=logging.INFO,
+    format='hyperkitty-archive-cleanup: %(levelname)s: %(message)s')
+LOG = logging.getLogger(__name__)
+
+
+def normalize_name(value):
+    return value.strip().casefold()
+
+
+def normalize_policy(value):
+    if hasattr(value, 'name'):
+        value = value.name
+    if isinstance(value, int):
+        return {0: 'never', 1: 'private', 2: 'public'}.get(value, str(value))
+    return str(value).strip().lower()
+
+
+def get_archive_policy(core_list):
+    try:
+        value = core_list.archive_policy
+    except AttributeError:
+        value = core_list.settings['archive_policy']
+    return normalize_policy(value)
+
+
+def load_core_lists(client):
+    """Fetch a complete Core snapshot before deciding what may be deleted."""
+    result = {}
+    page_number = 1
+
+    while True:
+        page = list(client.get_list_page(count=PAGE_SIZE, page=page_number))
+        if not page:
+            break
+
+        previous_count = len(result)
+        for core_list in page:
+            result[normalize_name(core_list.fqdn_listname)] = get_archive_policy(core_list)
+
+        if len(result) == previous_count:
+            raise RuntimeError(
+                'Mailman Core pagination returned no new lists on page {}'.format(
+                    page_number))
+        if len(page) < PAGE_SIZE:
+            break
+        page_number += 1
+
+    if not result:
+        raise RuntimeError(
+            'Mailman Core returned no lists; refusing to consider every archive orphaned')
+    return result
+
+
+def recheck_candidate(client, name):
+    """Return the current Core policy, or None only for a confirmed 404."""
+    try:
+        core_list = client.get_list(name)
+    except HTTPError as error:
+        if error.code == 404:
+            return None
+        raise
+    if core_list is None:
+        return None
+    return get_archive_policy(core_list)
+
+
+def find_candidates(core_lists, archive_lists):
+    candidates = []
+    for archive in archive_lists:
+        policy = core_lists.get(normalize_name(archive.name))
+        if policy is None and DELETE_DELETED_LISTS:
+            candidates.append((archive, 'deleted from Mailman Core'))
+        elif policy == 'never' and DELETE_DISABLED_LISTS:
+            candidates.append((archive, 'archive policy is never'))
+    return candidates
+
+
+def confirm_candidates(client, candidates):
+    confirmed = []
+    for archive, original_reason in candidates:
+        current_policy = recheck_candidate(client, archive.name)
+        if current_policy is None and DELETE_DELETED_LISTS:
+            confirmed.append((archive, 'deleted from Mailman Core'))
+        elif current_policy == 'never' and DELETE_DISABLED_LISTS:
+            confirmed.append((archive, 'archive policy is never'))
+        else:
+            LOG.warning(
+                'skipping %s: Core changed since the initial snapshot (%s)',
+                archive.name, original_reason)
+    return confirmed
+
+
+def delete_archives(candidates, dry_run):
+    from django.core.cache import cache
+    from django.db import transaction
+    from django.db.models.signals import post_delete, pre_delete
+    from hyperkitty.models import Email, MailingList, Thread, Vote
+    from hyperkitty.signals import (
+        Email_on_post_delete,
+        Email_on_pre_delete,
+        Thread_on_post_delete,
+        Vote_on_post_delete,
+    )
+
+    signal_bindings = (
+        (pre_delete, Email_on_pre_delete, Email),
+        (post_delete, Email_on_post_delete, Email),
+        (post_delete, Thread_on_post_delete, Thread),
+        (post_delete, Vote_on_post_delete, Vote),
+    )
+
+    if dry_run:
+        for archive, reason in candidates:
+            LOG.info(
+                'would delete %s (%s; %d threads, %d messages)',
+                archive.name, reason, archive.threads.count(), archive.emails.count())
+        return 0
+
+    disconnected = []
+    try:
+        # Bulk archive deletion must not enqueue cache rebuilds for objects that
+        # cease to exist before django-q executes the tasks (HyperKitty #440).
+        for signal, receiver, sender in signal_bindings:
+            if signal.disconnect(receiver, sender=sender):
+                disconnected.append((signal, receiver, sender))
+
+        deleted = 0
+        for archive, reason in candidates:
+            with transaction.atomic():
+                try:
+                    locked_archive = MailingList.objects.select_for_update().get(
+                        pk=archive.pk)
+                except MailingList.DoesNotExist:
+                    LOG.warning('archive %s disappeared before deletion', archive.name)
+                    continue
+
+                thread_count = locked_archive.threads.count()
+                message_count = locked_archive.emails.count()
+                # Match HyperKitty's administrative delete view: deleting
+                # threads explicitly also works around backend FK constraints.
+                locked_archive.threads.all().delete()
+                locked_archive.delete()
+                deleted += 1
+                LOG.info(
+                    'deleted %s (%s; %d threads, %d messages)',
+                    archive.name, reason, thread_count, message_count)
+    finally:
+        for signal, receiver, sender in disconnected:
+            signal.connect(receiver, sender=sender)
+
+    if deleted:
+        cache.clear()
+    return deleted
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument(
+        '--dry-run', action='store_true',
+        help='report candidates without deleting archives')
+    arguments = parser.parse_args()
+
+    os.chdir(PROJECT_DIR)
+    sys.path.insert(0, PROJECT_DIR)
+    os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'settings')
+
+    import django
+    django.setup()
+
+    from django_mailman3.lib.mailman import get_mailman_client
+    from hyperkitty.models import MailingList
+    from mailmanclient import MailmanConnectionError
+
+    try:
+        with open(LOCK_FILE, 'w') as lock_handle:
+            try:
+                fcntl.flock(lock_handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
+            except OSError:
+                LOG.error('another cleanup process holds %s', LOCK_FILE)
+                return 1
+
+            try:
+                client = get_mailman_client()
+                core_lists = load_core_lists(client)
+                archives = list(MailingList.objects.all().order_by('name'))
+                candidates = find_candidates(core_lists, archives)
+                candidates = confirm_candidates(client, candidates)
+            except (HTTPError, MailmanConnectionError, RuntimeError) as error:
+                LOG.error('cannot obtain a reliable Mailman Core view: %s', error)
+                return 1
+
+            if MAX_DELETIONS > 0 and len(candidates) > MAX_DELETIONS:
+                LOG.error(
+                    'refusing to delete %d archives; safety limit is %d',
+                    len(candidates), MAX_DELETIONS)
+                for archive, reason in candidates:
+                    LOG.error('candidate %s (%s)', archive.name, reason)
+                return 1
+
+            deleted = delete_archives(candidates, arguments.dry_run)
+            LOG.info(
+                '%d Core lists, %d HyperKitty archives, %d candidates, %d deleted',
+                len(core_lists), len(archives), len(candidates), deleted)
+            return 0
+    except OSError as error:
+        LOG.error('cannot use cleanup lock %s: %s', LOCK_FILE, error)
+        return 1
+
+
+if __name__ == '__main__':
+    sys.exit(main())

templates/hyperkitty-archive-cleanup.service.systemd.j2

@@ -0,0 +1,15 @@
+[Unit]
+Description=Remove obsolete HyperKitty archives
+After=network-online.target mailman.service
+Wants=network-online.target
+ConditionPathExists={{ mailman_postorius_dir }}/settings.py
+
+[Service]
+Type=oneshot
+User={{ mailman_user }}
+Group={{ mailman_user }}
+WorkingDirectory={{ mailman_postorius_dir }}
+ExecStart={{ mailman_hyperkitty_archive_cleanup_script }}
+TimeoutStartSec=2h
+Nice=10
+IOSchedulingClass=idle

templates/hyperkitty-archive-cleanup.timer.systemd.j2

@@ -0,0 +1,11 @@
+[Unit]
+Description=Weekly cleanup of obsolete HyperKitty archives
+
+[Timer]
+OnCalendar={{ mailman_hyperkitty_archive_cleanup_on_calendar }}
+AccuracySec=5min
+Persistent=false
+Unit=hyperkitty-archive-cleanup.service
+
+[Install]
+WantedBy=timers.target

templates/mailman-legacy-hyperkitty-compatibility.py.j2

@@ -7,6 +7,7 @@ import gettext
 import glob
 import hashlib
 import os
+import re
 import stat
 import struct
 import sys
@@ -156,6 +157,22 @@ MAILMAN_HYPERKITTY_SEND_NEW = '''            message_text = msg.as_bytes()
         except (MessageError, KeyError, UnicodeEncodeError) as error:
 '''
 
+HYPERKITTY_TASKS_WITH_MAILING_LIST = (
+    '_rebuild_mailinglist_cache_recent',
+    '_rebuild_mailinglist_cache_for_month',
+    '_rebuild_cache_popular_threads',
+    '_update_from_mailman',
+)
+HYPERKITTY_TASKS_GET_OLD = (
+    "    mlist = MailingList.objects.get(name=mlist_name)\n"
+)
+HYPERKITTY_TASKS_GET_NEW = '''    try:
+        mlist = MailingList.objects.get(name=mlist_name)
+    except MailingList.DoesNotExist:
+        log.warning('MailingList with name %s does not exist', mlist_name)
+        return
+'''
+
 
 def distribution(name):
     dist = pkg_resources.get_distribution(name)
@@ -253,6 +270,38 @@ def patch_mailman_hyperkitty(source):
         'mailman-hyperkitty byte delivery'), True
 
 
+def patch_hyperkitty_tasks(source):
+    """Backport the upstream stale-list guards to all affected tasks."""
+    updated = source
+    changed = False
+    for function_name in HYPERKITTY_TASKS_WITH_MAILING_LIST:
+        pattern = re.compile(
+            r'^def {}\([^\n]*\):\n.*?(?=^def |\Z)'.format(
+                re.escape(function_name)),
+            re.MULTILINE | re.DOTALL)
+        matches = list(pattern.finditer(updated))
+        if len(matches) != 1:
+            raise RuntimeError(
+                'HyperKitty tasks: expected exactly one {} function, found {}'.format(
+                    function_name, len(matches)))
+
+        function_source = matches[0].group(0)
+        if 'except MailingList.DoesNotExist:' in function_source:
+            continue
+        if function_source.count(HYPERKITTY_TASKS_GET_OLD) != 1:
+            raise RuntimeError(
+                'HyperKitty tasks: {} does not match the expected source'.format(
+                    function_name))
+
+        patched_function = function_source.replace(
+            HYPERKITTY_TASKS_GET_OLD, HYPERKITTY_TASKS_GET_NEW, 1)
+        updated = (
+            updated[:matches[0].start()] + patched_function +
+            updated[matches[0].end():])
+        changed = True
+    return updated, changed
+
+
 def load_source(path):
     with open(path, 'rb') as source_file:
         return source_file.read().decode('utf-8')
@@ -307,18 +356,21 @@ def main():
     mailman_hyperkitty = distribution('mailman-hyperkitty')
 
     incoming_path = os.path.join(hyperkitty.location, 'hyperkitty', 'lib', 'incoming.py')
+    tasks_path = os.path.join(hyperkitty.location, 'hyperkitty', 'tasks.py')
     scrub_path = os.path.join(
         django_mailman3.location, 'django_mailman3', 'lib', 'scrub.py')
     mailman_hyperkitty_path = os.path.join(
         mailman_hyperkitty.location, 'mailman_hyperkitty', '__init__.py')
 
     incoming_source, incoming_changed = patch_hyperkitty(load_source(incoming_path))
+    tasks_source, tasks_changed = patch_hyperkitty_tasks(load_source(tasks_path))
     scrub_source, scrub_changed = patch_scrubber(load_source(scrub_path))
     mailman_hyperkitty_source, mailman_hyperkitty_changed = (
         patch_mailman_hyperkitty(load_source(mailman_hyperkitty_path)))
 
     # Compile all transformed files before replacing any one of them.
     compile(incoming_source, incoming_path, 'exec')
+    compile(tasks_source, tasks_path, 'exec')
     compile(scrub_source, scrub_path, 'exec')
     compile(mailman_hyperkitty_source, mailman_hyperkitty_path, 'exec')
 
@@ -332,6 +384,9 @@ def main():
     if incoming_changed:
         atomic_write(incoming_path, incoming_source)
         print('CHANGED: patched {}'.format(incoming_path))
+    if tasks_changed:
+        atomic_write(tasks_path, tasks_source)
+        print('CHANGED: patched {}'.format(tasks_path))
     if scrub_changed:
         atomic_write(scrub_path, scrub_source)
         print('CHANGED: patched {}'.format(scrub_path))
@@ -343,7 +398,7 @@ def main():
         print('CHANGED: quarantined {} as {} ({})'.format(
             catalog_path, destination, error))
 
-    if (not incoming_changed and not scrub_changed and
+    if (not incoming_changed and not tasks_changed and not scrub_changed and
             not mailman_hyperkitty_changed and not catalogs):
         print('OK: compatibility fixes are already applied')