From 7ec9aaf05cd4b8a0b7b645bbef3206a15b9af6a7 Mon Sep 17 00:00:00 2001 From: EgorE Date: Fri, 9 Oct 2026 21:22:01 +0000 Subject: [PATCH 1/5] docs: propose optional HA active-node monitoring --- docs/HA_ACTIVE_NODE_ALERT_DESIGN.md | 170 ++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 docs/HA_ACTIVE_NODE_ALERT_DESIGN.md diff --git a/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md b/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md new file mode 100644 index 0000000..fc9b546 --- /dev/null +++ b/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md @@ -0,0 +1,170 @@ +# Optional CyberController HA Active-Node Alert — Design Proposal + +**Status:** Proposed for review; design only, no implementation +**Date:** 2026-10-09 +**Target:** Radware/CyberController-Container-Alert, baseline v1.5.4 (main: a4ddfed) +**Delivery:** One small implementation PR after design approval + +## 1. Goal and decision + +Customer request: alert when CyberController HA changes its active node, including a manual switchover, automatic failover, or failback. The alert should identify the old and new active nodes, not speculate about the cause. + +**Decision:** Add one opt-in HA role-change check to the existing Container Watchdog. Read the existing HA exporter directly over host-local HTTP. Reuse the watchdog's polling loop, persistent state, and existing syslog/SMTP/SNMP/Slack delivery. Do not introduce another daemon, Docker container, metric exporter, firewall port, or Pacemaker control capability. + +This is an **observed active-node change** alert, not a claim to identify why a failover happened or that all CyberController applications are healthy. + +## 2. Verified appliance evidence (read-only inspection) + +On the two-node HA environment ha-test-admin-ea-iad1, Pacemaker reports nodes 10.199.0.241 and 10.199.0.242 online. The HA Operator, OpenSearch, PostgreSQL, MariaDB, and Failback promotable resources are currently **Promoted on .241** and **Unpromoted on .242**. The Pacemaker DC is .242; **DC is not the CyberController active node**. + +- HA exporter: **ha_cluster_exporter.service**, active on .241, serving port **9664** and reachable at **http://127.0.0.1:9664/metrics** (HTTP 200). +- Promoted role metric: **ha_cluster_pacemaker_resources** includes resource=HAOperatorRep-Res, role=promoted, status=active, node=10.199.0.241, value 1. +- Host identity metric: **ha_cluster_corosync_member_votes** includes local=true, node=10.199.0.241. +- Data-quality signals: **ha_cluster_scrape_success{collector="pacemaker"} = 1** and **ha_cluster_corosync_quorate = 1**. +- Existing Watchdog v1.5.4 runs on .241 in **host network** mode, polls every 60 seconds, has persistent state, and currently delivers syslog. +- Prometheus's native HA target is misconfigured: it scrapes **localhost:9002**, while the exporter serves **9664**. The existing rule **ha_failover_occured** compares per-node location-constraint sum vs average and is not reliable for an actual role change. Correcting the native configuration is a **separate Radware/R&D action**, not part of this feature. +- Standby .242 did **not** accept cross-node connections to 9664/9099/9093. It has not been inspected locally, so its exporter/watchdog installation is **unverified**. Release testing must cover both nodes. + +## 3. Small architecture + +~~~text +Per CyberController node: + Existing ha_cluster_exporter :9664/metrics (localhost only) + | + | bounded HTTP GET in the existing 60s watchdog poll + v + Optional HA role check: + identify local node + unique promoted HA Operator + validate quorum / scrape health + compare with persisted last confirmed active node + confirm role change on 2 consecutive successful polls + | + | only the newly promoted node sends + v + Existing watchdog alert dispatcher + syslog / SMTP / SNMP / Slack + +Both HA nodes must run the watchdog and have their own local exporter. +No inter-node polling or new firewall opening is required. +~~~ + +The check must execute independently of Docker container probes: a Docker API exception must not suppress the HA check, and a failed HA read must not break Docker monitoring. The small HA check runs within the existing process; no worker framework, queue, or additional process is needed. + +### Signal selection + +**Active owner:** accept exactly one node for which the HA exporter reports: + +~~~text +ha_cluster_pacemaker_resources{ + resource="HAOperatorRep-Res", + role="promoted", + status="active", + node="" +} 1 +~~~ + +The check must not use the Pacemaker DC, host virtual IP alone, or location-constraint scores as evidence of an actual promotion. This resource was verified in the current CC version; release testing must confirm the resource name is consistent on the targeted CC versions. + +**Sender identity:** derive the local node from the exporter's **ha_cluster_corosync_member_votes{local="true",node=...}** series. Do not guess local identity from WATCHDOG_HOST, container hostname, or the Prometheus instance label. + +**Validity:** require a successful HTTP response, valid/parseable metrics, successful Pacemaker collection, quorum present, exactly one promoted HA Operator resource, and a unique local-node identity. Otherwise retain previous state, log a bounded diagnostic warning, and do not announce a completed failover. + +### Proposed configuration (disabled by default) + +~~~yaml +ha_monitor: + enabled: false + exporter_url: http://127.0.0.1:9664/metrics +~~~ + +Inherit **check_interval_seconds** (60 seconds in current deployments). Keep the implementation constants small: HTTP timeout 3 seconds, a bounded response size, and two consecutive good polls to confirm a new owner. Avoid extra user-configurable knobs unless validation demonstrates a need. + +Use only the local exporter in v1. No firewall rule, container exec, host bind mount, database query, REST credential, or Pacemaker write command is required. + +## 4. State and alert behavior + +Retain a **small optional ha_monitor section in the existing state.json** (backward-compatible with current container alert state). Track the last confirmed active node and the last notified transition. This state must survive watchdog restart, use existing atomic/serialized persistence, and must not be treated as a Docker container entry. + +| Observation | Required behavior | +|---|---| +| Disabled, or no HA configured | Existing watchdog behavior unchanged; no HA requests | +| First valid poll without prior HA state | Record baseline silently; no historical or startup alert | +| Same active node | Refresh/retain baseline; no notification | +| Candidate active node changes A -> B | Confirm on next good poll (two consecutive observations) | +| Confirmed A -> B | Update persistent baseline; **only watchdog on B** sends one WARNING | +| Confirmed B -> A (failback) | Same behavior; notify once from A | +| Watchdog restarted after notification | Do not repeat previous transition | +| Exporter timeout, 404/500, invalid/incomplete series, lost quorum, zero/multiple promoted owners | Do not change baseline or claim a completed failover; log a rate-limited local warning | +| Watchdog newly installed after the transition | Baseline silently; historical detection not guaranteed | +| Host reboot/grace window | Respect existing boot-grace behavior; do not mix HA alerts into the container reboot-summary. Reconfirm role after grace | +| Both nodes running watchdog | Both can observe role state, but only the newly promoted node emits the role-change notification | + +**Operator notification** (example, not an observed event): + +~~~text +[WARNING] CyberController HA active node changed +Previous active: 10.199.0.241 +New active: 10.199.0.242 +Observed at: +Evidence: HAOperatorRep-Res promoted on 10.199.0.242 (Pacemaker exporter) +Next step: Confirm expected switchover and application health. +~~~ + +Treat this as an event, **not a persistent container failure**: do not send a spurious "container recovered" alert. Keep the normal container alert subject/fields unchanged. Make only the minimum HA-specific display adjustment needed so email/Slack/syslog/SNMP do not misleadingly label it as a crashed container. Preserve existing notification formats for container alerts. + +**Delivery semantics:** one best-effort notification per confirmed observed transition, with state-based deduplication across restarts. Exactly-once delivery to external syslog/SNMP/SMTP cannot be guaranteed. Detection time is an observation timestamp, not necessarily the exact Pacemaker event time. + +## 5. Implementation scope — one development PR, after approval + +| Existing component | Small change | +|---|---| +| **watchdog.py / DEFAULT_CONFIG** | Optional ha_monitor block, disabled by default | +| **watchdog.py / Watchdog._poll_loop** | Invoke HA poll separately, with its own error handling, on the existing interval | +| **HA reader/transition helper** | Bounded local HTTP read; parse only required metrics; validate; confirm active change; deduplicate | +| **WatchdogState persistence** | Optional HA baseline and notified transition, without breaking existing v1 state files or container recovery | +| **AlertPayload/dispatch_alert** | Reuse channels; ensure HA event is clearly labelled and not folded into container reboot/recovery logic | +| **watchdog-config.yaml.example, README.md** | Add brief opt-in example, behavior, limitations, and both-node setup requirements | +| **tests/test_ha_monitor.py + existing regression suite** | Unit tests, mocks, notification checks | + +No changes to **install.sh**, Compose files, Python dependencies, port mappings, Docker privileges, Pacemaker services, or Prometheus are expected. If validation reveals a real need for one, bring it back for design review rather than expanding scope silently. + +## 6. Tests and Definition of Done (single implementation increment) + +### Automated tests (CI, no live HA action) + +- [ ] Feature disabled by default: zero exporter requests and no changes to existing container alerts. +- [ ] Parse metrics with different label orders; select only HAOperatorRep-Res that is active and promoted (value 1), and identify local=true node. +- [ ] Initial valid sample establishes baseline without alert. +- [ ] Stable A -> B across two polls sends **one** HA alert from B; the old node A sends none. +- [ ] B -> A produces one new notification; stable subsequent polls do not repeat. +- [ ] Duplicate/wrong-role resource series, missing local identity, missing Pacemaker-success metric, lost quorum, and zero/multiple promoted owners never create false failover alerts. +- [ ] HTTP timeout, connection refused, malformed metrics and empty response are bounded, leave baseline intact, and do not prevent Docker polling. +- [ ] Persist and restore role and notification deduplication; existing state v1 loads unchanged, and concurrent state writes remain valid. +- [ ] Restart/reboot/grace-window behavior does not generate stale duplicate alerts or contaminate the container reboot-summary. +- [ ] Notification shows previous/new node and UTC observation, is formatted correctly for the configured channels, and does not report fake container failure/recovery. +- [ ] Existing regression tests, Python syntax checks, and Compose/CI checks pass with no new dependencies. + +### Controlled two-node validation (manual QA, authorized window) + +- [ ] Watchdog and local HA exporter are **running and reachable locally on both nodes**; active role is visible from both local metric endpoints. +- [ ] Confirm normal steady state emits no failover alerts; initial installation/restart remains quiet. +- [ ] With an **explicitly authorized controlled switchover**, verify one notification from newly active node with correct A/B details and no duplicate from old active. +- [ ] Controlled failback produces one opposite-direction notification. +- [ ] Verify a loss-of-active-node scenario, or an approved equivalent fault simulation, demonstrates the surviving node can alert without the original node being reachable. +- [ ] Temporarily unavailable local exporter (test fixture or approved fault scenario) does not produce false failover alerts and Docker monitoring continues. +- [ ] Verify syslog end-to-end; test other configured alert channels as applicable. +- [ ] Observe bounded CPU/memory/HTTP impact and document expected limitations, rollback, and compatibility. + +### Definition of Done + +The implementation is ready for review/release when all automated tests and CI pass, controlled two-node role-change and failback tests produce the expected one-per-transition notifications, an active-node loss is covered by the surviving watchdog, resource use remains within current watchdog limits, documentation shows how to enable/disable the option on both nodes, and existing non-HA functionality is unchanged. No HA commands that modify cluster state or customer production services are introduced. + +## 7. Release gates, risks, and intentional exclusions + +**Before implementation/release:** confirm the standby .242 has its own exporter on localhost:9664 and watchdog running; verify both nodes can send syslog/other customer notifications. The remote connection refusal on .242 is **not proof** its local service is absent; it may be a firewall rule. No cross-node port exposure is required for the proposed design. + +**Known limitations:** If the surviving node has no running watchdog/exporter or has never recorded the original active node, this check cannot reconstruct the failover after the fact. Polling can miss a transition that changes twice between samples. Metrics cannot distinguish planned switchover vs unexpected failover, and promotion does not guarantee the entire application is healthy. + +**Out of scope:** HA orchestration/control, restarting or promoting resources, automatic remediation, logging every Pacemaker transition, live log parsing, new monitoring dashboards/exporters, historical HA event reconstruction, generalized multi-service health monitoring, and remediation of the native Prometheus 9002 vs 9664 mismatch. The latter should be handled as a separate CyberController native monitoring correction. + +**Review request:** Approve (or revise) the exact signal (**HAOperatorRep-Res promoted/active**), two-poll confirmation, single notification from the newly active node, state persistence, and the requirement to deploy on both nodes. Implementation must **not** start until design approval. From 2685612cf1d876708dca705f2963e1bbae4f4802 Mon Sep 17 00:00:00 2001 From: rdwr-egore Date: Fri, 9 Oct 2026 21:47:27 +0000 Subject: [PATCH 2/5] feat: optional HA active-node change alert via existing exporter --- DEPLOYMENT.md | 28 ++++ Dockerfile | 2 +- README.md | 41 +++++ ha_monitor.py | 153 ++++++++++++++++++ tests/test_ha_monitor.py | 299 +++++++++++++++++++++++++++++++++++ watchdog-config.yaml.example | 10 ++ watchdog.py | 131 +++++++++++++-- 7 files changed, 650 insertions(+), 14 deletions(-) create mode 100644 ha_monitor.py create mode 100644 tests/test_ha_monitor.py diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index 899164f..c126922 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -410,6 +410,34 @@ docker exec docker-container-watchdog tail -f /var/log/watchdog/watchdog.log --- +### Optional HA role-change verification + +This check is only for **two-node CyberController HA** installations and is +disabled by default. On **each HA host**: + +1. Verify the local exporter is active: curl -fsS http://127.0.0.1:9664/metrics. +2. Confirm metrics for Pacemaker resources, quorum, collector scrape success, + and local node identity are present. Port 9664 is the verified lab setting; + read /etc/ha_cluster_exporter.yaml if different. +3. Enable ha_monitor.enabled: true in watchdog-config.yaml on **both** + nodes; restart only the watchdog service using the normal documented + configuration-change procedure. +4. Verify the baseline is stored under ha_monitor.active_node in the persistent + watchdog state.json; startup should send **no** HA event. +5. During an approved HA maintenance window, perform one controlled switchover + and failback using the supported CyberController operations procedure. + Confirm exactly one notification from the newly active node for each + transition; check docker logs docker-container-watchdog on both hosts. +6. Disable by setting ha_monitor.enabled: false and restarting the watchdog. + No HA service or Pacemaker configuration changes are necessary. + +Do not attempt a network-partition simulation on a cluster with STONITH +disabled: split-brain and database inconsistency are possible. The native +Prometheus port mismatch (9002 vs exporter 9664) is a separate appliance +configuration issue; it does not block the watchdog's direct localhost check. + +--- + ## 5. Rollback ### 5.1 Roll Back a Failed Deployment diff --git a/Dockerfile b/Dockerfile index 6109679..6c11b0b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,7 +11,7 @@ RUN pip install --no-cache-dir \ "cryptography>=42.0" # Copy agent -COPY watchdog.py . +COPY watchdog.py ha_monitor.py ./ # UID 1000 account for install.sh's non-root hardening path (docker-compose.yaml # defaults to root; install.sh instead sets WATCHDOG_UID/GID to run as this user). diff --git a/README.md b/README.md index 30a825f..56a7220 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,47 @@ Two additional deduplication rules suppress redundant alerts at the event level: --- +### Optional HA active-node change alert + +On CyberController HA installations, the watchdog can optionally alert when the +**promoted HA Operator resource** moves from one cluster node to another. +This is an additional application event check; it does not replace or modify +existing Docker container health checks. + +Enable the following block on **both** HA nodes, after verifying that each +node has a local Pacemaker HA cluster exporter: + +~~~yaml +ha_monitor: + enabled: true + exporter_url: http://127.0.0.1:9664/metrics +~~~ + +The watchdog reads HA cluster resource metrics and validates local-node identity, +cluster quorum, and Pacemaker scrape success. The first valid reading +establishes a baseline without alerting. A new active node must be confirmed +on two consecutive successful polls (usually 60 seconds apart). Only the +newly active node sends one **WARNING: CyberController HA active node changed**, +using the already configured Syslog, SMTP, Slack, and/or SNMP channels. +Failback is handled the same way; HA state persists in the normal state.json. + +This check is **off by default**. It does not install an HA exporter, open a +firewall port, execute Pacemaker commands, or modify cluster roles. Both +watchdogs/exporters must be running and able to send notifications for detection +of a failure of the original active node. If a node is newly installed after +the failover, the first reading is a quiet baseline and **cannot reconstruct +the earlier event**. Loss of quorum or exporter connectivity is logged locally, +not misreported as a completed failover. Observed time may be later than the +actual Pacemaker transition. A confirmed promotion does not guarantee all +CyberController services have recovered. + +The exporter may use port **9664** even when the built-in CyberController +Prometheus configuration references **9002**. This optional check reads the +exporter directly and makes **no changes** to native Prometheus configuration. +See [design, test plan and release gates](docs/HA_ACTIVE_NODE_ALERT_DESIGN.md). + +--- + ## 2. System Overview ### Project Structure diff --git a/ha_monitor.py b/ha_monitor.py new file mode 100644 index 0000000..c2c52ed --- /dev/null +++ b/ha_monitor.py @@ -0,0 +1,153 @@ +"""Optional, read-only CyberController HA active-node monitoring. + +Uses the existing Pacemaker HA exporter. No Pacemaker commands, privileges, +network ports, or third-party Prometheus parsing dependency are required. +""" +from dataclasses import dataclass +import json +import math +import re +from urllib.parse import urlsplit + + +MAX_METRICS_BYTES = 256 * 1024 +_LABEL = re.compile(r'([a-zA-Z_][a-zA-Z_0-9]*)="((?:\\.|[^"\\])*)"(?:,|$)') +_SAMPLE = re.compile(r'^([a-zA-Z_:][a-zA-Z_0-9:]*)(?:\{(.*)\})?\s+(\S+)(?:\s+\S+)?$') +_WANTED = { + "ha_cluster_pacemaker_resources", + "ha_cluster_corosync_member_votes", + "ha_cluster_scrape_success", + "ha_cluster_corosync_quorate", +} + + +@dataclass(frozen=True) +class HAObservation: + active_node: str + local_node: str + + +@dataclass(frozen=True) +class HAChange: + previous: str + current: str + local_node: str + + @property + def should_notify(self) -> bool: + return self.local_node == self.current + + +def _labels(raw: str) -> dict[str, str]: + if not raw: + return {} + labels: dict[str, str] = {} + offset = 0 + while offset < len(raw): + match = _LABEL.match(raw, offset) + if not match or match.group(1) in labels: + raise ValueError("invalid or repeated Prometheus metric label") + # JSON-style escaping matches the Prometheus text label syntax. + value = json.loads(chr(34) + match.group(2) + chr(34)) + labels[match.group(1)] = value + offset = match.end() + return labels + + +def parse_ha_metrics(text: str) -> HAObservation: + """Reject incomplete or ambiguous cluster state instead of guessing.""" + promoted: list[str] = [] + local: list[str] = [] + quorum: list[float] = [] + pacemaker_ok: list[float] = [] + for line in text.splitlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + name = line.split("{", 1)[0].split(None, 1)[0] + if name not in _WANTED: + continue + match = _SAMPLE.match(line) + if not match: + raise ValueError("invalid HA metric sample") + labels = _labels(match.group(2) or "") + try: + value = float(match.group(3)) + except ValueError as exc: + raise ValueError("non-numeric HA metric") from exc + if not math.isfinite(value): + raise ValueError("non-finite HA metric") + if name == "ha_cluster_scrape_success" and labels.get("collector") == "pacemaker": + pacemaker_ok.append(value) + elif name == "ha_cluster_corosync_quorate": + quorum.append(value) + elif name == "ha_cluster_corosync_member_votes" and labels.get("local") == "true" and value > 0: + local.append(labels.get("node", "")) + elif (name == "ha_cluster_pacemaker_resources" + and labels.get("resource") == "HAOperatorRep-Res" + and labels.get("role") == "promoted" + and labels.get("status") == "active" + and value == 1): + promoted.append(labels.get("node", "")) + + if pacemaker_ok != [1] or quorum != [1]: + raise ValueError("Pacemaker scrape failed or cluster has no quorum") + if len(local) != 1 or not local[0] or len(local[0]) > 255: + raise ValueError("cannot identify unique local HA node") + if len(promoted) != 1 or not promoted[0] or len(promoted[0]) > 255: + raise ValueError("cannot identify unique promoted HA Operator") + return HAObservation(active_node=promoted[0], local_node=local[0]) + + +def read_ha_observation(session, exporter_url: str, timeout_seconds: float = 3) -> HAObservation: + """Read bounded, fresh local metrics without any new host permissions.""" + parts = urlsplit(exporter_url) + if (parts.scheme != "http" or parts.hostname not in ("127.0.0.1", "localhost", "::1") + or parts.username or parts.password or parts.query or parts.fragment + or parts.path != "/metrics" or not parts.port): + raise ValueError("HA exporter URL must be local HTTP /metrics") + response = session.get(exporter_url, timeout=timeout_seconds, stream=True, allow_redirects=False) + try: + if response.status_code != 200: + raise ValueError(f"HA exporter returned HTTP {response.status_code}") + raw = bytearray() + for chunk in response.iter_content(chunk_size=16384): + raw.extend(chunk) + if len(raw) > MAX_METRICS_BYTES: + raise ValueError("HA exporter response exceeded size limit") + return parse_ha_metrics(raw.decode("utf-8")) + finally: + response.close() + + +class HARoleTracker: + """Two-good-poll confirmation; persist only confirmed active owner.""" + def __init__(self, previous_active: str = ""): + self.confirmed = previous_active + self.candidate = "" + self.candidate_count = 0 + + def reset_candidate(self) -> None: + self.candidate = "" + self.candidate_count = 0 + + def observe(self, obs: HAObservation) -> tuple[bool, HAChange | None]: + """Return (confirmed_state_changed, optional role-change event).""" + if not self.confirmed: + self.confirmed = obs.active_node + self.reset_candidate() + return True, None # Baseline, never alert on install/restart. + if obs.active_node == self.confirmed: + self.reset_candidate() + return False, None + if self.candidate != obs.active_node: + self.candidate = obs.active_node + self.candidate_count = 1 + return False, None + self.candidate_count += 1 + if self.candidate_count < 2: + return False, None + previous = self.confirmed + self.confirmed = obs.active_node + self.reset_candidate() + return True, HAChange(previous=previous, current=obs.active_node, local_node=obs.local_node) diff --git a/tests/test_ha_monitor.py b/tests/test_ha_monitor.py new file mode 100644 index 0000000..c4ddc44 --- /dev/null +++ b/tests/test_ha_monitor.py @@ -0,0 +1,299 @@ +"""Opt-in HA monitor tests. No live cluster, Docker daemon or network required.""" +import json +import tempfile +import threading +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import Mock, patch + +import watchdog +from ha_monitor import ( + HAObservation, HARoleTracker, MAX_METRICS_BYTES, + parse_ha_metrics, read_ha_observation, +) + +A = "10.199.0.241" +B = "10.199.0.242" + + +def metrics(owner=A, local=A, *, pacemaker=1, quorum=1, extras=""): + # Deliberately put labels in a different order from the live exporter. + return f""" +# HELP ha_cluster_pacemaker_resources Cluster role state +ha_cluster_scrape_success{{collector="corosync"}} 1 +ha_cluster_scrape_success{{collector="pacemaker"}} {pacemaker} +ha_cluster_corosync_quorate {quorum} +ha_cluster_corosync_member_votes{{node="{local}",local="true",node_id="1"}} 1 +ha_cluster_corosync_member_votes{{node="{B if local == A else A}",local="false",node_id="2"}} 1 +ha_cluster_pacemaker_resources{{node="{owner}",role="promoted",resource="HAOperatorRep-Res",status="active"}} 1 +ha_cluster_pacemaker_resources{{node="{B if owner == A else A}",role="unpromoted",resource="HAOperatorRep-Res",status="active"}} 1 +ha_cluster_pacemaker_resources{{node="{owner}",role="promoted",resource="MariaDbRep-Res",status="active"}} 1 +{extras} +""" + + +class FakeResponse: + def __init__(self, content: bytes, status=200): + self.content = content + self.status_code = status + self.closed = False + + def iter_content(self, chunk_size=16384): + yield self.content + + def close(self): + self.closed = True + + +class HAReaderTests(unittest.TestCase): + def test_parse_realistic_exporter_snapshot(self): + self.assertEqual(parse_ha_metrics(metrics(owner=A, local=B)), + HAObservation(active_node=A, local_node=B)) + + def test_fail_closed_on_invalid_state(self): + cases = [ + metrics(pacemaker=0), metrics(quorum=0), + metrics().replace('collector="pacemaker"', 'collector="unknown"'), + metrics().replace('local="true"', 'local="false"'), + metrics().replace('resource="HAOperatorRep-Res"', 'resource="Failback-Res"'), + metrics(extras=f'ha_cluster_pacemaker_resources{{node="{B}",role="promoted",resource="HAOperatorRep-Res",status="active"}} 1'), + metrics(extras=f'ha_cluster_corosync_member_votes{{node="{B}",local="true"}} 1'), + metrics(extras='ha_cluster_corosync_quorate 1'), + metrics().replace('ha_cluster_corosync_quorate 1', 'ha_cluster_corosync_quorate NaN'), + metrics().replace('ha_cluster_corosync_quorate 1', 'ha_cluster_corosync_quorate not-a-number'), + "", + ] + for sample in cases: + with self.subTest(sample=sample[-75:]), self.assertRaises(ValueError): + parse_ha_metrics(sample) + + def test_ignores_non_matching_roles_and_untrusted_other_metric(self): + sample = metrics(owner=A, local=A, extras= + 'ha_cluster_pacemaker_resources{node="x",resource="HAOperatorRep-Res",role="promoted",status="active"} 0\n' + 'other_metric{malformed} 9999') + self.assertEqual(parse_ha_metrics(sample).active_node, A) + + def test_http_is_local_bounded_and_closed(self): + response = FakeResponse(metrics().encode()) + session = Mock(get=Mock(return_value=response)) + result = read_ha_observation(session, "http://127.0.0.1:9664/metrics") + self.assertEqual(result.active_node, A) + self.assertTrue(response.closed) + session.get.assert_called_once_with( + "http://127.0.0.1:9664/metrics", timeout=3, stream=True, allow_redirects=False) + + def test_rejects_remote_or_wrong_path_endpoints(self): + for url in ("http://10.199.0.242:9664/metrics", + "http://localhost:9664/admin", "https://localhost:9664/metrics", + "http://localhost:9664/metrics?x=1"): + with self.subTest(url=url), self.assertRaises(ValueError): + read_ha_observation(Mock(), url) + + def test_http_error_oversized_response_or_connection_failure(self): + for response in (FakeResponse(b"", status=500), + FakeResponse(b"X" * (MAX_METRICS_BYTES + 1))): + session = Mock(get=Mock(return_value=response)) + with self.assertRaises(ValueError): + read_ha_observation(session, "http://localhost:9664/metrics") + self.assertTrue(response.closed) + session = Mock(get=Mock(side_effect=TimeoutError("timeout"))) + with self.assertRaises(TimeoutError): + read_ha_observation(session, "http://localhost:9664/metrics") + + +class HATrackerTests(unittest.TestCase): + def test_initial_baseline_and_stable_state_are_silent(self): + tracker = HARoleTracker() + self.assertEqual(tracker.observe(HAObservation(A, A)), (True, None)) + self.assertEqual(tracker.observe(HAObservation(A, A)), (False, None)) + + def test_change_requires_two_good_polls_and_sends_only_from_new_owner(self): + for local in (A, B): + tracker = HARoleTracker(A) + self.assertEqual(tracker.observe(HAObservation(B, local)), (False, None)) + state_changed, change = tracker.observe(HAObservation(B, local)) + self.assertTrue(state_changed) + self.assertEqual((change.previous, change.current), (A, B)) + self.assertEqual(change.should_notify, local == B) + self.assertEqual(tracker.observe(HAObservation(B, local)), (False, None)) + + def test_failback_is_new_event(self): + tracker = HARoleTracker(A) + tracker.observe(HAObservation(B, B)) + tracker.observe(HAObservation(B, B)) + self.assertIsNone(tracker.observe(HAObservation(A, A))[1]) + _, change = tracker.observe(HAObservation(A, A)) + self.assertTrue(change.should_notify) + self.assertEqual((change.previous, change.current), (B, A)) + + def test_flapping_and_failed_check_reset_candidate(self): + tracker = HARoleTracker(A) + tracker.observe(HAObservation(B, B)) + tracker.observe(HAObservation(A, A)) + self.assertIsNone(tracker.observe(HAObservation(B, B))[1]) + tracker.reset_candidate() # unavailable exporter / no quorum + self.assertIsNone(tracker.observe(HAObservation(B, B))[1]) + self.assertTrue(tracker.observe(HAObservation(B, B))[1].should_notify) + + +class WatchdogHAIntegrationTests(unittest.TestCase): + def create_shell(self, state_file): + w = object.__new__(watchdog.Watchdog) + w.state = watchdog.WatchdogState(state_file) + w.state.load() + w.cfg = {"ha_monitor": {"enabled": True}} + w._ha_cfg = {"enabled": True, "exporter_url": "http://127.0.0.1:9664/metrics"} + w._ha_enabled = True + w._ha_tracker = HARoleTracker(w.state.get_ha_node()) + w._ha_error_log_at = 0.0 + w._session = Mock() + w._boot_time = 123 + w._grace_until = 0.0 + w.host = "CC-HA-test" + w.runbook_base = "https://example.invalid/runbook" + w._dispatch = Mock() + return w + + def test_real_watchdog_event_and_persistence(self): + with tempfile.TemporaryDirectory() as td: + path = f"{td}/state.json" + w = self.create_shell(path) + with patch("watchdog.read_ha_observation", side_effect=[ + HAObservation(A, B), HAObservation(B, B), + HAObservation(B, B), HAObservation(B, B), + ]): + for _ in range(4): + w._poll_ha() + w._dispatch.assert_called_once() + alert = w._dispatch.call_args.args[0] + self.assertEqual(alert.failure_type, "ha-failover") + self.assertEqual(alert.severity, "WARNING") + self.assertEqual((alert.ha_previous_node, alert.ha_new_node), (A, B)) + self.assertNotIn("Container", alert.subject()) + self.assertIn(f"{A} -> {B}", alert.subject()) + saved = json.loads(Path(path).read_text()) + self.assertEqual(saved["version"], 1) + self.assertEqual(saved["ha_monitor"]["active_node"], B) + self.assertEqual(saved["containers"], {}) + + restarted = self.create_shell(path) + with patch("watchdog.read_ha_observation", + return_value=HAObservation(B, B)): + restarted._poll_ha() + restarted._dispatch.assert_not_called() + + def test_failed_exporter_does_not_overwrite_state_or_alert(self): + with tempfile.TemporaryDirectory() as td: + w = self.create_shell(f"{td}/state.json") + w.state.set_ha_node(A) + w.state.save(w._boot_time) + w._ha_tracker = HARoleTracker(A) + with patch("watchdog.read_ha_observation", + side_effect=[HAObservation(B, B), TimeoutError("offline"), + HAObservation(B, B), HAObservation(B, B)]): + for _ in range(4): + w._poll_ha() + self.assertEqual(w.state.get_ha_node(), B) + w._dispatch.assert_called_once() + + def test_grace_period_delays_detection_and_no_boot_summary_pollution(self): + with tempfile.TemporaryDirectory() as td: + w = self.create_shell(f"{td}/state.json") + w._ha_tracker = HARoleTracker(A) + w._grace_until = float("inf") + with patch("watchdog.read_ha_observation") as read: + w._poll_ha() + read.assert_not_called() + w._grace_until = 0 + with patch("watchdog.read_ha_observation", + side_effect=[HAObservation(B, B), HAObservation(B, B)]): + w._poll_ha() + w._poll_ha() + w._dispatch.assert_called_once() + + def test_docker_poll_failure_does_not_suppress_ha_poll(self): + w = object.__new__(watchdog.Watchdog) + w.cfg = {"check_interval_seconds": 60} + w._ha_enabled = True + w._stop_event = Mock() + w._stop_event.is_set.side_effect = [False, True] + w._poll_all_containers = Mock(side_effect=RuntimeError("docker outage")) + w._poll_ha = Mock() + with patch("watchdog.log.error"): + w._poll_loop() + w._poll_ha.assert_called_once() + + def test_disabled_by_default(self): + self.assertFalse(watchdog.DEFAULT_CONFIG["ha_monitor"]["enabled"]) + w = object.__new__(watchdog.Watchdog) + w.cfg = {"check_interval_seconds": 60} + w._ha_enabled = False + w._stop_event = Mock() + w._stop_event.is_set.side_effect = [False, True] + w._poll_all_containers = Mock() + w._poll_ha = Mock() + w._poll_loop() + w._poll_ha.assert_not_called() + + def test_existing_state_schema_and_container_recovery_preserved(self): + with tempfile.TemporaryDirectory() as td: + path = f"{td}/state.json" + Path(path).write_text(json.dumps({ + "version": 1, "boot_time": 123, "containers": { + "old-service": {"alerted_for": "unhealthy", "last_alert_time": 42}, + }, + })) + state = watchdog.WatchdogState(path) + containers, boot = state.load() + self.assertIn("old-service", containers) + self.assertEqual(boot, 123) + self.assertEqual(state.get_ha_node(), "") + state.set_ha_node(A) + st = state.get("old-service") + st.alerted_for = "unhealthy" + st.last_alert_time = 42 + threads = [threading.Thread(target=lambda: [state.save(123) for _ in range(20)]) + for _ in range(3)] + for t in threads: t.start() + for t in threads: t.join() + saved = json.loads(Path(path).read_text()) + self.assertIn("old-service", saved["containers"]) + self.assertEqual(saved["ha_monitor"]["active_node"], A) + + +class NotificationTests(unittest.TestCase): + def make_alert(self): + return watchdog.AlertPayload( + "WARNING", "(HA cluster)", "", "CC-HA-test", "ha-failover", + None, "", "Verify service health.", "https://example.invalid/runbook", + f"Previous active: {A}; New active: {B}", + ha_previous_node=A, ha_new_node=B) + + def test_slack_ha_fields_and_no_false_container(self): + alert = self.make_alert() + with patch.dict("os.environ", {"SLACK_WEBHOOK_URL": "https://example.invalid/slack"}): + with patch("watchdog.requests.post") as post: + watchdog.send_slack(alert, {"slack": {"enabled": True}}) + fields = post.call_args.kwargs["json"]["attachments"][0]["fields"] + keys = [f["title"] for f in fields] + self.assertIn("Previous active", keys) + self.assertIn("New active", keys) + self.assertNotIn("Container", keys) + + def test_syslog_logging_includes_transition_and_dispatches_snmp(self): + alert = self.make_alert() + with patch("watchdog.log.warning") as warn, patch("watchdog.send_snmp_trap") as trap: + watchdog.dispatch_alert(alert, {"alert_channels": ["snmp_trap", "syslog"]}) + self.assertIn(f"{A} -> {B}", warn.call_args.args[1]) + trap.assert_called_once() + + def test_container_alert_subject_unchanged(self): + alert = watchdog.AlertPayload( + "HIGH", "config_postgres_1", "abc", "host", "unhealthy", + None, "", "Investigate", "") + self.assertIn("Container UNHEALTHY", alert.subject()) + + +if __name__ == "__main__": + unittest.main() diff --git a/watchdog-config.yaml.example b/watchdog-config.yaml.example index ad27b58..fdc49f7 100644 --- a/watchdog-config.yaml.example +++ b/watchdog-config.yaml.example @@ -127,6 +127,16 @@ auto_health_check: - / # fallback timeout_seconds: 2 # discovery uses 1s (half), cached checks use 2s +# ── Optional CyberController HA active-node change alert ───────────────────── +# Run this watchdog on BOTH HA nodes, with the HA exporter active locally. +# The HA exporter is normally configured on port 9664 on the appliance. +# No Prometheus scrape, Pacemaker command, host bind mount, or firewall change +# is required. First successful sample sets a quiet baseline; a confirmed +# active-node change alerts only from the newly promoted node. +ha_monitor: + enabled: false + exporter_url: http://127.0.0.1:9664/metrics + # ── Manual health checks (override auto-discovery for specific containers) ──── # Leave empty — all containers use auto-discovery container_health_checks: {} \ No newline at end of file diff --git a/watchdog.py b/watchdog.py index aab65fa..72d319d 100644 --- a/watchdog.py +++ b/watchdog.py @@ -26,6 +26,8 @@ import socket import yaml +from ha_monitor import HARoleTracker, read_ha_observation + # ── Logging setup ───────────────────────────────────────────────────────────── LOG_FORMAT = "%(asctime)s [%(levelname)s] %(name)s: %(message)s" SYSLOG_FORMAT = "watchdog[%(process)d]: %(levelname)s %(name)s: %(message)s" @@ -101,6 +103,7 @@ def setup_logging(level: str, log_file: str | None, syslog_cfg: dict | None = No "log_level": "INFO", # Alert state survives restarts so recovery alerts still fire after a reboot. "state_file": "/var/log/watchdog/state.json", + "ha_monitor": {"enabled": False, "exporter_url": "http://127.0.0.1:9664/metrics"}, # After a host reboot, hold alerts this long and emit one summary instead. "boot_grace_seconds": 180, "syslog": { @@ -172,6 +175,7 @@ def __init__(self, state_file: str = ""): self._save_lock = threading.Lock() self._states: dict[str, ContainerState] = {} self._state_file = state_file + self._ha_state: dict[str, str] = {} # independent of container failure state def get(self, name: str) -> ContainerState: with self._lock: @@ -179,6 +183,14 @@ def get(self, name: str) -> ContainerState: self._states[name] = ContainerState(name) return self._states[name] + def get_ha_node(self) -> str: + with self._lock: + return self._ha_state.get("active_node", "") + + def set_ha_node(self, node: str) -> None: + with self._lock: + self._ha_state = {"active_node": node} + def cleanup_stale(self, active_names: set, retain_open: bool = False) -> None: with self._lock: stale = [ @@ -213,6 +225,10 @@ def load(self) -> tuple[dict, int]: log.warning("State file %s has version %s (expected %d) — ignoring", self._state_file, data.get("version"), _STATE_VERSION) return {}, 0 + ha = data.get("ha_monitor") + if isinstance(ha, dict) and isinstance(ha.get("active_node"), str): + with self._lock: + self._ha_state = {"active_node": ha["active_node"][:255]} return data.get("containers") or {}, int(data.get("boot_time") or 0) except Exception as exc: # A corrupt state file must never stop monitoring. @@ -228,6 +244,7 @@ def save(self, boot_time: int) -> None: with self._save_lock: with self._lock: states = list(self._states.values()) + ha_state = dict(self._ha_state) containers: dict[str, dict] = {} for st in states: with st.lock: @@ -242,6 +259,8 @@ def save(self, boot_time: int) -> None: "saved_at": time.time(), "containers": containers, } + if ha_state: + record["ha_monitor"] = ha_state tmp = f"{self._state_file}.tmp" try: parent = os.path.dirname(self._state_file) @@ -283,6 +302,7 @@ def _default_probe_type(failure_type: str) -> str: "oom": "Docker event (OOM)", "unhealthy": "Docker HEALTHCHECK", "restart-loop": "Restart-loop tracking", + "ha-failover": "Pacemaker HA exporter", }.get(failure_type, "") @@ -299,7 +319,11 @@ def __init__( recommended_action: str, runbook_url: str, probe_detail: str = "", + ha_previous_node: str = "", + ha_new_node: str = "", ): + self.ha_previous_node = ha_previous_node + self.ha_new_node = ha_new_node self.severity = severity # CRITICAL / HIGH / WARNING self.container_name = container_name self.container_id = container_id[:12] if container_id else "N/A" @@ -314,6 +338,9 @@ def __init__( self.timestamp = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") def subject(self) -> str: + if self.failure_type == "ha-failover": + return (f"[{self.severity}] CyberController HA active node changed: " + f"{self.ha_previous_node} -> {self.ha_new_node} on {self.host}") probe = f" ({self.probe_type})" if self.probe_type else "" return ( f"[{self.severity}] Container {self.failure_type.upper()}{probe}: " @@ -359,11 +386,21 @@ def send_slack(payload: AlertPayload, cfg: dict) -> None: if payload.probe_detail else [] ) + ha_event = payload.failure_type == "ha-failover" + ha_fields = [ + {"title": "Event", "value": "HA active-node change", "short": True}, + {"title": "Previous active", "value": payload.ha_previous_node, "short": True}, + {"title": "New active", "value": payload.ha_new_node, "short": True}, + {"title": "Host", "value": payload.host, "short": True}, + {"title": "Time (UTC)", "value": payload.timestamp, "short": True}, + *probe_field, + {"title": "Action", "value": payload.recommended_action, "short": False}, + ] body = { "attachments": [{ "color": color, "title": payload.subject(), - "fields": [ + "fields": ha_fields if ha_event else [ {"title": "Container", "value": payload.container_name, "short": True}, {"title": "Host", "value": payload.host, "short": True}, {"title": "Failure", "value": payload.failure_type, "short": True}, @@ -375,7 +412,8 @@ def send_slack(payload: AlertPayload, cfg: dict) -> None: {"title": "Action", "value": payload.recommended_action, "short": False}, {"title": "Runbook", "value": payload.runbook_url, "short": False}, ], - "footer": f"Container ID: {payload.container_id}", + "footer": ("Source: HA cluster exporter" if ha_event + else f"Container ID: {payload.container_id}"), "ts": int(time.time()), }] } @@ -392,11 +430,15 @@ def dispatch_alert(payload: AlertPayload, cfg: dict) -> None: """Send alert to all configured channels.""" channels = cfg.get("alert_channels", ["slack"]) log_fn = log.info if payload.failure_type == "recovered" else log.warning - log_fn( - "ALERT [%s] %s — %s (channels: %s)", - payload.severity, payload.container_name, - payload.failure_type, channels, - ) + if payload.failure_type == "ha-failover": + log_fn("ALERT %s (channels: %s) — %s", + payload.subject(), channels, payload.probe_detail) + else: + log_fn( + "ALERT [%s] %s — %s (channels: %s)", + payload.severity, payload.container_name, + payload.failure_type, channels, + ) if "slack" in channels: send_slack(payload, cfg) if "smtp" in channels: send_smtp(payload, cfg) if "snmp_trap" in channels: send_snmp_trap(payload, cfg) @@ -457,8 +499,19 @@ def send_smtp(payload: AlertPayload, cfg: dict) -> None: return # ── Build message - subject = f"[{payload.severity}] {payload.container_name}: {payload.failure_type}" - body = f"""Container: {payload.container_name} + ha_event = payload.failure_type == "ha-failover" + subject = (payload.subject() if ha_event else + f"[{payload.severity}] {payload.container_name}: {payload.failure_type}") + if ha_event: + body = (f"CyberController HA active node changed\n" + f"Previous active: {payload.ha_previous_node}\n" + f"New active: {payload.ha_new_node}\n" + f"Host: {payload.host}\n" + f"Observed (UTC): {payload.timestamp}\n" + f"Evidence: {payload.probe_detail}\n\n" + f"Action: {payload.recommended_action}") + else: + body = f"""Container: {payload.container_name} Host: {payload.host} Status: {payload.failure_type} Severity: {payload.severity} @@ -855,10 +908,9 @@ def send_snmp_trap(payload: AlertPayload, cfg: dict) -> None: mp_model = 0 if version == "v1" else 1 # 0=SNMPv1, 1=SNMPv2c security_data = CommunityData(community, mpModel=mp_model) - summary = ( - f"[{payload.severity}] {payload.container_name} on {payload.host}: " - f"{payload.failure_type}" - ) + summary = (payload.subject() if payload.failure_type == "ha-failover" else + f"[{payload.severity}] {payload.container_name} on {payload.host}: " + f"{payload.failure_type}") # ── 6. Log the effective engine ID (needed for snmptrapd createUser -e) ─── if _eid_source == "config": @@ -1246,6 +1298,11 @@ def __init__(self, cfg: dict): self._manual_exec_lock = threading.Lock() self._manual_exec_inflight: set[str] = set() self._restore_state() + _ha = cfg.get("ha_monitor") + self._ha_cfg = _ha if isinstance(_ha, dict) else {} + self._ha_enabled = self._ha_cfg.get("enabled") is True + self._ha_tracker = HARoleTracker(self.state.get_ha_node()) + self._ha_error_log_at = float("-inf") # ── State restore ───────────────────────────────────────────────────────── def _restore_state(self) -> None: @@ -1492,6 +1549,49 @@ def _handle_event(self, event: dict) -> None: state.unhealthy_cycles = 0 self._maybe_alert_recovery(container, state) + def _poll_ha(self) -> None: + """Check HA independently of Docker health probes; do not alter Pacemaker.""" + if self._grace_until and time.time() < self._grace_until: + return # Never fold HA events into the host reboot-summary. + url = self._ha_cfg.get("exporter_url", "http://127.0.0.1:9664/metrics") + try: + observation = read_ha_observation(self._session, url) + except Exception as exc: + self._ha_tracker.reset_candidate() + if time.monotonic() - self._ha_error_log_at >= 300: + self._ha_error_log_at = time.monotonic() + log.warning("HA check unavailable (%s): %s", type(exc).__name__, exc) + return + + changed, change = self._ha_tracker.observe(observation) + if changed: + self.state.set_ha_node(self._ha_tracker.confirmed) + self.state.save(self._boot_time) + if change is None: + return + if not change.should_notify: + log.info("HA role changed %s -> %s; new owner will notify", + change.previous, change.current) + return + detail = (f"Previous active: {change.previous}; New active: {change.current}; " + "HAOperatorRep-Res is promoted/active on the new node " + "(Pacemaker HA exporter)") + payload = AlertPayload( + severity="WARNING", + container_name="(HA cluster)", + container_id="", + host=self.host, + failure_type="ha-failover", + exit_code=None, + log_tail="", + recommended_action="Verify whether the HA switchover was expected and check service health.", + runbook_url=self.runbook_base, + probe_detail=detail, + ha_previous_node=change.previous, + ha_new_node=change.current, + ) + self._dispatch(payload) + # ── Poll loop ───────────────────────────────────────────────────────────── def _poll_loop(self) -> None: interval = self.cfg.get("check_interval_seconds", 60) @@ -1501,6 +1601,11 @@ def _poll_loop(self) -> None: self._poll_all_containers() except Exception as exc: log.error("Poll error: %s", exc) + if self._ha_enabled: + try: + self._poll_ha() + except Exception as exc: + log.error("HA poll error: %s", exc) self._stop_event.wait(interval) def _poll_all_containers(self) -> None: From 6402641fe6acb977116ecc6580dc58203c20be73 Mon Sep 17 00:00:00 2001 From: rdwr-egore Date: Fri, 9 Oct 2026 21:51:59 +0000 Subject: [PATCH 3/5] test: document lab validation and pending HA acceptance gates --- docs/HA_ACTIVE_NODE_ALERT_DESIGN.md | 10 +++---- docs/HA_VALIDATION.md | 41 +++++++++++++++++++++++++++++ tests/test_ha_monitor.py | 18 +++++++++++++ 3 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 docs/HA_VALIDATION.md diff --git a/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md b/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md index fc9b546..399706c 100644 --- a/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md +++ b/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md @@ -1,9 +1,9 @@ # Optional CyberController HA Active-Node Alert — Design Proposal -**Status:** Proposed for review; design only, no implementation -**Date:** 2026-10-09 -**Target:** Radware/CyberController-Container-Alert, baseline v1.5.4 (main: a4ddfed) -**Delivery:** One small implementation PR after design approval +**Status:** Implementation approved and completed on feature branch; controlled two-node validation pending +**Date:** 2026-10-09 +**Target:** Radware/CyberController-Container-Alert, baseline v1.5.4 (main: a4ddfed) +**Delivery:** One small implementation PR, with release blocked on two-node QA ## 1. Goal and decision @@ -167,4 +167,4 @@ The implementation is ready for review/release when all automated tests and CI p **Out of scope:** HA orchestration/control, restarting or promoting resources, automatic remediation, logging every Pacemaker transition, live log parsing, new monitoring dashboards/exporters, historical HA event reconstruction, generalized multi-service health monitoring, and remediation of the native Prometheus 9002 vs 9664 mismatch. The latter should be handled as a separate CyberController native monitoring correction. -**Review request:** Approve (or revise) the exact signal (**HAOperatorRep-Res promoted/active**), two-poll confirmation, single notification from the newly active node, state persistence, and the requirement to deploy on both nodes. Implementation must **not** start until design approval. +**Implementation status:** Approved by the user on 2026-10-09 and implemented on the feature branch. Do not merge or release until the two-node acceptance gates in [HA_VALIDATION.md](HA_VALIDATION.md) are satisfied. diff --git a/docs/HA_VALIDATION.md b/docs/HA_VALIDATION.md new file mode 100644 index 0000000..294907b --- /dev/null +++ b/docs/HA_VALIDATION.md @@ -0,0 +1,41 @@ +# Optional HA active-node alert — validation status + +**Branch:** feature/optional-ha-failover-alert +**Status:** Implementation complete for review; controlled two-node failover and release DoD **pending** +**Date:** 2026-10-09 + +## Automated verification + +- **25 unit/regression tests pass** on the development workstation, covering HA metric parsing, state validation, transition confirmation, failback, newly-active-node-only sending, persistence/duplicate suppression, bad/unavailable metrics, disabled-by-default behavior, existing Docker monitoring independence, Slack/SMTP/syslog/SNMP dispatch semantics and previous watchdog regressions. +- **24 tests passed** in an isolated Docker container on the HA test appliance using the existing watchdog:1.5.4 runtime image with the new source mounted read-only (prior to adding the extra SMTP test). +- Python syntax and existing shell-script syntax checks pass. +- The local clean Docker build was **not completed** because Docker Hub returned HTTP 504 while fetching the python:3.11-slim base image. New Dockerfile includes both watchdog.py and ha_monitor.py; full clean image build remains for CI. + +## Actual appliance checks — read-only + +Active side (lab host 10.199.0.241): + +- Pacemaker sees two online nodes, 11 HA resource instances; HAOperatorRep-Res promoted on .241. +- Direct HA exporter port 9664 returns HTTP 200. The new parser identifies active=.241, local=.241; one read took approximately **78 ms** in the isolated test container. +- Calling Watchdog._poll_ha() against the **real** exporter twice establishes and saves state format v1 containing ha_monitor.active_node=10.199.0.241, without sending an initial alert. +- No Docker socket, Pacemaker control, configuration change, notification receiver, or running-watchdog deployment was used in the isolated tests. + +Standby side (10.199.0.242): + +- Node is online and its local HA exporter returns HTTP 200 from localhost:9664. +- No Container Watchdog deployment or watchdog:1.5.4 image was found there during read-only inspection. +- Native Prometheus HA scrape target 9002 vs HA exporter 9664 mismatch remains a separate appliance defect; this feature bypasses Prometheus and uses the existing local exporter directly. + +## Outstanding acceptance gates (not claimed complete) + +1. Deploy the approved watchdog build and enable ha_monitor **on both HA nodes**, with persistent local state and outbound alert destinations. +2. Verify both nodes learn the same initial active owner *before* the failover. +3. In an approved maintenance window, perform a **supported controlled switchover** and then failback; verify one notification comes only from the newly active node each time, with correct timestamps and old/new node details. +4. Confirm that losing the original active node does not prevent the surviving node from sending the notification; avoid artificial network partitions while STONITH fencing is disabled. +5. Verify at least syslog delivery end-to-end, performance, clean image build/CI, and disabled-option backward compatibility. + +**Why no forced failover yet:** Testing a cluster-wide role change before the surviving node has a watchdog would not validate the requested operator notification. Because STONITH is disabled, a null-route or network partition is additionally unsafe (split-brain risk). No cluster state or firewall settings were changed. + +## Rollback + +Because no live agent was replaced or configured, there is currently nothing to roll back on the HA appliance. When deploying for controlled QA, keep the previous watchdog image/configuration; disable ha_monitor (or roll back to v1.5.4) to revert the feature without touching the HA cluster. diff --git a/tests/test_ha_monitor.py b/tests/test_ha_monitor.py index c4ddc44..bed9099 100644 --- a/tests/test_ha_monitor.py +++ b/tests/test_ha_monitor.py @@ -288,6 +288,24 @@ def test_syslog_logging_includes_transition_and_dispatches_snmp(self): self.assertIn(f"{A} -> {B}", warn.call_args.args[1]) trap.assert_called_once() + def test_smtp_ha_notification_identifies_role_change(self): + from email.parser import BytesParser + from email.policy import default + alert = self.make_alert() + cfg = {"smtp": {"enabled": True, "auth": False, "tls": False, + "host": "mail.invalid", "port": 25, + "sender": "alerts@example.invalid", + "recipients": ["ops@example.invalid"]}} + with patch("smtplib.SMTP") as smtp: + watchdog.send_smtp(alert, cfg) + sent = smtp.return_value.__enter__.return_value.sendmail.call_args.args[2] + parsed = BytesParser(policy=default).parsebytes(sent) + self.assertIn("HA active node changed", parsed["Subject"]) + body = parsed.get_body(preferencelist=("plain",)).get_content() + self.assertIn(f"Previous active: {A}", body) + self.assertIn(f"New active: {B}", body) + self.assertNotIn("Container:", body) + def test_container_alert_subject_unchanged(self): alert = watchdog.AlertPayload( "HIGH", "config_postgres_1", "abc", "host", "unhealthy", From a73f90e2c24c3110a784f5888e3f4fe8b55ee663 Mon Sep 17 00:00:00 2001 From: rdwr-egore Date: Fri, 9 Oct 2026 21:53:13 +0000 Subject: [PATCH 4/5] docs: record successful clean-image CI verification --- docs/HA_VALIDATION.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/HA_VALIDATION.md b/docs/HA_VALIDATION.md index 294907b..e276060 100644 --- a/docs/HA_VALIDATION.md +++ b/docs/HA_VALIDATION.md @@ -9,7 +9,7 @@ - **25 unit/regression tests pass** on the development workstation, covering HA metric parsing, state validation, transition confirmation, failback, newly-active-node-only sending, persistence/duplicate suppression, bad/unavailable metrics, disabled-by-default behavior, existing Docker monitoring independence, Slack/SMTP/syslog/SNMP dispatch semantics and previous watchdog regressions. - **24 tests passed** in an isolated Docker container on the HA test appliance using the existing watchdog:1.5.4 runtime image with the new source mounted read-only (prior to adding the extra SMTP test). - Python syntax and existing shell-script syntax checks pass. -- The local clean Docker build was **not completed** because Docker Hub returned HTTP 504 while fetching the python:3.11-slim base image. New Dockerfile includes both watchdog.py and ha_monitor.py; full clean image build remains for CI. +- **GitHub Actions CI passed** on draft PR #4, including a clean image build, shell/Python syntax, regression tests and Compose checks. The earlier local build encountered an external Docker Hub HTTP 504; CI subsequently built the image successfully. The Dockerfile includes both watchdog.py and ha_monitor.py. ## Actual appliance checks — read-only @@ -32,7 +32,7 @@ Standby side (10.199.0.242): 2. Verify both nodes learn the same initial active owner *before* the failover. 3. In an approved maintenance window, perform a **supported controlled switchover** and then failback; verify one notification comes only from the newly active node each time, with correct timestamps and old/new node details. 4. Confirm that losing the original active node does not prevent the surviving node from sending the notification; avoid artificial network partitions while STONITH fencing is disabled. -5. Verify at least syslog delivery end-to-end, performance, clean image build/CI, and disabled-option backward compatibility. +5. Verify at least syslog delivery end-to-end, performance and disabled-option behavior on both real HA nodes (CI already passes). **Why no forced failover yet:** Testing a cluster-wide role change before the surviving node has a watchdog would not validate the requested operator notification. Because STONITH is disabled, a null-route or network partition is additionally unsafe (split-brain risk). No cluster state or firewall settings were changed. From 31d3f0347df93ddc4a38970bd3df399cd38b8abb Mon Sep 17 00:00:00 2001 From: rdwr-egore Date: Tue, 13 Oct 2026 22:26:47 +0000 Subject: [PATCH 5/5] fix: isolate HA polling and reset baseline when disabled --- DEPLOYMENT.md | 15 ++++- README.md | 18 ++++++ docs/HA_ACTIVE_NODE_ALERT_DESIGN.md | 19 +++--- docs/HA_VALIDATION.md | 56 +++++++++--------- tests/test_ha_monitor.py | 92 +++++++++++++++++++++++++---- watchdog.py | 44 ++++++++++---- 6 files changed, 183 insertions(+), 61 deletions(-) diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index c126922..c6b97ac 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -429,8 +429,15 @@ disabled by default. On **each HA host**: Confirm exactly one notification from the newly active node for each transition; check docker logs docker-container-watchdog on both hosts. 6. Disable by setting ha_monitor.enabled: false and restarting the watchdog. + This clears its saved HA baseline; re-enabling starts with a fresh, quiet + baseline (no notification for transitions that occurred while disabled). + An ordinary restart while HA remains enabled keeps the confirmed baseline. No HA service or Pacemaker configuration changes are necessary. +Delivery is best-effort. Test the actual alert receiver on **both** nodes: +an HA alert in watchdog logs does not prove a syslog or SMTP destination +received it. This release has no persisted notification retry queue. + Do not attempt a network-partition simulation on a cluster with STONITH disabled: split-brain and database inconsistency are possible. The native Prometheus port mismatch (9002 vs exporter 9664) is a separate appliance @@ -853,9 +860,15 @@ Configuration for the SNMP Traps channel is in [2.4.4 SNMP Traps](#244-snmp-trap | `1.3.6.1.4.1.89.110.1.1.0` | `cwHost` | Hostname of the alerting node | | `1.3.6.1.4.1.89.110.1.2.0` | `cwSummary` | Human-readable alert summary | | `1.3.6.1.4.1.89.110.1.3.0` | `cwContainerName` | Container name | -| `1.3.6.1.4.1.89.110.1.4.0` | `cwFailureType` | `crashed` / `oom` / `unhealthy` / `restart-loop` / `recovered` | +| `1.3.6.1.4.1.89.110.1.4.0` | `cwFailureType` | `crashed` / `oom` / `unhealthy` / `restart-loop` / `recovered` / `ha-failover` | | `1.3.6.1.4.1.89.110.1.5.0` | `cwProbeDetail` | Probe failure detail | +**HA trap mapping:** this release uses the existing Container Watchdog MIB. +For HA, cwContainerName is (HA cluster), cwFailureType is ha-failover, and the +summary/detail fields contain previous and new active nodes. NMS rules should +match cwFailureType instead of treating all traps as container failures. No +new trap OID is introduced. + **SNMPv3 notes:** - **Engine ID:** On the first run the watchdog logs an auto-stable engine ID derived from the hostname (look for `SNMP engine ID: 0x...` in the logs). Copy it into `v3_local_engine_id` and register it in `snmptrapd.conf` with `createUser -e 0x watchdog-user SHA "..." AES "..."`. This keeps the ID stable across container restarts. diff --git a/README.md b/README.md index 56a7220..a59b486 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,24 @@ on two consecutive successful polls (usually 60 seconds apart). Only the newly active node sends one **WARNING: CyberController HA active node changed**, using the already configured Syslog, SMTP, Slack, and/or SNMP channels. Failback is handled the same way; HA state persists in the normal state.json. +HA polling runs in its own lightweight thread with a separate HTTP session, so +slow or unavailable Docker API probes do not block HA detection. + +An ordinary watchdog restart with HA enabled retains its baseline. If HA +monitoring is **disabled and the watchdog restarted**, the old HA baseline is +cleared; re-enabling and restarting establishes a new baseline silently rather +than reporting an unobserved, potentially old role change. + +**Delivery is best-effort:** after confirming a change, the watchdog saves its +new baseline before dispatching. If SMTP/Slack/SNMP/syslog delivery fails, +this event is logged but is **not retried** by the HA monitor. Validate the +notification path on both HA nodes. A failed state-file save may result in a +duplicate alert after a restart. + +**SNMP compatibility:** HA events use the existing Container Watchdog MIB with +cwFailureType=`ha-failover`, cwContainerName=`(HA cluster)`, and old/new +node values in cwSummary/cwProbeDetail. NMS integrations should distinguish +HA events by cwFailureType, not by the container-name field. This check is **off by default**. It does not install an HA exporter, open a firewall port, execute Pacemaker commands, or modify cluster roles. Both diff --git a/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md b/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md index 399706c..8d3b261 100644 --- a/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md +++ b/docs/HA_ACTIVE_NODE_ALERT_DESIGN.md @@ -1,6 +1,6 @@ # Optional CyberController HA Active-Node Alert — Design Proposal -**Status:** Implementation approved and completed on feature branch; controlled two-node validation pending +**Status:** Feature under review; manual switchover verified, failback and alert delivery pending **Date:** 2026-10-09 **Target:** Radware/CyberController-Container-Alert, baseline v1.5.4 (main: a4ddfed) **Delivery:** One small implementation PR, with release blocked on two-node QA @@ -9,7 +9,7 @@ Customer request: alert when CyberController HA changes its active node, including a manual switchover, automatic failover, or failback. The alert should identify the old and new active nodes, not speculate about the cause. -**Decision:** Add one opt-in HA role-change check to the existing Container Watchdog. Read the existing HA exporter directly over host-local HTTP. Reuse the watchdog's polling loop, persistent state, and existing syslog/SMTP/SNMP/Slack delivery. Do not introduce another daemon, Docker container, metric exporter, firewall port, or Pacemaker control capability. +**Decision:** Add one opt-in HA role-change check to the existing Container Watchdog. Read the existing HA exporter directly over host-local HTTP. Reuse the watchdog's polling interval, persistent state, and existing syslog/SMTP/SNMP/Slack delivery. Do not introduce another daemon, Docker container, metric exporter, firewall port, or Pacemaker control capability. This is an **observed active-node change** alert, not a claim to identify why a failover happened or that all CyberController applications are healthy. @@ -31,7 +31,7 @@ On the two-node HA environment ha-test-admin-ea-iad1, Pacemaker reports nodes 10 Per CyberController node: Existing ha_cluster_exporter :9664/metrics (localhost only) | - | bounded HTTP GET in the existing 60s watchdog poll + | bounded HTTP GET from independent HA polling thread (60s) v Optional HA role check: identify local node + unique promoted HA Operator @@ -48,7 +48,7 @@ Both HA nodes must run the watchdog and have their own local exporter. No inter-node polling or new firewall opening is required. ~~~ -The check must execute independently of Docker container probes: a Docker API exception must not suppress the HA check, and a failed HA read must not break Docker monitoring. The small HA check runs within the existing process; no worker framework, queue, or additional process is needed. +The HA check runs in its own lightweight thread with a dedicated HTTP session. Slow/blocking Docker probes cannot delay HA polling, and failed HA reads cannot break Docker monitoring. This is a single additional thread within the existing process, not a new service, container, queue, or worker framework. ### Signal selection @@ -93,7 +93,8 @@ Retain a **small optional ha_monitor section in the existing state.json** (backw | Candidate active node changes A -> B | Confirm on next good poll (two consecutive observations) | | Confirmed A -> B | Update persistent baseline; **only watchdog on B** sends one WARNING | | Confirmed B -> A (failback) | Same behavior; notify once from A | -| Watchdog restarted after notification | Do not repeat previous transition | +| Watchdog restarted while HA stays enabled | Restore confirmed owner; do not repeat previous transition | +| HA intentionally disabled and watchdog restarted | Clear saved HA owner; re-enable establishes a silent new baseline | | Exporter timeout, 404/500, invalid/incomplete series, lost quorum, zero/multiple promoted owners | Do not change baseline or claim a completed failover; log a rate-limited local warning | | Watchdog newly installed after the transition | Baseline silently; historical detection not guaranteed | | Host reboot/grace window | Respect existing boot-grace behavior; do not mix HA alerts into the container reboot-summary. Reconfirm role after grace | @@ -112,14 +113,16 @@ Next step: Confirm expected switchover and application health. Treat this as an event, **not a persistent container failure**: do not send a spurious "container recovered" alert. Keep the normal container alert subject/fields unchanged. Make only the minimum HA-specific display adjustment needed so email/Slack/syslog/SNMP do not misleadingly label it as a crashed container. Preserve existing notification formats for container alerts. -**Delivery semantics:** one best-effort notification per confirmed observed transition, with state-based deduplication across restarts. Exactly-once delivery to external syslog/SNMP/SMTP cannot be guaranteed. Detection time is an observation timestamp, not necessarily the exact Pacemaker event time. +**Delivery semantics (accepted v1 limitation):** one best-effort notification per confirmed observed transition, with state-based deduplication across restarts. The confirmed owner is persisted before dispatch; a delivery failure is logged but is not automatically retried. External syslog/SNMP delivery acknowledgement and exactly-once semantics are not available; adding a persisted retry queue would require changing the existing dispatcher contract and is deferred. A failed state-file save can lead to a duplicate after restart. Verify alert routing on both nodes. Detection time is an observation timestamp, not necessarily the exact Pacemaker event time. + +**SNMP compatibility (accepted v1 mapping):** the existing Container Watchdog MIB is reused. For HA notifications, cwContainerName is `(HA cluster)`, cwFailureType is `ha-failover`, and cwSummary/cwProbeDetail contain the old/new active nodes. An NMS should distinguish the HA event by cwFailureType; no new trap identity or OIDs are introduced in this release. ## 5. Implementation scope — one development PR, after approval | Existing component | Small change | |---|---| | **watchdog.py / DEFAULT_CONFIG** | Optional ha_monitor block, disabled by default | -| **watchdog.py / Watchdog._poll_loop** | Invoke HA poll separately, with its own error handling, on the existing interval | +| **watchdog.py / Watchdog.start, _ha_poll_loop** | Start independent HA polling thread with its own HTTP session and stop event; Docker polling remains unchanged | | **HA reader/transition helper** | Bounded local HTTP read; parse only required metrics; validate; confirm active change; deduplicate | | **WatchdogState persistence** | Optional HA baseline and notified transition, without breaking existing v1 state files or container recovery | | **AlertPayload/dispatch_alert** | Reuse channels; ensure HA event is clearly labelled and not folded into container reboot/recovery logic | @@ -161,7 +164,7 @@ The implementation is ready for review/release when all automated tests and CI p ## 7. Release gates, risks, and intentional exclusions -**Before implementation/release:** confirm the standby .242 has its own exporter on localhost:9664 and watchdog running; verify both nodes can send syslog/other customer notifications. The remote connection refusal on .242 is **not proof** its local service is absent; it may be a firewall rule. No cross-node port exposure is required for the proposed design. +**Before release:** confirm both HA nodes can send notifications end-to-end. Both watchdogs and local exporters were confirmed running during the manual .242 -> .241 test. The standby's earlier *remote* exporter connection refusal reflected an inaccessible cross-node port, not a missing local service. No inter-node exporter access is required. **Known limitations:** If the surviving node has no running watchdog/exporter or has never recorded the original active node, this check cannot reconstruct the failover after the fact. Polling can miss a transition that changes twice between samples. Metrics cannot distinguish planned switchover vs unexpected failover, and promotion does not guarantee the entire application is healthy. diff --git a/docs/HA_VALIDATION.md b/docs/HA_VALIDATION.md index e276060..1682f8f 100644 --- a/docs/HA_VALIDATION.md +++ b/docs/HA_VALIDATION.md @@ -1,41 +1,39 @@ # Optional HA active-node alert — validation status **Branch:** feature/optional-ha-failover-alert -**Status:** Implementation complete for review; controlled two-node failover and release DoD **pending** -**Date:** 2026-10-09 +**Status:** Draft PR #4; tested in the two-node lab, not approved for merge +**Updated:** 2026-10-13 -## Automated verification +## Automated tests and build -- **25 unit/regression tests pass** on the development workstation, covering HA metric parsing, state validation, transition confirmation, failback, newly-active-node-only sending, persistence/duplicate suppression, bad/unavailable metrics, disabled-by-default behavior, existing Docker monitoring independence, Slack/SMTP/syslog/SNMP dispatch semantics and previous watchdog regressions. -- **24 tests passed** in an isolated Docker container on the HA test appliance using the existing watchdog:1.5.4 runtime image with the new source mounted read-only (prior to adding the extra SMTP test). -- Python syntax and existing shell-script syntax checks pass. -- **GitHub Actions CI passed** on draft PR #4, including a clean image build, shell/Python syntax, regression tests and Compose checks. The earlier local build encountered an external Docker Hub HTTP 504; CI subsequently built the image successfully. The Dockerfile includes both watchdog.py and ha_monitor.py. +- **26 tests pass** after the PR review fixes. Added regression coverage for an HA thread running while the Docker poll is blocked and raises, and disable -> role change -> re-enable with a quiet baseline. +- The new HA thread owns its HTTP session, uses the existing stop event and global polling interval, and does not change Docker health checks or require new dependencies. +- Earlier PR #4 CI passed, including the clean Docker build. **CI must pass again on the updated commit** before merging. +- The existing state.json v1 format and Docker container alert state are retained. -## Actual appliance checks — read-only +## Real two-node environment — user-initiated test (2026-10-10 UTC) -Active side (lab host 10.199.0.241): +- **Both nodes have HA exporters and watchdog builds running** during the user test, not just the original active node. These installations were done by the user, not by this code review. +- At about 02:53:46 UTC the user initiated a manual switchover on .242, which moved the promoted HA Operator resource to .241. +- .242 logged at **02:55:17**: "HA role changed 10.199.0.242 -> 10.199.0.241; new owner will notify." +- .241 logged at **02:55:37**: one WARNING, "CyberController HA active node changed: 10.199.0.242 -> 10.199.0.241". +- Both nodes stayed online and .241's promoted HA resources became active. The actual switch was detected correctly, without duplicate HA events observed. +- **Actual notification delivery still needs verification.** .241 had syslog configured to 127.0.0.1:514; receiving NMS confirmation was not established. .242 used SMTP, but smtp.radware.com did not resolve, so its container failure messages were not emailed. +- During the planned demotion, the HA Operator explicitly removed vDirect, TED and Assist Service on .242. The watchdog generated separate CRITICAL crash alerts for vDirect and Assist Service. Narrowly scoped suppression is **out of scope** for PR #4 and should have a separate code review. +- A **reverse failback (.241 -> .242)**, end-to-end receiver confirmation, and a safe surviving-node test after failure of the original active node have not been confirmed. -- Pacemaker sees two online nodes, 11 HA resource instances; HAOperatorRep-Res promoted on .241. -- Direct HA exporter port 9664 returns HTTP 200. The new parser identifies active=.241, local=.241; one read took approximately **78 ms** in the isolated test container. -- Calling Watchdog._poll_ha() against the **real** exporter twice establishes and saves state format v1 containing ha_monitor.active_node=10.199.0.241, without sending an initial alert. -- No Docker socket, Pacemaker control, configuration change, notification receiver, or running-watchdog deployment was used in the isolated tests. +## Release gates still open -Standby side (10.199.0.242): +1. CI passes for the current head, with new concurrency/re-enable tests. +2. Test a controlled **failback** from .241 to .242, verifying a single reverse alert. +3. Verify the configured receiving systems, not just local watchdog logs; resolve .242 SMTP DNS or configure a working syslog route. +4. Validate a supported active-node-loss scenario without deliberately partitioning a cluster where STONITH is disabled. +5. Confirm targeted CC-version compatibility and ordinary watchdog restart/disabled option behavior. -- Node is online and its local HA exporter returns HTTP 200 from localhost:9664. -- No Container Watchdog deployment or watchdog:1.5.4 image was found there during read-only inspection. -- Native Prometheus HA scrape target 9002 vs HA exporter 9664 mismatch remains a separate appliance defect; this feature bypasses Prometheus and uses the existing local exporter directly. +## Accepted v1 limitations -## Outstanding acceptance gates (not claimed complete) +- One-shot best-effort dispatch: transition state is persisted **before** notification. Failed SMTP/Slack/SNMP/syslog delivery does not automatically retry. A failed state write may cause a duplicate after restart. Do not add a durable delivery queue to this PR without changing the dispatch API. +- HA SNMP events use the existing Container Watchdog MIB: cwContainerName=(HA cluster), cwFailureType=ha-failover. NMS integrations should classify by failure type. A separate MIB identity is not included in v1. +- Planned HA service removals currently produce container failure alerts. Suppression is a separate, narrowly scoped enhancement. -1. Deploy the approved watchdog build and enable ha_monitor **on both HA nodes**, with persistent local state and outbound alert destinations. -2. Verify both nodes learn the same initial active owner *before* the failover. -3. In an approved maintenance window, perform a **supported controlled switchover** and then failback; verify one notification comes only from the newly active node each time, with correct timestamps and old/new node details. -4. Confirm that losing the original active node does not prevent the surviving node from sending the notification; avoid artificial network partitions while STONITH fencing is disabled. -5. Verify at least syslog delivery end-to-end, performance and disabled-option behavior on both real HA nodes (CI already passes). - -**Why no forced failover yet:** Testing a cluster-wide role change before the surviving node has a watchdog would not validate the requested operator notification. Because STONITH is disabled, a null-route or network partition is additionally unsafe (split-brain risk). No cluster state or firewall settings were changed. - -## Rollback - -Because no live agent was replaced or configured, there is currently nothing to roll back on the HA appliance. When deploying for controlled QA, keep the previous watchdog image/configuration; disable ha_monitor (or roll back to v1.5.4) to revert the feature without touching the HA cluster. +**No HA host, Docker service, Pacemaker, firewall, or monitoring configuration changes were made during this code-review update.** diff --git a/tests/test_ha_monitor.py b/tests/test_ha_monitor.py index bed9099..4e8cb5c 100644 --- a/tests/test_ha_monitor.py +++ b/tests/test_ha_monitor.py @@ -164,7 +164,7 @@ def test_real_watchdog_event_and_persistence(self): HAObservation(B, B), HAObservation(B, B), ]): for _ in range(4): - w._poll_ha() + w._poll_ha(w._session) w._dispatch.assert_called_once() alert = w._dispatch.call_args.args[0] self.assertEqual(alert.failure_type, "ha-failover") @@ -180,7 +180,7 @@ def test_real_watchdog_event_and_persistence(self): restarted = self.create_shell(path) with patch("watchdog.read_ha_observation", return_value=HAObservation(B, B)): - restarted._poll_ha() + restarted._poll_ha(restarted._session) restarted._dispatch.assert_not_called() def test_failed_exporter_does_not_overwrite_state_or_alert(self): @@ -193,7 +193,7 @@ def test_failed_exporter_does_not_overwrite_state_or_alert(self): side_effect=[HAObservation(B, B), TimeoutError("offline"), HAObservation(B, B), HAObservation(B, B)]): for _ in range(4): - w._poll_ha() + w._poll_ha(w._session) self.assertEqual(w.state.get_ha_node(), B) w._dispatch.assert_called_once() @@ -203,26 +203,92 @@ def test_grace_period_delays_detection_and_no_boot_summary_pollution(self): w._ha_tracker = HARoleTracker(A) w._grace_until = float("inf") with patch("watchdog.read_ha_observation") as read: - w._poll_ha() + w._poll_ha(w._session) read.assert_not_called() w._grace_until = 0 with patch("watchdog.read_ha_observation", side_effect=[HAObservation(B, B), HAObservation(B, B)]): - w._poll_ha() - w._poll_ha() + w._poll_ha(w._session) + w._poll_ha(w._session) w._dispatch.assert_called_once() - def test_docker_poll_failure_does_not_suppress_ha_poll(self): + def test_slow_broken_docker_poll_cannot_block_ha(self): w = object.__new__(watchdog.Watchdog) w.cfg = {"check_interval_seconds": 60} + w.host = "CC-HA-test" + w.state = Mock() + w._boot_time = 123 w._ha_enabled = True - w._stop_event = Mock() - w._stop_event.is_set.side_effect = [False, True] - w._poll_all_containers = Mock(side_effect=RuntimeError("docker outage")) - w._poll_ha = Mock() + w._stop_event = threading.Event() + w._session = Mock() # Existing Docker probe HTTP session. + w._event_listener = Mock() + docker_entered = threading.Event() + release_docker = threading.Event() + docker_timed_out = threading.Event() + ha_ran = threading.Event() + ha_sessions = [] + + def broken_docker(): + docker_entered.set() + if not release_docker.wait(timeout=3): + docker_timed_out.set() + raise RuntimeError("Docker API became unavailable") + + def ha_tick(session): + # This runs while the main thread is still inside Docker polling. + if docker_entered.wait(timeout=2): + ha_sessions.append(session) + ha_ran.set() + release_docker.set() + w._stop_event.set() + + w._poll_all_containers = broken_docker + w._poll_ha = ha_tick with patch("watchdog.log.error"): - w._poll_loop() - w._poll_ha.assert_called_once() + w.start() + self.assertTrue(docker_entered.is_set()) + self.assertTrue(ha_ran.is_set(), "HA must run before Docker returns") + self.assertFalse(docker_timed_out.is_set(), "HA must release blocked Docker") + self.assertEqual(len(ha_sessions), 1) + self.assertIsNot(ha_sessions[0], w._session) + self.assertTrue(w._stop_event.is_set()) + + def test_disable_then_reenable_uses_fresh_quiet_baseline(self): + with tempfile.TemporaryDirectory() as td: + path = f"{td}/state.json" + state = watchdog.WatchdogState(path) + state.set_ha_node(A) + state.save(123) + + settings = {"state_file": path, "boot_grace_seconds": 0} + with patch("watchdog.docker.from_env", return_value=Mock()), \ + patch("watchdog._host_boot_time", return_value=123): + # A real disabled watchdog startup erases its previous HA state. + disabled = watchdog.Watchdog({ + **settings, "ha_monitor": {"enabled": False} + }) + self.assertEqual(disabled.state.get_ha_node(), "") + self.assertNotIn("ha_monitor", json.loads(Path(path).read_text())) + + # While disabled, the role changes from A to B. + enabled_cfg = {**settings, "ha_monitor": {"enabled": True}} + enabled = watchdog.Watchdog(enabled_cfg) + enabled._dispatch = Mock() + self.assertEqual(enabled._ha_tracker.confirmed, "") + with patch("watchdog.read_ha_observation", + return_value=HAObservation(B, B)): + enabled._poll_ha(Mock()) + enabled._dispatch.assert_not_called() + self.assertEqual(enabled.state.get_ha_node(), B) + + # Ordinary enabled restarts preserve B and remain silent. + restarted = watchdog.Watchdog(enabled_cfg) + self.assertEqual(restarted._ha_tracker.confirmed, B) + restarted._dispatch = Mock() + with patch("watchdog.read_ha_observation", + return_value=HAObservation(B, B)): + restarted._poll_ha(Mock()) + restarted._dispatch.assert_not_called() def test_disabled_by_default(self): self.assertFalse(watchdog.DEFAULT_CONFIG["ha_monitor"]["enabled"]) diff --git a/watchdog.py b/watchdog.py index 72d319d..9df7782 100644 --- a/watchdog.py +++ b/watchdog.py @@ -191,6 +191,10 @@ def set_ha_node(self, node: str) -> None: with self._lock: self._ha_state = {"active_node": node} + def clear_ha_node(self) -> None: + with self._lock: + self._ha_state.clear() + def cleanup_stale(self, active_names: set, retain_open: bool = False) -> None: with self._lock: stale = [ @@ -973,7 +977,7 @@ def _suppress_invalid_state(loop, context): (f"{_ENTERPRISE_ARC}.1.2.0", OctetString(summary.encode("utf-8", errors="replace"))), # cwContainerName (.1.3.0) — STRING — container name (f"{_ENTERPRISE_ARC}.1.3.0", OctetString(payload.container_name.encode("utf-8", errors="replace"))), - # cwFailureType (.1.4.0) — STRING — crashed | oom | unhealthy | restart-loop + # cwFailureType (.1.4.0) — STRING — container failure or ha-failover (f"{_ENTERPRISE_ARC}.1.4.0", OctetString(payload.failure_type.encode("utf-8", errors="replace"))), # cwProbeDetail (.1.5.0) — STRING — what probe failed and why (f"{_ENTERPRISE_ARC}.1.5.0", OctetString((payload.probe_detail or "").encode("utf-8", errors="replace"))), @@ -1301,6 +1305,11 @@ def __init__(self, cfg: dict): _ha = cfg.get("ha_monitor") self._ha_cfg = _ha if isinstance(_ha, dict) else {} self._ha_enabled = self._ha_cfg.get("enabled") is True + if not self._ha_enabled and self.state.get_ha_node(): + # An explicit disabled run breaks the observation history. On the + # next enable, establish a fresh quiet baseline, not a stale alert. + self.state.clear_ha_node() + self.state.save(self._boot_time) self._ha_tracker = HARoleTracker(self.state.get_ha_node()) self._ha_error_log_at = float("-inf") @@ -1549,13 +1558,13 @@ def _handle_event(self, event: dict) -> None: state.unhealthy_cycles = 0 self._maybe_alert_recovery(container, state) - def _poll_ha(self) -> None: - """Check HA independently of Docker health probes; do not alter Pacemaker.""" + def _poll_ha(self, session: requests.Session) -> None: + """Read HA state without Docker API calls; do not alter Pacemaker.""" if self._grace_until and time.time() < self._grace_until: return # Never fold HA events into the host reboot-summary. url = self._ha_cfg.get("exporter_url", "http://127.0.0.1:9664/metrics") try: - observation = read_ha_observation(self._session, url) + observation = read_ha_observation(session, url) except Exception as exc: self._ha_tracker.reset_candidate() if time.monotonic() - self._ha_error_log_at >= 300: @@ -1592,6 +1601,18 @@ def _poll_ha(self) -> None: ) self._dispatch(payload) + def _ha_poll_loop(self) -> None: + """Separate HA scheduling and HTTP session so Docker probes cannot delay it.""" + interval = self.cfg.get("check_interval_seconds", 60) + log.info("HA poll loop started — interval: %ds", interval) + with requests.Session() as session: + while not self._stop_event.is_set(): + try: + self._poll_ha(session) + except Exception as exc: + log.error("HA poll error: %s", exc) + self._stop_event.wait(interval) + # ── Poll loop ───────────────────────────────────────────────────────────── def _poll_loop(self) -> None: interval = self.cfg.get("check_interval_seconds", 60) @@ -1601,11 +1622,6 @@ def _poll_loop(self) -> None: self._poll_all_containers() except Exception as exc: log.error("Poll error: %s", exc) - if self._ha_enabled: - try: - self._poll_ha() - except Exception as exc: - log.error("HA poll error: %s", exc) self._stop_event.wait(interval) def _poll_all_containers(self) -> None: @@ -2061,13 +2077,21 @@ def start(self) -> None: target=self._event_listener, name="event-listener", daemon=True ) event_thread.start() - # Poll runs in the main thread + ha_thread = None + if self._ha_enabled: + ha_thread = threading.Thread( + target=self._ha_poll_loop, name="ha-poller", daemon=True + ) + ha_thread.start() + # Docker polling remains in the main thread, independent of HA. try: self._poll_loop() except KeyboardInterrupt: pass finally: self.stop() + if ha_thread: + ha_thread.join(timeout=5) def stop(self) -> None: log.info("Watchdog shutting down")