From dc9b470f28e58d783e63658af7c0cdac0fda160e Mon Sep 17 00:00:00 2001 From: Iain Earl Date: Mon, 30 May 2022 13:32:49 +0100 Subject: [PATCH 1/8] Add retry-count to container_push --- container/go/cmd/pusher/pusher.go | 14 ++++++++++++-- container/push.bzl | 6 ++++++ contrib/push-all.bzl | 3 +++ docs/container.md | 3 ++- 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/container/go/cmd/pusher/pusher.go b/container/go/cmd/pusher/pusher.go index ab0257414..cfdef4412 100644 --- a/container/go/cmd/pusher/pusher.go +++ b/container/go/cmd/pusher/pusher.go @@ -44,6 +44,7 @@ var ( format = flag.String("format", "", "The format of the uploaded image (Docker or OCI).") clientConfigDir = flag.String("client-config-dir", "", "The path to the directory where the client configuration files are located. Overiddes the value from DOCKER_CONFIG.") skipUnchangedDigest = flag.Bool("skip-unchanged-digest", false, "If set to true, will only push images where the digest has changed.") + retryCount = flag.Int("retry-count", 0, "Amount of times the push will be retried.") layers utils.ArrayStringFlags stampInfoFile utils.ArrayStringFlags ) @@ -126,8 +127,17 @@ func main() { log.Printf("Failed to digest image: %v", err) } - if err := push(stamped, img); err != nil { - log.Fatalf("Error pushing image to %s: %v", stamped, err) + for retry := 0; retry < *retryCount+1; retry++ { + err := push(stamped, img) + if err == nil { + break + } + + if *retryCount > 0 && retry < *retryCount { + log.Printf("Failed to push image on attempt %d: %v", retry+1, err) + } else { + log.Fatalf("Error pushing image to %s: %v", stamped, err) + } } digestStr := "" diff --git a/container/push.bzl b/container/push.bzl index c9cb9bfa0..c487444a2 100644 --- a/container/push.bzl +++ b/container/push.bzl @@ -90,6 +90,9 @@ def _impl(ctx): tag = tag, )) + if ctx.attr.retry_count > 0: + pusher_args.append("-retry-count={}".format(ctx.attr.retry_count)) + if ctx.attr.skip_unchanged_digest: pusher_args.append("-skip-unchanged-digest") digester_args += ["--dst", str(ctx.outputs.digest.path), "--format", str(ctx.attr.format)] @@ -168,6 +171,9 @@ container_push_ = rule( allow_single_file = True, doc = "The label of the file with repository value. Overrides 'repository'.", ), + "retry_count": attr.int( + doc = "Number of times to retry pushing the image.", + ), "skip_unchanged_digest": attr.bool( default = False, doc = "Check if the container registry already contain the image's digest. If yes, skip the push for that image. " + diff --git a/contrib/push-all.bzl b/contrib/push-all.bzl index 9ab5b739d..60cd5dd2d 100644 --- a/contrib/push-all.bzl +++ b/contrib/push-all.bzl @@ -107,6 +107,9 @@ container_push = rule( ], doc = "The form to push: Docker or OCI.", ), + "retry_count": attr.int( + doc = "Number of times to retry pushing an image", + ), "sequential": attr.bool( default = False, doc = "If true, push images sequentially.", diff --git a/docs/container.md b/docs/container.md index 474ebe335..c72398596 100644 --- a/docs/container.md +++ b/docs/container.md @@ -221,7 +221,7 @@ please use the bazel startup flag `--loading_phase_threads=1` in your bazel invo 
 container_push(name, extension, extract_config, format, image, incremental_load_template, registry,
-               repository, repository_file, skip_unchanged_digest, stamp, tag, tag_file, tag_tpl,
+               repository, repository_file, retry_count, skip_unchanged_digest, stamp, tag, tag_file, tag_tpl,
                windows_paths)
@@ -241,6 +241,7 @@ container_push(name, registry | The registry to which we are pushing. | String | required | | | repository | The name of the image. | String | required | | | repository_file | The label of the file with repository value. Overrides 'repository'. | Label | optional | None | +| retry_count | The number of times the push will be retried on failure. | Integer | optional | `0` | | skip_unchanged_digest | Check if the container registry already contain the image's digest. If yes, skip the push for that image. Default to False. Note that there is no transactional guarantee between checking for digest existence and pushing the digest. This means that you should try to avoid running the same container_push targets in parallel. | Boolean | optional | False | | stamp | Whether to encode build information into the output. Possible values:

- @io_bazel_rules_docker//stamp:always: Always stamp the build information into the output, even in [--nostamp][stamp] builds. This setting should be avoided, since it potentially causes cache misses remote caching for any downstream actions that depend on it.

- @io_bazel_rules_docker//stamp:never: Always replace build information by constant values. This gives good build result caching.

- @io_bazel_rules_docker//stamp:use_stamp_flag: Embedding of build information is controlled by the [--[no]stamp][stamp] flag. Stamped binaries are not rebuilt unless their dependencies change.

[stamp]: https://docs.bazel.build/versions/main/user-manual.html#flag--stamp | Label | optional | @io_bazel_rules_docker//stamp:use_stamp_flag | | tag | The tag of the image. | String | optional | "latest" | From 43f6d3fcc0cb6470962261a1de12950b554db22e Mon Sep 17 00:00:00 2001 From: Iain Earl Date: Mon, 30 May 2022 15:31:36 +0100 Subject: [PATCH 2/8] Run push-all through buildifier --- contrib/push-all.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/push-all.bzl b/contrib/push-all.bzl index 60cd5dd2d..5957c81fd 100644 --- a/contrib/push-all.bzl +++ b/contrib/push-all.bzl @@ -107,9 +107,9 @@ container_push = rule( ], doc = "The form to push: Docker or OCI.", ), - "retry_count": attr.int( - doc = "Number of times to retry pushing an image", - ), + "retry_count": attr.int( + doc = "Number of times to retry pushing an image", + ), "sequential": attr.bool( default = False, doc = "If true, push images sequentially.", From ddd69495f6b4dc089b13b4e91f5e7000b6265c37 Mon Sep 17 00:00:00 2001 From: Iain Earl Date: Mon, 30 May 2022 15:36:22 +0100 Subject: [PATCH 3/8] Regenerate docs --- docs/container.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/container.md b/docs/container.md index c72398596..a5a8662d6 100644 --- a/docs/container.md +++ b/docs/container.md @@ -221,8 +221,8 @@ please use the bazel startup flag `--loading_phase_threads=1` in your bazel invo 
 container_push(name, extension, extract_config, format, image, incremental_load_template, registry,
-               repository, repository_file, retry_count, skip_unchanged_digest, stamp, tag, tag_file, tag_tpl,
-               windows_paths)
+               repository, repository_file, retry_count, skip_unchanged_digest, stamp, tag, tag_file,
+               tag_tpl, windows_paths)
@@ -241,7 +241,7 @@ container_push(name, registry | The registry to which we are pushing. | String | required | | | repository | The name of the image. | String | required | | | repository_file | The label of the file with repository value. Overrides 'repository'. | Label | optional | None | -| retry_count | The number of times the push will be retried on failure. | Integer | optional | `0` | +| retry_count | Number of times to retry pushing the image. | Integer | optional | 0 | | skip_unchanged_digest | Check if the container registry already contain the image's digest. If yes, skip the push for that image. Default to False. Note that there is no transactional guarantee between checking for digest existence and pushing the digest. This means that you should try to avoid running the same container_push targets in parallel. | Boolean | optional | False | | stamp | Whether to encode build information into the output. Possible values:

- @io_bazel_rules_docker//stamp:always: Always stamp the build information into the output, even in [--nostamp][stamp] builds. This setting should be avoided, since it potentially causes cache misses remote caching for any downstream actions that depend on it.

- @io_bazel_rules_docker//stamp:never: Always replace build information by constant values. This gives good build result caching.

- @io_bazel_rules_docker//stamp:use_stamp_flag: Embedding of build information is controlled by the [--[no]stamp][stamp] flag. Stamped binaries are not rebuilt unless their dependencies change.

[stamp]: https://docs.bazel.build/versions/main/user-manual.html#flag--stamp | Label | optional | @io_bazel_rules_docker//stamp:use_stamp_flag | | tag | The tag of the image. | String | optional | "latest" | From 5a1f982e63f5c232828f5e694524b782c47c2f28 Mon Sep 17 00:00:00 2001 From: Iain Earl Date: Tue, 31 May 2022 09:39:41 +0100 Subject: [PATCH 4/8] Add missing retry flag during push all --- contrib/push-all.bzl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/push-all.bzl b/contrib/push-all.bzl index 5957c81fd..2496d78e3 100644 --- a/contrib/push-all.bzl +++ b/contrib/push-all.bzl @@ -48,6 +48,8 @@ def _impl(ctx): pusher_args, pusher_inputs = _gen_img_args(ctx, image, _get_runfile_path) pusher_args += ["--stamp-info-file=%s" % _get_runfile_path(ctx, f) for f in stamp_inputs] + if ctx.attr.retry_count > 0: + pusher_args.append("--retry-count={}".format(ctx.attr.retry_count)) if ctx.attr.skip_unchanged_digest: pusher_args.append("--skip-unchanged-digest") pusher_args.append("--dst={}".format(tag)) From 5e5bec0c3aac1ccb573d3b914b437cef86010137 Mon Sep 17 00:00:00 2001 From: Iain Earl Date: Wed, 8 Jun 2022 10:02:20 +0100 Subject: [PATCH 5/8] Update retry log to be more useful --- container/go/cmd/pusher/pusher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/container/go/cmd/pusher/pusher.go b/container/go/cmd/pusher/pusher.go index cfdef4412..7a9e8ed47 100644 --- a/container/go/cmd/pusher/pusher.go +++ b/container/go/cmd/pusher/pusher.go @@ -134,7 +134,7 @@ func main() { } if *retryCount > 0 && retry < *retryCount { - log.Printf("Failed to push image on attempt %d: %v", retry+1, err) + log.Printf("Error pushing image to %s (attempt %d): %v", stamped, retry+1, err) } else { log.Fatalf("Error pushing image to %s: %v", stamped, err) } From 4e540514fef7ddd56121f81c481993ab9d3843f6 Mon Sep 17 00:00:00 2001 From: Iain Earl Date: Wed, 8 Jun 2022 10:03:54 +0100 Subject: [PATCH 6/8] Validate -retry-count is not negative --- container/go/cmd/pusher/pusher.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/container/go/cmd/pusher/pusher.go b/container/go/cmd/pusher/pusher.go index 7a9e8ed47..cd3eff2d7 100644 --- a/container/go/cmd/pusher/pusher.go +++ b/container/go/cmd/pusher/pusher.go @@ -44,7 +44,7 @@ var ( format = flag.String("format", "", "The format of the uploaded image (Docker or OCI).") clientConfigDir = flag.String("client-config-dir", "", "The path to the directory where the client configuration files are located. Overiddes the value from DOCKER_CONFIG.") skipUnchangedDigest = flag.Bool("skip-unchanged-digest", false, "If set to true, will only push images where the digest has changed.") - retryCount = flag.Int("retry-count", 0, "Amount of times the push will be retried.") + retryCount = flag.Int("retry-count", 0, "Amount of times the push will be retried. This cannot be a negative number.") layers utils.ArrayStringFlags stampInfoFile utils.ArrayStringFlags ) @@ -85,6 +85,9 @@ func main() { if *imgTarball == "" && *imgConfig == "" { log.Fatalln("Neither --tarball nor --config was specified.") } + if *retryCount < 0 { + log.Fatalln("-retry-count cannot be a negative number.") + } // If the user provided a client config directory, ensure it's a valid // directory and instruct the keychain resolver to use it to look for the From f63e72a661d332f135c8e0ff90430299dfd43908 Mon Sep 17 00:00:00 2001 From: Iain Earl Date: Mon, 24 Jul 2023 11:59:37 +0100 Subject: [PATCH 7/8] Validate that retry_count is a positive int --- container/push.bzl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/container/push.bzl b/container/push.bzl index fbba5b97f..22a413c28 100644 --- a/container/push.bzl +++ b/container/push.bzl @@ -90,6 +90,9 @@ def _impl(ctx): tag = tag, )) + if ctx.attr.retry_count < 0: + fail("retry_count must be a positive integer") + if ctx.attr.retry_count > 0: pusher_args.append("-retry-count={}".format(ctx.attr.retry_count)) @@ -178,7 +181,7 @@ container_push_ = rule( doc = "The label of the file with repository value. Overrides 'repository'.", ), "retry_count": attr.int( - doc = "Number of times to retry pushing the image.", + doc = "Number of times to retry pushing the image. Only positive numbers are valid.", ), "skip_unchanged_digest": attr.bool( default = False, From 776c2d202938d8018ec962267d1428e796ef2cc3 Mon Sep 17 00:00:00 2001 From: Iain Earl Date: Mon, 24 Jul 2023 12:02:29 +0100 Subject: [PATCH 8/8] Update docs again --- docs/container.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/container.md b/docs/container.md index 3c4fe7553..95d464dce 100644 --- a/docs/container.md +++ b/docs/container.md @@ -242,7 +242,7 @@ container_push(name, registry | The registry to which we are pushing. | String | required | | | repository | The name of the image. | String | required | | | repository_file | The label of the file with repository value. Overrides 'repository'. | Label | optional | None | -| retry_count | Number of times to retry pushing the image. | Integer | optional | 0 | +| retry_count | Number of times to retry pushing the image. Only positive numbers are valid. | Integer | optional | 0 | | skip_unchanged_digest | Check if the container registry already contain the image's digest. If yes, skip the push for that image. Default to False. Note that there is no transactional guarantee between checking for digest existence and pushing the digest. This means that you should try to avoid running the same container_push targets in parallel. | Boolean | optional | False | | stamp | Whether to encode build information into the output. Possible values:

- @io_bazel_rules_docker//stamp:always: Always stamp the build information into the output, even in [--nostamp][stamp] builds. This setting should be avoided, since it potentially causes cache misses remote caching for any downstream actions that depend on it.

- @io_bazel_rules_docker//stamp:never: Always replace build information by constant values. This gives good build result caching.

- @io_bazel_rules_docker//stamp:use_stamp_flag: Embedding of build information is controlled by the [--[no]stamp][stamp] flag. Stamped binaries are not rebuilt unless their dependencies change.

[stamp]: https://docs.bazel.build/versions/main/user-manual.html#flag--stamp | Label | optional | @io_bazel_rules_docker//stamp:use_stamp_flag | | tag | The tag of the image. | String | optional | "latest" |