Fix bug in image puller execution timeout (#1495)

* repository_ctx.execute must also receive 'timeout' value otherwise limit is default 600s

* make test test by updating expected err, and add note about potential test flakiness

Author: Jonathon Belotti

container/pull.bzl

@@ -165,10 +165,15 @@ exports_files(["image.digest", "digest"])
     kwargs = {}
 
     if "PULLER_TIMEOUT" in repository_ctx.os.environ:
-        args += [
-            "-timeout",
-            repository_ctx.os.environ.get("PULLER_TIMEOUT"),
-        ]
+        timeout_in_secs = repository_ctx.os.environ["PULLER_TIMEOUT"]
+        if timeout_in_secs.isdigit():
+            args += [
+                "-timeout",
+                timeout_in_secs,
+            ]
+            kwargs["timeout"] = int(timeout_in_secs)
+        else:
+            fail("'%s' is invalid value for PULLER_TIMEOUT. Must be an integer." % (timeout_in_secs))
 
     result = repository_ctx.execute(args, **kwargs)
     if result.return_code:

testing/e2e/puller.sh

@@ -25,9 +25,12 @@ function test_puller_timeout() {
   # Ensure the puller respects the PULLER_TIMEOUT environment variable. Try
   # pulling a large image but set a very low timeout of 1s which should fail if
   # the puller is respecting timeouts.
+  # NOTE: Potential race condition between the Bazel timeout mechanism and the go puller's.
+  #       Could result in test flakes.
+  #       See: https://github.com/bazelbuild/rules_docker/pull/1495#issuecomment-627969114
   cd "${ROOT}"
-  EXPECT_CONTAINS "$(PULLER_TIMEOUT=1 bazel build @large_image_timeout_test//image 2>&1)" "i/o timeout"
+  EXPECT_CONTAINS "$(PULLER_TIMEOUT=1 bazel build @large_image_timeout_test//image 2>&1)" "ERROR: Pull command failed: Timed out"
   echo "test_puller_timeout PASSED!"
 }
 
-test_puller_timeout
+test_puller_timeout