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

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion go.mod
Expand Up @@ -31,7 +31,7 @@ require (
github.com/pquerna/ffjson v0.0.0-20190813045741-dac163c6c0a9 // indirect
github.com/sirupsen/logrus v1.7.0
github.com/stretchr/testify v1.7.0
github.com/ulikunitz/xz v0.5.9
github.com/ulikunitz/xz v0.5.10
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to be explicit, this c/storage change will drag in a bit more than just AddNames. It’s still on the 1.24 branch of c/storage, so hopefully fine.

github.com/vbatts/tar-split v0.11.1
github.com/vbauerster/mpb/v5 v5.4.0
github.com/xeipuuv/gojsonpointer v0.0.0-20190809123943-df4f5c81cb3b // indirect
Expand All @@ -44,3 +44,5 @@ require (
golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
)

replace github.com/containers/storage => github.com/flouthoc/storage v1.24.9-0.20220406143351-b050a8141d8d
3 changes: 3 additions & 0 deletions go.sum
Expand Up @@ -68,6 +68,8 @@ github.com/docker/go-units v0.4.0 h1:3uh0PgVws3nIA0Q+MwDC8yjEPf9zjRfZZWXZYDct3Tw
github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 h1:UhxFibDNY/bfvqU5CAUmr9zpesgbU6SWc8/B4mflAE4=
github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7/go.mod h1:cyGadeNEkKy96OOhEzfZl+yxihPEzKnqJwvfuSUqbZE=
github.com/flouthoc/storage v1.24.9-0.20220406143351-b050a8141d8d h1:vAlqUKp756AAI0Ns/EK1aOUlpRkY7yPaGpFe63PfqiU=
github.com/flouthoc/storage v1.24.9-0.20220406143351-b050a8141d8d/go.mod h1:5Gjxx8EqRRuTC6J2dbQ/5SMs43SHhe3Ky+BzyeNVPZM=
github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
Expand Down Expand Up @@ -228,6 +230,7 @@ github.com/tchap/go-patricia v2.3.0+incompatible h1:GkY4dP3cEfEASBPPkWd+AmjYxhmD
github.com/tchap/go-patricia v2.3.0+incompatible/go.mod h1:bmLyhP68RS6kStMGxByiQ23RP/odRBOTVjwp2cDyi6I=
github.com/ulikunitz/xz v0.5.9 h1:RsKRIA2MO8x56wkkcd3LbtcE/uMszhb6DpRf+3uwa3I=
github.com/ulikunitz/xz v0.5.9/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14=
github.com/ulikunitz/xz v0.5.10/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14=
github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
github.com/vbatts/tar-split v0.11.1 h1:0Odu65rhcZ3JZaPHxl7tCI3V/C/Q9Zf82UFravl02dE=
Expand Down
23 changes: 6 additions & 17 deletions storage/storage_image.go
Expand Up @@ -802,24 +802,13 @@ func (s *storageImageDestination) Commit(ctx context.Context, unparsedToplevel t
return errors.Wrapf(err, "error saving big data %q for image %q", blob.String(), img.ID)
}
}
// Set the reference's name on the image. We don't need to worry about avoiding duplicate
// values because SetNames() will deduplicate the list that we pass to it.
if name := s.imageRef.DockerReference(); len(oldNames) > 0 || name != nil {
names := []string{}
if name != nil {
names = append(names, name.String())
// Adds the reference's name on the image. We don't need to worry about avoiding duplicate
// values because AddNames() will deduplicate the list that we pass to it.
if name := s.imageRef.DockerReference(); name != nil {
if err := s.imageRef.transport.store.AddNames(img.ID, []string{name.String()}); err != nil {
return errors.Wrapf(err, "adding names %v to image %q", name, img.ID)
}
if len(oldNames) > 0 {
names = append(names, oldNames...)
}
if err := s.imageRef.transport.store.SetNames(img.ID, names); err != nil {
if _, err2 := s.imageRef.transport.store.DeleteImage(img.ID, true); err2 != nil {
logrus.Debugf("error deleting incomplete image %q: %v", img.ID, err2)
}
logrus.Debugf("error setting names %v on image %q: %v", names, img.ID, err)
return errors.Wrapf(err, "error setting names %v on image %q", names, img.ID)
}
logrus.Debugf("set names of image %q to %v", img.ID, names)
logrus.Debugf("added name %q to image %q", name, img.ID)
}
// Save the unparsedToplevel's manifest.
if len(toplevelManifest) != 0 {
Expand Down