Merge pull request #318 from Uninett/feature/clearflap-api

Add proper implementation of `CLEARFLAP` API command

README.md

@@ -13,7 +13,6 @@ to be ported:
 - Not all SNMP traps are logged in the same detail as in Zino 1.
 - No support for reading trap messages from a trap multiplexer like
   `straps`/`nmtrapd`.
-- The `CLEARFLAP` API command is not yet fully implemented.
 
 Development of Zino 2.0 is fully sponsored by [NORDUnet](https://nordu.net/),
 on behalf of the nordic NRENs.

changelog.d/+113.added.md

@@ -0,0 +1 @@
+Added full implementation of the `CLEARFLAP` API command

src/zino/api/legacy.py

@@ -451,19 +451,21 @@ async def do_pollintf(self, router_name: str, ifindex: Union[str, int]):
 
     @requires_authentication
     async def do_clearflap(self, router_name: str, ifindex: Union[str, int]):
-        """Implements a dummy CLEARFLAP command (for now)"""
+        """Clears the flapping state of an interface and its corresponding portstate event"""
         from zino.state import polldevs
 
         try:
-            _device = polldevs[router_name]
+            poll_device = polldevs[router_name]
         except KeyError:
             return self._respond_error(f"Router {router_name} unknown")
         try:
             ifindex = abs(int(ifindex))
         except ValueError:
             return self._respond_error(f"{ifindex} is an invalid ifindex value")
 
-        return self._respond_ok("not implemented")
+        self._state.flapping.clear_flap((router_name, ifindex), self.user, self._state, poll_device)
+
+        return self._respond_ok()
 
     def _translate_pm_id_to_pm(responder: callable):  # noqa
         """Decorates any command that works with planned maintenance adding verification of the

src/zino/flaps.py

@@ -6,7 +6,7 @@
 from __future__ import annotations
 
 import logging
-from datetime import datetime
+from datetime import datetime, timedelta
 from typing import TYPE_CHECKING, List, Optional, Tuple
 
 from pydantic import BaseModel, BeforeValidator, Field, PlainSerializer
@@ -22,6 +22,7 @@
 
 _logger = logging.getLogger(__name__)
 
+IMMEDIATELY = timedelta(seconds=0)
 # Constants from Zino 1
 FLAP_THRESHOLD = 35
 FLAP_CEILING = 256
@@ -151,6 +152,46 @@ def get_flap_value(self, interface: PortIndex) -> float:
             return 0
         return self.interfaces[interface].hist_val
 
+    def clear_flap(self, interface: PortIndex, user: str, state: ZinoState, polldev: PollDevice) -> None:
+        """Clears the internal flapping state for a port, if it exists, and also updates an existing portstate event"""
+        self._clear_flap_internal(interface, user, "Flapstate manually cleared", state=state)
+
+        router, ifindex = interface
+        try:
+            port = state.devices[router].ports[ifindex]
+        except KeyError:
+            return
+        port.state = InterfaceState.FLAPPING  # to get logged state change
+        from zino.tasks.linkstatetask import (
+            LinkStateTask,  # local import to avoid import cycles
+        )
+
+        poller = LinkStateTask(device=polldev, state=state)
+        poller.schedule_verification_of_single_port(ifindex, deadline=IMMEDIATELY, reason="clearflap")
+
+    def _clear_flap_internal(self, interface: PortIndex, user: str, reason: str, state: ZinoState) -> None:
+        """Clears the internal flapping state for a port, including its ongoing portstate event"""
+        router, ifindex = interface
+        event = state.events.get(router, ifindex, PortStateEvent)
+        if not event or event.flapstate != FlapState.FLAPPING:
+            return
+
+        try:
+            port = state.devices[router].ports[ifindex]
+        except KeyError:
+            return
+
+        event = state.events.checkout(event.id)
+        event.add_history(f"{user}\n{reason}")
+        event.flapstate = FlapState.STABLE
+        event.flaps = self.get_flap_count((router, ifindex))
+        msg = f'{router}: intf "{port.ifdescr}" ix {port.ifindex}({port.ifalias}) {reason}'
+        _logger.info(msg)
+        event.add_log(msg)
+        state.events.commit(event)
+
+        self.unflap(interface)
+
 
 async def age_flapping_states(state: ZinoState, polldevs: dict[str, PollDevice]):
     """Ages all flapping states in the given ZinoState.  Should be called every FLAP_DECREMENT_INTERVAL_SECONDS."""

tests/api/legacy_test.py

@@ -20,10 +20,15 @@
 from zino.statemodels import (
     BGPEvent,
     BGPOperState,
+    DeviceState,
     Event,
     EventState,
+    FlapState,
+    InterfaceState,
     MatchType,
     PmType,
+    Port,
+    PortStateEvent,
     ReachabilityEvent,
 )
 from zino.time import now
@@ -789,7 +794,8 @@ async def test_should_output_error_response_for_invalid_ifindex(self, authentica
 class TestZino1ServerProtocolClearflapCommand:
     @pytest.mark.asyncio
     @patch("zino.state.polldevs", dict())
-    async def test_it_should_respond_with_ok_but_not_implemented(self, authenticated_protocol):
+    async def test_it_should_set_event_flapstate_to_stable_and_respond_with_ok(self, authenticated_protocol):
+        # Arrange bigly
         from zino.state import polldevs
 
         router_name = "buick.lab.example.org"
@@ -802,10 +808,27 @@ async def test_it_should_respond_with_ok_but_not_implemented(self, authenticated
         )
         polldevs[device.name] = device
 
+        device_state = DeviceState(name=router_name)
+        port = Port(ifindex=1, ifdescr="eth0", ifalias="Test port", state=InterfaceState.FLAPPING)
+        device_state.ports[port.ifindex] = port
+
+        state = authenticated_protocol._state
+        state.devices.devices[router_name] = device_state
+
+        event = state.events.create_event(router_name, 1, PortStateEvent)
+        event.ifindex = 1
+        event.flapstate = FlapState.FLAPPING
+        state.events.commit(event)
+
+        # Act
         await authenticated_protocol.message_received(f"CLEARFLAP {router_name} 1")
 
+        # Assert
         output = authenticated_protocol.transport.data_buffer.getvalue()
-        assert "200 not implemented".encode() in output
+        assert "200 ".encode() in output
+        updated_event = state.events.get(router_name, 1, PortStateEvent)
+        assert updated_event
+        assert updated_event.flapstate == FlapState.STABLE
 
     @pytest.mark.asyncio
     @patch("zino.state.polldevs", dict())

tests/flaps_test.py

@@ -151,6 +151,107 @@ def test_when_flapping_stats_do_not_exist_get_flap_value_should_return_zero(self
         assert flapping_states.get_flap_value(1) == 0
 
 
+class TestFlappingStatesClearFlapInternal:
+    def test_when_event_with_flapstate_exists_it_should_reset_it_to_stable(
+        self, state_with_flapstats_and_portstate_event
+    ):
+        state = state_with_flapstats_and_portstate_event
+        port: Port = next(iter(state.devices.devices["localhost"].ports.values()))
+
+        state.flapping._clear_flap_internal(
+            ("localhost", port.ifindex), "nobody", "Flapstate manually cleared", state=state
+        )
+
+        updated_event = state.events.get("localhost", port.ifindex, PortStateEvent)
+        assert updated_event
+        assert updated_event.flapstate == FlapState.STABLE
+
+    def test_when_event_does_not_exist_it_should_do_nothing(
+        self,
+        state_with_flapstats,
+    ):
+        state = state_with_flapstats
+        port: Port = next(iter(state.devices.devices["localhost"].ports.values()))
+
+        state.flapping._clear_flap_internal(
+            ("localhost", port.ifindex), "nobody", "Flapstate manually cleared", state=state
+        )
+
+        assert not state.events.get("localhost", port.ifindex, PortStateEvent)
+
+    def test_when_event_but_not_port_exists_it_should_do_nothing(self, state_with_flapstats_and_portstate_event):
+        state = state_with_flapstats_and_portstate_event
+        port: Port = next(iter(state.devices.devices["localhost"].ports.values()))
+        # Remove the port from device state for this test
+        del state.devices.devices["localhost"].ports[port.ifindex]
+
+        state.flapping._clear_flap_internal(
+            ("localhost", port.ifindex), "nobody", "Flapstate manually cleared", state=state
+        )
+
+        event = state.events.get("localhost", port.ifindex, PortStateEvent)
+        assert event
+        assert event.flapstate == FlapState.FLAPPING  # still flapping!
+
+
+class TestFlappingStatesClearFlap:
+    def test_it_should_schedule_verification_of_single_port(
+        self,
+        state_with_flapstats_and_portstate_event,
+        polldevs_dict,
+        monkeypatch,
+    ):
+        state = state_with_flapstats_and_portstate_event
+        port: Port = next(iter(state.devices.devices["localhost"].ports.values()))
+        mock_schedule_verification_of_single_port = Mock()
+        monkeypatch.setattr(
+            LinkStateTask, "schedule_verification_of_single_port", mock_schedule_verification_of_single_port
+        )
+
+        state.flapping.clear_flap(("localhost", port.ifindex), "nobody", state, polldevs_dict["localhost"])
+
+        mock_schedule_verification_of_single_port.assert_called_once()
+        assert mock_schedule_verification_of_single_port.call_args[0][0] == port.ifindex
+
+    def test_when_port_does_not_exist_it_should_not_schedule_verification(
+        self,
+        state_with_flapstats_and_portstate_event,
+        polldevs_dict,
+        monkeypatch,
+    ):
+        state = state_with_flapstats_and_portstate_event
+        fake_ifindex = 999
+        mock_schedule_verification_of_single_port = Mock()
+        monkeypatch.setattr(
+            LinkStateTask, "schedule_verification_of_single_port", mock_schedule_verification_of_single_port
+        )
+
+        state.flapping.clear_flap(("localhost", fake_ifindex), "nobody", state, polldevs_dict["localhost"])
+
+        mock_schedule_verification_of_single_port.assert_not_called()
+
+
+@pytest.fixture
+def state_with_flapstats_and_portstate_event(state_with_flapstats) -> ZinoState:
+    port: Port = next(iter(state_with_flapstats.devices.devices["localhost"].ports.values()))
+    flapping_state = state_with_flapstats.flapping.interfaces[("localhost", port.ifindex)]
+    flapping_state.hist_val = FLAP_THRESHOLD
+
+    orig_event = state_with_flapstats.events.get_or_create_event("localhost", port.ifindex, PortStateEvent)
+    orig_event.flapstate = FlapState.FLAPPING
+    orig_event.flaps = 42
+    orig_event.port = port.ifdescr
+    orig_event.portstate = port.state
+    orig_event.router = "localhost"
+    orig_event.polladdr = "127.0.0.1"
+    orig_event.priority = 500
+    orig_event.ifindex = port.ifindex
+    orig_event.descr = port.ifalias
+
+    state_with_flapstats.events.commit(orig_event)
+    return state_with_flapstats
+
+
 class TestAgeSingleInterfaceFlappingState:
     @pytest.mark.asyncio
     async def test_it_should_decrease_hist_val(self, state_with_flapstats, polldevs_dict):
@@ -317,5 +418,5 @@ def mocked_out_poll_single_interface(monkeypatch):
     """Monkey patches LinkStateTask.poll_single_interface to do essentially nothing"""
     future = Future()
     future.set_result(None)
-    monkeypatch.setattr(LinkStateTask, "poll_single_interface", future)
+    monkeypatch.setattr(LinkStateTask, "poll_single_interface", Mock(return_value=future))
     yield monkeypatch