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 d7e59b5  use PureConfig for misc. runtimesManifest configuration 
(#3259)
d7e59b5 is described below

commit d7e59b5549525bbe68c1a0c8b8b28db91844b2d9
Author: David Grove <dgrove-...@users.noreply.github.com>
AuthorDate: Mon Feb 12 13:01:12 2018 -0500

    use PureConfig for misc. runtimesManifest configuration (#3259)
    
    Refactor runtimes.json so that the misc. simple configuration
    fields are specified using PureConfig (whisk.runtimes),
    leaving only the more complex arrays of runtimes and blackboxes
    in runtime.json.  Since we expect deployments to frequently
    override the set of runtimes, it makes sense to leave them as
    being defined by a json file that is not included in the
    Invoker/Controller docker image (simplify override).  The flags
    moved to PureConfig are simple fields, and thus easier to override
    via envvars if necessary.
    
    Restores the ability to easily override bypassPullForLocalImages.
    
    Fixes #3245.
---
 ansible/environments/docker-machine/group_vars/all |  2 +-
 ansible/environments/local/group_vars/all          |  2 +-
 ansible/files/runtimes.json                        |  3 --
 ansible/group_vars/all                             | 12 +-----
 ansible/roles/controller/tasks/deploy.yml          |  5 +++
 ansible/roles/invoker/tasks/deploy.yml             |  4 ++
 common/scala/src/main/resources/application.conf   |  7 ++++
 .../src/main/scala/whisk/core/WhiskConfig.scala    |  2 +
 .../scala/whisk/core/entity/ExecManifest.scala     | 37 ++++++++++++------
 .../main/scala/whisk/core/invoker/Invoker.scala    |  2 +-
 .../whisk/core/entity/test/ExecManifestTests.scala | 44 +++++++---------------
 11 files changed, 62 insertions(+), 58 deletions(-)

diff --git a/ansible/environments/docker-machine/group_vars/all 
b/ansible/environments/docker-machine/group_vars/all
index efd0b56..6f68e73 100644
--- a/ansible/environments/docker-machine/group_vars/all
+++ b/ansible/environments/docker-machine/group_vars/all
@@ -3,7 +3,7 @@ config_root_dir: /Users/Shared/wskconf
 whisk_logs_dir: /Users/Shared/wsklogs
 docker_registry: ""
 docker_dns: ""
-bypass_pull_for_local_images: true
+runtimes_bypass_pull_for_local_images: true
 
 env_hosts_dir: "{{ playbook_dir }}/environments/docker-machine"
 
diff --git a/ansible/environments/local/group_vars/all 
b/ansible/environments/local/group_vars/all
index fbed10e..bcec410 100755
--- a/ansible/environments/local/group_vars/all
+++ b/ansible/environments/local/group_vars/all
@@ -4,7 +4,7 @@ config_root_dir: "{{ openwhisk_tmp_dir }}/wskconf"
 whisk_logs_dir: "{{ openwhisk_tmp_dir }}/wsklogs"
 docker_registry: ""
 docker_dns: ""
-bypass_pull_for_local_images: true
+runtimes_bypass_pull_for_local_images: true
 
 db_prefix: whisk_local_
 
diff --git a/ansible/files/runtimes.json b/ansible/files/runtimes.json
index 8030fa2..42858d6 100644
--- a/ansible/files/runtimes.json
+++ b/ansible/files/runtimes.json
@@ -1,7 +1,4 @@
 {
-    "bypassPullForLocalImages": false,
-    "defaultImagePrefix": "openwhisk",
-    "defaultImageTag": "latest",
     "runtimes": {
         "nodejs": [
             {
diff --git a/ansible/group_vars/all b/ansible/group_vars/all
index 6978fb9..54c2d68 100644
--- a/ansible/group_vars/all
+++ b/ansible/group_vars/all
@@ -24,16 +24,8 @@ whisk:
   version:
     date: "{{ansible_date_time.iso8601}}"
 
-##
-# list of supported runtimes (see whisk.core.entity.ExecManifest for schema).
-# briefly:
-#   defaultImagePrefix: the default image prefix when not given explicitly
-#   defaultImageTag: the default image tag
-#   runtimes: set of language runtime families grouped by language (e.g., 
nodejs, python)
-#   blackboxes: list of pre-populated docker action images as "name" with 
optional "prefix" and "tag"
-#   bypassPullForLocalImages: optional, if true, allow images with a prefix 
that matches {{ docker.image.prefix }}
-#                             to skip docker pull in invoker even if the image 
is not part of the blackbox set
-#
+
+# list of supported runtimes (see whisk.core.entity.ExecManifest for schema)
 runtimesManifest: "{{ runtimes_manifest | default(lookup('file', '{{ 
openwhisk_home }}/ansible/files/runtimes.json') | from_json) }}"
 
 limits:
diff --git a/ansible/roles/controller/tasks/deploy.yml 
b/ansible/roles/controller/tasks/deploy.yml
index 8601a29..9c19dd4 100644
--- a/ansible/roles/controller/tasks/deploy.yml
+++ b/ansible/roles/controller/tasks/deploy.yml
@@ -133,6 +133,11 @@
       "CONFIG_whisk_activation_payload_max": "{{ limit_activation_payload | 
default() }}"
 
       "RUNTIMES_MANIFEST": "{{ runtimesManifest | to_json }}"
+      "CONFIG_whisk_runtimes_defaultImagePrefix": "{{ 
runtimes_default_image_prefix | default() }}"
+      "CONFIG_whisk_runtimes_defaultImageTag": "{{ runtimes_default_image_tag 
| default() }}"
+      "CONFIG_whisk_runtimes_bypassPullForLocalImages": "{{ 
runtimes_bypass_pull_for_local_images | default() }}"
+      "CONFIG_whisk_runtimes_localImagePrefix": "{{ 
runtimes_local_image_prefix | default() }}"
+
       "CONTROLLER_LOCALBOOKKEEPING": "{{ controller.localBookkeeping }}"
       "AKKA_CLUSTER_SEED_NODES": "{{seed_nodes_list | join(' ') }}"
       "CONTROLLER_HA": "{{ controller.ha }}"
diff --git a/ansible/roles/invoker/tasks/deploy.yml 
b/ansible/roles/invoker/tasks/deploy.yml
index c215e39..be2a251 100644
--- a/ansible/roles/invoker/tasks/deploy.yml
+++ b/ansible/roles/invoker/tasks/deploy.yml
@@ -165,6 +165,10 @@
         -e WHISK_API_HOST_PORT='{{ whisk_api_host_port | default('443') }}'
         -e WHISK_API_HOST_NAME='{{ whisk_api_host_name | 
default(groups['edge'] | first) }}'
         -e RUNTIMES_MANIFEST='{{ runtimesManifest | to_json }}'
+        -e CONFIG_whisk_runtimes_defaultImagePrefix='{{ 
runtimes_default_image_prefix | default() }}'
+        -e CONFIG_whisk_runtimes_defaultImageTag='{{ 
runtimes_default_image_tag | default() }}'
+        -e CONFIG_whisk_runtimes_bypassPullForLocalImages='{{ 
runtimes_bypass_pull_for_local_images | default() }}'
+        -e CONFIG_whisk_runtimes_localImagePrefix='{{ 
runtimes_local_image_prefix | default() }}'
         -e DOCKER_REGISTRY='{{ docker_registry }}'
         -e DOCKER_IMAGE_PREFIX='{{ docker.image.prefix }}'
         -e DOCKER_IMAGE_TAG='{{ docker.image.tag }}'
diff --git a/common/scala/src/main/resources/application.conf 
b/common/scala/src/main/resources/application.conf
index 97b1e63..8468286 100644
--- a/common/scala/src/main/resources/application.conf
+++ b/common/scala/src/main/resources/application.conf
@@ -94,6 +94,13 @@ whisk {
         stride = 1
         stride = ${?CONTROLLER_INSTANCES}
     }
+    # action runtimes configuration
+    runtimes {
+        default-image-prefix = "openwhisk"
+        default-image-tag = "latest"
+        bypass-pull-for-local-images = false
+        local-image-prefix = "whisk"
+    }
 
     activation {
         payload {
diff --git a/common/scala/src/main/scala/whisk/core/WhiskConfig.scala 
b/common/scala/src/main/scala/whisk/core/WhiskConfig.scala
index 22dfe23..5d3fada 100644
--- a/common/scala/src/main/scala/whisk/core/WhiskConfig.scala
+++ b/common/scala/src/main/scala/whisk/core/WhiskConfig.scala
@@ -245,6 +245,8 @@ object ConfigKeys {
   val activation = "whisk.activation"
   val activationPayload = s"$activation.payload"
 
+  val runtimes = "whisk.runtimes"
+
   val db = "whisk.db"
 
   val docker = "whisk.docker"
diff --git a/common/scala/src/main/scala/whisk/core/entity/ExecManifest.scala 
b/common/scala/src/main/scala/whisk/core/entity/ExecManifest.scala
index dc44691..668ef60 100644
--- a/common/scala/src/main/scala/whisk/core/entity/ExecManifest.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/ExecManifest.scala
@@ -17,10 +17,12 @@
 
 package whisk.core.entity
 
+import pureconfig.loadConfigOrThrow
+
 import scala.util.{Failure, Try}
 import spray.json._
 import spray.json.DefaultJsonProtocol._
-import whisk.core.WhiskConfig
+import whisk.core.{ConfigKeys, WhiskConfig}
 import whisk.core.entity.Attachments._
 import whisk.core.entity.Attachments.Attached._
 
@@ -41,11 +43,11 @@ protected[core] object ExecManifest {
    * singleton Runtime instance.
    *
    * @param config a valid configuration
-   * @param localDockerImagePrefix optional local docker prefix, permitting 
images matching prefix to bypass docker pull
    * @return the manifest if initialized successfully, or an failure
    */
-  protected[core] def initialize(config: WhiskConfig, localDockerImagePrefix: 
Option[String] = None): Try[Runtimes] = {
-    val mf = 
Try(config.runtimesManifest.parseJson.asJsObject).flatMap(runtimes(_, 
localDockerImagePrefix))
+  protected[core] def initialize(config: WhiskConfig): Try[Runtimes] = {
+    val rmc = loadConfigOrThrow[RuntimeManifestConfig](ConfigKeys.runtimes)
+    val mf = 
Try(config.runtimesManifest.parseJson.asJsObject).flatMap(runtimes(_, rmc))
     mf.foreach(m => manifest = Some(m))
     mf
   }
@@ -69,9 +71,10 @@ protected[core] object ExecManifest {
    * @param config a configuration object as JSON
    * @return Runtimes instance
    */
-  protected[entity] def runtimes(config: JsObject, localDockerImagePrefix: 
Option[String] = None): Try[Runtimes] = Try {
-    val prefix = 
config.fields.get("defaultImagePrefix").map(_.convertTo[String])
-    val tag = config.fields.get("defaultImageTag").map(_.convertTo[String])
+  protected[entity] def runtimes(config: JsObject, runtimeManifestConfig: 
RuntimeManifestConfig): Try[Runtimes] = Try {
+
+    val prefix = runtimeManifestConfig.defaultImagePrefix
+    val tag = runtimeManifestConfig.defaultImageTag
 
     val runtimes = config.fields
       .get("runtimes")
@@ -89,17 +92,27 @@ protected[core] object ExecManifest {
         ImageName(image.name, image.prefix.orElse(prefix), 
image.tag.orElse(tag))
       })
 
-    val bypassPullForLocalImages = config.fields
-      .get("bypassPullForLocalImages")
-      .map(_.convertTo[Boolean])
+    val bypassPullForLocalImages = 
runtimeManifestConfig.bypassPullForLocalImages
       .filter(identity)
-      .flatMap(_ => localDockerImagePrefix)
-      .map(_.trim)
+      .flatMap(_ => runtimeManifestConfig.localImagePrefix)
 
     Runtimes(runtimes.getOrElse(Set.empty), blackbox.getOrElse(Set.empty), 
bypassPullForLocalImages)
   }
 
   /**
+   * Misc options related to runtime manifests
+   * @param defaultImagePrefix the default image prefix when not given 
explicitly
+   * @param defaultImageTag the default image tag
+   * @param bypassPullForLocalImages if true, allow images with a prefix that 
matches localImagePrefix
+   *                                 to skip docker pull in invoker even if 
the image is not part of the blackbox set
+   * @param localImagePrefix image prefix for bypassPullForLocalImages
+   */
+  protected[core] case class RuntimeManifestConfig(defaultImagePrefix: 
Option[String] = None,
+                                                   defaultImageTag: 
Option[String] = None,
+                                                   bypassPullForLocalImages: 
Option[Boolean] = None,
+                                                   localImagePrefix: 
Option[String] = None)
+
+  /**
    * A runtime manifest describes the "exec" runtime support.
    *
    * @param kind the name of the kind e.g., nodejs:6
diff --git a/core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala 
b/core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala
index ae15e90..4ba2214 100644
--- a/core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala
+++ b/core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala
@@ -98,7 +98,7 @@ object Invoker {
       abort("Bad configuration, cannot start.")
     }
 
-    val execManifest = ExecManifest.initialize(config, localDockerImagePrefix 
= Some(config.dockerImagePrefix))
+    val execManifest = ExecManifest.initialize(config)
     if (execManifest.isFailure) {
       logger.error(this, s"Invalid runtimes manifest: 
${execManifest.failed.get}")
       abort("Bad configuration, cannot start.")
diff --git 
a/tests/src/test/scala/whisk/core/entity/test/ExecManifestTests.scala 
b/tests/src/test/scala/whisk/core/entity/test/ExecManifestTests.scala
index d232081..71f0237 100644
--- a/tests/src/test/scala/whisk/core/entity/test/ExecManifestTests.scala
+++ b/tests/src/test/scala/whisk/core/entity/test/ExecManifestTests.scala
@@ -17,15 +17,12 @@
 
 package whisk.core.entity.test
 
-import java.io.{BufferedWriter, File, FileWriter}
-
 import common.{StreamLogging, WskActorSystem}
 import org.junit.runner.RunWith
 import org.scalatest.{FlatSpec, Matchers}
 import org.scalatest.junit.JUnitRunner
 import spray.json.DefaultJsonProtocol._
 import spray.json._
-import whisk.core.WhiskConfig
 import whisk.core.entity.ExecManifest
 import whisk.core.entity.ExecManifest._
 
@@ -67,7 +64,7 @@ class ExecManifestTests extends FlatSpec with WskActorSystem 
with StreamLogging
     val k2 = RuntimeManifest("k2", ImageName("???"), default = Some(true))
     val p1 = RuntimeManifest("p1", ImageName("???"))
     val mf = manifestFactory(JsObject("ks" -> Set(k1, k2).toJson, "p1" -> 
Set(p1).toJson))
-    val runtimes = ExecManifest.runtimes(mf).get
+    val runtimes = ExecManifest.runtimes(mf, RuntimeManifestConfig()).get
 
     Seq("k1", "k2", "p1").foreach {
       runtimes.knownContainerRuntimes.contains(_) shouldBe true
@@ -89,11 +86,10 @@ class ExecManifestTests extends FlatSpec with 
WskActorSystem with StreamLogging
     val j1 = RuntimeManifest("j1", ImageName("???", Some("ppp"), Some("ttt")))
     val k1 = RuntimeManifest("k1", ImageName("???", None, Some("ttt")))
 
-    val mf = JsObject(
-      "defaultImagePrefix" -> "pre".toJson,
-      "defaultImageTag" -> "test".toJson,
-      "runtimes" -> JsObject("is" -> Set(i1, i2).toJson, "js" -> 
Set(j1).toJson, "ks" -> Set(k1).toJson))
-    val runtimes = ExecManifest.runtimes(mf).get
+    val mf =
+      JsObject("runtimes" -> JsObject("is" -> Set(i1, i2).toJson, "js" -> 
Set(j1).toJson, "ks" -> Set(k1).toJson))
+    val rmc = RuntimeManifestConfig(defaultImagePrefix = Some("pre"), 
defaultImageTag = Some("test"))
+    val runtimes = ExecManifest.runtimes(mf, rmc).get
 
     runtimes.resolveDefaultRuntime("i1").get.image.publicImageName shouldBe 
"pre/???:test"
     runtimes.resolveDefaultRuntime("i2").get.image.publicImageName shouldBe 
"ppp/???:test"
@@ -109,7 +105,7 @@ class ExecManifestTests extends FlatSpec with 
WskActorSystem with StreamLogging
       ImageName("???", None, Some("ttt")))
 
     val mf = JsObject("runtimes" -> JsObject(), "blackboxes" -> imgs.toJson)
-    val runtimes = ExecManifest.runtimes(mf).get
+    val runtimes = ExecManifest.runtimes(mf, RuntimeManifestConfig()).get
 
     runtimes.blackboxImages shouldBe imgs
     imgs.foreach(img => runtimes.skipDockerPull(img) shouldBe true)
@@ -123,12 +119,9 @@ class ExecManifestTests extends FlatSpec with 
WskActorSystem with StreamLogging
       ImageName("???", Some("ppp"), Some("ttt")),
       ImageName("???", None, Some("ttt")))
 
-    val mf = JsObject(
-      "defaultImagePrefix" -> "pre".toJson,
-      "defaultImageTag" -> "test".toJson,
-      "runtimes" -> JsObject(),
-      "blackboxes" -> imgs.toJson)
-    val runtimes = ExecManifest.runtimes(mf).get
+    val mf = JsObject("runtimes" -> JsObject(), "blackboxes" -> imgs.toJson)
+    val rmc = RuntimeManifestConfig(defaultImagePrefix = Some("pre"), 
defaultImageTag = Some("test"))
+    val runtimes = ExecManifest.runtimes(mf, rmc).get
 
     runtimes.blackboxImages shouldBe {
       Set(
@@ -147,7 +140,7 @@ class ExecManifestTests extends FlatSpec with 
WskActorSystem with StreamLogging
     val k2 = RuntimeManifest("k2", ImageName("???"), default = Some(true))
     val mf = manifestFactory(JsObject("ks" -> Set(k1, k2).toJson))
 
-    an[IllegalArgumentException] should be thrownBy 
ExecManifest.runtimes(mf).get
+    an[IllegalArgumentException] should be thrownBy ExecManifest.runtimes(mf, 
RuntimeManifestConfig()).get
   }
 
   it should "reject finding a default when none is specified for multiple 
versions" in {
@@ -155,7 +148,7 @@ class ExecManifestTests extends FlatSpec with 
WskActorSystem with StreamLogging
     val k2 = RuntimeManifest("k2", ImageName("???"))
     val mf = manifestFactory(JsObject("ks" -> Set(k1, k2).toJson))
 
-    an[IllegalArgumentException] should be thrownBy 
ExecManifest.runtimes(mf).get
+    an[IllegalArgumentException] should be thrownBy ExecManifest.runtimes(mf, 
RuntimeManifestConfig()).get
   }
 
   it should "prefix image name with overrides" in {
@@ -179,18 +172,9 @@ class ExecManifestTests extends FlatSpec with 
WskActorSystem with StreamLogging
   }
 
   it should "indicate image is local if it matches deployment docker prefix" 
in {
-    val config_manifest = """{"bypassPullForLocalImages":true}"""
-    val file = File.createTempFile("cxt", ".txt")
-    file.deleteOnExit()
-
-    val bw = new BufferedWriter(new FileWriter(file))
-    bw.write(WhiskConfig.runtimesManifest + s"=$config_manifest\n")
-    bw.close()
-
-    val props = Map(WhiskConfig.runtimesManifest -> null)
-    val manifest =
-      ExecManifest.initialize(new WhiskConfig(props, Set(), file), 
localDockerImagePrefix = Some("localpre"))
-    manifest should be a 'success
+    val mf = JsObject()
+    val rmc = RuntimeManifestConfig(bypassPullForLocalImages = Some(true), 
localImagePrefix = Some("localpre"))
+    val manifest = ExecManifest.runtimes(mf, rmc)
 
     manifest.get.skipDockerPull(ImageName(prefix = Some("x"), name = "y")) 
shouldBe false
     manifest.get.skipDockerPull(ImageName(prefix = Some("localpre"), name = 
"y")) shouldBe true

-- 
To stop receiving notification emails like this one, please contact
rab...@apache.org.

Reply via email to