AMBARI-11559 - Consider Alert Definition Removals When Associated Services/Components Are Removed (jonathanhurley)

ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertServiceStateListener.java

@@ -18,6 +18,7 @@
 package org.apache.ambari.server.events.listeners.alerts;
 
 import java.text.MessageFormat;
+import java.util.List;
 import java.util.Set;
 
 import org.apache.ambari.server.AmbariException;
@@ -36,8 +37,8 @@ import org.apache.ambari.server.orm.entities.AlertDefinitionEntity;
 import org.apache.ambari.server.orm.entities.AlertGroupEntity;
 import org.apache.ambari.server.state.alert.AlertDefinition;
 import org.apache.ambari.server.state.alert.AlertDefinitionFactory;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.eventbus.AllowConcurrentEvents;
 import com.google.common.eventbus.Subscribe;
@@ -57,7 +58,7 @@ public class AlertServiceStateListener {
   /**
    * Logger.
    */
-  private static Log LOG = LogFactory.getLog(AlertServiceStateListener.class);
+  private static Logger LOG = LoggerFactory.getLogger(AlertServiceStateListener.class);
 
   /**
    * Services metainfo; injected lazily as a {@link Provider} since JPA is not
@@ -84,7 +85,7 @@ public class AlertServiceStateListener {
 
   /**
    * Used when a service is installed to insert {@link AlertDefinitionEntity}
-   * into the database.
+   * into the database or when a service is removed to delete the definition.
    */
   @Inject
   private AlertDefinitionDAO m_definitionDao;
@@ -116,7 +117,7 @@ public class AlertServiceStateListener {
   @Subscribe
   @AllowConcurrentEvents
   public void onAmbariEvent(ServiceInstalledEvent event) {
-    LOG.debug(event);
+    LOG.debug("{} received {}", AlertServiceStateListener.class, event);
 
     long clusterId = event.getClusterId();
     String stackName = event.getStackName();
@@ -129,7 +130,8 @@ public class AlertServiceStateListener {
     try {
       m_alertDispatchDao.createDefaultGroup(clusterId, serviceName);
     } catch (AmbariException ambariException) {
-      LOG.error(ambariException);
+      LOG.error("Unable to create a default alert group for {}", event.getServiceName(),
+          ambariException);
     }
 
     // populate alert definitions for the new service from the database, but
@@ -156,8 +158,7 @@ public class AlertServiceStateListener {
   }
 
   /**
-   * Removes any current alerts associated with the specified service and the
-   * service's default alert group.
+   * Removes all alert data associated with the removed serviced.
    *
    * @param event
    *          the published event being handled (not {@code null}).
@@ -165,17 +166,25 @@ public class AlertServiceStateListener {
   @Subscribe
   @AllowConcurrentEvents
   public void onAmbariEvent(ServiceRemovedEvent event) {
-    LOG.debug(event);
+    LOG.debug("{} received {}", AlertServiceStateListener.class, event);
 
-    // remove any current alerts
-    m_alertsDao.removeCurrentByService(event.getServiceName());
+    List<AlertDefinitionEntity> definitions = m_definitionDao.findByService(event.getClusterId(),
+        event.getServiceName());
 
-    // remove the default group for the service
-    AlertGroupEntity group = m_alertDispatchDao.findGroupByName(
-        event.getClusterId(), event.getServiceName());
+    for (AlertDefinitionEntity definition : definitions) {
+      try {
+        m_definitionDao.remove(definition);
 
-    if (null != group && group.isDefault()) {
-      m_alertDispatchDao.remove(group);
+        // remove the default group for the service
+        AlertGroupEntity group = m_alertDispatchDao.findGroupByName(event.getClusterId(),
+            event.getServiceName());
+
+        if (null != group && group.isDefault()) {
+          m_alertDispatchDao.remove(group);
+        }
+      } catch (Exception exception) {
+        LOG.error("Unable to remove alert definition {}", definition.getDefinitionName(), exception);
+      }
     }
   }
 }

ambari-server/src/test/java/org/apache/ambari/server/events/EventsTest.java

@@ -26,6 +26,8 @@ import junit.framework.Assert;
 import org.apache.ambari.server.orm.GuiceJpaInitializer;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
 import org.apache.ambari.server.orm.OrmTestHelper;
+import org.apache.ambari.server.orm.dao.AlertDefinitionDAO;
+import org.apache.ambari.server.orm.entities.AlertDefinitionEntity;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.Host;
@@ -67,6 +69,7 @@ public class EventsTest {
   private ServiceComponentHostFactory m_schFactory;
   private MockEventListener m_listener;
   private OrmTestHelper m_helper;
+  private AlertDefinitionDAO m_definitionDao;
 
   /**
    *
@@ -87,6 +90,7 @@ public class EventsTest {
     m_serviceFactory = m_injector.getInstance(ServiceFactory.class);
     m_componentFactory = m_injector.getInstance(ServiceComponentFactory.class);
     m_schFactory = m_injector.getInstance(ServiceComponentHostFactory.class);
+    m_definitionDao = m_injector.getInstance(AlertDefinitionDAO.class);
 
     m_clusterName = "foo";
     StackId stackId = new StackId("HDP", "2.0.6");
@@ -149,6 +153,42 @@ public class EventsTest {
     Assert.assertTrue(m_listener.isAmbariEventReceived(eventClass));
   }
 
+  /**
+   * Tests that {@link ServiceRemovedEvent}s are fired correctly and alerts are
+   * removed.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testServiceRemovedEventForAlerts() throws Exception {
+    Class<? extends AmbariEvent> eventClass = ServiceRemovedEvent.class;
+    Assert.assertFalse(m_listener.isAmbariEventReceived(eventClass));
+    installHdfsService();
+
+    // check that there are alert definitions
+    Assert.assertTrue(m_definitionDao.findAll(m_cluster.getClusterId()).size() > 0);
+
+    // get all definitions for HDFS
+    List<AlertDefinitionEntity> hdfsDefinitions = m_definitionDao.findByService(
+        m_cluster.getClusterId(), "HDFS");
+
+    // make sure there are at least 1
+    Assert.assertTrue(hdfsDefinitions.size() > 0);
+
+    AlertDefinitionEntity definition = hdfsDefinitions.get(0);
+
+    // delete HDFS
+    m_cluster.getService("HDFS").delete();
+
+    // verify the event was received
+    Assert.assertTrue(m_listener.isAmbariEventReceived(eventClass));
+
+    // verify that the definitions were removed
+    hdfsDefinitions = m_definitionDao.findByService(m_cluster.getClusterId(), "HDFS");
+
+    Assert.assertEquals(0, hdfsDefinitions.size());
+  }
+
   /**
    * Tests that {@link ServiceComponentUninstalledEvent}s are fired correctly.
    *