[GitHub] csantanapr commented on a change in pull request #3616: Use ‘docker.host’ java property as a way to set Docker access for Runtime unit tests

[GitBox <gi...@apache.org>]

csantanapr commented on a change in pull request #3616: Use ‘docker.host’ java property as a way to set Docker access for Runtime unit tests
URL: https://github.com/apache/incubator-openwhisk/pull/3616#discussion_r186275965
 
 

 ##########
 File path: tests/src/test/scala/actionContainers/ActionContainer.scala
 ##########
 @@ -88,12 +88,37 @@ object ActionContainer {
   }
 
   private lazy val dockerCmd: String = {
-    val version = WhiskProperties.getProperty("whisk.version.name")
-    // Check if we are running on docker-machine env.
-    val hostStr = if (version.toLowerCase().contains("mac")) {
-      s" --host tcp://${WhiskProperties.getMainDockerEndpoint} "
+    /*
+     * The docker host is set to a provided property 'docker.host' if it's
+     * available; otherwise we check with WhiskProperties to see whether we are
+     * running on a docker-machine.
+     *
+     * IMPLICATION:  The test must EITHER have the 'docker.host' system
+     * property set OR the 'OPENWHISK_HOME' environment variable set and a
+     * valid 'whisk.properties' file generated.  The 'docker.host' system
+     * property takes precedence.
+     *
+     * WARNING:  Adding a non-docker-machine environment that contains 'mac'
+     * (i.e. 'environments/local-mac') will likely break things.
+     *
+     * The plan is to move builds to using 'gradle-docker-plugin', which know
+     * its docker socket and to have it pass the docker socket implicitly using
+     * 'systemProperty "docker.host", docker.url'.  Eventually, we will also
+     * need to handle TLS certificates here.  Again, 'gradle-docker-plugin'
+     * knows where they are; we will just add system properties to get the
+     * information onto the docker command line.
+     */
+    val dockerUrl = Option(System.getProperty("docker.host"))
+    val hostStr = if (dockerUrl.isDefined) {
+      s" --host ${dockerUrl.get} "
     } else {
-      " "
+      val version = WhiskProperties.getProperty("whisk.version.name")
+      // Check if we are running on docker-machine env.
+      val hostStr = if (version.toLowerCase().contains("mac")) {
 
 Review comment:
   remove `val hostStr`
   ```
   if (version.toLowerCase().contains("mac")) {
           s" --host tcp://${WhiskProperties.getMainDockerEndpoint()} "
         } else {
           " "
         }
   ```

----------------------------------------------------------------
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