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

DO NOT MERGE: Vendor unmerged https://github.com/containers/storage/pull/1399 #4350

Closed
wants to merge 1 commit into from

Conversation

mtrmac
Copy link
Contributor

@mtrmac mtrmac commented Oct 18, 2022

Signed-off-by: Miloslav Trmač [email protected]

What type of PR is this?

/kind bug

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 18, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mtrmac
Once this PR has been reviewed and has the lgtm label, please assign umohnani8 for approval by writing /assign @umohnani8 in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mtrmac mtrmac force-pushed the locking-test branch 2 times, most recently from 603153e to d6558e5 Compare October 19, 2022 01:10
@mtrmac
Copy link
Contributor Author

mtrmac commented Oct 19, 2022

Now includes #4352 to avoid callers of .Locked().

@mtrmac
Copy link
Contributor Author

mtrmac commented Oct 19, 2022

Consistent failures with

not ok 65 build: test race in updating image name while performing parallel commits
[+0303s] # (from function `assert' in file ./helpers.bash, line 423,
[+0303s] #  in test file ./bud.bats, line 603)
[+0303s] #   `assert "$(cat ${TEST_SCRATCH_DIR}/id.* | wc -c)" = 1775 "Total chars in all id.* files"' failed
[+0303s] # /var/tmp/go/src/github.com/containers/buildah/tests /var/tmp/go/src/github.com/containers/buildah/tests
[+0303s] # # [checking for: docker.io/library/alpine]
[+0303s] # # [restoring from cache: /var/tmp/buildah-image-cache.8993 / docker.io/library/alpine]
[+0303s] # Getting image source signatures
[+0303s] # Copying blob sha256:9d16cba9fb961d1aafec9542f2bf7cb64acfc55245f9e4eb5abecd4cdc38d749
[+0303s] # Copying config sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4
[+0303s] # Writing manifest to image destination
[+0303s] # Storing signatures
[+0303s] # Error: mounting new container: mounting build container "003f239f34edaf56f1073e31c8d9deec1fc258bbff6306dbf879456f9075c9ea": layer not known

(with varying layer IDs). That’s bad. What did I break?

mtrmac added a commit to mtrmac/libpod that referenced this pull request Oct 19, 2022
DO NOT MERGE: Vendor containers/buildah#4350

Remove uses of Lockfile.Locked():
One of them is entirely unnecessary, and the other one
is avoidable.

[NO NEW TESTS NEEDED]

Signed-off-by: Miloslav Trmač <[email protected]>
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 21, 2022
@mtrmac
Copy link
Contributor Author

mtrmac commented Oct 24, 2022

That’s bad. What did I break?

For the record, containers/storage#1400 is what I broke, and hopefully fixed.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 30, 2022
@openshift-merge-robot
Copy link
Collaborator

@mtrmac: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mtrmac
Copy link
Contributor Author

mtrmac commented Nov 1, 2022

containers/storage#1399 was now merged.

@mtrmac mtrmac closed this Nov 1, 2022
@mtrmac mtrmac deleted the locking-test branch November 1, 2022 01:33
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-merge/work-in-progress locked - please file new issue/PR needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants