From 2d833746a0de3c652fbc0c26d1ded83e23eb3d34 Mon Sep 17 00:00:00 2001 From: Jonathon Belotti Date: Wed, 27 May 2020 22:38:52 +1000 Subject: [PATCH] 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 --- container/pull.bzl | 13 +++++++++---- testing/e2e/puller.sh | 7 +++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/container/pull.bzl b/container/pull.bzl index e1dbc82c7..48c3b91ff 100644 --- a/container/pull.bzl +++ b/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: diff --git a/testing/e2e/puller.sh b/testing/e2e/puller.sh index 22f5b7db2..7bb405b4b 100755 --- a/testing/e2e/puller.sh +++ b/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 \ No newline at end of file +test_puller_timeout