Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retry_count to container_push #2101

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions container/go/cmd/pusher/pusher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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. This cannot be a negative number.")
layers utils.ArrayStringFlags
stampInfoFile utils.ArrayStringFlags
insecureRepository = flag.Bool("insecure-repository", false, "If set to true, the repository is assumed to be insecure (http vs https)")
Expand Down Expand Up @@ -85,6 +86,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
Expand Down Expand Up @@ -132,8 +136,17 @@ func main() {
opts = append(opts, name.Insecure)
}

if err := push(stamped, img, opts...); err != nil {
log.Fatalf("Error pushing image to %s: %v", stamped, err)
for retry := 0; retry < *retryCount+1; retry++ {
err := push(stamped, img, opts...)
if err == nil {
break
}

if *retryCount > 0 && retry < *retryCount {
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)
}
uhthomas marked this conversation as resolved.
Show resolved Hide resolved
}

digestStr := ""
Expand Down
9 changes: 9 additions & 0 deletions container/push.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ 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))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe there should be some validation here? Might be confusing if a user provides -1 and nothing happens.

I think we can merge given this change.


if ctx.attr.skip_unchanged_digest:
pusher_args.append("-skip-unchanged-digest")
if ctx.attr.insecure_repository:
Expand Down Expand Up @@ -174,6 +180,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. Only positive numbers are valid.",
),
"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. " +
Expand Down
5 changes: 5 additions & 0 deletions contrib/push-all.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,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))
Expand Down Expand Up @@ -112,6 +114,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.",
Expand Down
5 changes: 3 additions & 2 deletions docs/container.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ please use the bazel startup flag `--loading_phase_threads=1` in your bazel invo

<pre>
container_push(<a href="#container_push-name">name</a>, <a href="#container_push-extension">extension</a>, <a href="#container_push-extract_config">extract_config</a>, <a href="#container_push-format">format</a>, <a href="#container_push-image">image</a>, <a href="#container_push-incremental_load_template">incremental_load_template</a>,
<a href="#container_push-insecure_repository">insecure_repository</a>, <a href="#container_push-registry">registry</a>, <a href="#container_push-repository">repository</a>, <a href="#container_push-repository_file">repository_file</a>, <a href="#container_push-skip_unchanged_digest">skip_unchanged_digest</a>,
<a href="#container_push-stamp">stamp</a>, <a href="#container_push-tag">tag</a>, <a href="#container_push-tag_file">tag_file</a>, <a href="#container_push-tag_tpl">tag_tpl</a>, <a href="#container_push-windows_paths">windows_paths</a>)
<a href="#container_push-insecure_repository">insecure_repository</a>, <a href="#container_push-registry">registry</a>, <a href="#container_push-repository">repository</a>, <a href="#container_push-repository_file">repository_file</a>, <a href="#container_push-retry_count">retry_count</a>,
<a href="#container_push-skip_unchanged_digest">skip_unchanged_digest</a>, <a href="#container_push-stamp">stamp</a>, <a href="#container_push-tag">tag</a>, <a href="#container_push-tag_file">tag_file</a>, <a href="#container_push-tag_tpl">tag_tpl</a>, <a href="#container_push-windows_paths">windows_paths</a>)
</pre>


Expand All @@ -242,6 +242,7 @@ container_push(<a href="#container_push-name">name</a>, <a href="#container_push
| <a id="container_push-registry"></a>registry | The registry to which we are pushing. | String | required | |
| <a id="container_push-repository"></a>repository | The name of the image. | String | required | |
| <a id="container_push-repository_file"></a>repository_file | The label of the file with repository value. Overrides 'repository'. | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
| <a id="container_push-retry_count"></a>retry_count | Number of times to retry pushing the image. Only positive numbers are valid. | Integer | optional | 0 |
| <a id="container_push-skip_unchanged_digest"></a>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 |
| <a id="container_push-stamp"></a>stamp | Whether to encode build information into the output. Possible values:<br><br> - <code>@io_bazel_rules_docker//stamp:always</code>: 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.<br><br> - <code>@io_bazel_rules_docker//stamp:never</code>: Always replace build information by constant values. This gives good build result caching.<br><br> - <code>@io_bazel_rules_docker//stamp:use_stamp_flag</code>: Embedding of build information is controlled by the [--[no]stamp][stamp] flag. Stamped binaries are not rebuilt unless their dependencies change.<br><br> [stamp]: https://docs.bazel.build/versions/main/user-manual.html#flag--stamp | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | @io_bazel_rules_docker//stamp:use_stamp_flag |
| <a id="container_push-tag"></a>tag | The tag of the image. | String | optional | "latest" |
Expand Down