[DNM] test: add regression coverage for the RGW datalog segfault

.github/workflows/tests.yml

@@ -133,6 +133,10 @@ jobs:
             name: Test MicroCeph Cluster features.
             suite: cluster-tests
 
+          - id: rgw-datalog-crash-tests
+            name: Regression test for RGW datalog admin request segfault (issue #810)
+            suite: rgw-datalog-crash-tests
+
           - id: rbd-replication-test
             name: Test MicroCeph RBD Remote Replication features.
             suite: rbd-replication-test

snap/snapcraft.yaml

@@ -12,7 +12,7 @@ confinement: strict
 
 package-repositories:
   - type: apt
-    ppa: lmlogiudice/ceph-tentacle-rc
+    ppa: johnramsden/noble-caracal-ceph-tentacle
 
 plugs:
   load-rbd:

tests/robot/README.md

@@ -73,13 +73,14 @@ Each directory under `tests/robot/` is a suite:
 api-tests                          nfs-test
 availability-zone-tests            nfs-multinode-test
 cephadm-adopt-test                 rbd-replication-test
-cephfs-replication-test            single-system-tests
-cluster-tests                      static-checks
-dsl-functional-tests               test-maintenance-modes
-loop-file-tests                    test-sequential-mon-host-refresh
-messenger-v2-tests                 unit-tests
-multi-node-tests                   upgrade-reef-tests
+cephfs-replication-test            rgw-datalog-crash-tests
+cluster-tests                      single-system-tests
+dsl-functional-tests               static-checks
+loop-file-tests                    test-maintenance-modes
+messenger-v2-tests                 test-sequential-mon-host-refresh
+multi-node-tests                   unit-tests
 multi-node-tests-with-custom-microceph-ip
+                                   upgrade-reef-tests
                                    wal-db-tests
                                    wiping-test
 ```

tests/robot/resources/microceph_harness.py

@@ -10,6 +10,7 @@
 import ipaddress
 import json
 import re
+import shlex
 import subprocess
 import tempfile
 import time
@@ -28,6 +29,7 @@
     rbd_primary_image_count,
     rbd_synced_image_count,
 )
+from rgw_admin_ops import rgw_user_s3_credentials
 from snap_services import enabled_active_services
 from streaming_process import run_streaming_process
 
@@ -848,6 +850,90 @@ def read_base64_file_from_container(self, container, path):
         # Non-raising (check=False), matching the previous run_in_container_unchecked call.
         return self.exec_in_container(container, "sudo", "base64", "-w0", path, timeout=30, quiet=True).stdout.strip()
 
+    # -----------------------------------------------------------------------
+    # RGW admin-ops API
+    # -----------------------------------------------------------------------
+
+    def create_rgw_system_user(self, uid, display_name):
+        """Creates a system RGW user on the outer VM, returning its S3 key pair.
+
+        A system user is what the /admin/ endpoints need. RGW checks the per-op
+        cap first but falls back to is_admin() when it is absent, and is_admin()
+        is true for a --system user, so no --caps argument is required. An
+        already-existing uid is reused rather than failing the keyword.
+
+        Runs quiet: the secret key still reaches log.html but is kept off the
+        CI console.
+        """
+        existing = self.run_in_vm(
+            f"sudo microceph.radosgw-admin user info --uid={uid}", 60, quiet=True)
+        if existing.rc == 0:
+            return rgw_user_s3_credentials(existing.stdout)
+        created = self.run_in_vm_and_check(
+            f"sudo microceph.radosgw-admin user create --uid={uid} "
+            f"--display-name={display_name} --system",
+            60, quiet=True)
+        return rgw_user_s3_credentials(created.stdout)
+
+    def send_signed_rgw_admin_request(self, access_key, secret_key, query, timeout=60):
+        """Issues one SigV4-signed GET against the outer VM's RGW admin-ops API.
+
+        *query* is the query string appended to /admin/log?. Returns the raw
+        exec result; callers split its stdout with Curl Body And Status.
+
+        Deliberately non-raising: a radosgw that dies mid-request makes curl
+        exit 52 ("Empty reply from server"), and that outcome has to reach the
+        assertions rather than abort the keyword. The command is a single curl
+        with no pipeline, so bash's -e/-o pipefail cannot alter the rc that
+        curl itself returns.
+
+        The credential is shell-quoted rather than interpolated: RGW secrets
+        are drawn from an alphabet that includes '/' and '+', and Ceph's JSON
+        formatter escapes the slashes, so nothing here should depend on the
+        generated key happening to be free of shell metacharacters.
+        """
+        credential = shlex.quote(f"{access_key}:{secret_key}")
+        return self.run_in_vm(
+            f"curl -sS --max-time 20 -w '\\n%{{http_code}}' "
+            f'--aws-sigv4 "aws:amz:us-east-1:s3" --user {credential} '
+            f'"http://localhost:80/admin/log?{query}"',
+            timeout,
+        )
+
+    # -----------------------------------------------------------------------
+    # Daemon crash forensics
+    # -----------------------------------------------------------------------
+
+    def get_snap_service_restart_count(self, service):
+        """Returns systemd's NRestarts counter for snap.microceph.<service>.service.
+
+        A daemon killed by a signal is restarted by systemd, so this counter
+        moving across a narrow window corroborates a crash independently of any
+        log wording.
+        """
+        result = self.run_in_vm(
+            f"systemctl show snap.microceph.{service}.service --property=NRestarts",
+            30, quiet=True)
+        return self._safe_int(result.stdout.partition("=")[2])
+
+    def get_vm_file_size(self, path):
+        """Returns the size of *path* in the outer VM in bytes, or 0 if absent."""
+        result = self.run_in_vm(f"sudo stat -c %s -- {path}", 30, quiet=True)
+        if result.rc != 0:
+            return 0
+        return self._safe_int(result.stdout)
+
+    def read_vm_file_from_offset(self, path, offset):
+        """Returns the contents of *path* in the outer VM from byte *offset* on.
+
+        Lets a test look at only what a daemon logged since a known point, so a
+        stale banner from an earlier crash cannot match. tail counts bytes from
+        1, hence the +1.
+        """
+        result = self.run_in_vm_and_check(
+            f"sudo tail -c +{int(offset) + 1} -- {path}", 30, quiet=True)
+        return result.stdout
+
     # -----------------------------------------------------------------------
     # OSD pollers
     # -----------------------------------------------------------------------

tests/robot/resources/microceph_harness.resource

@@ -9,6 +9,7 @@ Library         microceph_harness.py
 Library         streaming_process.py
 Library         snap_services.py
 Library         cephfs_replication.py
+Library         rgw_admin_ops.py
 
 *** Variables ***
 ${SNAP_PATH}          ${EMPTY}

tests/robot/resources/rgw_admin_ops.py

@@ -0,0 +1,126 @@
+"""Robot Framework library: parsing of RGW admin-ops API responses.
+
+Keeps the JSON shape checks for the /admin/log endpoints, and the rgw log
+crash-signature match the issue #810 regression suite relies on, out of the
+.robot suites as single unit-testable functions.
+
+The shape helpers deliberately raise rather than return a bool: a response that
+is empty, truncated, or an error document means the request never really
+reached the handler under test, and a regression suite that only asked "did
+anything come back" would pass in exactly that case.
+"""
+
+import json
+import re
+
+# Ceph's fatal-signal handler always writes this banner, independent of the
+# configured debug level, followed by a backtrace.
+CRASH_BANNER = "Caught signal (Segmentation fault)"
+
+# Frames unique to the issue #810 crash path. The banner alone would also match
+# an unrelated segfault elsewhere in the daemon, so a match requires both.
+CRASH_FRAMES = re.compile(r"run_coro|RGWOp_DATALog|rethrow_exception")
+
+
+def curl_body_and_status(curl_stdout):
+    """Split the stdout of ``curl -w '\\n%{http_code}'`` into (body, status).
+
+    curl writes the status code on its own final line after the response body,
+    so everything before the last newline is the body. A request whose server
+    died part-way through produces no body and the status ``000``.
+    """
+    body, _, status = curl_stdout.rpartition("\n")
+    return body.strip(), status.strip()
+
+
+def rgw_user_s3_credentials(user_json_text):
+    """Return the (access_key, secret_key) of the first S3 key of an RGW user.
+
+    *user_json_text* is the stdout of ``radosgw-admin user create`` or
+    ``radosgw-admin user info``.
+    """
+    doc = _decode(user_json_text, "radosgw-admin user")
+    keys = doc.get("keys")
+    if not keys:
+        raise AssertionError(f"radosgw-admin user has no S3 keys: {user_json_text!r}")
+    return keys[0]["access_key"], keys[0]["secret_key"]
+
+
+def datalog_num_objects(body):
+    """Return num_objects from a ``GET /admin/log?type=data`` response.
+
+    The healthy document is ``{"num_objects": <n>}``, written by
+    RGWOp_DATALog_Info::send_response.
+    """
+    doc = _decode(body, "datalog info")
+    if "num_objects" not in doc:
+        raise AssertionError(f'datalog info response has no "num_objects": {body!r}')
+    return doc["num_objects"]
+
+
+def datalog_shard_info_marker(body):
+    """Return the marker from a ``GET /admin/log?type=data&id=<n>&info`` response.
+
+    The healthy document is ``{"marker": <str>, "last_update": <str>}``.
+    RGWOp_DATALog_ShardInfo::send_response writes it as
+    ``encode_json("info", RGWDataChangesLogInfo)``, but "info" is the root
+    section and Ceph's JSON formatter drops the root section's name, so the
+    dumped fields land at the top level rather than nested under it -- the same
+    reason ``open_object_section("num_objects")`` yields ``{"num_objects": <n>}``
+    and not ``{"num_objects": {"num_objects": <n>}}``.
+
+    An idle shard's marker is legitimately empty, so callers must not treat ""
+    as a failure -- returning at all is what this asserts.
+    """
+    doc = _decode(body, "datalog shard info")
+    missing = [key for key in ("marker", "last_update") if key not in doc]
+    if missing:
+        raise AssertionError(f"datalog shard info is missing {missing}: {body!r}")
+    return doc["marker"]
+
+
+def datalog_list_entry_count(body):
+    """Return the entry count of a ``GET /admin/log?type=data&id=<n>`` response.
+
+    The healthy document is ``{"marker": ..., "last_update": ..., "truncated":
+    <bool>, "entries": [...]}``, written by RGWOp_DATALog_List::send_response.
+    An idle shard returns an empty array, so 0 is a valid result.
+    """
+    doc = _decode(body, "datalog list")
+    entries = doc.get("entries")
+    if not isinstance(entries, list):
+        raise AssertionError(f'datalog list response has no "entries" array: {body!r}')
+    return len(entries)
+
+
+def datalog_crash_signature(log_text, context_lines=20):
+    """Return the issue #810 SIGSEGV excerpt in *log_text*, or "" if absent.
+
+    A match needs the fatal-signal banner followed within *context_lines* by a
+    frame from the crashing path, so an unrelated segfault elsewhere in radosgw
+    is not misreported as this bug.
+
+    *log_text* must be only the log written since the request under test --
+    passing a whole log file would match a stale banner from an earlier crash.
+    """
+    lines = log_text.splitlines()
+    for index, line in enumerate(lines):
+        if CRASH_BANNER not in line:
+            continue
+        excerpt = "\n".join(lines[index:index + context_lines])
+        if CRASH_FRAMES.search(excerpt):
+            return excerpt
+    return ""
+
+
+def _decode(body, what):
+    """json.loads *body*, reporting a parse failure as a response-shape error."""
+    if not body:
+        raise AssertionError(f"{what} response body is empty -- nothing came back")
+    try:
+        doc = json.loads(body)
+    except ValueError as exc:
+        raise AssertionError(f"{what} response is not JSON ({exc}): {body!r}")
+    if not isinstance(doc, dict):
+        raise AssertionError(f"{what} response is not a JSON object: {body!r}")
+    return doc