You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/03/05 20:02:44 UTC

[GitHub] dubeejw closed pull request #134: fix random test failure and other minor cleanup

dubeejw closed pull request #134: fix random test failure and other minor cleanup
URL: https://github.com/apache/incubator-openwhisk-package-alarms/pull/134
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/provider/lib/health.js b/provider/lib/health.js
index 9157e82..5255a5a 100644
--- a/provider/lib/health.js
+++ b/provider/lib/health.js
@@ -11,6 +11,8 @@ module.exports = function(logger, utils) {
     var triggerName;
     var monitorStatus;
     var alarmTypes = ['interval', 'date', 'cron'];
+    var alarmTypeIndex = 0;
+    var monitorStages = ['triggerStarted', 'triggerFired', 'triggerStopped'];
     var healthMonitor = this;
 
     // Health Logic
@@ -49,6 +51,14 @@ module.exports = function(logger, utils) {
 
         if (triggerName) {
             monitorStatus = Object.assign({}, utils.monitorStatus);
+            utils.monitorStatus = {};
+
+            var monitorStatusSize = Object.keys(monitorStatus).length;
+            if (monitorStatusSize < 5) {
+                //we have a failure in one of the stages
+                var stageFailed = monitorStages[monitorStatusSize - 2];
+                monitorStatus[stageFailed] = 'failed';
+            }
             var existingID = `${apikey}/_/${triggerName}`;
 
             //delete trigger feed from database
@@ -60,46 +70,41 @@ module.exports = function(logger, utils) {
                 triggerID: existingID
             };
             utils.sanitizer.deleteTrigger(dataTrigger, auth, 0)
-                .then((info) => {
-                    logger.info(method, existingID, info);
-                })
-                .catch(err => {
-                    logger.error(method, existingID, err);
-                });
+            .then((info) => {
+                logger.info(method, existingID, info);
+            })
+            .catch(err => {
+                logger.error(method, existingID, err);
+            });
+
+            var existingAlarmIndex = alarmTypes.indexOf(monitorStatus.triggerType);
+            alarmTypeIndex = existingAlarmIndex !== 2 ? existingAlarmIndex + 1 : 0;
         }
 
         //create new alarm trigger
         triggerName = 'alarms_' + utils.worker + utils.host + '_' + Date.now();
+        var alarmType = alarmTypes[alarmTypeIndex];
+
+        //update status monitor object
+        utils.monitorStatus.triggerName = triggerName;
+        utils.monitorStatus.triggerType = alarmType;
 
         var triggerURL = utils.uriHost + '/api/v1/namespaces/_/triggers/' + triggerName;
         var triggerID = `${apikey}/_/${triggerName}`;
         healthMonitor.createTrigger(triggerURL, auth)
-            .then((info) => {
-                logger.info(method, triggerID, info);
-                var newTrigger = healthMonitor.createAlarmTrigger(triggerID, triggerName, apikey);
-                healthMonitor.createTriggerInDB(triggerID, newTrigger);
-            })
-            .catch(err => {
-                logger.error(method, triggerID, err);
-            });
+        .then((info) => {
+            logger.info(method, triggerID, info);
+            var newTrigger = healthMonitor.createAlarmTrigger(triggerID, apikey, alarmType);
+            healthMonitor.createTriggerInDB(triggerID, newTrigger);
+        })
+        .catch(err => {
+            logger.error(method, triggerID, err);
+        });
     };
 
-    this.createAlarmTrigger = function(triggerID, triggerName, apikey) {
+    this.createAlarmTrigger = function(triggerID, apikey, alarmType) {
         var method = 'createAlarmTrigger';
 
-        var existingTypeIndex = -1;
-        if (monitorStatus && alarmTypes.indexOf(monitorStatus.triggerType) !== 2) {
-            existingTypeIndex = alarmTypes.indexOf(monitorStatus.triggerType);
-        }
-        var alarmType = alarmTypes[existingTypeIndex + 1];
-
-        //update status monitor object
-        utils.monitorStatus.triggerName = triggerName;
-        utils.monitorStatus.triggerType = alarmType;
-        utils.monitorStatus.triggerStarted = false;
-        utils.monitorStatus.triggerFired = false;
-        utils.monitorStatus.triggerUpdated = false;
-
         var newTrigger = {
             apikey: apikey,
             name: triggerName,
diff --git a/provider/lib/utils.js b/provider/lib/utils.js
index a428a46..3200189 100644
--- a/provider/lib/utils.js
+++ b/provider/lib/utils.js
@@ -165,7 +165,7 @@ module.exports = function(logger, triggerDB, redisClient) {
         var method = 'handleFiredTrigger';
 
         if (isMonitorTrigger) {
-            utils.monitorStatus.triggerFired = true;
+            utils.monitorStatus.triggerFired = "success";
         }
 
         var triggerIdentifier = dataTrigger.triggerID;
@@ -343,10 +343,10 @@ module.exports = function(logger, triggerDB, redisClient) {
 
                 if (utils.triggers[triggerIdentifier]) {
                     if (doc.status && doc.status.active === false) {
+                        utils.stopTrigger(triggerIdentifier);
                         if (doc.monitor && doc.monitor === utils.host) {
-                            utils.monitorStatus.triggerUpdated = true;
+                            utils.monitorStatus.triggerStopped = "success";
                         }
-                        utils.stopTrigger(triggerIdentifier);
                     }
                 }
                 else {
@@ -358,7 +358,7 @@ module.exports = function(logger, triggerDB, redisClient) {
                             logger.info(method, triggerIdentifier, 'created successfully');
 
                             if (doc.monitor && doc.monitor === utils.host) {
-                                utils.monitorStatus.triggerStarted = true;
+                                utils.monitorStatus.triggerStarted = "success";
                             }
 
                             if (cachedTrigger.intervalHandle && utils.shouldFireTrigger(cachedTrigger)) {
diff --git a/tests/src/test/scala/system/packages/AlarmsFeedTests.scala b/tests/src/test/scala/system/packages/AlarmsFeedTests.scala
index 59c4b0a..56e7e74 100644
--- a/tests/src/test/scala/system/packages/AlarmsFeedTests.scala
+++ b/tests/src/test/scala/system/packages/AlarmsFeedTests.scala
@@ -83,11 +83,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when alarm action does not include cron parameter" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "alarm"
 
             // the package alarms should be there
@@ -112,11 +111,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when alarms once action does not include date parameter" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "once"
 
             // the package alarms should be there
@@ -141,11 +139,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when alarm action includes invalid cron parameter" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "alarm"
 
             // the package alarms should be there
@@ -173,11 +170,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when alarms once action includes an invalid date parameter" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "once"
 
             // the package alarms should be there
@@ -203,11 +199,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when alarms once action date parameter is not a future date" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "once"
 
             // the package alarms should be there
@@ -235,11 +230,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when alarms startDate parameter is not a future date" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "alarm"
 
             // the package alarms should be there
@@ -267,11 +261,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when alarms stopDate parameter is not greater than startDate" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "alarm"
 
             // the package alarms should be there
@@ -284,7 +277,7 @@ class AlarmsFeedTests
                 (pkg, name) => pkg.bind("/whisk.system/alarms", name)
             }
 
-            val stopDate = System.currentTimeMillis + 5000
+            val stopDate = System.currentTimeMillis + 30000
             val startDate = stopDate
 
             // create trigger with feed
@@ -301,11 +294,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when interval action does not include minutes parameter" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "interval"
 
             // the package alarms should be there
@@ -330,11 +322,10 @@ class AlarmsFeedTests
     }
 
     it should "return error message when interval action includes invalid minutes parameter" in withAssetCleaner(wskprops) {
-
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "interval"
 
             // the package alarms should be there
@@ -456,8 +447,8 @@ class AlarmsFeedTests
     it should "return error message when limitCronFields is true and 6 cron fields are used" in withAssetCleaner(wskprops) {
         (wp, assetHelper) =>
             implicit val wskprops = wp // shadow global props and make implicit
-            val triggerName = s"dummyCloudantTrigger-${System.currentTimeMillis}"
-            val packageName = "dummyCloudantPackage"
+            val triggerName = s"dummyAlarmsTrigger-${System.currentTimeMillis}"
+            val packageName = "dummyAlarmsPackage"
             val feed = "alarm"
 
             // the package alarms should be there


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services