Merge pull request #430 from joesondow/ASGARD-1261-monitor-bucket-none

ASGARD-1261 - Create monitoring bucket type of "none"
Netflix · Dec 9, 2013 · af714bc · af714bc
2 parents 103012a + 9fe5794
commit af714bc
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 12 deletions.
diff --git a/grails-app/services/com/netflix/asgard/ApplicationService.groovy b/grails-app/services/com/netflix/asgard/ApplicationService.groovy
@@ -232,17 +232,17 @@ class ApplicationService implements CacheInitializer, InitializingBean {
     }
 
     /**
-     * Provides a string to use for monitoring bucket, either provided cluster name or app name based on the
-     * application settings. If the application is not found, the app name is returned.
+     * Provides a string to use for monitoring bucket, either provided an empty string, cluster name or app name based
+     * on the application settings.
      *
      * @param userContext who, where, why
-     * @param appName application name to lookup
+     * @param appName application name to look up, and the value to return if the bucket type is 'application'
      * @param clusterName value to return if the application's monitor bucket type is 'cluster'
-     * @return appName or clusterName parameter, based on the application's monitorBucketType
+     * @return appName or clusterName or empty string, based on the application's monitorBucketType
      */
     String getMonitorBucket(UserContext userContext, String appName, String clusterName) {
-        MonitorBucketType bucketType = getRegisteredApplication(userContext, appName)?.monitorBucketType
-        (bucketType == MonitorBucketType.cluster) ? clusterName : appName
+        MonitorBucketType type = getRegisteredApplication(userContext, appName)?.monitorBucketType
+        type == MonitorBucketType.application ? appName : type == MonitorBucketType.cluster ? clusterName : ''
     }
 }
 

diff --git a/grails-app/views/application/create.gsp b/grails-app/views/application/create.gsp
@@ -100,7 +100,7 @@
             <td class="value">
               <select id="monitorBucketType" name="monitorBucketType">
                 <g:each var="bucketType" in="${MonitorBucketType.values()}">
-                  <option ${MonitorBucketType.getDefaultForNewApps() == bucketType.name() ? 'selected' : ''} value="${bucketType.name()}">${bucketType.description}</option>
+                  <option ${MonitorBucketType.getDefaultForNewApps() == bucketType ? 'selected' : ''} value="${bucketType.name()}">${bucketType.description}</option>
                 </g:each>
               </select>
             </td>

diff --git a/src/groovy/com/netflix/asgard/model/MonitorBucketType.groovy b/src/groovy/com/netflix/asgard/model/MonitorBucketType.groovy
@@ -20,10 +20,17 @@ package com.netflix.asgard.model
  */
 enum MonitorBucketType {
 
-    application, cluster
+    none('none (Do not configure monitoring)'),
+    application('application (Aggregate monitoring data by application)'),
+    cluster('cluster (Aggregate monitoring data by cluster)')
 
-    String getDescription() {
-        "Aggregate monitoring data by ${name()}"
+    /**
+     * Human-readable explanation of the type for display on web pages.
+     */
+    String description
+
+    MonitorBucketType(String description) {
+        this.description = description
     }
 
     /**
@@ -36,7 +43,13 @@ enum MonitorBucketType {
         return name ? MonitorBucketType.values().find { it.name() == name } as MonitorBucketType : null
     }
 
-    static MonitorBucketType getDefaultForOldApps() { application }
+    /**
+     * @return the type that should be used if an application does not have any monitor bucket type specified
+     */
+    static MonitorBucketType getDefaultForOldApps() { none }
 
-    static MonitorBucketType getDefaultForNewApps() { cluster }
+    /**
+     * @return the type that should be prepopulated when creating a new application via a web form
+     */
+    static MonitorBucketType getDefaultForNewApps() { none }
 }
diff --git a/test/unit/com/netflix/asgard/ApplicationServiceUnitSpec.groovy b/test/unit/com/netflix/asgard/ApplicationServiceUnitSpec.groovy
@@ -19,6 +19,7 @@ import com.amazonaws.services.simpledb.AmazonSimpleDB
 import com.netflix.asgard.mock.Mocks
 import com.netflix.asgard.model.MonitorBucketType
 import spock.lang.Specification
+import spock.lang.Unroll
 
 @SuppressWarnings("GroovyAssignabilityCheck")
 class ApplicationServiceUnitSpec extends Specification {
@@ -86,4 +87,20 @@ class ApplicationServiceUnitSpec extends Specification {
         1 * applicationService.simpleDbClient.putAttributes(_)
         notThrown(NullPointerException)
     }
+
+    @Unroll('monitor bucket should be "#monitorBucket" if monitor bucket type is #type')
+    def 'monitor bucket value should depend on monitor bucket type'() {
+
+        AppRegistration app = new AppRegistration(name: 'hello', monitorBucketType: MonitorBucketType.byName(type))
+        applicationService.getRegisteredApplication(_, _) >> app
+
+        expect:
+        monitorBucket == applicationService.getMonitorBucket(UserContext.auto(), 'hello', 'hello-there')
+
+        where:
+        type          | monitorBucket
+        'none'        | ''
+        'application' | 'hello'
+        'cluster'     | 'hello-there'
+    }
 }
diff --git a/test/unit/com/netflix/asgard/NetflixAdvancedUserDataProviderSpec.groovy b/test/unit/com/netflix/asgard/NetflixAdvancedUserDataProviderSpec.groovy
@@ -20,6 +20,7 @@ import com.amazonaws.services.ec2.model.Tag
 import com.netflix.asgard.model.AutoScalingGroupBeanOptions
 import com.netflix.asgard.model.LaunchConfigurationBeanOptions
 import com.netflix.asgard.model.LaunchContext
+import com.netflix.asgard.model.MonitorBucketType
 import com.netflix.asgard.plugin.UserDataProvider
 import javax.xml.bind.DatatypeConverter
 import spock.lang.Specification
@@ -99,6 +100,42 @@ class NetflixAdvancedUserDataProviderSpec extends Specification {
         111.1       | helloStandardUserData
     }
 
+    @Unroll('monitor bucket should be "#monitorBucket" if monitor bucket type is #type')
+    def 'monitor bucket should be empty, cluster, or app name as requested'() {
+
+        String description = "blah blah blah, ancestor_version=nflx-base-2.0-12345-h24"
+        launchContext.image = new Image(description: description)
+        AppRegistration app = new AppRegistration(name: 'hi', monitorBucketType: MonitorBucketType.byName(type))
+        launchContext.application = app
+        launchContext.autoScalingGroup = new AutoScalingGroupBeanOptions(autoScalingGroupName: 'hi-dev-v001')
+        launchContext.launchConfiguration = new LaunchConfigurationBeanOptions(
+                launchConfigurationName: 'hi-dev-v001-1234567')
+        netflixAdvancedUserDataProvider.applicationService = Spy(ApplicationService) {
+            getRegisteredApplication(_, _) >> app
+        }
+
+        when:
+        String userData = decode(netflixAdvancedUserDataProvider.buildUserData(launchContext))
+
+        then:
+        userData == """\
+                export NETFLIX_ENVIRONMENT=test
+                export NETFLIX_MONITOR_BUCKET=${monitorBucket ?: ''}
+                export NETFLIX_APP=hi
+                export NETFLIX_STACK=dev
+                export NETFLIX_CLUSTER=hi-dev
+                export NETFLIX_AUTO_SCALE_GROUP=hi-dev-v001
+                export NETFLIX_LAUNCH_CONFIG=hi-dev-v001-1234567
+                export EC2_REGION=us-west-2
+                """.stripIndent()
+
+        where:
+        type          | monitorBucket
+        'none'        | ''
+        'cluster'     | 'hi-dev'
+        'application' | 'hi'
+    }
+
     def 'should build user data with legacy format if AMI description does not match the standard pattern'() {
 
         launchContext.image = new Image(description: 'blah blah blah')