Enabling pre-submit by addressing the comments received from DevOps team

private-cicd/README.md

@@ -28,10 +28,11 @@ current implementation contract, see
 
 ## How it works
 
-The production Jenkins job loads [`Jenkinsfile`](Jenkinsfile) and helper scripts
-from the protected NetApp `main` branch. Every five minutes a short discovery
-build polls GitHub and self-queues a worker for each unseen PR head SHA. Each
-worker checks out the exact commit into a separate `cloudstack-src` directory,
+The production job is the multibranch folder `cloudstack-ontap-presubmit-mb`.
+Jenkins scans GitHub about once a minute. That scan is not a build. Each
+non-draft origin pull request into `main` gets its own child job. The child
+loads [`Jenkinsfile`](Jenkinsfile) and helper scripts from protected `main`,
+then checks the exact pull-request head SHA out into `cloudstack-src`. A child
 then:
 
 1. validates the request, builder, credentials, and VM inventory;
@@ -48,10 +49,10 @@ replacing the underlying build or test result.
 
 ## Entry points
 
-- **Pull request:** `SOURCE_MODE=discover` notices an open non-draft PR update
-  to `main` and self-queues at most five `SOURCE_MODE=pull_request` workers in
-  the same job. Extra SHAs wait for a later poll. Workers then wait on the
-  `cloudstack-presubmit-vm` lock for a lab VM.
+- **Pull request:** a folder scan creates one child job per open non-draft
+  origin pull request into `main`. Drafts get no job. Compiles run together.
+  Integration waits on the `cloudstack-presubmit-vm` lock. There is no
+  five-worker cap. GitHub cannot call this Jenkins, so there is no webhook.
 - **Manual branch:** a separate triggerless job uses `SOURCE_MODE=branch` with
   a remote branch and exact 40-character commit SHA.
 - **Local validation:** `scripts/validate-local.sh` checks the CI files without

private-cicd/docs/CREATE-PRESUBMIT-JOB.md

@@ -19,12 +19,48 @@
 
 # Create the auto-triggered ONTAP presubmit job
 
-This guide creates **one** Pipeline job, `cloudstack-ontap-presubmit`. Timer
-builds poll GitHub every five minutes and self-queue one worker build for each
-new eligible PR head SHA. Worker builds compile and package concurrently, then
-wait for an available `cloudstack-presubmit-vm` Lockable Resource before
-integration. Until `feature/CSTACKEX-223` is merged, the job loads trusted CI
-from that branch. After merge, change only **Branches to build** to `*/main`.
+The current job is the multibranch folder `cloudstack-ontap-presubmit-mb`.
+It scans GitHub about once a minute and creates one child job for every open
+non-draft origin pull request into `main`. Remote Jenkinsfile Provider loads
+`private-cicd/Jenkinsfile` from the trusted CI branch. Match branches stays off.
+Draft pull requests get no job. Do not add a webhook.
+
+The trusted CI branch is `main`. Both the Remote Jenkinsfile Provider branch and
+`trustedCiBranch()` in the Jenkinsfile must name that branch. During the PR-119
+pilot the provider branch was `feature/CSTACKEX-340` and the folder include
+filter was `PR-119`. Do not switch the provider branch to `main` until PR-119 is
+merged: the previous `main` Jenkinsfile knows nothing about `CHANGE_ID` and
+defaults to `SOURCE_MODE=discover`. After the merge, change the provider branch
+to `main` and the filter to `PR-*`, and keep `feature/CSTACKEX-340` until then.
+
+The numbered sections below that create a single Pipeline job with a five-minute
+`SOURCE_MODE=discover` timer (11 to 16) are retired. Sections on plugins,
+credentials, the inventory, Lockable Resources, and the GitHub App still apply.
+Keep `cloudstack-ontap-presubmit` disabled after the folder works. Do not delete
+the GitHub App, credentials, or the VM lock until a multibranch pull request has
+passed.
+
+## Multibranch folder configuration
+
+Open `cloudstack-ontap-presubmit-mb` > **Configure**:
+
+| Setting | Value |
+| --- | --- |
+| Branch Source | GitHub, credential `cloudstack-presubmit-github-app`, repository `NetApp/cloudstack` |
+| Behaviours | Discover pull requests from origin only, strategy **The current pull request revision**; remove branch and fork discovery |
+| Filter by name | `PR-*` (`PR-119` during the pilot) |
+| Build Configuration | **by Remote Jenkinsfile Provider**, repository `https://github.com/NetApp/cloudstack.git`, branch `main` (`feature/CSTACKEX-340` during the pilot), file `private-cicd/Jenkinsfile`, **Match branches off** |
+| Scan Repository Triggers | Periodically, 1 to 5 minutes |
+| Orphaned items | Keep 14 days, 50 items |
+
+Run **Scan Repository Now** and expect one child for each open non-draft pull
+request into `main` (only `PR-119` during the pilot). The first build may
+stop on Script Security. Approve only signatures requested by this Jenkinsfile
+and rebuild.
+
+The single-branch Pipeline job is now only the manual job for
+`SOURCE_MODE=branch` and `diagnose`. Point it at the same trusted branch, add no
+triggers, and never run `SOURCE_MODE=discover`.
 
 Do not create a GitHub repository webhook. OpenLab Jenkins is not reachable
 from GitHub.com, so inbound `pull_request` deliveries fail with
@@ -36,8 +72,8 @@ item for this presubmit: that would run `Jenkinsfile` from the PR branch. This
 design loads trusted CI from a fixed branch and checks the PR SHA out
 separately.
 
-Do not create a separate poller, Multibranch Pipeline, or second production
-copy of this job.
+The multibranch folder above is the production job. Do not create another
+poller or a second production copy.
 
 This guide starts before the Jenkins item exists and ends after polling and
 required Check have been verified. Follow sections 1 through 21 in order. Do

private-cicd/docs/PRIVATE-CICD-GUIDE.md

@@ -30,9 +30,9 @@ This is the consolidated operator and developer reference. The current [`Jenkins
 
 The guide covers:
 
-- the trusted production Pipeline-from-SCM job with five-minute GitHub API
-  discovery, including first-PR cutover from `feature/CSTACKEX-223` to `main`
-  ([`CREATE-PRESUBMIT-JOB.md`](CREATE-PRESUBMIT-JOB.md));
+- the multibranch folder `cloudstack-ontap-presubmit-mb`, which scans about
+  once a minute and runs trusted CI from `main` for every non-draft origin
+  pull request into `main` ([`CREATE-PRESUBMIT-JOB.md`](CREATE-PRESUBMIT-JOB.md));
 - the separate triggerless manual branch job;
 - local CI-file validation and direct disposable-VM validation;
 - Stage 1 build, unit-test, and Debian package handoff;
@@ -68,12 +68,10 @@ VM lock.
 ## 3. Architecture and data flow
 
 ```text
-Jenkins timer -> discover mode (every 5 minutes)
-  -> GitHub API -> unseen PR head SHAs
-  -> self-queue at most MAX_ACTIVE_WORKERS pull_request builds
-  -> extra SHAs deferred with watermark held
-  -> self-queued worker builds in the same job
-    -> trusted Jenkinsfile/scripts from protected main
+Folder scan about once a minute (not a pipeline build)
+  -> one child job per open non-draft origin PR into main
+  -> trusted Jenkinsfile/scripts from protected main
+  -> exact indexed PR head SHA, confirmed with the pull request API
   -> request, eligibility, credential, inventory, and builder checks
   -> exact PR or branch SHA checkout into cloudstack-src
   -> Maven clean install and unit tests
@@ -156,12 +154,10 @@ Lightweight checkout: off
 PR code cannot replace trusted CI helpers before credentials are used. The source under test is checked out
 separately.
 
-Until `feature/CSTACKEX-223` is merged, that Jenkinsfile is not on `main`. Create one job named
-`cloudstack-ontap-presubmit` that loads CI from `*/feature/CSTACKEX-223`, using
-[`CREATE-PRESUBMIT-JOB.md`](CREATE-PRESUBMIT-JOB.md). Its timer builds discover work and self-queue workers in the
-same item. After merge, change only **Branches to build** to `*/main`. Keep the same job, GitHub App, credentials,
-resources, and Check. Do not create a GitHub repository webhook: OpenLab Jenkins is not reachable from GitHub.com.
-Do not create a GitHub Organization folder for this presubmit.
+Production is the multibranch folder `cloudstack-ontap-presubmit-mb`. Remote Jenkinsfile
+Provider loads this Jenkinsfile from `main` with Match branches off. A folder scan,
+not a webhook, finds pull requests. Keep the old `cloudstack-ontap-presubmit` job
+disabled. Do not create a GitHub repository webhook or a GitHub Organization folder.
 
 ### Manual-job trust boundary
 

private-cicd/scripts/generate-presubmit-report.py

@@ -21,9 +21,11 @@
 import argparse
 import html
 import json
+import os
 import re
 import shutil
-from datetime import datetime, timezone
+from datetime import datetime, timedelta, timezone
+from functools import lru_cache
 from pathlib import Path
 from urllib.parse import quote
 
@@ -71,6 +73,16 @@ def parse_time(value):
     return datetime.fromisoformat(value.replace("Z", "+00:00"))
 
 
+IST = timezone(timedelta(hours=5, minutes=30), "IST")
+
+
+def format_ist(value):
+    """Show a parsed time in IST. Stored timestamps and durations stay UTC."""
+    if value is None:
+        return "—"
+    return value.astimezone(IST).strftime("%Y-%m-%d %H:%M:%S IST")
+
+
 def format_duration(start, finish):
     if not start or not finish:
         return "—"
@@ -121,7 +133,115 @@ def html_document(title, body):
 """
 
 
-def log_to_html(source, destination):
+FAILURE_HEADER = re.compile(r"^(ERROR|FAIL):\s*(.*)$")
+DESCRIBED_TEST = re.compile(
+    r"^(.*?)\s*\.\.\.\s*=== TestName:\s+(\S+)\s+\|\s+Status\s+:\s+(\S+)")
+JOB_ERROR = re.compile(
+    r"errorcode\s*:\s*(\d+)\s*,\s*errortext\s*:\s*'(.*?)(?:'\s*[,}]|$)")
+MAX_FAILURE_LINES = 80
+MAX_ERROR_CHARS = 400
+
+
+@lru_cache(maxsize=None)
+def parse_suite_failures(results_txt):
+    """Return the ERROR and FAIL blocks of one Marvin suite results.txt.
+
+    Marvin prints each block with the test's docstring as its title, while the
+    summary lines pair the same docstring with the test name, so the two are
+    joined on that text. A block that cannot be joined (for example a
+    setUpClass error) keeps an empty name list and is still shown.
+    """
+    try:
+        text = ANSI_ESCAPE.sub(
+            "", results_txt.read_text(encoding="utf-8", errors="replace"))
+    except OSError:
+        return ()
+    lines = text.splitlines()
+    names = {}
+    for line in lines:
+        described = DESCRIBED_TEST.match(line)
+        if described:
+            names.setdefault(described.group(1).strip(), []).append(
+                described.group(2))
+    failures = []
+    index = 0
+    while index < len(lines):
+        header = FAILURE_HEADER.match(lines[index])
+        if not (header and index > 0 and lines[index - 1].startswith("=====")):
+            index += 1
+            continue
+        block = []
+        index += 1
+        while index < len(lines):
+            line = lines[index]
+            if (line.startswith("=====") or line.startswith("Ran ")
+                    or ">> begin captured" in line):
+                break
+            block.append(line)
+            index += 1
+        while block and (not block[0].strip()
+                         or set(block[0].strip()) == {"-"}):
+            block.pop(0)
+        while block and (not block[-1].strip()
+                         or set(block[-1].strip()) == {"-"}):
+            block.pop()
+        description = header.group(2).strip()
+        error = block[-1].strip() if block else ""
+        # A failed CloudStack async job prints a long object of account and
+        # domain IDs before the useful text, so lead with the job's own message.
+        job = JOB_ERROR.search(error)
+        if job:
+            error = f"Job failed (code {job.group(1)}): {job.group(2)}"
+        if len(error) > MAX_ERROR_CHARS:
+            error = error[:MAX_ERROR_CHARS] + "…"
+        failures.append({
+            "kind": header.group(1),
+            "description": description,
+            "names": tuple(names.get(description, ())),
+            "lines": tuple(block[:MAX_FAILURE_LINES]),
+            "error": error,
+        })
+    return tuple(failures)
+
+
+def failure_anchor(test_name):
+    return f"fail-{slug(test_name)}"
+
+
+def render_failures(failures, related=()):
+    if not failures:
+        return ""
+    sections = []
+    if related:
+        links = " &middot; ".join(
+            f'<a href="{html.escape(quote(path))}">{html.escape(label)}</a>'
+            for label, path in related)
+        sections.append(
+            "<p>The API error above is often only a symptom. The cause is "
+            f"usually in the CloudStack logs: {links}</p>")
+    for number, failure in enumerate(failures, 1):
+        names = failure["names"] or (f"unmapped-{number}",)
+        anchors = "".join(
+            f'<a id="{failure_anchor(name)}"></a>' for name in names)
+        title = ", ".join(failure["names"]) or failure["description"]
+        sections.append(
+            f'{anchors}<h3 style="color:#b42318">'
+            f'{html.escape(failure["kind"])}: {html.escape(title)}</h3>'
+            f'<pre style="white-space:pre-wrap;word-break:break-word;'
+            f'background:#fef3f2;border:1px solid #fda29b;padding:16px">'
+            f'{html.escape(chr(10).join(failure["lines"]))}</pre>')
+    return "<h2>Failure details</h2>" + "".join(sections) + "<h2>Suite log</h2>"
+
+
+def failure_for_test(summary_path, test):
+    suite_dir = summary_path.parent / "suites" / test.get("tag", "")
+    for failure in parse_suite_failures(suite_dir / "results.txt"):
+        if test.get("name") in failure["names"]:
+            return failure
+    return None
+
+
+def log_to_html(source, destination, failures=(), related=()):
     lines = source.read_text(encoding="utf-8", errors="replace").splitlines()
     rendered = []
     test_pattern = re.compile(r"TestName:\s+(\S+)\s+\|\s+Status\s+:\s+(\S+)")
@@ -154,7 +274,8 @@ def log_to_html(source, destination):
         rendered.append(f"{anchor}{escaped}")
     body = (
         f"<h2>{html.escape(source.name)}</h2>"
-        f'<pre style="white-space:pre-wrap;word-break:break-word;'
+        + render_failures(failures, related)
+        + f'<pre style="white-space:pre-wrap;word-break:break-word;'
         f'background:#f9fafb;border:1px solid #d0d5dd;padding:16px">'
         + "\n".join(rendered)
         + "</pre>"
@@ -175,6 +296,12 @@ def generate_log_artifacts(results, debian_log):
         results / "phase2" / "configure-cloudstack.log",
         results / "phase2" / "health-check.log",
     ]
+    runtime = results / "phase2" / "runtime-logs"
+    related_logs = (
+        ("Management server log",
+         runtime / "var-log-cloudstack-management" / "management-server.log"),
+        ("Agent log", runtime / "var-log-cloudstack-agent" / "agent.log"),
+    )
     marvin = results / "phase2" / "marvin"
     candidates.extend(marvin.glob("*.log"))
     candidates.extend(
@@ -183,7 +310,15 @@ def generate_log_artifacts(results, debian_log):
     for source in sorted(set(candidates)):
         if not source.is_file():
             continue
-        log_to_html(source, Path(f"{source}.html"))
+        failures = (
+            parse_suite_failures(source.with_name("results.txt"))
+            if source.name == "stdout.log" else ())
+        destination = Path(f"{source}.html")
+        related = tuple(
+            (label, os.path.relpath(path, destination.parent))
+            for label, path in related_logs if path.is_file()
+        ) if failures else ()
+        log_to_html(source, destination, failures, related)
 
 
 def parse_stage_events(stage_events, results):
@@ -263,8 +398,7 @@ def stage_rows(events, results, build_url):
             log_link = (
                 f'<a href="{html.escape(console_url)}">'
                 "Console</a>")
-        started_text = (
-            started.strftime("%Y-%m-%d %H:%M:%S UTC") if started else "—")
+        started_text = format_ist(started)
         rows.append(
             "<tr>"
             f"<td>{html.escape(stage)}</td>"
@@ -339,20 +473,34 @@ def test_rows(tests, results, build_url):
     for test in tests:
         status = display_test_status(test.get("status"))
         log_path = test_log_for_summary(test["_summary"], test)
+        failure = None
+        if status not in ("PASS", "SKIP"):
+            failure = failure_for_test(test["_summary"], test)
         if log_path:
             relative = log_path.relative_to(results.parent)
-            link = artifact_url(
-                build_url, relative, f"test-{slug(test.get('name', 'test'))}")
+            name = test.get("name", "test")
+            # The failure section sits at the top of the suite page, so a
+            # failed test links straight to its exception.
+            fragment = (
+                failure_anchor(name)
+                if failure and log_path.name == "stdout.log.html"
+                else f"test-{slug(name)}")
+            link = artifact_url(build_url, relative, fragment)
             log_cell = f'<a href="{html.escape(link)}">HTML log</a>'
         else:
             log_cell = "—"
+        error_cell = (
+            f'<span style="font-size:12px;color:#b42318">'
+            f'{html.escape(failure["error"])}</span>'
+            if failure and failure["error"] else "")
         suite = str(test.get("label") or test.get("tag") or "")
         rows.append(
             "<tr>"
             f"<td>{html.escape(str(test.get('_protocol', '')))}</td>"
             f"<td>{html.escape(suite)}</td>"
             f"<td><code>{html.escape(str(test.get('name', '')))}</code></td>"
             f'<td style="{status_style(status)}">{html.escape(status)}</td>'
+            f"<td>{error_cell}</td>"
             f"<td>{log_cell}</td>"
             "</tr>")
     return "\n".join(rows)
@@ -415,9 +563,9 @@ def main():
 <tr><td><strong>VM</strong></td>
 <td>{html.escape(args.vm or 'not assigned')}</td></tr>
 <tr><td><strong>Started</strong></td>
-<td>{html.escape(args.started_at)}</td></tr>
+<td>{html.escape(format_ist(started))}</td></tr>
 <tr><td><strong>Finished</strong></td>
-<td>{html.escape(args.finished_at)}</td></tr>
+<td>{html.escape(format_ist(finished))}</td></tr>
 <tr><td><strong>Total duration</strong></td>
 <td>{html.escape(format_duration(started, finished))}</td></tr>
 </table>
@@ -437,7 +585,8 @@ def main():
         "<h2>ONTAP tests</h2>"
         "<p>Failures are listed first. Select a log link to jump to the "
         "test.</p>"
-        + table(["Protocol", "Suite", "Test", "Status", "Details"], tests_html)
+        + table(["Protocol", "Suite", "Test", "Status", "Error", "Details"],
+              tests_html)
         if tests else
         "<h2>ONTAP tests</h2><p>No Marvin test summary was available.</p>")
     body = (