This is an automated email from the ASF dual-hosted git repository. markusthoemmes pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/openwhisk.git
The following commit(s) were added to refs/heads/master by this push: new 9bef49f KCF: propagate cf_ca_extraArgs_env_i into user containers (#4570) 9bef49f is described below commit 9bef49fcd47f7922e4226ff7a97385f1313f7d64 Author: David Grove <dgrove-...@users.noreply.github.com> AuthorDate: Wed Aug 7 11:22:17 2019 -0400 KCF: propagate cf_ca_extraArgs_env_i into user containers (#4570) * KCF: propagate cf_ca_extraArgs_env_i into user containers Partial fix for #4569. Add logic to KubernetesContainerFactory to propagate the set of key=value pairs) from containerFactory.containerArgs.extraArgs.envN into the environment of all action containers. * review feedback: generalize by adding explicit extraEnvVars to conf. * restore testing of extra-args --- ansible/roles/invoker/tasks/deploy.yml | 2 +- .../core/containerpool/ContainerFactory.scala | 13 +++++++++++- .../core/mesos/MesosContainerFactory.scala | 2 +- core/invoker/src/main/resources/application.conf | 1 + .../docker/DockerContainerFactory.scala | 2 +- .../kubernetes/KubernetesContainerFactory.scala | 4 +++- .../docker/test/DockerContainerFactoryTests.scala | 24 ++++++++++++++++++---- .../mesos/test/MesosContainerFactoryTest.scala | 2 ++ .../yarn/test/YARNContainerFactoryTests.scala | 1 + 9 files changed, 42 insertions(+), 9 deletions(-) diff --git a/ansible/roles/invoker/tasks/deploy.yml b/ansible/roles/invoker/tasks/deploy.yml index 0adda6f..1f86ece 100644 --- a/ansible/roles/invoker/tasks/deploy.yml +++ b/ansible/roles/invoker/tasks/deploy.yml @@ -270,7 +270,7 @@ "CONFIG_whisk_activation_payload_max": "{{ limit_activation_payload | default() }}" "CONFIG_whisk_transactions_header": "{{ transactions.header }}" "CONFIG_whisk_containerPool_akkaClient": "{{ container_pool_akka_client | default('false') | string }}" - "CONFIG_whisk_containerFactory_containerArgs_extraArgs_env_0": "__OW_ALLOW_CONCURRENT={{ runtimes_enable_concurrency | default('false') }}" + "CONFIG_whisk_containerFactory_containerArgs_extraEnvVars_0": "__OW_ALLOW_CONCURRENT={{ runtimes_enable_concurrency | default('false') }}" "CONFIG_whisk_invoker_protocol": "{{ invoker.protocol }}" "CONFIG_whisk_invoker_https_keystorePath": "/conf/{{ invoker.ssl.keystore.name }}" "CONFIG_whisk_invoker_https_keystorePassword": "{{ invoker.ssl.keystore.password }}" diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerFactory.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerFactory.scala index d246a40..921cabe 100644 --- a/common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerFactory.scala +++ b/common/scala/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerFactory.scala @@ -30,7 +30,18 @@ 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) + extraEnvVars: Seq[String] = Seq.empty, + extraArgs: Map[String, Set[String]] = Map.empty) { + + val extraEnvVarMap: Map[String, String] = + extraEnvVars.flatMap { + _.split("=", 2) match { + case Array(key) => Some(key -> "") + case Array(key, value) => Some(key -> value) + case _ => None + } + }.toMap +} case class ContainerPoolConfig(userMemory: ByteSize, concurrentPeekFactor: Double, akkaClient: Boolean) { require( diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/mesos/MesosContainerFactory.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/mesos/MesosContainerFactory.scala index 11573bb..8cd66e1 100644 --- a/common/scala/src/main/scala/org/apache/openwhisk/core/mesos/MesosContainerFactory.scala +++ b/common/scala/src/main/scala/org/apache/openwhisk/core/mesos/MesosContainerFactory.scala @@ -149,7 +149,7 @@ class MesosContainerFactory(config: WhiskConfig, userProvidedImage = userProvidedImage, memory = memory, cpuShares = cpuShares, - environment = Map("__OW_API_HOST" -> config.wskApiHost), + environment = Map("__OW_API_HOST" -> config.wskApiHost) ++ containerArgs.extraEnvVarMap, network = containerArgs.network, dnsServers = containerArgs.dnsServers, name = Some(name), diff --git a/core/invoker/src/main/resources/application.conf b/core/invoker/src/main/resources/application.conf index 4f36209..9bd29d7 100644 --- a/core/invoker/src/main/resources/application.conf +++ b/core/invoker/src/main/resources/application.conf @@ -93,6 +93,7 @@ whisk { dns-servers: [] dns-search: [] dns-options: [] + extra-env-vars: [] # sequence of `key` and/or `key=value` bindings to add to all user action container environments extra-args: {} # to pass additional args to 'docker run'; format is `{key1: [v1, v2], key2: [v1, v2]}` } runtimes-registry { diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala index 94e5119..15f7e0c 100644 --- a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala +++ b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala @@ -66,7 +66,7 @@ class DockerContainerFactory(instance: InvokerInstanceId, if (userProvidedImage) Left(actionImage) else Right(actionImage.localImageName(runtimesRegistryConfig.url)), memory = memory, cpuShares = cpuShares, - environment = Map("__OW_API_HOST" -> config.wskApiHost), + environment = Map("__OW_API_HOST" -> config.wskApiHost) ++ containerArgsConfig.extraEnvVarMap, network = containerArgsConfig.network, dnsServers = containerArgsConfig.dnsServers, dnsSearch = containerArgsConfig.dnsSearch, diff --git a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/kubernetes/KubernetesContainerFactory.scala b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/kubernetes/KubernetesContainerFactory.scala index 49f8edc..173c9e8 100644 --- a/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/kubernetes/KubernetesContainerFactory.scala +++ b/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/kubernetes/KubernetesContainerFactory.scala @@ -28,6 +28,7 @@ import org.apache.openwhisk.common.Logging import org.apache.openwhisk.common.TransactionId import org.apache.openwhisk.core.containerpool.{ Container, + ContainerArgsConfig, ContainerFactory, ContainerFactoryProvider, RuntimesRegistryConfig @@ -40,6 +41,7 @@ import org.apache.openwhisk.core.{ConfigKeys, WhiskConfig} class KubernetesContainerFactory( label: String, config: WhiskConfig, + containerArgsConfig: ContainerArgsConfig = loadConfigOrThrow[ContainerArgsConfig](ConfigKeys.containerArgs), runtimesRegistryConfig: RuntimesRegistryConfig = loadConfigOrThrow[RuntimesRegistryConfig]( ConfigKeys.runtimesRegistry))(implicit actorSystem: ActorSystem, ec: ExecutionContext, logging: Logging) extends ContainerFactory { @@ -82,7 +84,7 @@ class KubernetesContainerFactory( image, userProvidedImage, memory, - environment = Map("__OW_API_HOST" -> config.wskApiHost), + environment = Map("__OW_API_HOST" -> config.wskApiHost) ++ containerArgsConfig.extraEnvVarMap, labels = Map("invoker" -> label)) } } diff --git a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/docker/test/DockerContainerFactoryTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/docker/test/DockerContainerFactoryTests.scala index b0ef1bc..8483a48 100644 --- a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/docker/test/DockerContainerFactoryTests.scala +++ b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/docker/test/DockerContainerFactoryTests.scala @@ -86,16 +86,26 @@ class DockerContainerFactoryTests "net1", "-e", "__OW_API_HOST=", + "-e", + "k1=v1", + "-e", + "k2=v2", + "-e", + "k3=", "--dns", "dns1", "--dns", "dns2", "--name", "testContainer", - "--env", + "--extra1", "e1", - "--env", - "e2"), + "--extra1", + "e2", + "--extra2", + "e3", + "--extra2", + "e4"), *) .returning(Future.successful { ContainerId("fakecontainerid") }) //setup inspect expectation @@ -117,7 +127,13 @@ class DockerContainerFactoryTests new DockerContainerFactory( InvokerInstanceId(0, userMemory = defaultUserMemory), Map.empty, - ContainerArgsConfig("net1", Seq("dns1", "dns2"), Seq.empty, Seq.empty, Map("env" -> Set("e1", "e2"))), + ContainerArgsConfig( + "net1", + Seq("dns1", "dns2"), + Seq.empty, + Seq.empty, + Seq("k1=v1", "k2=v2", "k3"), + Map("extra1" -> Set("e1", "e2"), "extra2" -> Set("e3", "e4"))), runtimesRegistryConfig, DockerContainerFactoryConfig(true))(actorSystem, executionContext, logging, dockerApiStub, mock[RuncApi]) diff --git a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/mesos/test/MesosContainerFactoryTest.scala b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/mesos/test/MesosContainerFactoryTest.scala index d980fb7..72f21a9 100644 --- a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/mesos/test/MesosContainerFactoryTest.scala +++ b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/mesos/test/MesosContainerFactoryTest.scala @@ -94,6 +94,7 @@ class MesosContainerFactoryTest Seq("dns1", "dns2"), Seq.empty, Seq.empty, + Seq.empty, Map("extra1" -> Set("e1", "e2"), "extra2" -> Set("e3", "e4"))) private var factory: MesosContainerFactory = _ @@ -274,6 +275,7 @@ class MesosContainerFactoryTest Seq.empty, Seq.empty, Seq.empty, + Seq.empty, Map("extra1" -> Set("e1", "e2"), "extra2" -> Set("e3", "e4"))), mesosConfig = mesosConfig, clientFactory = (system, mesosConfig) => probe.testActor, diff --git a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/yarn/test/YARNContainerFactoryTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/yarn/test/YARNContainerFactoryTests.scala index 66b6916..188cefd 100644 --- a/tests/src/test/scala/org/apache/openwhisk/core/containerpool/yarn/test/YARNContainerFactoryTests.scala +++ b/tests/src/test/scala/org/apache/openwhisk/core/containerpool/yarn/test/YARNContainerFactoryTests.scala @@ -84,6 +84,7 @@ class YARNContainerFactoryTests Seq("dns1", "dns2"), Seq.empty, Seq.empty, + Seq.empty, Map("extra1" -> Set("e1", "e2"), "extra2" -> Set("e3", "e4"))) val yarnConfig = YARNConfig(