[incubator-openwhisk] branch master updated: Fix endless collapsing of failed pulls. (#2713)

[ra...@apache.org]

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 5f84bb9  Fix endless collapsing of failed pulls. (#2713)
5f84bb9 is described below

commit 5f84bb9d46452dd2df23ca1dc92b9b17eb95efc5
Author: Markus Thömmes <ma...@me.com>
AuthorDate: Fri Sep 8 05:38:18 2017 +0200

    Fix endless collapsing of failed pulls. (#2713)
    
    The pull-future needs to be removed from the caching Map once the underlying runCmd finishes, either successfully or failed.
---
 .../core/containerpool/docker/DockerClient.scala   |  2 +-
 .../docker/test/DockerClientTests.scala            | 32 +++++++++++++++++++++-
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerClient.scala b/core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerClient.scala
index 8ab2a86..915e7f6 100644
--- a/core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerClient.scala
+++ b/core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerClient.scala
@@ -97,7 +97,7 @@ class DockerClient(dockerHost: Option[String] = None)(executionContext: Executio
   private val pullsInFlight = TrieMap[String, Future[Unit]]()
   def pull(image: String)(implicit transid: TransactionId): Future[Unit] =
     pullsInFlight.getOrElseUpdate(image, {
-      runCmd("pull", image).map(_ => pullsInFlight.remove(image))
+      runCmd("pull", image).map(_ => ()).andThen { case _ => pullsInFlight.remove(image) }
     })
 
   private def runCmd(args: String*)(implicit transid: TransactionId): Future[String] = {
diff --git a/tests/src/test/scala/whisk/core/containerpool/docker/test/DockerClientTests.scala b/tests/src/test/scala/whisk/core/containerpool/docker/test/DockerClientTests.scala
index 3ad8f06..9146034 100644
--- a/tests/src/test/scala/whisk/core/containerpool/docker/test/DockerClientTests.scala
+++ b/tests/src/test/scala/whisk/core/containerpool/docker/test/DockerClientTests.scala
@@ -48,7 +48,8 @@ class DockerClientTests extends FlatSpec with Matchers with StreamLogging with B
   implicit val transid = TransactionId.testing
   val id = ContainerId("Id")
 
-  def await[A](f: Future[A], timeout: FiniteDuration = 500.milliseconds) = Await.result(f, timeout)
+  val commandTimeout = 500.milliseconds
+  def await[A](f: Future[A], timeout: FiniteDuration = commandTimeout) = Await.result(f, timeout)
 
   val dockerCommand = "docker"
 
@@ -112,6 +113,35 @@ class DockerClientTests extends FlatSpec with Matchers with StreamLogging with B
     }
   }
 
+  it should "properly clean up failed pulls" in {
+    // Delay execution of the pull command
+    val pullPromise = Promise[String]()
+    var commandsRun = 0
+    val dc = dockerClient {
+      commandsRun += 1
+      pullPromise.future
+    }
+
+    val image = "testimage"
+
+    // Pull first, command should be run
+    dc.pull(image)
+    commandsRun shouldBe 1
+
+    // Pull again, command should not be run
+    dc.pull(image)
+    commandsRun shouldBe 1
+
+    // Finish the pulls above
+    pullPromise.failure(new Throwable())
+
+    retry {
+      // Pulling again should execute the command again
+      Await.ready(dc.pull(image), commandTimeout)
+      commandsRun shouldBe 2
+    }
+  }
+
   it should "write proper log markers on a successful command" in {
     // a dummy string works here as we do not assert any output
     // from the methods below

-- 
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>'].