fix: use correct ns8-sendmail signature (runagent ns8-sendmail -s -f <to>)

ns8_backup_monitor/notifier.py

@@ -1,44 +1,34 @@
 #!/usr/bin/env python3
 """
-notifier.py - Sends a single classified notification email via ns8-sendmail.
+notifier.py - Sends email via ns8-sendmail.
 
-Delivery strategy:
+Correct invocation (from working reference code):
-  1. ns8-sendmail -t   (uses NS8 configured relay - preferred)
+    runagent ns8-sendmail -s <subject> -f <from> <to> [<to> ...]
-  2. sendmail -t       (fallback if ns8-sendmail not found)
 
-No SMTP configuration needed: ns8-sendmail already knows the relay.
+Body is passed on stdin as plain text.
+ns8-sendmail does NOT read To:/From:/Subject: from headers.
 """
-
 import logging
-import shutil
 import subprocess
 from datetime import datetime, timezone
-from email.mime.multipart import MIMEMultipart
-from email.mime.text import MIMEText
 from typing import Optional
 
 log = logging.getLogger(__name__)
 
-OUTCOME_LABEL = {
+OUTCOME_LABEL = {"SUCCESS": "OK", "PARTIAL": "WARNING", "REPO_FAILURE": "CRITICAL"}
-    "SUCCESS": "OK",
+OUTCOME_COLOR = {"SUCCESS": "#2e7d32", "PARTIAL": "#e65100", "REPO_FAILURE": "#b71c1c"}
-    "PARTIAL": "WARNING",
-    "REPO_FAILURE": "CRITICAL",
-}
-
-OUTCOME_COLOR = {
-    "SUCCESS": "#2e7d32",
-    "PARTIAL": "#e65100",
-    "REPO_FAILURE": "#b71c1c",
-}
 
 
-def _sendmail_binary() -> str:
+def _send_cmd(subject: str, mail_from: str, mail_to: list) -> list:
-    """Return the path to ns8-sendmail, falling back to sendmail."""
+    """
-    for cmd in ("ns8-sendmail", "sendmail"):
+    Build argv for ns8-sendmail.
-        path = shutil.which(cmd)
+    runagent must be in PATH (it is, for root on NS8 nodes).
-        if path:
+    """
-            return path
+    cmd = ["runagent", "ns8-sendmail", "-s", subject]
-    raise FileNotFoundError("Neither ns8-sendmail nor sendmail found in PATH")
+    if mail_from:
+        cmd += ["-f", mail_from]
+    cmd += mail_to
+    return cmd
 
 
 def _build_text(correlation: dict, repo_status: Optional[dict]) -> str:
@@ -46,17 +36,15 @@ def _build_text(correlation: dict, repo_status: Optional[dict]) -> str:
     lines = [
         f"NS8 Backup Monitor - {OUTCOME_LABEL[outcome]}: {outcome}",
         f"Time: {datetime.now(timezone.utc).isoformat()}",
-        f"Plans checked: {', '.join(correlation.get('backup_ids', []))}",
+        f"Plans: {', '.join(correlation.get('backup_ids', []))}",
         f"Modules: {correlation['succeeded']} OK / {correlation['failed']} FAILED / {correlation['total']} total",
         "",
     ]
-
     if correlation["failed_modules"]:
         lines.append("Failed modules:")
         for m in correlation["failed_modules"]:
-            lines.append(f"  - [{m['module_id']}] backup_id={m['backup_id']}: {m.get('error', 'unknown error')}")
+            lines.append(f"  - [{m['module_id']}] {m['backup_id']}: {m.get('error', '?')}")
         lines.append("")
-
     if repo_status:
         lines.append("Repository check:")
         for dest in repo_status.get("destinations", []):
@@ -64,10 +52,8 @@ def _build_text(correlation: dict, repo_status: Optional[dict]) -> str:
         if repo_status.get("note"):
             lines.append(f"  NOTE: {repo_status['note']}")
         lines.append("")
-
     if correlation.get("note"):
         lines.append(f"Note: {correlation['note']}")
-
     return "\n".join(lines)
 
 
@@ -77,128 +63,74 @@ def _build_html(correlation: dict, repo_status: Optional[dict]) -> str:
     label = OUTCOME_LABEL[outcome]
     ts = datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M UTC")
     plan_ids = ", ".join(correlation.get("backup_ids", [])) or "N/A"
-
     rows = ""
     for m in correlation.get("modules", []):
         bg = "#e8f5e9" if m["result"] == "success" else "#ffebee"
         icon = "&#10003;" if m["result"] == "success" else "&#10007;"
-        rows += (
+        rows += (f'<tr style="background:{bg}"><td style="padding:4px 8px">{icon}</td>'
-            f'<tr style="background:{bg}">'
+                 f'<td style="padding:4px 8px">{m["module_id"]}</td>'
-            f'<td style="padding:4px 8px">{icon}</td>'
+                 f'<td style="padding:4px 8px">{m["backup_id"]}</td>'
-            f'<td style="padding:4px 8px">{m["module_id"]}</td>'
+                 f'<td style="padding:4px 8px">{m.get("timestamp","")}</td>'
-            f'<td style="padding:4px 8px">{m["backup_id"]}</td>'
+                 f'<td style="padding:4px 8px">{m.get("error","") or ""}</td></tr>')
-            f'<td style="padding:4px 8px">{m.get("timestamp", "")}</td>'
-            f'<td style="padding:4px 8px">{m.get("error", "") or ""}</td>'
-            '</tr>'
-        )
-
     repo_section = ""
     if repo_status:
-        repo_rows = ""
+        rr = ""
         for dest in repo_status.get("destinations", []):
             bg = "#e8f5e9" if dest["status"] == "OK" else "#ffebee"
-            repo_rows += (
+            rr += (f'<tr style="background:{bg}"><td style="padding:4px 8px">{dest["repo_id"]}</td>'
-                f'<tr style="background:{bg}">'
+                   f'<td style="padding:4px 8px"><b>{dest["status"]}</b></td>'
-                f'<td style="padding:4px 8px">{dest["repo_id"]}</td>'
+                   f'<td style="padding:4px 8px">{dest.get("error","")}</td></tr>')
-                f'<td style="padding:4px 8px"><b>{dest["status"]}</b></td>'
+        if rr:
-                f'<td style="padding:4px 8px">{dest.get("error", "")}</td>'
+            repo_section = ("<h3 style='margin-top:24px'>Repository check</h3>"
-                '</tr>'
+                            "<table border='1' cellspacing='0' style='border-collapse:collapse;font-size:13px;width:100%'>"
-            )
+                            "<thead><tr style='background:#f5f5f5'>"
-        if repo_rows:
+                            "<th style='padding:4px 8px'>Repo</th><th style='padding:4px 8px'>Status</th>"
-            repo_section = f"""
+                            f"<th style='padding:4px 8px'>Detail</th></tr></thead><tbody>{rr}</tbody></table>")
-            <h3 style="margin-top:24px">Repository check</h3>
-            <table border="1" cellspacing="0" cellpadding="0" style="border-collapse:collapse;font-size:13px;width:100%">
-              <thead><tr style="background:#f5f5f5">
-                <th style="padding:4px 8px">Repo ID</th>
-                <th style="padding:4px 8px">Status</th>
-                <th style="padding:4px 8px">Detail</th>
-              </tr></thead>
-              <tbody>{repo_rows}</tbody>
-            </table>
-            """
         if repo_status.get("note"):
-            repo_section += f'<p style="color:#777;font-size:12px">{repo_status["note"]}</p>'
+            repo_section += f"<p style='color:#777;font-size:12px'>{repo_status['note']}</p>"
-
+    return (f"<html><body style='font-family:monospace;font-size:14px;max-width:800px;margin:auto'>"
-    return f"""
+            f"<div style='background:{color};color:#fff;padding:16px 20px;border-radius:6px 6px 0 0'>"
-    <html><body style="font-family:monospace;font-size:14px;max-width:800px;margin:auto">
+            f"<b>NS8 Backup Monitor &mdash; {label}: {outcome}</b><br>"
-      <div style="background:{color};color:#fff;padding:16px 20px;border-radius:6px 6px 0 0">
+            f"<small>{ts} &bull; Plans: {plan_ids}</small></div>"
-        <b>NS8 Backup Monitor &mdash; {label}: {outcome}</b><br>
+            f"<div style='border:1px solid #ddd;border-top:none;padding:16px 20px;border-radius:0 0 6px 6px'>"
-        <small>{ts} &bull; Plans: {plan_ids}</small>
+            f"<p><b>{correlation['succeeded']}</b> OK &nbsp;|&nbsp;"
-      </div>
+            f"<b>{correlation['failed']}</b> FAILED &nbsp;|&nbsp;"
-      <div style="border:1px solid #ddd;border-top:none;padding:16px 20px;border-radius:0 0 6px 6px">
+            f"<b>{correlation['total']}</b> total</p>"
-        <p>
+            "<table border='1' cellspacing='0' style='border-collapse:collapse;font-size:13px;width:100%'>"
-          <b>{correlation['succeeded']}</b> module(s) OK &nbsp;|&nbsp;
+            "<thead><tr style='background:#f5f5f5'><th style='padding:4px 8px'></th>"
-          <b>{correlation['failed']}</b> FAILED &nbsp;|&nbsp;
+            "<th style='padding:4px 8px'>Module</th><th style='padding:4px 8px'>Backup ID</th>"
-          <b>{correlation['total']}</b> total
+            "<th style='padding:4px 8px'>Timestamp</th><th style='padding:4px 8px'>Error</th>"
-        </p>
+            f"</tr></thead><tbody>{rows}</tbody></table>{repo_section}</div></body></html>")
-        <table border="1" cellspacing="0" cellpadding="0" style="border-collapse:collapse;font-size:13px;width:100%">
-          <thead><tr style="background:#f5f5f5">
-            <th style="padding:4px 8px"></th>
-            <th style="padding:4px 8px">Module</th>
-            <th style="padding:4px 8px">Backup ID</th>
-            <th style="padding:4px 8px">Timestamp</th>
-            <th style="padding:4px 8px">Error</th>
-          </tr></thead>
-          <tbody>{rows}</tbody>
-        </table>
-        {repo_section}
-      </div>
-    </body></html>
-    """
 
 
-def send_notification(
+def send_notification(config: dict, alerts: list, correlation: dict, repo_status: Optional[dict] = None):
-    config: dict,
-    alerts: list,
-    correlation: dict,
-    repo_status: Optional[dict] = None
-):
     outcome = correlation["outcome"]
     mail_cfg = config.get("mail", {})
     subject_prefix = mail_cfg.get("subject_prefix", "[NS8 Backup]")
-    subject = (
+    subject = (f"{subject_prefix} {OUTCOME_LABEL[outcome]}: {outcome} - "
-        f"{subject_prefix} {OUTCOME_LABEL[outcome]}: {outcome} - "
+               f"{datetime.now(timezone.utc).strftime('%Y-%m-%d')}")
-        f"{datetime.now(timezone.utc).strftime('%Y-%m-%d')}"
-    )
 
     mail_to = mail_cfg.get("to", [])
     if not mail_to:
-        log.error("No mail.to recipients configured - cannot send notification")
+        log.error("No mail.to recipients configured")
         return
-
     mail_from = mail_cfg.get("from", "")
 
-    text_body = _build_text(correlation, repo_status)
+    # ns8-sendmail accepts plain text on stdin; send the text body
-    html_body = _build_html(correlation, repo_status)
+    body = _build_text(correlation, repo_status)
-
+    cmd = _send_cmd(subject, mail_from, mail_to)
-    msg = MIMEMultipart("alternative")
-    msg["Subject"] = subject
-    msg["To"] = ", ".join(mail_to)
-    if mail_from:
-        msg["From"] = mail_from
-    msg.attach(MIMEText(text_body, "plain"))
-    msg.attach(MIMEText(html_body, "html"))
 
+    log.debug("send cmd: %s", cmd)
     try:
-        binary = _sendmail_binary()
+        proc = subprocess.run(cmd, input=body, text=True, capture_output=True, timeout=30)
-    except FileNotFoundError as e:
-        log.error(str(e))
-        return
-
-    try:
-        proc = subprocess.run(
-            [binary, "-t"],
-            input=msg.as_bytes(),
-            capture_output=True,
-            timeout=30,
-        )
         if proc.returncode != 0:
-            log.error(
+            err = proc.stderr.strip() or proc.stdout.strip() or f"exit {proc.returncode}"
-                f"ns8-sendmail exited {proc.returncode}: {proc.stderr.decode().strip()}"
+            log.error("ns8-sendmail failed: %s", err)
-            )
         else:
-            log.info(f"Notification sent via {binary}: {subject} -> {mail_to}")
+            log.info("notification sent: %s -> %s", subject, mail_to)
+    except FileNotFoundError:
+        log.error("'runagent' not found in PATH - is this an NS8 node?")
     except subprocess.TimeoutExpired:
-        log.error(f"ns8-sendmail timed out after 30s")
+        log.error("ns8-sendmail timed out after 30s")
     except Exception as e:
-        log.error(f"Failed to invoke {binary}: {e}")
+        log.error("failed to send notification: %s", e)