You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by ra...@apache.org on 2018/02/20 15:36:59 UTC
[incubator-openwhisk] branch master updated: Handle trigger
activations with inactive rules (#3262)
This is an automated email from the ASF dual-hosted git repository.
rabbah pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push:
new b20778f Handle trigger activations with inactive rules (#3262)
b20778f is described below
commit b20778f37e052893818c3c39f0307783d7e5024a
Author: Mark Deuser <md...@us.ibm.com>
AuthorDate: Tue Feb 20 10:36:55 2018 -0500
Handle trigger activations with inactive rules (#3262)
If trigger has no associated active rules; return a 204. Also no trigger activation record is created. Updated swagger to reflect new 204 response from POST trigger.
When a trigger activation is created, log all inactive rules as well.
---
.../src/main/scala/whisk/http/ErrorResponse.scala | 5 ++
.../src/main/resources/apiv1swagger.json | 6 +++
.../scala/whisk/core/controller/Triggers.scala | 26 ++++++---
.../test/scala/system/basic/WskBasicTests.scala | 62 ++++++++++++++++++++++
.../core/controller/test/TriggersApiTests.scala | 9 ++++
5 files changed, 102 insertions(+), 6 deletions(-)
diff --git a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
index f161618..e599a1f 100644
--- a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
@@ -111,6 +111,11 @@ object Messages {
val notAllowedOnBinding = "Operation not permitted on package binding."
def packageNameIsReserved(name: String) = s"Package name '$name' is reserved."
+ /** Error messages for triggers */
+ def triggerWithInactiveRule(rule: String, action: String) = {
+ s"Rule '$rule' is inactive, action '$action' was not activated."
+ }
+
/** Error messages for sequence activations. */
def sequenceRetrieveActivationTimeout(id: ActivationId) =
s"Timeout reached when retrieving activation $id for sequence component."
diff --git a/core/controller/src/main/resources/apiv1swagger.json b/core/controller/src/main/resources/apiv1swagger.json
index efb553a..81636a5 100644
--- a/core/controller/src/main/resources/apiv1swagger.json
+++ b/core/controller/src/main/resources/apiv1swagger.json
@@ -940,6 +940,9 @@
"$ref": "#/definitions/ItemId"
}
},
+ "204": {
+ "$ref": "#/responses/NoActiveRules"
+ },
"401": {
"$ref": "#/responses/UnauthorizedRequest"
},
@@ -2109,6 +2112,9 @@
"schema": {
"$ref": "#/definitions/ErrorMessage"
}
+ },
+ "NoActiveRules": {
+ "description": "Trigger has no active rules"
}
}
}
diff --git a/core/controller/src/main/scala/whisk/core/controller/Triggers.scala b/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
index 873dcfd..8302d74 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
@@ -26,7 +26,7 @@ import akka.actor.ActorSystem
import akka.http.scaladsl.Http
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
import akka.http.scaladsl.model.HttpMethods.POST
-import akka.http.scaladsl.model.StatusCodes.{Accepted, BadRequest, InternalServerError, OK, ServerError}
+import akka.http.scaladsl.model.StatusCodes.{Accepted, BadRequest, InternalServerError, NoContent, OK, ServerError}
import akka.http.scaladsl.model.Uri.Path
import akka.http.scaladsl.model.headers.{Authorization, BasicHttpCredentials}
import akka.http.scaladsl.model._
@@ -42,6 +42,7 @@ import whisk.core.entitlement.Collection
import whisk.core.entity._
import whisk.core.entity.types.{ActivationStore, EntityStore}
import whisk.http.ErrorResponse
+import whisk.http.Messages
/** A trait implementing the triggers API. */
trait WhiskTriggersApi extends WhiskCollectionAPI {
@@ -129,7 +130,7 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
if (activeRules.nonEmpty) {
val args: JsObject = trigger.parameters.merge(payload).getOrElse(JsObject())
- activateRules(user, args, activeRules)
+ activateRules(user, args, trigger.rules.getOrElse(Map.empty))
.map(results => triggerActivation.withLogs(ActivationLogs(results.map(_.toJson.compactPrint).toVector)))
.recover {
case e =>
@@ -146,9 +147,14 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
case Failure(t) =>
logging.error(this, s"[POST] storing trigger activation $triggerActivationId failed: ${t.getMessage}")
}
+ complete(Accepted, triggerActivationId.toJsObject)
+ } else {
+ logging
+ .debug(
+ this,
+ s"[POST] trigger without an active rule was activated; no trigger activation record created for $entityName")
+ complete(NoContent)
}
-
- complete(Accepted, triggerActivationId.toJsObject)
})
}
}
@@ -287,13 +293,21 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
}
/**
- * Iterates through each active rule and invoke each mapped action.
+ * Iterates through each rule and invoking each active rule's mapped action.
*/
private def activateRules(user: Identity,
args: JsObject,
rulesToActivate: Map[FullyQualifiedEntityName, ReducedRule])(
implicit transid: TransactionId): Future[Iterable[RuleActivationResult]] = {
val ruleResults = rulesToActivate.map {
+ case (ruleName, rule) if (rule.status != Status.ACTIVE) =>
+ Future.successful {
+ RuleActivationResult(
+ ActivationResponse.ApplicationError,
+ ruleName,
+ rule.action,
+ Left(Messages.triggerWithInactiveRule(ruleName.asString, rule.action.asString)))
+ }
case (ruleName, rule) =>
// Invoke the action. Retain action results for inclusion in the trigger activation record
postActivation(user, rule, args)
@@ -346,7 +360,7 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
/**
* Posts an action activation. Currently done by posting internally to the controller.
- * TODO: use a poper path that does not route through HTTP.
+ * TODO: use a proper path that does not route through HTTP.
*
* @param rule the name of the rule that is activated
* @param args the arguments to post to the action
diff --git a/tests/src/test/scala/system/basic/WskBasicTests.scala b/tests/src/test/scala/system/basic/WskBasicTests.scala
index 4c8e197..1fd93fd 100644
--- a/tests/src/test/scala/system/basic/WskBasicTests.scala
+++ b/tests/src/test/scala/system/basic/WskBasicTests.scala
@@ -39,6 +39,7 @@ import common.WskProps
import common.WskTestHelpers
import spray.json._
import spray.json.DefaultJsonProtocol._
+import whisk.http.Messages
@RunWith(classOf[JUnitRunner])
class WskBasicTests extends TestHelpers with WskTestHelpers {
@@ -754,6 +755,67 @@ class WskBasicTests extends TestHelpers with WskTestHelpers {
}
}
+ it should "create and fire a trigger having an active rule and an inactive rule" in withAssetCleaner(wskprops) {
+ (wp, assetHelper) =>
+ val ruleName1 = withTimestamp("r1toa1")
+ val ruleName2 = withTimestamp("r2toa2")
+ val triggerName = withTimestamp("t1tor1r2")
+ val actionName1 = withTimestamp("a1")
+ val actionName2 = withTimestamp("a2")
+ val ns = wsk.namespace.whois()
+
+ assetHelper.withCleaner(wsk.trigger, triggerName) { (trigger, _) =>
+ trigger.create(triggerName)
+ trigger.create(triggerName, update = true)
+ }
+
+ assetHelper.withCleaner(wsk.action, actionName1) { (action, name) =>
+ action.create(name, defaultAction)
+ }
+ assetHelper.withCleaner(wsk.action, actionName2) { (action, name) =>
+ action.create(name, defaultAction)
+ }
+
+ assetHelper.withCleaner(wsk.rule, ruleName1) { (rule, name) =>
+ rule.create(name, trigger = triggerName, action = actionName1)
+ }
+ assetHelper.withCleaner(wsk.rule, ruleName2) { (rule, name) =>
+ rule.create(name, trigger = triggerName, action = actionName2)
+ rule.disable(ruleName2)
+ }
+
+ val run = wsk.trigger.fire(triggerName)
+ withActivation(wsk.activation, run) { activation =>
+ activation.duration shouldBe 0L // shouldn't exist but CLI generates it
+ activation.end shouldBe Instant.EPOCH // shouldn't exist but CLI generates it
+ activation.logs shouldBe defined
+ activation.logs.get.size shouldBe 2
+
+ val logEntry1 = activation.logs.get(0).parseJson.asJsObject
+ val logEntry2 = activation.logs.get(1).parseJson.asJsObject
+ val logs = JsArray(logEntry1, logEntry2)
+ val ruleActivationId: String = if (logEntry1.getFields("activationId").size == 1) {
+ logEntry1.getFields("activationId")(0).convertTo[String]
+ } else {
+ logEntry2.getFields("activationId")(0).convertTo[String]
+ }
+ val expectedLogs = JsArray(
+ JsObject(
+ "statusCode" -> JsNumber(0),
+ "activationId" -> JsString(ruleActivationId),
+ "success" -> JsBoolean(true),
+ "rule" -> JsString(ns + "/" + ruleName1),
+ "action" -> JsString(ns + "/" + actionName1)),
+ JsObject(
+ "statusCode" -> JsNumber(1),
+ "success" -> JsBoolean(false),
+ "error" -> JsString(Messages.triggerWithInactiveRule(s"$ns/$ruleName2", s"$ns/$actionName2")),
+ "rule" -> JsString(ns + "/" + ruleName2),
+ "action" -> JsString(ns + "/" + actionName2)))
+ logs shouldBe expectedLogs
+ }
+ }
+
behavior of "Wsk Rule REST"
it should "create rule, get rule, update rule and list rule" in withAssetCleaner(wskprops) { (wp, assetHelper) =>
diff --git a/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
index 59e9e7e..fdfcf4f 100644
--- a/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
@@ -366,6 +366,15 @@ class TriggersApiTests extends ControllerTestCommon with WhiskTriggersApi {
}
}
+ it should "not fire a trigger without a rule" in {
+ implicit val tid = transid()
+ val trigger = WhiskTrigger(namespace, aname())
+ put(entityStore, trigger)
+ Post(s"$collectionPath/${trigger.name}") ~> Route.seal(routes(creds)) ~> check {
+ status shouldBe NoContent
+ }
+ }
+
//// invalid resource
it should "reject invalid resource" in {
implicit val tid = transid()
--
To stop receiving notification emails like this one, please contact
rabbah@apache.org.