You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/02/27 16:50:24 UTC
[GitHub] rabbah closed pull request #3215: adding namePrefix config to ContainerArgsConfig
rabbah closed pull request #3215: adding namePrefix config to ContainerArgsConfig
URL: https://github.com/apache/incubator-openwhisk/pull/3215
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/common/scala/src/main/scala/whisk/core/containerpool/ContainerFactory.scala b/common/scala/src/main/scala/whisk/core/containerpool/ContainerFactory.scala
index 5c70d5d8d3..25be520070 100644
--- a/common/scala/src/main/scala/whisk/core/containerpool/ContainerFactory.scala
+++ b/common/scala/src/main/scala/whisk/core/containerpool/ContainerFactory.scala
@@ -50,6 +50,15 @@ trait ContainerFactory {
def cleanup(): Unit
}
+object ContainerFactory {
+
+ /** include the instance name, if specified and strip invalid chars before attempting to use them in the container name */
+ def containerNamePrefix(instanceId: InstanceId): String =
+ s"wsk${instanceId.name.getOrElse("")}${instanceId.toInt}"
+ .replaceAll("[^a-zA-Z0-9_\\.\\-]", "") // based on https://github.com/moby/moby/issues/3138 and https://github.com/moby/moby/blob/master/daemon/names/names.go
+
+}
+
/**
* An SPI for ContainerFactory creation
* All impls should use the parameters specified as additional args to "docker run" commands
diff --git a/common/scala/src/main/scala/whisk/core/entity/InstanceId.scala b/common/scala/src/main/scala/whisk/core/entity/InstanceId.scala
index 743cda5cd5..8087b8b7bd 100644
--- a/common/scala/src/main/scala/whisk/core/entity/InstanceId.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/InstanceId.scala
@@ -19,10 +19,10 @@ package whisk.core.entity
import spray.json.DefaultJsonProtocol
-case class InstanceId(val instance: Int) {
+case class InstanceId(val instance: Int, name: Option[String] = None) {
def toInt: Int = instance
}
object InstanceId extends DefaultJsonProtocol {
- implicit val serdes = jsonFormat1(InstanceId.apply)
+ implicit val serdes = jsonFormat2(InstanceId.apply)
}
diff --git a/core/invoker/src/main/scala/whisk/core/containerpool/ContainerProxy.scala b/core/invoker/src/main/scala/whisk/core/containerpool/ContainerProxy.scala
index 23d0965107..b17a816dca 100644
--- a/core/invoker/src/main/scala/whisk/core/containerpool/ContainerProxy.scala
+++ b/core/invoker/src/main/scala/whisk/core/containerpool/ContainerProxy.scala
@@ -18,7 +18,6 @@
package whisk.core.containerpool
import java.time.Instant
-
import scala.concurrent.Future
import scala.concurrent.duration._
import scala.util.Success
@@ -428,7 +427,8 @@ object ContainerProxy {
* @return a unique container name
*/
def containerName(instance: InstanceId, prefix: String, suffix: String) =
- s"wsk${instance.toInt}_${containerCount.next()}_${prefix}_${suffix}".replaceAll("[^a-zA-Z0-9_]", "")
+ s"${ContainerFactory.containerNamePrefix(instance)}_${containerCount.next()}_${prefix}_${suffix}"
+ .replaceAll("[^a-zA-Z0-9_]", "")
/**
* Creates a WhiskActivation ready to be sent via active ack.
diff --git a/core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerContainerFactory.scala b/core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerContainerFactory.scala
index fbcd230bd0..ff0dfb5f78 100644
--- a/core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerContainerFactory.scala
+++ b/core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerContainerFactory.scala
@@ -106,24 +106,26 @@ class DockerContainerFactory(config: WhiskConfig,
@throws(classOf[InterruptedException])
private def removeAllActionContainers(): Unit = {
implicit val transid = TransactionId.invoker
- val cleaning = docker.ps(filters = Seq("name" -> s"wsk${instance.toInt}_"), all = true).flatMap { containers =>
- logging.info(this, s"removing ${containers.size} action containers.")
- val removals = containers.map { id =>
- (if (config.invokerUseRunc) {
- runc.resume(id)
- } else {
- docker.unpause(id)
- })
- .recoverWith {
- // Ignore resume failures and try to remove anyway
- case _ => Future.successful(())
- }
- .flatMap { _ =>
- docker.rm(id)
+ val cleaning =
+ docker.ps(filters = Seq("name" -> s"${ContainerFactory.containerNamePrefix(instance)}_"), all = true).flatMap {
+ containers =>
+ logging.info(this, s"removing ${containers.size} action containers.")
+ val removals = containers.map { id =>
+ (if (config.invokerUseRunc) {
+ runc.resume(id)
+ } else {
+ docker.unpause(id)
+ })
+ .recoverWith {
+ // Ignore resume failures and try to remove anyway
+ case _ => Future.successful(())
+ }
+ .flatMap { _ =>
+ docker.rm(id)
+ }
}
+ Future.sequence(removals)
}
- Future.sequence(removals)
- }
Await.ready(cleaning, 30.seconds)
}
}
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 ae15e90abe..2a1fc0eebb 100644
--- a/core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala
+++ b/core/invoker/src/main/scala/whisk/core/invoker/Invoker.scala
@@ -120,7 +120,7 @@ object Invoker {
}
val cmdLineArgs = parse(args.toList, CmdLineArgs())
logger.info(this, "Command line arguments parsed to yield " + cmdLineArgs)
-
+ val invokerName = cmdLineArgs.name.orElse(if (config.invokerName.trim.isEmpty) None else Some(config.invokerName))
val assignedInvokerId = cmdLineArgs.id
.map { id =>
logger.info(this, s"invokerReg: using proposedInvokerId ${id}")
@@ -130,8 +130,7 @@ object Invoker {
if (config.zookeeperHosts.startsWith(":") || config.zookeeperHosts.endsWith(":")) {
abort(s"Must provide valid zookeeper host and port to use dynamicId assignment (${config.zookeeperHosts})")
}
- val invokerName = cmdLineArgs.name.getOrElse(config.invokerName)
- if (invokerName.trim.isEmpty) {
+ if (invokerName.isEmpty || invokerName.get.trim.isEmpty) {
abort("Invoker name can't be empty to use dynamicId assignment.")
}
@@ -177,7 +176,7 @@ object Invoker {
assignedId
}
- val invokerInstance = InstanceId(assignedInvokerId)
+ val invokerInstance = InstanceId(assignedInvokerId, invokerName)
val msgProvider = SpiLoader.get[MessagingProvider]
if (!msgProvider.ensureTopic(config, topic = "invoker" + assignedInvokerId, topicConfig = "invoker")) {
abort(s"failure during msgProvider.ensureTopic for topic invoker$assignedInvokerId")
diff --git a/tests/src/test/scala/whisk/core/containerpool/test/ContainerProxyTests.scala b/tests/src/test/scala/whisk/core/containerpool/test/ContainerProxyTests.scala
index b278871d57..b624a49af0 100644
--- a/tests/src/test/scala/whisk/core/containerpool/test/ContainerProxyTests.scala
+++ b/tests/src/test/scala/whisk/core/containerpool/test/ContainerProxyTests.scala
@@ -185,17 +185,17 @@ class ContainerProxyTests
val container = new TestContainer
val factory = createFactory(Future.successful(container))
val store = createStore
-
val machine =
childActorOf(
- ContainerProxy.props(factory, createAcker, store, createCollector(), InstanceId(0), pauseGrace = timeout))
+ ContainerProxy
+ .props(factory, createAcker, store, createCollector(), InstanceId(0, Some("myname")), pauseGrace = timeout))
registerCallback(machine)
preWarm(machine)
factory.calls should have size 1
val (tid, name, _, _, memory) = factory.calls(0)
tid shouldBe TransactionId.invokerWarmup
- name should fullyMatch regex """wsk\d+_\d+_prewarm_actionKind"""
+ name should fullyMatch regex """wskmyname\d+_\d+_prewarm_actionKind"""
memory shouldBe memoryLimit
}
----------------------------------------------------------------
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