NMS-19979: clear data collection alarms after a restart or collectd reload

opennms-services/src/main/java/org/opennms/netmgt/collectd/CollectableService.java

@@ -156,7 +156,10 @@ protected CollectableService(OnmsIpInterface iface, IpInterfaceDao ifaceDao, Col
         m_persisterFactory = persisterFactory;
 
         m_nodeId = iface.getNode().getId().intValue();
-        m_status = CollectionStatus.SUCCEEDED;
+        // Start out with no opinion rather than assuming success, so the first successful collection is a
+        // transition and emits the event that clears an alarm left over from a previous scheduling
+        // generation. A restart and a collectd reload both discard and rebuild every CollectableService.
+        m_status = CollectionStatus.UNKNOWN;
 
         m_updates = new CollectorUpdates();
 

opennms-services/src/test/java/org/opennms/netmgt/collectd/CollectableServiceTest.java

@@ -61,6 +61,8 @@
 import org.opennms.netmgt.dao.api.IpInterfaceDao;
 import org.opennms.netmgt.dao.api.ResourceStorageDao;
 import org.opennms.netmgt.dao.mock.MockEventIpcManager;
+import org.opennms.netmgt.events.api.EventConstants;
+import org.opennms.netmgt.events.api.EventIpcManager;
 import org.opennms.netmgt.events.api.EventIpcManagerFactory;
 import org.opennms.netmgt.model.OnmsIpInterface;
 import org.opennms.netmgt.rrd.RrdRepository;
@@ -69,6 +71,7 @@
 import org.opennms.netmgt.scheduler.Scheduler;
 import org.opennms.netmgt.snmp.InetAddrUtils;
 import org.opennms.netmgt.threshd.api.ThresholdingService;
+import org.opennms.netmgt.xml.event.Event;
 import org.opennms.test.FileAnticipator;
 import org.springframework.transaction.PlatformTransactionManager;
 
@@ -245,6 +248,45 @@ public void thresholdingSessionIsCreatedWhenExplicitlyEnabled() throws Exception
         verify(thresholdingService, times(1)).createSession(anyInt(), any(), any(), any());
     }
 
+    /**
+     * A CollectableService starts with no known status, so the first successful collection is a transition
+     * and emits dataCollectionSucceeded. That is what clears a dataCollectionFailed alarm raised before a
+     * restart or a collectd reload, both of which rebuild every CollectableService. See NMS-19979.
+     */
+    @Test
+    public void sendsSucceededEventOnFirstSuccessfulCollection() throws CollectionInitializationException, CollectionException, IOException {
+        EventIpcManager eventIpcManager = mock(EventIpcManager.class);
+        EventIpcManagerFactory.setIpcManager(eventIpcManager);
+
+        createCollectableService();
+        when(spec.collect(any())).thenReturn(null);
+
+        service.run();
+
+        ArgumentCaptor<Event> eventCaptor = ArgumentCaptor.forClass(Event.class);
+        verify(eventIpcManager, times(1)).sendNow(eventCaptor.capture());
+        assertEquals(EventConstants.DATA_COLLECTION_SUCCEEDED_EVENT_UEI, eventCaptor.getValue().getUei());
+    }
+
+    /**
+     * Only the transition emits, so a service that keeps collecting successfully stays quiet after the
+     * first pass rather than sending an event per collection cycle.
+     */
+    @Test
+    public void sendsNoFurtherEventWhileCollectionKeepsSucceeding() throws CollectionInitializationException, CollectionException, IOException {
+        EventIpcManager eventIpcManager = mock(EventIpcManager.class);
+        EventIpcManagerFactory.setIpcManager(eventIpcManager);
+
+        createCollectableService();
+        when(spec.collect(any())).thenReturn(null);
+
+        service.run();
+        service.run();
+        service.run();
+
+        verify(eventIpcManager, times(1)).sendNow(any(Event.class));
+    }
+
     private void createCollectableService() throws CollectionInitializationException, IOException {
         // Disable thresholding
         Map<String, Object> paramsMap = new HashMap<>();

opennms-services/src/test/java/org/opennms/netmgt/collectd/CollectdIT.java

@@ -88,6 +88,7 @@
 import org.opennms.netmgt.scheduler.mock.MockScheduler;
 import org.opennms.netmgt.threshd.api.ThresholdingService;
 import org.opennms.netmgt.threshd.api.ThresholdingSession;
+import org.opennms.netmgt.xml.event.Event;
 import org.opennms.test.JUnitConfigurationEnvironment;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.io.ClassPathResource;
@@ -336,6 +337,9 @@ public void testOneMatchingSpec() throws Exception {
 
         verify(m_eventIpcManager, times(1)).addEventListener(eq(m_collectd), (Collection<String>)isA(Collection.class));
         verify(m_eventIpcManager, times(1)).removeEventListener(m_collectd);
+        // This test actually collects, and the first success transitions out of UNKNOWN and emits
+        // dataCollectionSucceeded. Account for it here so tearDown's verifyNoMoreInteractions passes.
+        verify(m_eventIpcManager, times(1)).sendNow(isA(Event.class));
     }
 
     /**

opennms-services/src/test/java/org/opennms/netmgt/collectd/ThresholdIT.java

@@ -204,6 +204,10 @@ public void canTriggerThreshold() throws Exception {
 
         EventAnticipator eventAnticipator = mockEventIpcManager.getEventAnticipator();
 
+        // Each scheduling generation starts out UNKNOWN, so its first successful collection emits this
+        // once. Anticipate before the service is scheduled below, or the collection can beat us to it.
+        anticipateDataCollectionSucceeded(eventAnticipator);
+
         // Let's send a nodeGainedService event
         EventBuilder bldr = new EventBuilder(EventConstants.NODE_GAINED_SERVICE_EVENT_UEI, "Test");
         bldr.setNodeid(1);
@@ -273,6 +277,9 @@ public void canTriggerThreshold() throws Exception {
 
         eventAnticipator.reset();
 
+        // The category change above rescheduled the service, so a fresh generation reports success again.
+        anticipateDataCollectionSucceeded(eventAnticipator);
+
         // Again, Assert 2 collections are performed and that Threshold is no longer triggered
         collector.resetLatch(2);
         if (!collector.getLatch().await(30, TimeUnit.SECONDS)) {
@@ -284,6 +291,14 @@ public void canTriggerThreshold() throws Exception {
         collectd.stop();
     }
 
+    private static void anticipateDataCollectionSucceeded(EventAnticipator eventAnticipator) {
+        EventBuilder bldr = new EventBuilder(EventConstants.DATA_COLLECTION_SUCCEEDED_EVENT_UEI, "OpenNMS.Collectd");
+        bldr.setNodeid(1);
+        bldr.setInterface(addr("192.168.1.1"));
+        bldr.setService("Mock");
+        eventAnticipator.anticipateEvent(bldr.getEvent());
+    }
+
     private void initThreshdFactories(String threshd, String thresholds) throws Exception {
         thresholdingDao.overrideConfig(getClass().getResourceAsStream(thresholds));
         threshdDao.overrideConfig(getClass().getResourceAsStream(threshd));