From d18e4674849f0ffc0d48d2aa1ae21136a8265888 Mon Sep 17 00:00:00 2001 From: vptechops Date: Sun, 6 Sep 2026 06:49:26 -0500 Subject: [PATCH] =?UTF-8?q?fix(plant):=20review-gate=20fixes=20=E2=80=94?= =?UTF-8?q?=20level-based=20UPS=20re-polls,=20test=20hardening=20[#790=20#?= =?UTF-8?q?344]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review verdict FAIL -> addressed: runtime-low + shutdown chain re-poll /5 min (edge-crossing could miss a page mid-outage); suppression stamp set after notify; plan body names all 7 hosts explicitly; test fixes (pipefail SIGPIPE, comment-only assertion). Reviewer P1 'tripp_lite slug' refuted with live entity registry evidence (entity_id derives from name; sensor exists, 8f48730 aligned). On-box ha core check passed with the changed files. Review thread: https://git.knownelement.com/KNEL/KNELBMS/pulls/3 --- automations.yaml | 21 ++++++++++++---- packages/ups_shutdown.yaml | 51 ++++++++++++++++++++++++++++---------- tests/test_sdlc.sh | 19 +++++++++++--- 3 files changed, 70 insertions(+), 21 deletions(-) diff --git a/automations.yaml b/automations.yaml index b11658c..5f54ce0 100644 --- a/automations.yaml +++ b/automations.yaml @@ -79,14 +79,25 @@ - sensor.pfv_ups_runtime_minutes - sensor.pfv_tripp_lite_runtime_minutes below: 10 + # 5-min level re-check: numeric_state alone fires only on the threshold + # CROSSING — a blip at that minute (or an HA restart mid-outage) would + # silence the page for the whole discharge. Level-based polling turns + # this into an escalation ladder while the outage persists. + - trigger: time_pattern + minutes: /5 conditions: - condition: template value_template: >- - {% set status_map = { - 'sensor.pfv_ups_runtime_minutes': 'sensor.tsys1_ups_status', - 'sensor.pfv_tripp_lite_runtime_minutes': 'sensor.tsys1_triplite_status'} %} - {{ states(status_map.get(trigger.entity_id, 'sensor.tsys1_ups_status')) - not in ['Online', 'OL', 'unknown', 'unavailable'] }} + {% set ns = namespace(hit=false) %} + {% for rt, st in [ + ('sensor.pfv_ups_runtime_minutes', 'sensor.tsys1_ups_status'), + ('sensor.pfv_tripp_lite_runtime_minutes', 'sensor.tsys1_triplite_status')] %} + {% if is_number(states(rt)) and (states(rt) | float) < 10 + and states(st) not in ['Online', 'OL', 'unknown', 'unavailable'] %} + {% set ns.hit = true %} + {% endif %} + {% endfor %} + {{ ns.hit }} actions: - action: notify.send_message target: diff --git a/packages/ups_shutdown.yaml b/packages/ups_shutdown.yaml index 29202e6..b4ca765 100644 --- a/packages/ups_shutdown.yaml +++ b/packages/ups_shutdown.yaml @@ -20,6 +20,10 @@ # ============================================================================ input_boolean: + # NOTE: `initial:` wins over restored state at EVERY HA start — a deploy + # mid-test silently reverts these to the safe defaults (armed OFF, + # dry-run ON). Safe direction, but re-arm after any restart during the + # supervised Sept 11-12 live test. pfv_ups_shutdown_armed: name: UPS shutdown chain ARMED description: Founder-level arm. ON + dry-run OFF = real shutdown dispatch. @@ -61,26 +65,39 @@ automation: - sensor.tsys1_ups_battery_charge - sensor.tsys1_triplite_battery_charge below: 50 + # 5-min level re-check — same edge-vs-level reasoning as the + # runtime-low alert: never let a threshold-crossing blip silence + # the plan page for an entire discharge. + - trigger: time_pattern + minutes: /5 conditions: - condition: template value_template: >- - {% set sm = { - 'sensor.pfv_ups_runtime_minutes': 'sensor.tsys1_ups_status', - 'sensor.pfv_tripp_lite_runtime_minutes': 'sensor.tsys1_triplite_status', - 'sensor.tsys1_ups_battery_charge': 'sensor.tsys1_ups_status', - 'sensor.tsys1_triplite_battery_charge': 'sensor.tsys1_triplite_status'} %} - {{ states(sm.get(trigger.entity_id, 'sensor.tsys1_ups_status')) - not in ['Online', 'OL', 'unknown', 'unavailable'] }} + {% set ns = namespace(hit=false) %} + {% for rt, st in [ + ('sensor.pfv_ups_runtime_minutes', 'sensor.tsys1_ups_status'), + ('sensor.pfv_tripp_lite_runtime_minutes', 'sensor.tsys1_triplite_status')] %} + {% if is_number(states(rt)) and (states(rt) | float) < 20 + and states(st) not in ['Online', 'OL', 'unknown', 'unavailable'] %} + {% set ns.hit = true %} + {% endif %} + {% endfor %} + {% for ch, st in [ + ('sensor.tsys1_ups_battery_charge', 'sensor.tsys1_ups_status'), + ('sensor.tsys1_triplite_battery_charge', 'sensor.tsys1_triplite_status')] %} + {% if is_number(states(ch)) and (states(ch) | float) < 50 + and states(st) not in ['Online', 'OL', 'unknown', 'unavailable'] %} + {% set ns.hit = true %} + {% endif %} + {% endfor %} + {{ ns.hit }} + # Status 'unavailable' suppresses (anti-noise); UPS COMMS LOST covers + # that failure mode separately after 5 min. - condition: template value_template: >- {{ states('input_datetime.pfv_ups_shutdown_last_page') in ['unknown', 'unavailable', 'none'] or as_timestamp(now()) - as_timestamp(states('input_datetime.pfv_ups_shutdown_last_page')) > 21600 }} actions: - - action: input_datetime.set_datetime - target: - entity_id: input_datetime.pfv_ups_shutdown_last_page - data: - datetime: "{{ now().strftime('%Y-%m-%d %H:%M:%S') }}" - variables: execute_mode: >- {{ is_state('input_boolean.pfv_ups_shutdown_armed', 'on') @@ -97,7 +114,8 @@ automation: (ca LAST — signing path for everything above) 5. pfv-bms VM 100 LAST of the VMs — the brain stays up until now 6. Hypervisors: iDRAC graceful off tsys6/tsys7 (PowerEdge); - tsys1/3/4/5/9 idle out on UPS exhaustion (OptiPlex/Precision) + tsys1, tsys3, tsys4, tsys5, tsys9 idle out on UPS exhaustion + (OptiPlex/Precision, no BMC) mode_note: >- {{ 'MODE: EXECUTE (armed, live dispatch)' if execute_mode else 'MODE: DRY RUN (plan only — nothing will be shut down)' }} @@ -122,3 +140,10 @@ automation: data: title: "{{ 'UPS SHUTDOWN EXECUTING' if execute_mode else 'UPS SHUTDOWN PLAN (dry run)' }}" message: "{{ plan }}" + # Suppression stamp set LAST — a failed notify must not suppress the + # next 5-min re-page cycle. + - action: input_datetime.set_datetime + target: + entity_id: input_datetime.pfv_ups_shutdown_last_page + data: + datetime: "{{ now().strftime('%Y-%m-%d %H:%M:%S') }}" diff --git a/tests/test_sdlc.sh b/tests/test_sdlc.sh index fe7aee5..0e5d40a 100644 --- a/tests/test_sdlc.sh +++ b/tests/test_sdlc.sh @@ -63,17 +63,30 @@ t "trend sensors fed from smoothed sources" \ t "runtime-low covers Tripp Lite" \ "sed -n '/id: pfv_plant_ups_runtime_low/,/^ mode:/p' automations.yaml | grep -q 'sensor.pfv_tripp_lite_runtime_minutes'" t "runtime-low status check maps per-UPS" \ - "sed -n '/id: pfv_plant_ups_runtime_low/,/^ mode:/p' automations.yaml | grep -q 'status_map'" + "sed -n '/id: pfv_plant_ups_runtime_low/,/^ mode:/p' automations.yaml | grep -q \"sensor.pfv_tripp_lite_runtime_minutes', 'sensor.tsys1_triplite_status'\"" echo "=== UPS shutdown chain (#790 — dry-run is the shipped mode) ===" t "shutdown chain package present, dry-run default ON" \ "grep -q 'id: pfv_ups_shutdown_plan' packages/ups_shutdown.yaml && grep -A4 'pfv_ups_shutdown_dry_run:' packages/ups_shutdown.yaml | grep -q 'initial: true'" t "armed switch default OFF" \ "grep -A4 'pfv_ups_shutdown_armed:' packages/ups_shutdown.yaml | grep -q 'initial: false'" -t "plan covers all seven hypervisors" \ - "for h in tsys1 tsys3 tsys4 tsys5 tsys6 tsys7 tsys9; do grep -q \$h packages/ups_shutdown.yaml || exit 1; done" +# Host coverage must hold in the PLAN BODY, not the header comment — +# scope the grep to the plan template variable. +PLAN_OK=1 +for h in tsys1 tsys3 tsys4 tsys5 tsys6 tsys7 tsys9; do + # full-read grep (not -q): the script runs with pipefail — -q's early + # exit SIGPIPEs sed and flips the pipeline status to 141 + sed -n '/plan: >-/,/mode_note:/p' packages/ups_shutdown.yaml | grep "$h" >/dev/null || PLAN_OK=0 +done +t "plan covers all seven hypervisors (plan body)" "[ \$PLAN_OK -eq 1 ]" t "pfv-bms VM 100 ordered last of the VMs (#455)" \ "grep -q 'pfv-bms VM 100 LAST' packages/ups_shutdown.yaml" +echo "=== level-based re-checks (review finding: edge-only triggers miss pages) ===" +t "runtime-low re-polls every 5 min" \ + "sed -n '/id: pfv_plant_ups_runtime_low/,/^ mode:/p' automations.yaml | grep -q 'minutes: /5'" +t "shutdown chain re-polls every 5 min" \ + "grep -A30 'id: pfv_ups_shutdown_plan' packages/ups_shutdown.yaml | grep -q 'minutes: /5'" + echo "=== SDLC SUITE: $FAIL failures ===" exit "$FAIL"