AMBARI-8035 - Alerts: Creating WEB Alert Definitions via REST API Drops URI (jonathanhurley)

ambari-agent/src/main/python/ambari_agent/alerts/metric_alert.py

@@ -39,8 +39,12 @@ class MetricAlert(BaseAlert):
       self.metric_info = JmxMetric(alert_source_meta['jmx'])
 
     # extract any lookup keys from the URI structure
-    self.uri_property_keys = self._lookup_uri_property_keys(alert_source_meta['uri'])
-      
+    self.uri_property_keys = None
+    if 'uri' in alert_source_meta:
+      uri = alert_source_meta['uri']
+      self.uri_property_keys = self._lookup_uri_property_keys(uri)
+
+
   def _collect(self):
     if self.metric_info is None:
       raise Exception("Could not determine result. Specific metric collector is not defined.")

ambari-agent/src/main/python/ambari_agent/alerts/web_alert.py

@@ -33,7 +33,10 @@ class WebAlert(BaseAlert):
     super(WebAlert, self).__init__(alert_meta, alert_source_meta)
     
     # extract any lookup keys from the URI structure
-    self.uri_property_keys = self._lookup_uri_property_keys(alert_source_meta['uri'])
+    self.uri_property_keys = None
+    if 'uri' in alert_source_meta:
+      uri = alert_source_meta['uri']
+      self.uri_property_keys = self._lookup_uri_property_keys(uri)
 
       
   def _collect(self):

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

@@ -19,6 +19,7 @@ package org.apache.ambari.server.controller.internal;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -54,6 +55,7 @@ import org.apache.ambari.server.state.alert.SourceType;
 import org.apache.commons.lang.StringUtils;
 
 import com.google.gson.Gson;
+import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
@@ -77,10 +79,6 @@ public class AlertDefinitionResourceProvider extends AbstractControllerResourceP
 
   protected static final String ALERT_DEF_SOURCE = "AlertDefinition/source";
   protected static final String ALERT_DEF_SOURCE_TYPE = "AlertDefinition/source/type";
-  protected static final String ALERT_DEF_SOURCE_REPORTING = "AlertDefinition/source/reporting";
-  protected static final String ALERT_DEF_SOURCE_REPORTING_OK = "AlertDefinition/source/reporting/ok";
-  protected static final String ALERT_DEF_SOURCE_REPORTING_WARNING = "AlertDefinition/source/reporting/warning";
-  protected static final String ALERT_DEF_SOURCE_REPORTING_CRITICAL = "AlertDefinition/source/reporting/critical";
 
   protected static final String ALERT_DEF_ACTION_RUN_NOW = "AlertDefinition/run_now";
 
@@ -429,67 +427,51 @@ public class AlertDefinitionResourceProvider extends AbstractControllerResourceP
       throw new IllegalArgumentException("Service name must be specified");
     }
 
+    // on creation, source type is required
     if (bCreate && null == sourceType) {
       throw new IllegalArgumentException(String.format(
           "Source type must be specified and one of %s",
           EnumSet.allOf(SourceType.class)));
     }
 
-    // !!! Alert structures contain nested objects; reconstruct a valid
-    // JSON from the flat, exploded properties so that a Source instance can
-    // be properly persisted
-    JsonObject source = new JsonObject();
-    JsonObject reporting = new JsonObject();
-    JsonObject reportingOk = new JsonObject();
-    JsonObject reportingWarning = new JsonObject();
-    JsonObject reportingCritical = new JsonObject();
+    // !!! The AlertDefinition "Source" field is a nested JSON object;
+    // build a JSON representation from the flat properties and then
+    // serialize that JSON object as a string
+    Map<String,JsonObject> jsonObjectMapping = new HashMap<String, JsonObject>();
 
+    // for every property in the request, if it's a source property, then
+    // add it to the JSON model
     for (Entry<String, Object> entry : requestMap.entrySet()) {
-      String propCat = PropertyHelper.getPropertyCategory(entry.getKey());
-      String propName = PropertyHelper.getPropertyName(entry.getKey());
+      String propertyKey = entry.getKey();
 
-      if (propCat.equals(ALERT_DEF) && "source".equals(propName)) {
-        source.addProperty(propName, entry.getValue().toString());
+      // only handle "source" subproperties
+      if (!propertyKey.startsWith(ALERT_DEF_SOURCE)) {
+        continue;
       }
 
-      if (propCat.equals(ALERT_DEF_SOURCE)) {
-        source.addProperty(propName, entry.getValue().toString());
-      }
+      // gets a JSON object to add the property to; this will create the
+      // property and all of its parent properties recursively if necessary
+      JsonObject jsonObject = getJsonObjectMapping(ALERT_DEF_SOURCE,
+          jsonObjectMapping, propertyKey);
 
-      if (propCat.equals(ALERT_DEF_SOURCE_REPORTING)) {
-        reporting.addProperty(propName, entry.getValue().toString());
-      }
+      String propertyName = PropertyHelper.getPropertyName(propertyKey);
+      Object entryValue = entry.getValue();
 
-      if (propCat.equals(ALERT_DEF_SOURCE_REPORTING_OK)) {
-        reportingOk.addProperty(propName, entry.getValue().toString());
-      }
-
-      if (propCat.equals(ALERT_DEF_SOURCE_REPORTING_WARNING)) {
-        reportingWarning.addProperty(propName, entry.getValue().toString());
-      }
-
-      if (propCat.equals(ALERT_DEF_SOURCE_REPORTING_CRITICAL)) {
-        reportingCritical.addProperty(propName, entry.getValue().toString());
+      if( entryValue instanceof Collection<?> ){
+        JsonElement jsonElement = gson.toJsonTree(entryValue);
+        jsonObject.add(propertyName, jsonElement);
+      } else {
+        if (entryValue instanceof Number) {
+          jsonObject.addProperty(propertyName, (Number) entryValue);
+        } else {
+          jsonObject.addProperty(propertyName, entryValue.toString());
+        }
       }
     }
 
-    if (reportingOk.entrySet().size() > 0) {
-      reporting.add("ok", reportingOk);
-    }
-
-    if (reportingWarning.entrySet().size() > 0) {
-      reporting.add("warning", reportingWarning);
-    }
-
-    if (reportingCritical.entrySet().size() > 0) {
-      reporting.add("critical", reportingCritical);
-    }
-
-    if (reporting.entrySet().size() > 0) {
-      source.add("reporting", reporting);
-    }
-
-    if (bCreate && 0 == source.entrySet().size()) {
+    // "source" must be filled in when creating
+    JsonObject source = jsonObjectMapping.get(ALERT_DEF_SOURCE);
+    if (bCreate && (null == source || 0 == source.entrySet().size())) {
       throw new IllegalArgumentException("Source must be specified");
     }
 
@@ -540,6 +522,75 @@ public class AlertDefinitionResourceProvider extends AbstractControllerResourceP
     entity.setHash(UUID.randomUUID().toString());
   }
 
+  /**
+   * Gets a {@link JsonObject} that the specified flattened property should be
+   * set on, creating all parent {@link JsonObject} instances as needed. This
+   * will turn {code AlertDefinition/source/reporting/critical/text} into
+   *
+   * <pre>
+   * AlertDefinition/source: {
+   *   reporting: {
+   *     critical: {
+   *       text
+   *     }
+   *   }
+   * }
+   * </pre>
+   *
+   * Where the the following {@link JsonObject} instances are created:
+   *
+   * <pre>
+   * AlertDefinition/source
+   *   Reporting
+   *     Critical
+   *       property 'text',
+   *       property 'value',
+   *       etc
+   * </pre>
+   *
+   * @param root
+   *          the flattened property which will be considered the root.
+   * @param jsonObjectMapping
+   *          a mapping of flattened property to {@link JsonObject}.
+   * @param propertyKey
+   *          the key to get the {@link JsonObject} for.
+   * @return the {@link JsonObject} that cooresponds to the specified key.
+   */
+  private JsonObject getJsonObjectMapping(String root,
+      Map<String, JsonObject> jsonObjectMapping, String propertyKey) {
+
+    // if there is already a mapping for the key, return the mapping
+    if (jsonObjectMapping.containsKey(propertyKey)) {
+      return jsonObjectMapping.get(propertyKey);
+    }
+
+    if (root.equals(propertyKey)) {
+      JsonObject jsonRoot = jsonObjectMapping.get(root);
+      if (null == jsonRoot) {
+        jsonRoot = new JsonObject();
+        jsonObjectMapping.put(root, jsonRoot);
+      }
+
+      return jsonRoot;
+    }
+
+    String propertyCategory = PropertyHelper.getPropertyCategory(propertyKey);
+    JsonObject categoryJson = jsonObjectMapping.get(propertyCategory);
+
+    if (null == categoryJson) {
+      JsonObject parent = getJsonObjectMapping(root, jsonObjectMapping,
+          propertyCategory);
+
+      categoryJson = new JsonObject();
+      jsonObjectMapping.put(propertyCategory, categoryJson);
+
+      String categoryName = PropertyHelper.getPropertyName(propertyCategory);
+      parent.add(categoryName, categoryJson);
+    }
+
+    return categoryJson;
+  }
+
   private Resource toResource(boolean isCollection, String clusterName,
       AlertDefinitionEntity entity, Set<String> requestedIds) {
     Resource resource = new ResourceImpl(Resource.Type.AlertDefinition);

ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProviderTest.java

@@ -55,6 +55,7 @@ import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.alert.AlertDefinition;
 import org.apache.ambari.server.state.alert.AlertDefinitionFactory;
 import org.apache.ambari.server.state.alert.AlertDefinitionHash;
+import org.apache.ambari.server.state.alert.MetricSource;
 import org.apache.ambari.server.state.alert.Scope;
 import org.apache.ambari.server.state.alert.Source;
 import org.apache.ambari.server.state.alert.SourceType;
@@ -193,7 +194,7 @@ public class AlertDefinitionResourceProviderTest {
 
     Source source = getMockSource();
     String okJson = source.getReporting().getOk().getText();
-    Object reporting = r.getPropertyValue(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE_REPORTING);
+    Object reporting = r.getPropertyValue("AlertDefinition/source/reporting");
 
     Assert.assertTrue(reporting.toString().contains(okJson));
 
@@ -242,7 +243,7 @@ public class AlertDefinitionResourceProviderTest {
     Assert.assertEquals("my_def",
         resource.getPropertyValue(AlertDefinitionResourceProvider.ALERT_DEF_NAME));
 
-    Map<String, String> reporting = (Map<String, String>) resource.getPropertyValue(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE_REPORTING);
+    Map<String, String> reporting = (Map<String, String>) resource.getPropertyValue("AlertDefinition/source/reporting");
 
     Assert.assertTrue(reporting.containsKey("ok"));
     Assert.assertTrue(reporting.containsKey("critical"));
@@ -263,7 +264,8 @@ public class AlertDefinitionResourceProviderTest {
         "my_def",
         resource.getPropertyValue(AlertDefinitionResourceProvider.ALERT_DEF_NAME));
 
-    reporting = (Map<String, String>) resource.getPropertyValue(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE_REPORTING);
+    reporting = (Map<String, String>) resource.getPropertyValue("AlertDefinition/source/reporting");
+
     Assert.assertNull(reporting);
   }
 
@@ -291,8 +293,7 @@ public class AlertDefinitionResourceProviderTest {
     replay(amc, clusters, cluster, dao, definitionHash);
 
     Gson gson = m_factory.getGson();
-    Source source = getMockSource();
-    String sourceJson = gson.toJson(source);
+    MetricSource source = (MetricSource)getMockSource();
     AlertDefinitionResourceProvider provider = createProvider(amc);
 
     Map<String, Object> requestProps = new HashMap<String, Object>();
@@ -303,17 +304,42 @@ public class AlertDefinitionResourceProviderTest {
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SERVICE_NAME,
         "HDFS");
 
-    requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE,
-        sourceJson);
+    requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_LABEL,
+        "Mock Label (Create)");
 
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE_TYPE,
         SourceType.METRIC.name());
 
-    requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_LABEL,
-        "Mock Label (Create)");
+    // JMX
+    requestProps.put("AlertDefinition/source/jmx/value",
+        source.getJmxInfo().getValue());
+    requestProps.put("AlertDefinition/source/jmx/property_list",
+        source.getJmxInfo().getPropertyList());
+
+    // URI
+    requestProps.put("AlertDefinition/source/uri/http",
+        source.getUri().getHttpUri());
+    requestProps.put("AlertDefinition/source/uri/https",
+        source.getUri().getHttpsUri());
+    requestProps.put("AlertDefinition/source/uri/https_property",
+        source.getUri().getHttpsProperty());
+    requestProps.put("AlertDefinition/source/uri/https_property_value",
+        source.getUri().getHttpsPropertyValue());
+
+    // reporting
+    requestProps.put("AlertDefinition/source/reporting/critical/text",
+        source.getReporting().getCritical().getText());
+    requestProps.put("AlertDefinition/source/reporting/critical/value",
+        source.getReporting().getCritical().getValue());
+    requestProps.put("AlertDefinition/source/reporting/ok/text",
+        source.getReporting().getOk().getText());
+    requestProps.put("AlertDefinition/source/reporting/warning/text",
+        source.getReporting().getWarning().getText());
+    requestProps.put("AlertDefinition/source/reporting/warning/value",
+        source.getReporting().getWarning().getValue());
 
-    Request request = PropertyHelper.getCreateRequest(Collections.singleton(requestProps), null);
 
+    Request request = PropertyHelper.getCreateRequest(Collections.singleton(requestProps), null);
     provider.createResources(request);
 
     Assert.assertTrue(entityCapture.hasCaptured());
@@ -333,20 +359,30 @@ public class AlertDefinitionResourceProviderTest {
 
     // verify Source
     Assert.assertNotNull(entity.getSource());
-    Source actualSource = gson.fromJson(entity.getSource(), Source.class);
+    MetricSource actualSource = gson.fromJson(entity.getSource(),
+        MetricSource.class);
+
     Assert.assertNotNull(actualSource);
 
     assertEquals(source.getReporting().getOk().getText(),
-        source.getReporting().getOk().getText());
+        actualSource.getReporting().getOk().getText());
 
     assertEquals(source.getReporting().getWarning().getText(),
-        source.getReporting().getWarning().getText());
+        actualSource.getReporting().getWarning().getText());
 
     assertEquals(source.getReporting().getCritical().getText(),
-        source.getReporting().getCritical().getText());
+        actualSource.getReporting().getCritical().getText());
 
-    verify(amc, clusters, cluster, dao);
+    Assert.assertNotNull(source.getUri().getHttpUri());
+    Assert.assertNotNull(source.getUri().getHttpsUri());
+
+    assertEquals(source.getUri().getHttpUri(),
+        actualSource.getUri().getHttpUri());
+
+    assertEquals(source.getUri().getHttpsUri(),
+        actualSource.getUri().getHttpsUri());
 
+    verify(amc, clusters, cluster, dao);
   }
 
   /**
@@ -354,8 +390,6 @@ public class AlertDefinitionResourceProviderTest {
    */
   @Test
   public void testUpdateResources() throws Exception {
-    Gson gson = m_factory.getGson();
-
     AmbariManagementController amc = createMock(AmbariManagementController.class);
     Clusters clusters = createMock(Clusters.class);
     Cluster cluster = createMock(Cluster.class);
@@ -376,8 +410,7 @@ public class AlertDefinitionResourceProviderTest {
 
     replay(amc, clusters, cluster, dao, definitionHash);
 
-    Source source = getMockSource();
-    String sourceString = gson.toJson(source);
+    MetricSource source = (MetricSource) getMockSource();
 
     Map<String, Object> requestProps = new HashMap<String, Object>();
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_CLUSTER_NAME, "c1");
@@ -387,12 +420,37 @@ public class AlertDefinitionResourceProviderTest {
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SERVICE_NAME, "HDFS");
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE_TYPE, "METRIC");
 
-    requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE,
-        sourceString);
-
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_ENABLED,
         Boolean.TRUE.toString());
 
+    // JMX
+    requestProps.put("AlertDefinition/source/jmx/value",
+        source.getJmxInfo().getValue());
+    requestProps.put("AlertDefinition/source/jmx/property_list",
+        source.getJmxInfo().getPropertyList());
+
+    // URI
+    requestProps.put("AlertDefinition/source/uri/http",
+        source.getUri().getHttpUri());
+    requestProps.put("AlertDefinition/source/uri/https",
+        source.getUri().getHttpsUri());
+    requestProps.put("AlertDefinition/source/uri/https_property",
+        source.getUri().getHttpsProperty());
+    requestProps.put("AlertDefinition/source/uri/https_property_value",
+        source.getUri().getHttpsPropertyValue());
+
+    // reporting
+    requestProps.put("AlertDefinition/source/reporting/critical/text",
+        source.getReporting().getCritical().getText());
+    requestProps.put("AlertDefinition/source/reporting/critical/value",
+        source.getReporting().getCritical().getValue());
+    requestProps.put("AlertDefinition/source/reporting/ok/text",
+        source.getReporting().getOk().getText());
+    requestProps.put("AlertDefinition/source/reporting/warning/text",
+        source.getReporting().getWarning().getText());
+    requestProps.put("AlertDefinition/source/reporting/warning/value",
+        source.getReporting().getWarning().getValue());
+
     Request request = PropertyHelper.getCreateRequest(Collections.singleton(requestProps), null);
 
     AlertDefinitionResourceProvider provider = createProvider(amc);
@@ -430,8 +488,15 @@ public class AlertDefinitionResourceProviderTest {
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SERVICE_NAME, "HDFS");
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE_TYPE, "METRIC");
 
-    requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_SOURCE,
-        sourceString.replaceAll("CPU", "CPU2"));
+    // new URI
+    requestProps.put("AlertDefinition/source/uri/http",
+        source.getUri().getHttpUri() + "_foobarbaz");
+    requestProps.put("AlertDefinition/source/uri/https",
+        source.getUri().getHttpsUri() + "_foobarbaz");
+    requestProps.put("AlertDefinition/source/uri/https_property",
+        source.getUri().getHttpsProperty() + "_foobarbaz");
+    requestProps.put("AlertDefinition/source/uri/https_property_value",
+        source.getUri().getHttpsPropertyValue() + "_foobarbaz");
 
     requestProps.put(AlertDefinitionResourceProvider.ALERT_DEF_ENABLED,
         Boolean.FALSE.toString());
@@ -445,7 +510,7 @@ public class AlertDefinitionResourceProviderTest {
     Assert.assertFalse(oldInterval.equals(entity.getScheduleInterval()));
     Assert.assertFalse(oldEnabled == entity.getEnabled());
     Assert.assertFalse(oldSource.equals(entity.getSource()));
-    Assert.assertTrue(entity.getSource().contains("CPU2"));
+    Assert.assertTrue(entity.getSource().contains("_foobarbaz"));
 
     verify(amc, clusters, cluster, dao);
   }