diff --git a/sonic-thermalctld/scripts/thermalctld b/sonic-thermalctld/scripts/thermalctld index 6298f2c..1047bd2 100644 --- a/sonic-thermalctld/scripts/thermalctld +++ b/sonic-thermalctld/scripts/thermalctld @@ -679,7 +679,9 @@ class LiquidCoolingUpdater(threading.Thread, logger.Logger): leak_start_time = self.leaking_sensors[sensor_name] profile = sensor.get_leak_profile() max_minor_secs = profile.get_leak_max_minor_duration_sec() if profile else inf - if max_minor_secs < inf: + # A value of 0 (or negative) means the platform does not support + # time-based escalation from MINOR to CRITICAL; skip escalation. + if 0 < max_minor_secs < inf: max_minor = leak_start_time + timedelta(seconds=max_minor_secs) if max_minor <= now: sensor_leak_severity = LeakSeverity.CRITICAL @@ -728,6 +730,9 @@ class LiquidCoolingUpdater(threading.Thread, logger.Logger): leaking = 'N/A' if not sensor_is_ok else 'Yes' if sensor_is_leak else 'No' sensor_type = try_get(sensor.get_leak_sensor_type, 'unknown') sensor_location = try_get(sensor.get_leak_sensor_location, 'unknown') + # Add the timestamp to the leak sensor entry to track when leak occured + # But don't include in fvs_tuple which is used for comparing values + # Hence create a new one fvs_with_ts for updating sesor_table fvs_tuple = ( ('name', sensor_name), ('leaking', leaking), @@ -737,10 +742,13 @@ class LiquidCoolingUpdater(threading.Thread, logger.Logger): ('leak_sensor_status', 'Good' if sensor_is_ok else 'Fault'), ('type', str(sensor_type)), ('location', str(sensor_location)), - ('leak_severity', str(sensor_leak_severity)), + ('leak_severity', sensor_leak_severity.value if sensor_leak_severity is not None else 'None'), ) if self.last_sensor_fvs.get(sensor_name) != fvs_tuple: - self.sensor_table.set(sensor_name, swsscommon.FieldValuePairs(list(fvs_tuple))) + fvs_with_ts = list(fvs_tuple) + [ + ('timestamp', now.strftime('%Y%m%d %H:%M:%S')), + ] + self.sensor_table.set(sensor_name, swsscommon.FieldValuePairs(fvs_with_ts)) self.last_sensor_fvs[sensor_name] = fvs_tuple if status == LeakSeverity.CRITICAL and self.last_leak_status != LeakSeverity.CRITICAL: diff --git a/sonic-thermalctld/tests/test_thermalctld.py b/sonic-thermalctld/tests/test_thermalctld.py index 61f28ba..aee79cb 100644 --- a/sonic-thermalctld/tests/test_thermalctld.py +++ b/sonic-thermalctld/tests/test_thermalctld.py @@ -435,6 +435,9 @@ def test_refresh_leak_status_no_leak(self, mock_try_get): sensor_name, fvp = call[0] assert sensor_name in ["leakage1", "leakage2"] assert fvp.fv_dict['leaking'] == 'No' + # timestamp must be present on the initial create write + assert 'timestamp' in fvp.fv_dict + assert fvp.fv_dict['timestamp'] # non-empty calls_sys = liquid_cooling_updater.system_table.set.call_args_list for call in calls_sys: @@ -555,6 +558,44 @@ def test_refresh_status_with_multiple_leaks(self, mock_try_get): elif index == 1: assert fvp.fv_dict['device_leak_status'] == 'CRITICAL' + @mock.patch('thermalctld.try_get') + def test_refresh_status_minor_no_escalation_when_duration_zero(self, mock_try_get): + """A get_leak_max_minor_duration_sec() value of 0 means the platform does + not support time-based escalation from MINOR to CRITICAL. The severity + must stay MINOR even after the leak has persisted across polls.""" + mock_chassis = MockChassis() + mock_chassis.get_liquid_cooling().make_sensor_leak(0) + + liquid_cooling_updater = thermalctld.LiquidCoolingUpdater(mock_chassis, 0.5) + + sensor = mock_chassis.get_liquid_cooling().leakage_sensors[0] + sensor.get_leak_severity = mock.MagicMock(return_value=LeakSeverity.MINOR) + mock_profile = mock.MagicMock() + mock_profile.get_leak_max_minor_duration_sec = mock.MagicMock(return_value=0) + mock_profile.get_type = mock.MagicMock(return_value='mock_sensor') + sensor.get_leak_profile = mock.MagicMock(return_value=mock_profile) + + liquid_cooling_updater.log_error = mock.MagicMock() + liquid_cooling_updater.log_notice = mock.MagicMock() + liquid_cooling_updater.sensor_table = mock.MagicMock() + liquid_cooling_updater.system_table = mock.MagicMock() + liquid_cooling_updater.event_logger = mock.MagicMock() + liquid_cooling_updater.leaking_sensors = {} + + mock_try_get.side_effect = lambda func, default: func() + + liquid_cooling_updater._refresh_leak_status() + # Poll again after a short delay; even after wall-clock advances, a value + # of 0 must NOT be treated as "immediately escalate". + time.sleep(1.1) + liquid_cooling_updater._refresh_leak_status() + + assert liquid_cooling_updater.last_leak_status == LeakSeverity.MINOR + assert "leakage1" not in liquid_cooling_updater.critical_sensors + # No "escalated from MINOR to CRITICAL" error should have been logged. + for call in liquid_cooling_updater.event_logger.log_error.call_args_list: + assert 'escalated from MINOR to CRITICAL' not in call[0][0] + @mock.patch('thermalctld.try_get') def test_refresh_status_with_long_leak(self, mock_try_get): """Test _refresh_leak_status when one sensor leaks for an extended period""" @@ -650,6 +691,80 @@ def test_refresh_leak_status_platform_severity_bump(self, mock_try_get): assert 'CRITICAL leak reported' not in call[0][0] assert 'escalated from MINOR to CRITICAL' not in call[0][0] + @mock.patch('thermalctld.try_get') + def test_refresh_leak_status_timestamp_on_transitions_only(self, mock_try_get): + """Timestamp is written on initial create and on real state changes, + but not rewritten on unchanged polls (row is skipped entirely).""" + mock_chassis = MockChassis() + mock_chassis.get_liquid_cooling().make_sensor_leak(0) + liquid_cooling_updater = thermalctld.LiquidCoolingUpdater(mock_chassis, 0.5) + + sensor = mock_chassis.get_liquid_cooling().leakage_sensors[0] + sensor.get_leak_severity = mock.MagicMock(return_value=LeakSeverity.MINOR) + # Long max-minor so time-based escalation cannot fire + sensor.get_leak_profile().get_leak_max_minor_duration_sec = mock.MagicMock(return_value=86400) + + liquid_cooling_updater.event_logger = mock.MagicMock() + liquid_cooling_updater.sensor_table = mock.MagicMock() + liquid_cooling_updater.system_table = mock.MagicMock() + liquid_cooling_updater.leaking_sensors = {} + liquid_cooling_updater.critical_sensors = set() + liquid_cooling_updater.last_sensor_fvs = {} + + mock_try_get.side_effect = lambda func, default: func() + + # Cycle 1: initial create — both sensors written, timestamp present + liquid_cooling_updater._refresh_leak_status() + assert liquid_cooling_updater.sensor_table.set.call_count == 2 + first_calls = {c[0][0]: dict(c[0][1].fv_dict) for c in + liquid_cooling_updater.sensor_table.set.call_args_list} + assert 'timestamp' in first_calls['leakage1'] + ts1_leak1 = first_calls['leakage1']['timestamp'] + assert first_calls['leakage1']['leak_severity'] == LeakSeverity.MINOR.value + + # Cycle 2: nothing changed — no write at all (diff-gate skips) + liquid_cooling_updater.sensor_table.reset_mock() + liquid_cooling_updater._refresh_leak_status() + assert liquid_cooling_updater.sensor_table.set.call_count == 0, \ + "unchanged poll must not rewrite the sensor row" + + # Cycle 3: platform escalates severity — write triggers with fresh timestamp + liquid_cooling_updater.sensor_table.reset_mock() + sensor.get_leak_severity = mock.MagicMock(return_value=LeakSeverity.CRITICAL) + # Ensure a distinct timestamp value (strftime resolution = 1s) + import time as _time + _time.sleep(1.1) + liquid_cooling_updater._refresh_leak_status() + # Only the changed sensor (leakage1) is rewritten; leakage2 unchanged + assert liquid_cooling_updater.sensor_table.set.call_count == 1 + name, fvp = liquid_cooling_updater.sensor_table.set.call_args_list[0][0] + assert name == 'leakage1' + assert fvp.fv_dict['leak_severity'] == LeakSeverity.CRITICAL.value + assert 'timestamp' in fvp.fv_dict + assert fvp.fv_dict['timestamp'] != ts1_leak1, \ + "escalation must record a fresh timestamp" + + @mock.patch('thermalctld.try_get') + def test_refresh_leak_status_timestamp_not_in_diff_cache(self, mock_try_get): + """last_sensor_fvs cache must exclude 'timestamp' so unchanged polls + aren't triggered by the ever-advancing clock.""" + mock_chassis = MockChassis() + liquid_cooling_updater = thermalctld.LiquidCoolingUpdater(mock_chassis, 0.5) + liquid_cooling_updater.event_logger = mock.MagicMock() + liquid_cooling_updater.sensor_table = mock.MagicMock() + liquid_cooling_updater.system_table = mock.MagicMock() + liquid_cooling_updater.leaking_sensors = {} + liquid_cooling_updater.critical_sensors = set() + liquid_cooling_updater.last_sensor_fvs = {} + + mock_try_get.side_effect = lambda func, default: func() + + liquid_cooling_updater._refresh_leak_status() + for sensor_name, cached_tuple in liquid_cooling_updater.last_sensor_fvs.items(): + fields = dict(cached_tuple) + assert 'timestamp' not in fields, \ + "timestamp must not be in the diff cache for {}".format(sensor_name) + @mock.patch('thermalctld.try_get') def test_refresh_leak_status_platform_severity_downgrade(self, mock_try_get): """Platform downgrades CRITICAL -> MINOR while still leaking.