additional container factory dns configuration (#4176)

Allow configuration of the --dns-search and --dns-opt (--dns-option)
arguments to the docker run command of the DockerContainerFactory.
Motivated by better support of event providers in Kubernetes when
using the DockerContainerFactory.

Author: dgrove-oss

common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerFactory.scala

@@ -27,6 +27,8 @@ import scala.concurrent.Future
 
 case class ContainerArgsConfig(network: String,
                                dnsServers: Seq[String] = Seq.empty,
+                               dnsSearch: Seq[String] = Seq.empty,
+                               dnsOptions: Seq[String] = Seq.empty,
                                extraArgs: Map[String, Set[String]] = Map.empty)
 
 case class ContainerPoolConfig(userMemory: ByteSize, concurrentPeekFactor: Double, akkaClient: Boolean) {

core/invoker/src/main/resources/application.conf

@@ -28,6 +28,7 @@ whisk {
       inspect: 1 minute
       pause: 10 seconds
       unpause: 10 seconds
+      version: 10 seconds
     }
   }
 
@@ -68,7 +69,10 @@ whisk {
   # args for 'docker run' to use
   container-factory.container-args {
     network: bridge
+    # See https://docs.docker.com/config/containers/container-networking/#dns-services for documentation of dns-*
     dns-servers: []
+    dns-search: []
+    dns-options: []
     extra-args: {}   # to pass additional args to 'docker run'; format is `{key1: [v1, v2], key2: [v1, v2]}`
   }
 

core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerClient.scala

@@ -27,7 +27,7 @@ import akka.actor.ActorSystem
 import scala.collection.concurrent.TrieMap
 import scala.concurrent.blocking
 import scala.concurrent.ExecutionContext
-import scala.concurrent.Future
+import scala.concurrent.{Await, Future}
 import scala.util.Failure
 import scala.util.Success
 import scala.util.Try
@@ -61,6 +61,7 @@ case class DockerClientTimeoutConfig(run: Duration,
                                      ps: Duration,
                                      pause: Duration,
                                      unpause: Duration,
+                                     version: Duration,
                                      inspect: Duration)
 
 /**
@@ -98,6 +99,20 @@ class DockerClient(dockerHost: Option[String] = None,
     Seq(dockerBin) ++ host
   }
 
+  // Invoke docker CLI to determine client version.
+  // If the docker client version cannot be determined, an exception will be thrown and instance initialization will fail.
+  // Rationale: if we cannot invoke `docker version` successfully, it is unlikely subsequent `docker` invocations will succeed.
+  protected def getClientVersion(): String = {
+    val vf = executeProcess(dockerCmd ++ Seq("version", "--format", "{{.Client.Version}}"), config.timeouts.version)
+      .andThen {
+        case Success(version) => log.info(this, s"Detected docker client version $version")
+        case Failure(e) =>
+          log.error(this, s"Failed to determine docker client version: ${e.getClass} - ${e.getMessage}")
+      }
+    Await.result(vf, 2 * config.timeouts.version)
+  }
+  val clientVersion: String = getClientVersion()
+
   protected val maxParallelRuns = config.parallelRuns
   protected val runSemaphore =
     new Semaphore( /* permits= */ if (maxParallelRuns > 0) maxParallelRuns else Int.MaxValue, /* fair= */ true)
@@ -197,6 +212,13 @@ class DockerClient(dockerHost: Option[String] = None,
 
 trait DockerApi {
 
+  /**
+   * The version number of the docker client cli
+   *
+   * @return The version of the docker client cli being used by the invoker
+   */
+  def clientVersion: String
+
   /**
    * Spawns a container in detached mode.
    *

core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainer.scala

@@ -65,6 +65,8 @@ object DockerContainer {
              environment: Map[String, String] = Map.empty,
              network: String = "bridge",
              dnsServers: Seq[String] = Seq.empty,
+             dnsSearch: Seq[String] = Seq.empty,
+             dnsOptions: Seq[String] = Seq.empty,
              name: Option[String] = None,
              useRunc: Boolean = true,
              dockerRunParameters: Map[String, Set[String]])(implicit docker: DockerApiWithFileAccess,
@@ -82,6 +84,8 @@ object DockerContainer {
       case (key, valueList) => valueList.toList.flatMap(Seq(key, _))
     }
 
+    // NOTE: --dns-option on modern versions of docker, but is --dns-opt on docker 1.12
+    val dnsOptString = if (docker.clientVersion.startsWith("1.12")) { "--dns-opt" } else { "--dns-option" }
     val args = Seq(
       "--cpu-shares",
       cpuShares.toString,
@@ -93,6 +97,8 @@ object DockerContainer {
       network) ++
       environmentArgs ++
       dnsServers.flatMap(d => Seq("--dns", d)) ++
+      dnsSearch.flatMap(d => Seq("--dns-search", d)) ++
+      dnsOptions.flatMap(d => Seq(dnsOptString, d)) ++
       name.map(n => Seq("--name", n)).getOrElse(Seq.empty) ++
       params
 

core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala

@@ -69,6 +69,8 @@ class DockerContainerFactory(instance: InvokerInstanceId,
       environment = Map("__OW_API_HOST" -> config.wskApiHost),
       network = containerArgsConfig.network,
       dnsServers = containerArgsConfig.dnsServers,
+      dnsSearch = containerArgsConfig.dnsSearch,
+      dnsOptions = containerArgsConfig.dnsOptions,
       name = Some(name),
       useRunc = dockerContainerFactoryConfig.useRunc,
       parameters ++ containerArgsConfig.extraArgs.map { case (k, v) => ("--" + k, v) })

tests/src/test/scala/org/apache/openwhisk/core/containerpool/docker/test/DockerClientTests.scala

@@ -67,6 +67,7 @@ class DockerClientTests
   /** Returns a DockerClient with a mocked result for 'executeProcess' */
   def dockerClient(execResult: => Future[String]) = new DockerClient()(global) {
     override val dockerCmd = Seq(dockerCommand)
+    override def getClientVersion() = "mock-test-client"
     override def executeProcess(args: Seq[String], timeout: Duration)(implicit ec: ExecutionContext, as: ActorSystem) =
       execResult
   }
@@ -189,6 +190,7 @@ class DockerClientTests
     var runCmdCount = 0
     val dc = new DockerClient()(global) {
       override val dockerCmd = Seq(dockerCommand)
+      override def getClientVersion() = "mock-test-client"
       override def executeProcess(args: Seq[String], timeout: Duration)(implicit ec: ExecutionContext,
                                                                         as: ActorSystem) = {
         runCmdCount += 1
@@ -237,6 +239,7 @@ class DockerClientTests
     var runCmdCount = 0
     val dc = new DockerClient()(global) {
       override val dockerCmd = Seq(dockerCommand)
+      override def getClientVersion() = "mock-test-client"
       override def executeProcess(args: Seq[String], timeout: Duration)(implicit ec: ExecutionContext,
                                                                         as: ActorSystem) = {
         runCmdCount += 1

tests/src/test/scala/org/apache/openwhisk/core/containerpool/docker/test/DockerClientWithFileAccessTests.scala

@@ -73,6 +73,7 @@ class DockerClientWithFileAccessTestsIp
                    readResult: Future[JsObject] = Future.successful(dockerConfig)) =
     new DockerClientWithFileAccess()(global) {
       override val dockerCmd = Seq(dockerCommand)
+      override def getClientVersion() = "mock-test-client"
       override def executeProcess(args: Seq[String], timeout: Duration)(implicit ec: ExecutionContext,
                                                                         as: ActorSystem) = execResult
       override def configFileContents(configFile: File) = readResult
@@ -129,6 +130,7 @@ class DockerClientWithFileAccessTestsOom
   def dockerClient(readResult: Future[JsObject]) =
     new DockerClientWithFileAccess()(global) {
       override val dockerCmd = Seq("docker")
+      override def getClientVersion() = "mock-test-client"
       override def configFileContents(configFile: File) = readResult
     }
 

tests/src/test/scala/org/apache/openwhisk/core/containerpool/docker/test/DockerContainerFactoryTests.scala

@@ -102,12 +102,16 @@ class DockerContainerFactoryTests
       .rm(_: ContainerId)(_: TransactionId))
       .expects(ContainerId("fakecontainerid"), *)
       .returning(Future.successful(Unit))
+    //setup clientVersion exceptation
+    (dockerApiStub.clientVersion _)
+      .expects()
+      .returning("mock_test_client")
 
     val factory =
       new DockerContainerFactory(
         InvokerInstanceId(0, userMemory = defaultUserMemory),
         Map.empty,
-        ContainerArgsConfig("net1", Seq("dns1", "dns2"), Map("env" -> Set("e1", "e2"))),
+        ContainerArgsConfig("net1", Seq("dns1", "dns2"), Seq.empty, Seq.empty, Map("env" -> Set("e1", "e2"))),
         DockerContainerFactoryConfig(true))(actorSystem, executionContext, logging, dockerApiStub, mock[RuncApi])
 
     val cf = factory.createContainer(tid, "testContainer", image, false, 10.MB, 32)

tests/src/test/scala/org/apache/openwhisk/core/containerpool/docker/test/DockerContainerTests.scala

@@ -807,6 +807,8 @@ class DockerContainerTests
     var pulls = mutable.Buffer.empty[String]
     var rawContainerLogsInvocations = mutable.Buffer.empty[(ContainerId, Long, Option[FiniteDuration])]
 
+    def clientVersion: String = "mock-test-client"
+
     def run(image: String, args: Seq[String] = Seq.empty[String])(
       implicit transid: TransactionId): Future[ContainerId] = {
       runs += ((image, args))

tests/src/test/scala/org/apache/openwhisk/core/containerpool/mesos/test/MesosContainerFactoryTest.scala

@@ -86,7 +86,12 @@ class MesosContainerFactoryTest
   val mesosCpus = poolConfig.cpuShare(actionMemory) / 1024.0
 
   val containerArgsConfig =
-    new ContainerArgsConfig("net1", Seq("dns1", "dns2"), Map("extra1" -> Set("e1", "e2"), "extra2" -> Set("e3", "e4")))
+    new ContainerArgsConfig(
+      "net1",
+      Seq("dns1", "dns2"),
+      Seq.empty,
+      Seq.empty,
+      Map("extra1" -> Set("e1", "e2"), "extra2" -> Set("e3", "e4")))
 
   override def beforeEach() = {
     stream.reset()
@@ -247,7 +252,12 @@ class MesosContainerFactoryTest
         system,
         logging,
         Map("--arg1" -> Set("v1", "v2"), "--arg2" -> Set("v3", "v4"), "other" -> Set("v5", "v6")),
-        new ContainerArgsConfig("bridge", Seq.empty, Map("extra1" -> Set("e1", "e2"), "extra2" -> Set("e3", "e4"))),
+        new ContainerArgsConfig(
+          "bridge",
+          Seq.empty,
+          Seq.empty,
+          Seq.empty,
+          Map("extra1" -> Set("e1", "e2"), "extra2" -> Set("e3", "e4"))),
         mesosConfig,
         (system, mesosConfig) => probe.testActor,
         testTaskId _)