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.

Sign up for GitHub

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

Jump to bottom

[v4.0-rhel backport] bump to race-free c/image and c/storage along with test to verify concurrent/parallel builds #13623

Merged
merged 3 commits into from Mar 24, 2022
Merged

[v4.0-rhel backport] bump to race-free c/image and c/storage along with test to verify concurrent/parallel builds #13623

merged 3 commits into from Mar 24, 2022

Conversation

flouthoc
Copy link
Collaborator

@flouthoc flouthoc commented Mar 23, 2022 •
edited

Commits explained in bullet points.

New Names API in c/storage and c/image ensures that Name modification operations are race-free.

  • Adds a test to verify parallel/concurrent builds via podman don't race against each other.

For reference actual upstream PR: containers/image#1480 and containers/storage#1153

@flouthoc
vendor: bump c/storage to v1.38.3
a54be69 
Bump c/storage to v1.38.3 so podman could use new `race-free`
`AddNames` and `RemoveNames` api

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc
vendor: bump c/image to v5.19.2
c9a44ef 
Bump c/image to upstream v5.19.2 so podman could use new race-free
code.

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc
test: add a test to verify race free concurrent/parallel builds
1ea5724 
Invoking parallel/concurrent builds from podman race against each other
following behviour was fixed in
containers/storage#1153 and containers/image#1480

Test verifies if following bug is fixed in new race-free API or not.
Read more about this issue, see bz 2055487 for more details.

Test manually backported from: containers@63f92d0

Signed-off-by: Aditya R <arajan@redhat.com>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 23, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc

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

The pull request process is described 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 23, 2022
@flouthoc
Copy link
Collaborator Author

flouthoc commented Mar 23, 2022 •
edited

@mheon @TomSweeneyRedHat fixes for v4.0-rhel. PTAL

@TomSweeneyRedHat
Copy link
Member

Won't we need Buildah bumped too? Otherwise LGTM.

@TomSweeneyRedHat
Copy link
Member

And we'll need this in the 4.0 branch.

@TomSweeneyRedHat
Copy link
Member

At least partially addresses: https://bugzilla.redhat.com/show_bug.cgi?id=2066519 RHEL 8.6 BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2055487

@flouthoc
Copy link
Collaborator Author

Won't we need Buildah bumped too? Otherwise LGTM.

@TomSweeneyRedHat Buildah vendor is not necessary needed to fix this in podman ( i.e for podman build .. commands ), since only dependencies are updated.

But if we are also planning to release a new back-ported binary of buildah then c/image and c/storage can be back-ported to older version of buildah as well but no reason to block this PR.

@flouthoc
Copy link
Collaborator Author

And we'll need this in the 4.0 branch.

Just created a PR for that in v4.0 branch #13630

@TomSweeneyRedHat
Copy link
Member

Exceptions approved: RHEL 9.0 BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2066519 RHEL 8.6 BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2055487

@TomSweeneyRedHat
Copy link
Member

@mheon @bbaude care to add your lgtm and merge this?

@mheon
Copy link
Member

mheon commented Mar 24, 2022

/lgtm

@flouthoc
Copy link
Collaborator Author

@mheon @rhatdan Any idea why /lgtm is not reacting.

@rhatdan rhatdan added the lgtm Indicates that a PR is ready to be merged. label Mar 24, 2022
@openshift-merge-robot openshift-merge-robot merged commit bb1e6e6 into containers:v4.0-rhel Mar 24, 2022
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@flouthoc @TomSweeneyRedHat @mheon @rhatdan @openshift-merge-robot