Review

apache · Feb 29, 2020 · fc63c07 · fc63c07
1 parent 2af74d8
commit fc63c07
Show file tree

Hide file tree

Showing 15 changed files with 52 additions and 171 deletions.
diff --git a/ansible/group_vars/all b/ansible/group_vars/all
@@ -203,8 +203,6 @@ invoker:
     runcdir: "{{ invoker_runcdir | default('/run/docker/runtime-runc/moby') }}"
     volumes: "{{ invoker_docker_volumes | default([]) }}"
   loglevel: "{{ invoker_loglevel | default(whisk_loglevel) | default('INFO') }}"
-  username: "{{ invoker_username | default('invoker.user') }}"
-  password: "{{ invoker_password | default('invoker.pass') }}"
   jmxremote:
     jvmArgs: "{% if inventory_hostname in groups['invokers'] %}
     {{ jmx.jvmCommonArgs }} -Djava.rmi.server.hostname={{ invokerHostname }} -Dcom.sun.management.jmxremote.rmi.port={{ jmx.rmiBasePortInvoker + groups['invokers'].index(inventory_hostname) }} -Dcom.sun.management.jmxremote.port={{ jmx.basePortInvoker + groups['invokers'].index(inventory_hostname) }}

diff --git a/ansible/roles/invoker/tasks/deploy.yml b/ansible/roles/invoker/tasks/deploy.yml
@@ -266,8 +266,6 @@
       "CONFIG_whisk_timeLimit_min": "{{ limit_action_time_min | default() }}"
       "CONFIG_whisk_timeLimit_max": "{{ limit_action_time_max | default() }}"
       "CONFIG_whisk_timeLimit_std": "{{ limit_action_time_std | default() }}"
-      "CONFIG_whisk_credentials_invoker_username": "{{ invoker.username }}"
-      "CONFIG_whisk_credentials_invoker_password": "{{ invoker.password }}"
       "CONFIG_whisk_concurrencyLimit_min": "{{ limit_action_concurrency_min | default() }}"
       "CONFIG_whisk_concurrencyLimit_max": "{{ limit_action_concurrency_max | default() }}"
       "CONFIG_whisk_concurrencyLimit_std": "{{ limit_action_concurrency_std | default() }}"

diff --git 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 }}
@@ -60,8 +57,6 @@ controller.hosts={{ groups["controllers"] | map('extract', hostvars, 'ansible_ho
 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/common/scala/src/main/scala/org/apache/openwhisk/common/ComponentCredentials.scala b/common/scala/src/main/scala/org/apache/openwhisk/common/ComponentCredentials.scala
@@ -18,5 +18,3 @@
 package org.apache.openwhisk.common
 
 case class ControllerCredentials(username: String, password: String)
-
-case class InvokerCredentials(username: String, password: String)
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala
@@ -268,5 +268,4 @@ object ConfigKeys {
   val apacheClientConfig = "whisk.apache-client"
 
   val controllerCredentials = "whisk.credentials.controller"
-  val invokerCredentials = "whisk.credentials.invoker"
 }
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
@@ -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/containerpool/ContainerPool.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
@@ -75,6 +75,7 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
   var busyPool = immutable.Map.empty[ActorRef, ContainerData]
   var prewarmedPool = immutable.Map.empty[ActorRef, ContainerData]
   var prewarmStartingPool = immutable.Map.empty[ActorRef, (String, ByteSize)]
+  var latestPrewarmConfig = prewarmConfig
   // If all memory slots are occupied and if there is currently no container to be removed, than the actions will be
   // buffered here to keep order of computation.
   // Otherwise actions with small memory-limits could block actions with large memory limits.
@@ -285,14 +286,20 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
       freePool = freePool - sender()
       busyPool = busyPool - sender()
     case prewarmConfigList: PreWarmConfigList =>
-      // Delete prewarmedPool firstly
-      prewarmedPool foreach { element =>
-        val actor = element._1
-        actor ! Remove
-        prewarmedPool = prewarmedPool - actor
-      }
+      latestPrewarmConfig = prewarmConfigList.list
       prewarmConfigList.list foreach { config =>
-        logging.info(this, s"add extra pre-warming ${config.count} ${config.exec.kind} ${config.memoryLimit.toString}")(
+        // Delete matched prewarm container from prewarmedPool firstly
+        val kind = config.exec.kind
+        val memory = config.memoryLimit
+        prewarmedPool.filter {
+          case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+          case _                                          => false
+        } foreach { element =>
+          val actor = element._1
+          actor ! Remove
+          prewarmedPool = prewarmedPool - actor
+        }
+        logging.info(this, s"add pre-warming ${config.count} ${config.exec.kind} ${config.memoryLimit.toString}")(
           TransactionId.invokerWarmup)
         (1 to config.count).foreach { _ =>
           prewarmContainer(config.exec, config.memoryLimit)
@@ -325,7 +332,7 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
 
   /** Install prewarm containers up to the configured requirements for each kind/memory combination. */
   def backfillPrewarms(init: Boolean) = {
-    prewarmConfig.foreach { config =>
+    latestPrewarmConfig.foreach { config =>
       val kind = config.exec.kind
       val memory = config.memoryLimit
       val currentCount = prewarmedPool.count {

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
@@ -18,17 +18,10 @@
 package org.apache.openwhisk.core.invoker
 
 import akka.actor.ActorSystem
-import akka.http.scaladsl.model.StatusCodes
-import akka.http.scaladsl.model.headers.BasicHttpCredentials
 import akka.http.scaladsl.server.Route
-import org.apache.openwhisk.common.{InvokerCredentials, Logging, TransactionId}
-import org.apache.openwhisk.core.ConfigKeys
-import org.apache.openwhisk.core.containerpool.PrewarmingConfig
-import org.apache.openwhisk.core.entity.{CodeExecAsString, ExecManifest}
-import org.apache.openwhisk.http.BasicRasService
+import org.apache.openwhisk.common.{Logging, TransactionId}
 
-import pureconfig._
-import pureconfig.generic.auto._
+import org.apache.openwhisk.http.BasicRasService
 
 import scala.concurrent.ExecutionContext
 
@@ -40,38 +33,10 @@ class DefaultInvokerServer(val invoker: InvokerCore)(implicit val ec: ExecutionC
                                                      val logger: Logging)
     extends BasicRasService {
 
-  private val invokerCredentials = loadConfigOrThrow[InvokerCredentials](ConfigKeys.invokerCredentials)
-
   override def routes(implicit transid: TransactionId): Route = {
     super.routes ~ {
-      (path("config" / "runtime") & post) {
-        extractCredentials {
-          case Some(BasicHttpCredentials(username, password)) =>
-            if (username == invokerCredentials.username && password == invokerCredentials.password) {
-              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")
-                } else {
-                  val prewarmingConfigs: List[PrewarmingConfig] = execManifest.get.stemcells.flatMap {
-                    case (mf, cells) =>
-                      cells.map { cell =>
-                        PrewarmingConfig(cell.count, new CodeExecAsString(mf, "", None), cell.memory)
-                      }
-                  }.toList
-                  invoker.configRuntime(prewarmingConfigs)
-                }
-              }
-            } else {
-              complete("username or password is wrong")
-            }
-          case _ => complete(StatusCodes.Unauthorized)
-        }
-      } ~ {
-        (path("getRuntime") & get) {
-          invoker.getRuntime()
-        }
+      (path("getRuntime") & get) {
+        invoker.getRuntime()
       }
     }
   }

diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/Invoker.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/invoker/Invoker.scala
@@ -27,7 +27,7 @@ import org.apache.openwhisk.common.Https.HttpsConfig
 import org.apache.openwhisk.common._
 import org.apache.openwhisk.core.WhiskConfig._
 import org.apache.openwhisk.core.connector.{MessageProducer, MessagingProvider}
-import org.apache.openwhisk.core.containerpool.{Container, ContainerPoolConfig, PrewarmingConfig}
+import org.apache.openwhisk.core.containerpool.{Container, ContainerPoolConfig}
 import org.apache.openwhisk.core.entity._
 import org.apache.openwhisk.core.entity.size._
 import org.apache.openwhisk.core.{ConfigKeys, WhiskConfig}
@@ -213,7 +213,6 @@ trait InvokerProvider extends Spi {
 // this trait can be used to add common implementation
 trait InvokerCore {
   def getRuntime(): Route
-  def configRuntime(prewarmConfigList: List[PrewarmingConfig]): Route
 }
 
 /**

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
@@ -323,11 +323,6 @@ class InvokerReactive(
     }
   })
 
-  override def configRuntime(prewarmConfigList: List[PrewarmingConfig]): Route = {
-    pool ! PreWarmConfigList(prewarmConfigList)
-    complete(s"config runtime request is handling")
-  }
-
   override def getRuntime(): Route = {
     complete {
       pool

diff --git a/docs/operation.md b/docs/operation.md
@@ -18,33 +18,6 @@
 -->
 
 # Runtime configuration
-## Change runtime on assigned invoker, e.g:
-```
-curl -u ${username}:${password} -X POST http://${invokerAddress}:${invokerPort}/config/runtime -d '
-{
- "runtimes": {
-  "nodejs": [{
-   "kind": "nodejs:10",
-   "default": true,
-   "image": {
-    "prefix": "openwhisk",
-    "name": "action-nodejs-v10",
-    "tag": "nightly"
-   },
-   "deprecated": false,
-   "attached": {
-    "attachmentName": "codefile",
-    "attachmentType": "text/plain"
-   },
-   "stemCells": [{
-    "count": 2,
-    "memory": "128 MB"
-   }]
-  }]
- }
-}
-'
-```
 ## Change runtime to all managed invokers via controller. e.g.
 ```
 curl -u ${username}:${password} -X POST http://${controllerAddress}:${controllerPort}/config/runtime -d '{...}'

diff --git a/tests/src/test/resources/application.conf.j2 b/tests/src/test/resources/application.conf.j2
@@ -92,6 +92,13 @@ whisk {
             url = "{{ user_images_registry | default('') }}"
         }
     }
+
+    credentials {
+        controller {
+            username = "{{ controller.username }}"
+            password = "{{ controller.password }}"
+        }
+    }
 }
 
 #test-only overrides so that tests can override defaults in application.conf (todo: move all defaults to reference.conf)

diff --git 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];
     }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -18,5 +18,3 @@
		package org.apache.openwhisk.common

		case class ControllerCredentials(username: String, password: String)

		case class InvokerCredentials(username: String, password: String)