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

[v3.4.2 backport] bump to race-free c/image, c/storage and c/buildah along with test to verify concurrent/parallel builds #13793

Merged
merged 4 commits into from Apr 8, 2022
Merged

[v3.4.2 backport] bump to race-free c/image, c/storage and c/buildah along with test to verify concurrent/parallel builds #13793

merged 4 commits into from Apr 8, 2022

Conversation

flouthoc
Copy link
Collaborator

@flouthoc flouthoc commented Apr 6, 2022 •
edited by TomSweeneyRedHat

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

Needed for: BZ 2066364

@flouthoc
vendor: bump c/storage to race-free 1.36.3
f42162b 
Use race-free c/storage by bumping it to 1.36.3

Signed-off-by: Aditya R <arajan@redhat.com>
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 6, 2022
@flouthoc flouthoc marked this pull request as draft April 6, 2022 17:20
@flouthoc
Copy link
Collaborator Author

flouthoc commented Apr 6, 2022

@mtrmac FYI

@flouthoc flouthoc mentioned this pull request Apr 6, 2022
Merged
@flouthoc
Copy link
Collaborator Author

flouthoc commented Apr 6, 2022

Wait till upstream c/image is merged containers/image#1519

flouthoc and others added 2 commits April 7, 2022 01:06
@flouthoc
vendor: bump c/image to v5.16.0-rhel/8b06d33
8043e27 
Use race-free `AddNames` instead of `SetNames` by bumping `c/image`

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc @edsantiago
test: add a test to verify race free concurrent/parallel builds
4926556 
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.

More details here: containers/buildah#3794 and containers#13339

Co-authored-by: Ed Santiago <santiago@redhat.com>
Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc flouthoc mentioned this pull request Apr 6, 2022
Merged
@flouthoc
Copy link
Collaborator Author

flouthoc commented Apr 6, 2022

We might wanna vendor containers/buildah#3903 but its not necessary since affected deps are already bumped.

@flouthoc
Copy link
Collaborator Author

flouthoc commented Apr 6, 2022

@jnovy @TomSweeneyRedHat PTAL

@flouthoc flouthoc changed the title [WIP] [v3.4.2 backport] bump to race-free c/image and c/storage along with test to verify concurrent/parallel builds [v3.4.2 backport] bump to race-free c/image and c/storage along with test to verify concurrent/parallel builds Apr 6, 2022
@flouthoc flouthoc marked this pull request as ready for review April 7, 2022 03:01
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 7, 2022
@flouthoc
Copy link
Collaborator Author

flouthoc commented Apr 7, 2022

We might wanna vendor containers/buildah#3903 but its not necessary since affected deps are already bumped.

Its upto maintainers if we want to release and vendor the buildah but its not necessary since needed deps are updated this should fix the reported issue. This PR is good to go if we don't want to vendor in buildah.

mtrmac
mtrmac reviewed Apr 7, 2022
View reviewed changes
Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Looks good, for the record. (IIRC the current opinion is that this should be merged only with a Buildah backport.)

@flouthoc
vendor: bump buildah to v1.23.4
928df9d 
A new version of buildah was released after containers/buildah#3903
which is using race-free deps so use that.

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc flouthoc changed the title [v3.4.2 backport] bump to race-free c/image and c/storage along with test to verify concurrent/parallel builds [v3.4.2 backport] bump to race-free c/image, c/storage and c/buildah along with test to verify concurrent/parallel builds Apr 7, 2022
@flouthoc
Copy link
Collaborator Author

flouthoc commented Apr 7, 2022 •
edited

Vendor-ed buildah v1.23.4 in the most recent commit.

mtrmac
mtrmac reviewed Apr 8, 2022
View reviewed changes
Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 8, 2022
@rhatdan
Copy link
Member

rhatdan commented Apr 8, 2022

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 8, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, rhatdan

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 Apr 8, 2022
@openshift-merge-robot openshift-merge-robot merged commit b6bca81 into containers:v3.4.2-rhel Apr 8, 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

@mtrmac mtrmac mtrmac left review comments

Assignees

@mtrmac mtrmac

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
4 participants
@flouthoc @rhatdan @mtrmac @openshift-merge-robot