AMBARI-9538 - Some Alerts Have Stale Text And Do Not Recalculate Their State (jonathanhurley)

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java

@@ -101,8 +101,10 @@ public class AlertResourceProvider extends ReadOnlyResourceProvider {
     PROPERTY_IDS.add(ALERT_SCOPE);
 
     // keys
-    KEY_PROPERTY_IDS.put(Resource.Type.Cluster, ALERT_CLUSTER_NAME);
     KEY_PROPERTY_IDS.put(Resource.Type.Alert, ALERT_ID);
+    KEY_PROPERTY_IDS.put(Resource.Type.Cluster, ALERT_CLUSTER_NAME);
+    KEY_PROPERTY_IDS.put(Resource.Type.Service, ALERT_SERVICE);
+    KEY_PROPERTY_IDS.put(Resource.Type.Host, ALERT_HOST);
   }
 
   /**

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

@@ -21,6 +21,7 @@ import java.text.MessageFormat;
 
 import org.apache.ambari.server.EagerSingleton;
 import org.apache.ambari.server.events.AlertReceivedEvent;
+import org.apache.ambari.server.events.AlertStateChangeEvent;
 import org.apache.ambari.server.events.publishers.AlertEventPublisher;
 import org.apache.ambari.server.orm.dao.AlertSummaryDTO;
 import org.apache.ambari.server.orm.dao.AlertsDAO;
@@ -38,8 +39,12 @@ import com.google.inject.Singleton;
 
 /**
  * The {@link AlertAggregateListener} is used to listen for all incoming
- * {@link AlertReceivedEvent} instances and determine if there exists a
+ * {@link AlertStateChangeEvent} instances and determine if there exists a
  * {@link SourceType#AGGREGATE} alert that needs to run.
+ * <p/>
+ * This listener is only needed on state changes as aggregation of alerts is
+ * only performed against the state of an alert and not the values that
+ * contributed to that state.
  */
 @Singleton
 @EagerSingleton
@@ -70,7 +75,7 @@ public class AlertAggregateListener {
    * Consume an alert that was received.
    */
   @Subscribe
-  public void onAlertEvent(AlertReceivedEvent event) {
+  public void onAlertEvent(AlertStateChangeEvent event) {
     AlertDefinition aggregateDefinition = m_aggregateMapping.getAggregateDefinition(
         event.getClusterId(), event.getAlert().getName());
 
@@ -129,7 +134,6 @@ public class AlertAggregateListener {
 
     // make a new event and allow others to consume it
     AlertReceivedEvent aggEvent = new AlertReceivedEvent(event.getClusterId(), alert);
-
     m_publisher.publish(aggEvent);
   }
 }

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

@@ -179,6 +179,7 @@ public class AlertReceivedListener {
       current.setAlertHistory(history);
       current.setLatestTimestamp(Long.valueOf(alert.getTimestamp()));
       current.setOriginalTimestamp(Long.valueOf(alert.getTimestamp()));
+      current.setLatestText(alert.getText());
 
       current = m_alertsDao.merge(current);
 

ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java

@@ -207,8 +207,9 @@ public class AlertCurrentEntity {
   }
 
   /**
-   * Gets the associated {@link AlertHistoryEntity} entry for this current alert
-   * instance.
+   * Sets the associated {@link AlertHistoryEntity} entry for this current alert
+   * instance. This will update the internal fields of this current alert with
+   * those from the alertHistory.
    *
    * @param alertHistory
    *          the most recently received history entry (not {@code null}).
@@ -216,6 +217,7 @@ public class AlertCurrentEntity {
   public void setAlertHistory(AlertHistoryEntity alertHistory) {
     this.alertHistory = alertHistory;
     alertDefinition = alertHistory.getAlertDefinition();
+    latestText = alertHistory.getAlertText();
   }
 
   /**

ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java

@@ -299,6 +299,51 @@ public class AlertsDAOTest {
     assertEquals(0, currentAlerts.size());
   }
 
+  /**
+   * Tests that the {@link AlertCurrentEntity} fields are updated properly when
+   * a new {@link AlertHistoryEntity} is associated.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testAlertCurrentUpdatesViaHistory() throws Exception {
+    AlertDefinitionEntity hostDef = new AlertDefinitionEntity();
+    hostDef.setDefinitionName("Host Alert Definition ");
+    hostDef.setServiceName("YARN");
+    hostDef.setComponentName(null);
+    hostDef.setClusterId(m_cluster.getClusterId());
+    hostDef.setHash(UUID.randomUUID().toString());
+    hostDef.setScheduleInterval(Integer.valueOf(60));
+    hostDef.setScope(Scope.HOST);
+    hostDef.setSource("{\"type\" : \"SCRIPT\"}");
+    hostDef.setSourceType(SourceType.SCRIPT);
+    m_definitionDao.create(hostDef);
+
+    // history for the definition
+    AlertHistoryEntity history = new AlertHistoryEntity();
+    history.setServiceName(hostDef.getServiceName());
+    history.setClusterId(m_cluster.getClusterId());
+    history.setAlertDefinition(hostDef);
+    history.setAlertLabel(hostDef.getDefinitionName());
+    history.setAlertText(hostDef.getDefinitionName());
+    history.setAlertTimestamp(Long.valueOf(1L));
+    history.setHostName("h2");
+    history.setAlertState(AlertState.OK);
+
+    // current for the history
+    AlertCurrentEntity current = new AlertCurrentEntity();
+    current.setOriginalTimestamp(1L);
+    current.setLatestTimestamp(2L);
+    current.setAlertHistory(history);
+    m_dao.create(current);
+
+    assertEquals(history.getAlertText(), current.getLatestText());
+
+    history.setAlertText("foobar!");
+    current.setAlertHistory(history);
+    assertEquals(history.getAlertText(), current.getLatestText());
+  }
+
   /**
    *
    */

ambari-server/src/test/java/org/apache/ambari/server/state/cluster/AlertDataManagerTest.java

@@ -341,8 +341,8 @@ public class AlertDataManagerTest {
     aggDef.setScope(Scope.SERVICE);
 
     AggregateSource source = new AggregateSource();
-
     source.setAlertName("to_aggregate");
+
     // !!! type is protected
     Field field = Source.class.getDeclaredField("type");
     field.setAccessible(true);
@@ -425,8 +425,6 @@ public class AlertDataManagerTest {
 
     assertNotNull(aggregatedDefinition);
 
-    // any alert and event will do that is for the definition since an aggregate
-    // checks them all regardless of state
     Alert alert = new Alert(
         definition.getDefinitionName(),
         null,
@@ -434,8 +432,12 @@ public class AlertDataManagerTest {
         definition.getComponentName(),
         "h1",
         AlertState.OK);
-    AlertReceivedEvent event = new AlertReceivedEvent(m_cluster.getClusterId(),
-        alert);
+
+    AlertCurrentEntity current = m_dao.findCurrentByHostAndName(
+        m_cluster.getClusterId(), "h1", definition.getDefinitionName());
+
+    AlertStateChangeEvent event = new AlertStateChangeEvent(
+        m_cluster.getClusterId(), alert, current, AlertState.OK);
 
     listener.onAlertEvent(event);
     assertNotNull(ref.get());
@@ -443,8 +445,6 @@ public class AlertDataManagerTest {
     assertTrue(ref.get().getText().indexOf("0/4") > -1);
 
     // check if one is critical, still ok
-    AlertCurrentEntity current = m_dao.findCurrentByHostAndName(
-        m_cluster.getClusterId(), "h1", definition.getDefinitionName());
     current.getAlertHistory().setAlertState(AlertState.CRITICAL);
     m_dao.merge(current.getAlertHistory());
 
@@ -454,9 +454,9 @@ public class AlertDataManagerTest {
     assertTrue(ref.get().getText().indexOf("1/4") > -1);
 
     // two are either warning or critical, warning
-    current = m_dao.findCurrentByHostAndName(
-m_cluster.getClusterId(), "h2",
+    current = m_dao.findCurrentByHostAndName(m_cluster.getClusterId(), "h2",
         definition.getDefinitionName());
+
     current.getAlertHistory().setAlertState(AlertState.WARNING);
     m_dao.merge(current.getAlertHistory());
 
@@ -466,9 +466,9 @@ m_cluster.getClusterId(), "h2",
     assertTrue(ref.get().getText().indexOf("2/4") > -1);
 
     // three make it critical
-    current = m_dao.findCurrentByHostAndName(
-m_cluster.getClusterId(), "h3",
+    current = m_dao.findCurrentByHostAndName(m_cluster.getClusterId(), "h3",
         definition.getDefinitionName());
+
     current.getAlertHistory().setAlertState(AlertState.CRITICAL);
     m_dao.merge(current.getAlertHistory());