From 81caa3fc7f8767db50b860e3f0b9c0f2ca57e2a7 Mon Sep 17 00:00:00 2001 From: "ning.yougang" Date: Thu, 27 Feb 2020 13:56:10 +0800 Subject: [PATCH] Review --- ansible/templates/whisk.properties.j2 | 6 ----- .../core/controller/Controller.scala | 21 +++++++++------ .../core/invoker/DefaultInvokerServer.scala | 7 ++--- .../core/invoker/InvokerReactive.scala | 4 ++- .../test/scala/common/WhiskProperties.java | 26 ------------------- ....scala => RuntimeConfigurationTests.scala} | 19 ++++++++------ 6 files changed, 31 insertions(+), 52 deletions(-) rename tests/src/test/scala/org/apache/openwhisk/operation/{RuntimeConfiguration.scala => RuntimeConfigurationTests.scala} (88%) diff --git a/ansible/templates/whisk.properties.j2 b/ansible/templates/whisk.properties.j2 index c3e9329f803..00e85ba9719 100644 --- a/ansible/templates/whisk.properties.j2 +++ b/ansible/templates/whisk.properties.j2 @@ -46,10 +46,7 @@ kafka.hosts={{ kafka_connect_string }} redis.host={{ groups["redis"]|default([""])|first }} router.host={{ groups["edge"]|first }} zookeeper.hosts={{ zookeeper_connect_string }} -invoker.protocol={{ invoker.protocol }} invoker.hosts={{ groups["invokers"] | map('extract', hostvars, 'ansible_host') | list | join(",") }} -invoker.username={{ invoker.username }} -invoker.password={{ invoker.password }} edge.host.apiport=443 kafkaras.host.port={{ kafka.ras.port }} @@ -59,9 +56,6 @@ invoker.hosts.basePort={{ invoker.port }} controller.hosts={{ groups["controllers"] | map('extract', hostvars, 'ansible_host') | list | join(",") }} controller.host.basePort={{ controller.basePort }} controller.instances={{ controller.instances }} -controller.protocol={{ controller.protocol }} -controller.username={{ controller.username }} -controller.password={{ controller.password }} invoker.container.network=bridge invoker.container.policy={{ invoker_container_policy_name | default()}} diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala index 98949fcce7f..250ddc63c91 100644 --- a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala +++ b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala @@ -186,36 +186,41 @@ class Controller(val instance: ControllerInstanceId, entity(as[String]) { runtime => val execManifest = ExecManifest.initialize(runtime) if (execManifest.isFailure) { - logging.error(this, s"Received invalid runtimes manifest") - complete(s"Received invalid runtimes manifest") + logging.info(this, s"received invalid runtimes manifest") + complete(StatusCodes.BadRequest) } else { parameter('limit.?) { limit => limit match { case Some(targetValue) => - val pattern = "\\d+:\\d" + val pattern = """\d+:\d""" if (targetValue.matches(pattern)) { val invokerArray = targetValue.split(":") val beginIndex = invokerArray(0).toInt val finishIndex = invokerArray(1).toInt if (finishIndex < beginIndex) { - complete(s"finishIndex can't be less than beginIndex") + logging.info(this, "finishIndex can't be less than beginIndex") + complete(StatusCodes.BadRequest) } else { val targetInvokers = (beginIndex to finishIndex).toList loadBalancer.sendRuntimeToInvokers(runtime, Some(targetInvokers)) - complete(s"config runtime request is already sent to target invokers") + logging.info(this, "config runtime request is already sent to target invokers") + complete(StatusCodes.BadRequest) } } else { - complete(s"limit value can't match [beginIndex:finishIndex]") + logging.info(this, "limit value can't match [beginIndex:finishIndex]") + complete(StatusCodes.BadRequest) } case None => loadBalancer.sendRuntimeToInvokers(runtime, None) - complete(s"config runtime request is already sent to all managed invokers") + logging.info(this, "config runtime request is already sent to all managed invokers") + complete(StatusCodes.Accepted) } } } } } else { - complete("username or password is wrong") + logging.info(this, s"username or password is wrong") + complete(StatusCodes.Unauthorized) } case _ => complete(StatusCodes.Unauthorized) } diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala index 404b7937ef8..8422b90d073 100644 --- a/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala +++ b/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/DefaultInvokerServer.scala @@ -51,8 +51,8 @@ class DefaultInvokerServer(val invoker: InvokerCore)(implicit val ec: ExecutionC entity(as[String]) { prewarmRuntime => val execManifest = ExecManifest.initialize(prewarmRuntime) if (execManifest.isFailure) { - logger.error(this, s"Received invalid runtimes manifest:${execManifest.failed.get}") - complete(s"Received invalid runtimes manifest") + logger.info(this, s"received invalid runtimes manifest") + complete(StatusCodes.BadRequest) } else { val prewarmingConfigs: List[PrewarmingConfig] = execManifest.get.stemcells.flatMap { case (mf, cells) => @@ -64,7 +64,8 @@ class DefaultInvokerServer(val invoker: InvokerCore)(implicit val ec: ExecutionC } } } else { - complete("username or password is wrong") + logger.info(this, s"username or password is wrong") + complete(StatusCodes.Unauthorized) } case _ => complete(StatusCodes.Unauthorized) } diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala index 2c71b54a3d2..df2c624def0 100644 --- a/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala +++ b/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/InvokerReactive.scala @@ -24,6 +24,7 @@ import akka.Done import akka.actor.{ActorRefFactory, ActorSystem, CoordinatedShutdown, Props} import akka.event.Logging.InfoLevel import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._ +import akka.http.scaladsl.model.StatusCodes import akka.http.scaladsl.server.Directives._ import akka.http.scaladsl.server.Route import akka.stream.ActorMaterializer @@ -325,7 +326,8 @@ class InvokerReactive( override def configRuntime(prewarmConfigList: List[PrewarmingConfig]): Route = { pool ! PreWarmConfigList(prewarmConfigList) - complete(s"config runtime request is handling") + logging.info(this, "config runtime request is handling") + complete(StatusCodes.Accepted) } override def getRuntime(): Route = { diff --git a/tests/src/test/scala/common/WhiskProperties.java b/tests/src/test/scala/common/WhiskProperties.java index fcbfbcba65f..48dce757939 100644 --- a/tests/src/test/scala/common/WhiskProperties.java +++ b/tests/src/test/scala/common/WhiskProperties.java @@ -258,40 +258,14 @@ public static int getControllerBasePort() { return Integer.parseInt(whiskProperties.getProperty("controller.host.basePort")); } - public static String getControllerProtocol() { - return whiskProperties.getProperty("controller.protocol"); - } - public static String getBaseControllerHost() { return getControllerHosts().split(",")[0]; } - public static String getInvokerProtocol() { - return whiskProperties.getProperty("invoker.protocol"); - } - - public static String getBaseInvokerAddress(){ return getInvokerHosts()[0] + ":" + whiskProperties.getProperty("invoker.hosts.basePort"); } - public static String getControllerUsername() { - return whiskProperties.getProperty("controller.username"); - } - - public static String getControllerPassword() { - return whiskProperties.getProperty("controller.password"); - } - - - public static String getInvokerUsername() { - return whiskProperties.getProperty("invoker.username"); - } - - public static String getInvokerPassword() { - return whiskProperties.getProperty("invoker.password"); - } - public static String getBaseDBHost() { return getDBHosts().split(",")[0]; } diff --git a/tests/src/test/scala/org/apache/openwhisk/operation/RuntimeConfiguration.scala b/tests/src/test/scala/org/apache/openwhisk/operation/RuntimeConfigurationTests.scala similarity index 88% rename from tests/src/test/scala/org/apache/openwhisk/operation/RuntimeConfiguration.scala rename to tests/src/test/scala/org/apache/openwhisk/operation/RuntimeConfigurationTests.scala index 329c827ee2b..3eb99f15add 100644 --- a/tests/src/test/scala/org/apache/openwhisk/operation/RuntimeConfiguration.scala +++ b/tests/src/test/scala/org/apache/openwhisk/operation/RuntimeConfigurationTests.scala @@ -30,6 +30,7 @@ import org.junit.runner.RunWith import org.scalatest.Matchers import org.scalatest.concurrent.ScalaFutures import org.scalatest.junit.JUnitRunner +import pureconfig.loadConfigOrThrow import spray.json._ import system.rest.RestUtil @@ -76,16 +77,16 @@ class RuntimeConfigurationTests }""" } - val invokerProtocol = WhiskProperties.getInvokerProtocol + val invokerProtocol = loadConfigOrThrow[String]("whisk.invoker.protocol") val invokerAddress = WhiskProperties.getBaseInvokerAddress - val invokerUsername = WhiskProperties.getInvokerUsername - val invokerPassword = WhiskProperties.getInvokerPassword + val invokerUsername = loadConfigOrThrow[String]("whisk.credentials.invoker.username") + val invokerPassword = loadConfigOrThrow[String]("whisk.credentials.invoker.password") val invokerAuthHeader = Authorization(BasicHttpCredentials(invokerUsername, invokerPassword)) - val controllerProtocol = WhiskProperties.getControllerProtocol + val controllerProtocol = loadConfigOrThrow[String]("whisk.controller.protocol") val controllerAddress = WhiskProperties.getBaseControllerAddress - val controllerUsername = WhiskProperties.getControllerUsername - val controllerPassword = WhiskProperties.getControllerPassword + val controllerUsername = loadConfigOrThrow[String]("whisk.credentials.controller.username") + val controllerPassword = loadConfigOrThrow[String]("whisk.credentials.controller.password") val controllerAuthHeader = Authorization(BasicHttpCredentials(controllerUsername, controllerPassword)) val getRuntimeUrl = s"${invokerProtocol}://${invokerAddress}/getRuntime" @@ -104,9 +105,10 @@ class RuntimeConfigurationTests entity = HttpEntity(ContentTypes.`text/plain(UTF-8)`, getRuntimes)), connectionContext = HttpConnection.getContext(invokerProtocol)) .map { response => - response.status shouldBe StatusCodes.OK + response.status shouldBe StatusCodes.Accepted } + // Make sure previous http post call successfully Thread.sleep(5.seconds.toMillis) //Cal the prewarm container number whether right @@ -136,9 +138,10 @@ class RuntimeConfigurationTests entity = HttpEntity(ContentTypes.`text/plain(UTF-8)`, getRuntimes)), connectionContext = HttpConnection.getContext(controllerProtocol)) .map { response => - response.status shouldBe StatusCodes.OK + response.status shouldBe StatusCodes.Accepted } + // Make sure previous http post call successfully Thread.sleep(5.seconds.toMillis) //Cal the prewarm container number whether right