Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

container_push: add doc about no transaction #2060

Merged
merged 1 commit into from
May 26, 2022

Conversation

sluongng
Copy link
Contributor

When use container_push with skip_unchanged_digest,
user often rely on the push process to (1) check for existence
of the image in the registry and (2) push the image if (1) is false.

This leave a time window between (1) and (2) where the image
could have been pushed by an external process such as another CI
job running in parallel. In such situation, depends on the container
registry configuration, the push arrive later can either fail for attempting
to override an already pushed image, or it will override the previously
pushed image.

There is no transactional guarantee that can help coordinate (1) and (2) in
the current container registry spec. So let's document this edge case and
advise users to work around by using some external distributed lock for
running container_push target.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@linzhp
Copy link
Collaborator

linzhp commented May 5, 2022

Can you run buildifier to format the file?

@linzhp linzhp self-assigned this May 5, 2022
@linzhp
Copy link
Collaborator

linzhp commented May 25, 2022

Can you run buildifier to pass the CI check?

@sluongng
Copy link
Contributor Author

CI now passed but it seems like we run into another issue caused by bazelbuild/bazel#15558

A separate update to gpg key inside WORKSPACE is needed

@sluongng
Copy link
Contributor Author

created #2096 for this

When use container_push with skip_unchanged_digest,
user often rely on the push process to (1) check for existence
of the image in the registry and (2) push the image if (1) is false.

This leave a time window between (1) and (2) where the image
could have been pushed by an external process such as another CI
job running in parallel. In such situation, depends on the container
registry configuration, the push arrive later can either fail for attempting
to override an already pushed image, or it will override the previously
pushed image.

There is no transactional guarantee that can help coordinate (1) and (2) in
the current container registry spec. So let's document this edge case and
advise users to work around by using some external distributed lock for
running container_push target.
@linzhp linzhp merged commit aa3438b into bazelbuild:master May 26, 2022
St0rmingBr4in pushed a commit to St0rmingBr4in/rules_docker that referenced this pull request Oct 17, 2022
When use container_push with skip_unchanged_digest,
user often rely on the push process to (1) check for existence
of the image in the registry and (2) push the image if (1) is false.

This leave a time window between (1) and (2) where the image
could have been pushed by an external process such as another CI
job running in parallel. In such situation, depends on the container
registry configuration, the push arrive later can either fail for attempting
to override an already pushed image, or it will override the previously
pushed image.

There is no transactional guarantee that can help coordinate (1) and (2) in
the current container registry spec. So let's document this edge case and
advise users to work around by using some external distributed lock for
running container_push target.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants