storage: use race-free `AddNames` instead of `SetNames` #1480

flouthoc · 2022-02-24T11:06:53Z

Commits from parallel builds using SetNames removes names from
storage for other builds.

Use race-free atomic AddNames to prevent breaking of parallel builds.

Uses c/storage from PR: containers/storage#1153

Fixes issue:

Builds fail with image not known.

Reproducer

#!/bin/bash
x=1
sudo ./podman rmi -af
rm -f log*.*
sudo ./podman build -t first . &> logfirst.log
while [ $x -le 30 ]
do
  echo "$x times"
  sudo ./podman build --log-level debug -t $x . &> log$x.log &
  x=$(( $x + 1 ))
#  sleep 1
done

Dockerfile

FROM quay.io/jitesoft/alpine

PS: I am not sure how to ensure parallel builds in CI and produce a race so not sure how to test this in CI.

flouthoc · 2022-02-24T11:22:24Z

@giuseppe @vrothberg @mtrmac @nalind This fixes the reported issue. I have also shared a reproducer above which is fixed.

flouthoc · 2022-02-24T11:22:53Z

I have to point to actual c/storage version right now its my fork.

storage/storage_image.go

vrothberg

LGTM

storage/storage_image.go

flouthoc · 2022-02-24T19:18:38Z

Wait for containers/storage#1153 and tests to pass here: containers/podman#13339

mtrmac

LGTM — just so that it isn‘t forgotten, the replace in go.mod should be replaced.

After that is done, and tests pass, feel free to merge without another review.

mtrmac · 2022-02-25T18:32:02Z

Also: Thanks!

vrothberg

LGTM

go.mod

flouthoc · 2022-03-01T20:10:00Z

@mtrmac @vrothberg done.

mtrmac · 2022-03-01T21:00:44Z

LGTM. Can you rebase it over the release, please, so that it’s trivially clear which commits part of 5.20.0 and which commits come after? Feel free to merge afterwards.

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 <[email protected]> Signed-off-by: Aditya R <[email protected]>

flouthoc · 2022-03-23T10:32:50Z

@containers/image-maintainers @mtrmac @vrothberg I want to backport this commit to v5.16.0 but I was unable to find any maintenance branch for that. Is there any branch where i could raise a PR to get this backported.

Thanks

vrothberg · 2022-03-23T10:41:30Z

@flouthoc, I just created one: https://github.com/containers/image/tree/release-5.16

When backporting, please make sure to bump the version accordingly in version/version.go.

mtrmac

~~ACK to the code (but blocked on a c/storage release).~~

~~WRT the test failures, is it possible to fix that by updating SKOPEO_BRANCH on this branch to an appropriate consumer of c/image 5.16?~~

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 <[email protected]>

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/podman@63f92d0 Signed-off-by: Aditya R <[email protected]>

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 <[email protected]> Signed-off-by: Aditya R <[email protected]>

flouthoc force-pushed the race-free-commit branch from 72d094c to 1eaae7e Compare February 24, 2022 11:10

mtrmac reviewed Feb 24, 2022

View reviewed changes

storage/storage_image.go Outdated Show resolved Hide resolved

storage/storage_image.go Outdated Show resolved Hide resolved

storage/storage_image.go Show resolved Hide resolved

storage/storage_image.go Outdated Show resolved Hide resolved

flouthoc force-pushed the race-free-commit branch from 1eaae7e to aa17da2 Compare February 24, 2022 11:45

flouthoc requested a review from mtrmac February 24, 2022 11:46

flouthoc force-pushed the race-free-commit branch from aa17da2 to a939655 Compare February 24, 2022 11:47

vrothberg reviewed Feb 24, 2022

View reviewed changes

storage/storage_image.go Outdated Show resolved Hide resolved

storage/storage_image.go Outdated Show resolved Hide resolved

storage/storage_image.go Outdated Show resolved Hide resolved

flouthoc force-pushed the race-free-commit branch from a939655 to 7faba36 Compare February 24, 2022 12:18

flouthoc requested a review from vrothberg February 24, 2022 12:19

vrothberg reviewed Feb 24, 2022

View reviewed changes

storage/storage_image.go Outdated Show resolved Hide resolved

flouthoc force-pushed the race-free-commit branch from 7faba36 to b500288 Compare February 24, 2022 12:45

flouthoc requested a review from vrothberg February 24, 2022 12:55

vrothberg reviewed Feb 24, 2022

View reviewed changes

mtrmac reviewed Feb 24, 2022

View reviewed changes

storage/storage_image.go Outdated Show resolved Hide resolved

storage/storage_image.go Outdated Show resolved Hide resolved

flouthoc force-pushed the race-free-commit branch from b500288 to 0219501 Compare February 24, 2022 19:17

flouthoc requested a review from mtrmac February 24, 2022 19:17

flouthoc mentioned this pull request Feb 25, 2022

[Never Merge] Verify parallel build race fix and new storage API containers/podman#13339

Closed

mtrmac approved these changes Feb 25, 2022

View reviewed changes

flouthoc force-pushed the race-free-commit branch 2 times, most recently from 9e8da2d to b361ff3 Compare March 1, 2022 16:38

vrothberg approved these changes Mar 1, 2022

View reviewed changes

mtrmac reviewed Mar 1, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

flouthoc force-pushed the race-free-commit branch from b361ff3 to f118f32 Compare March 1, 2022 20:09

flouthoc requested review from mtrmac and vrothberg March 1, 2022 20:10

flouthoc mentioned this pull request Mar 11, 2022

Update vendor of containers/buildah and friends containers/podman#13479

Closed

flouthoc mentioned this pull request Mar 23, 2022

[v5.16 Backport] storage: use race-free AddNames instead of SetNames #1502

Closed

mtrmac reviewed Mar 23, 2022

View reviewed changes

flouthoc mentioned this pull request Mar 23, 2022

[v5.19, backport] storage: use race-free AddNames instead of SetNames #1503

Merged

flouthoc mentioned this pull request Mar 23, 2022

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

Merged

flouthoc mentioned this pull request Mar 24, 2022

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

Merged

flouthoc mentioned this pull request Mar 24, 2022

[v1.24 backport] bump to race-free c/image and c/storage containers/buildah#3850

Merged

flouthoc mentioned this pull request Apr 6, 2022

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

Merged

flouthoc mentioned this pull request Apr 6, 2022

[release-1.23 backport] bump to race-free c/image and c/storage containers/buildah#3903

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

storage: use race-free `AddNames` instead of `SetNames` #1480

storage: use race-free `AddNames` instead of `SetNames` #1480

flouthoc commented Feb 24, 2022

flouthoc commented Feb 24, 2022

flouthoc commented Feb 24, 2022 •

edited

Loading

vrothberg left a comment

flouthoc commented Feb 24, 2022

mtrmac left a comment

mtrmac commented Feb 25, 2022

vrothberg left a comment

flouthoc commented Mar 1, 2022

mtrmac commented Mar 1, 2022

flouthoc commented Mar 23, 2022

vrothberg commented Mar 23, 2022

mtrmac left a comment •

edited

Loading

storage: use race-free AddNames instead of SetNames #1480

storage: use race-free AddNames instead of SetNames #1480

Conversation

flouthoc commented Feb 24, 2022

Reproducer

Dockerfile

flouthoc commented Feb 24, 2022

flouthoc commented Feb 24, 2022 • edited Loading

vrothberg left a comment

Choose a reason for hiding this comment

flouthoc commented Feb 24, 2022

mtrmac left a comment

Choose a reason for hiding this comment

mtrmac commented Feb 25, 2022

vrothberg left a comment

Choose a reason for hiding this comment

flouthoc commented Mar 1, 2022

mtrmac commented Mar 1, 2022

flouthoc commented Mar 23, 2022

vrothberg commented Mar 23, 2022

mtrmac left a comment • edited Loading

Choose a reason for hiding this comment

storage: use race-free `AddNames` instead of `SetNames` #1480

storage: use race-free `AddNames` instead of `SetNames` #1480

flouthoc commented Feb 24, 2022 •

edited

Loading

mtrmac left a comment •

edited

Loading