Merge pull request #13404 from flouthoc/bump-to-race-free-deps

deps: bump to race-free `c/image` and `c/storage` along with test to verify `concurrent/parallel` builds
This commit is contained in:
OpenShift Merge Robot 2022-03-03 12:15:19 -05:00 committed by GitHub
commit 3cfb70f953
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 265 additions and 71 deletions

4
go.mod
View File

@ -14,10 +14,10 @@ require (
github.com/containers/buildah v1.24.2
github.com/containers/common v0.47.5-0.20220228211119-9880eb424fde
github.com/containers/conmon v2.0.20+incompatible
github.com/containers/image/v5 v5.19.2-0.20220224100137-1045fb70b094
github.com/containers/image/v5 v5.19.2-0.20220302121925-9a9cd9322006
github.com/containers/ocicrypt v1.1.2
github.com/containers/psgo v1.7.2
github.com/containers/storage v1.38.3-0.20220228132533-ebc90aba7d29
github.com/containers/storage v1.38.3-0.20220301151551-d06b0f81c0aa
github.com/coreos/go-systemd/v22 v22.3.2
github.com/coreos/stream-metadata-go v0.0.0-20210225230131-70edb9eb47b3
github.com/cyphar/filepath-securejoin v0.2.3

7
go.sum
View File

@ -358,8 +358,9 @@ github.com/containers/common v0.47.5-0.20220228211119-9880eb424fde/go.mod h1:pks
github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
github.com/containers/image/v5 v5.19.1/go.mod h1:ewoo3u+TpJvGmsz64XgzbyTHwHtM94q7mgK/pX+v2SE=
github.com/containers/image/v5 v5.19.2-0.20220224100137-1045fb70b094 h1:27NmJhSA35ldAiq0pV2cXjj6YERVrQU8ectvWqblFxE=
github.com/containers/image/v5 v5.19.2-0.20220224100137-1045fb70b094/go.mod h1:XoYK6kE0dpazFNcuS+a8lra+QfbC6s8tzv+cUuCrZpE=
github.com/containers/image/v5 v5.19.2-0.20220302121925-9a9cd9322006 h1:84BWsbDrrkzKG2nj3bA+/RyBcjJbb3vEwT2rwqsEWsw=
github.com/containers/image/v5 v5.19.2-0.20220302121925-9a9cd9322006/go.mod h1:S3gHB0QSB4IdADVtDbFbvFDdDq5v4ENMyXIo+Qi4cm8=
github.com/containers/libtrust v0.0.0-20190913040956-14b96171aa3b/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a h1:spAGlqziZjCJL25C6F1zsQY05tfCKE9F5YwtEWWe6hU=
github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
@ -373,8 +374,8 @@ github.com/containers/psgo v1.7.2/go.mod h1:SLpqxsPOHtTqRygjutCPXmeU2PoEFzV3gzJp
github.com/containers/storage v1.37.0/go.mod h1:kqeJeS0b7DO2ZT1nVWs0XufrmPFbgV3c+Q/45RlH6r4=
github.com/containers/storage v1.38.0/go.mod h1:lBzt28gAk5ADZuRtwdndRJyqX22vnRaXmlF+7ktfMYc=
github.com/containers/storage v1.38.2/go.mod h1:INP0RPLHWBxx+pTsO5uiHlDUGHDFvWZPWprAbAlQWPQ=
github.com/containers/storage v1.38.3-0.20220228132533-ebc90aba7d29 h1:jKxTQc8+kAoYi/oQoMOptWi7CXsicJ/i6DR5GZCyISw=
github.com/containers/storage v1.38.3-0.20220228132533-ebc90aba7d29/go.mod h1:LkkL34WRi4dI4jt9Cp+ImdZi/P5i36glSHimT5CP5zM=
github.com/containers/storage v1.38.3-0.20220301151551-d06b0f81c0aa h1:rdJIWaQ7PwS2N0ZWgn6NXDp+8KtvfSmPTJ3S5i6fJr4=
github.com/containers/storage v1.38.3-0.20220301151551-d06b0f81c0aa/go.mod h1:LkkL34WRi4dI4jt9Cp+ImdZi/P5i36glSHimT5CP5zM=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=

View File

@ -210,6 +210,30 @@ EOF
run_podman rmi -f build_test
}
@test "podman parallel build should not race" {
skip_if_remote "following test is not supported for remote clients"
# Run thirty parallel builds using the same Containerfile
cat >$PODMAN_TMPDIR/Containerfile <<EOF
FROM $IMAGE
RUN echo hi
EOF
local count=30
for i in $(seq --format '%02g' 1 $count); do
timeout --foreground -v --kill=10 60 \
$PODMAN build -t i$i $PODMAN_TMPDIR &>/dev/null &
done
# Wait for all background builds to complete. Note that this succeeds
# even if some of the individual builds fail! Our actual test is below.
wait
# Now delete all built images. If any image wasn't built, rmi will fail
# and test will fail.
run_podman rmi $(seq --format 'i%02g' 1 $count)
}
@test "podman build - URLs" {
tmpdir=$PODMAN_TMPDIR/build-test
mkdir -p $tmpdir

View File

@ -112,7 +112,7 @@ func (d *ociImageDestination) IgnoresEmbeddedDockerReference() bool {
// HasThreadSafePutBlob indicates whether PutBlob can be executed concurrently.
func (d *ociImageDestination) HasThreadSafePutBlob() bool {
return false
return true
}
// PutBlob writes contents of stream and returns data representing the result.

View File

@ -1197,21 +1197,13 @@ func (s *storageImageDestination) Commit(ctx context.Context, unparsedToplevel t
}
logrus.Debugf("saved image metadata %q", string(metadata))
}
// 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 {
logrus.Debugf("error setting names %v on image %q: %v", names, img.ID, err)
return errors.Wrapf(err, "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)
}
commitSucceeded = true

View File

@ -6,9 +6,9 @@ const (
// VersionMajor is for an API incompatible changes
VersionMajor = 5
// VersionMinor is for functionality in a backwards-compatible manner
VersionMinor = 19
VersionMinor = 20
// VersionPatch is for backwards-compatible bug fixes
VersionPatch = 2
VersionPatch = 1
// VersionDev indicates development branch. Releases will be empty string.
VersionDev = "-dev"

View File

@ -84,8 +84,17 @@ type ContainerStore interface {
// SetNames updates the list of names associated with the container
// with the specified ID.
// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
SetNames(id string, names []string) error
// AddNames adds the supplied values to the list of names associated with the container with
// the specified id.
AddNames(id string, names []string) error
// RemoveNames removes the supplied values from the list of names associated with the container with
// the specified id.
RemoveNames(id string, names []string) error
// Get retrieves information about a container given an ID or name.
Get(id string) (*Container, error)
@ -377,22 +386,40 @@ func (r *containerStore) removeName(container *Container, name string) {
container.Names = stringSliceWithoutValue(container.Names, name)
}
// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
func (r *containerStore) SetNames(id string, names []string) error {
names = dedupeNames(names)
if container, ok := r.lookup(id); ok {
for _, name := range container.Names {
delete(r.byname, name)
}
for _, name := range names {
if otherContainer, ok := r.byname[name]; ok {
r.removeName(otherContainer, name)
}
r.byname[name] = container
}
container.Names = names
return r.Save()
return r.updateNames(id, names, setNames)
}
func (r *containerStore) AddNames(id string, names []string) error {
return r.updateNames(id, names, addNames)
}
func (r *containerStore) RemoveNames(id string, names []string) error {
return r.updateNames(id, names, removeNames)
}
func (r *containerStore) updateNames(id string, names []string, op updateNameOperation) error {
container, ok := r.lookup(id)
if !ok {
return ErrContainerUnknown
}
return ErrContainerUnknown
oldNames := container.Names
names, err := applyNameOperation(oldNames, names, op)
if err != nil {
return err
}
for _, name := range oldNames {
delete(r.byname, name)
}
for _, name := range names {
if otherContainer, ok := r.byname[name]; ok {
r.removeName(otherContainer, name)
}
r.byname[name] = container
}
container.Names = names
return r.Save()
}
func (r *containerStore) Delete(id string) error {

View File

@ -1530,7 +1530,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
diffDir := path.Join(id, "diff")
opts = fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", strings.Join(relLowers, ":"), diffDir, workdir)
} else {
opts = fmt.Sprintf("lowerdir=%s", strings.Join(absLowers, ":"))
opts = fmt.Sprintf("lowerdir=%s", strings.Join(relLowers, ":"))
}
if len(optsList) > 0 {
opts = fmt.Sprintf("%s,%s", opts, strings.Join(optsList, ","))

View File

@ -1,6 +1,8 @@
package storage
import (
"errors"
"github.com/containers/storage/types"
)
@ -57,4 +59,7 @@ var (
ErrNotSupported = types.ErrNotSupported
// ErrInvalidMappings is returned when the specified mappings are invalid.
ErrInvalidMappings = types.ErrInvalidMappings
// ErrInvalidNameOperation is returned when updateName is called with invalid operation.
// Internal error
errInvalidUpdateNameOperation = errors.New("invalid update name operation")
)

View File

@ -136,8 +136,19 @@ type ImageStore interface {
// SetNames replaces the list of names associated with an image with the
// supplied values. The values are expected to be valid normalized
// named image references.
// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
SetNames(id string, names []string) error
// AddNames adds the supplied values to the list of names associated with the image with
// the specified id. The values are expected to be valid normalized
// named image references.
AddNames(id string, names []string) error
// RemoveNames removes the supplied values from the list of names associated with the image with
// the specified id. The values are expected to be valid normalized
// named image references.
RemoveNames(id string, names []string) error
// Delete removes the record of the image.
Delete(id string) error
@ -505,26 +516,44 @@ func (i *Image) addNameToHistory(name string) {
i.NamesHistory = dedupeNames(append([]string{name}, i.NamesHistory...))
}
// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
func (r *imageStore) SetNames(id string, names []string) error {
return r.updateNames(id, names, setNames)
}
func (r *imageStore) AddNames(id string, names []string) error {
return r.updateNames(id, names, addNames)
}
func (r *imageStore) RemoveNames(id string, names []string) error {
return r.updateNames(id, names, removeNames)
}
func (r *imageStore) updateNames(id string, names []string, op updateNameOperation) error {
if !r.IsReadWrite() {
return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to change image name assignments at %q", r.imagespath())
}
names = dedupeNames(names)
if image, ok := r.lookup(id); ok {
for _, name := range image.Names {
delete(r.byname, name)
}
for _, name := range names {
if otherImage, ok := r.byname[name]; ok {
r.removeName(otherImage, name)
}
r.byname[name] = image
image.addNameToHistory(name)
}
image.Names = names
return r.Save()
image, ok := r.lookup(id)
if !ok {
return errors.Wrapf(ErrImageUnknown, "error locating image with ID %q", id)
}
return errors.Wrapf(ErrImageUnknown, "error locating image with ID %q", id)
oldNames := image.Names
names, err := applyNameOperation(oldNames, names, op)
if err != nil {
return err
}
for _, name := range oldNames {
delete(r.byname, name)
}
for _, name := range names {
if otherImage, ok := r.byname[name]; ok {
r.removeName(otherImage, name)
}
r.byname[name] = image
image.addNameToHistory(name)
}
image.Names = names
return r.Save()
}
func (r *imageStore) Delete(id string) error {

View File

@ -221,8 +221,17 @@ type LayerStore interface {
// SetNames replaces the list of names associated with a layer with the
// supplied values.
// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
SetNames(id string, names []string) error
// AddNames adds the supplied values to the list of names associated with the layer with the
// specified id.
AddNames(id string, names []string) error
// RemoveNames remove the supplied values from the list of names associated with the layer with the
// specified id.
RemoveNames(id string, names []string) error
// Delete deletes a layer with the specified name or ID.
Delete(id string) error
@ -1040,25 +1049,43 @@ func (r *layerStore) removeName(layer *Layer, name string) {
layer.Names = stringSliceWithoutValue(layer.Names, name)
}
// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
func (r *layerStore) SetNames(id string, names []string) error {
return r.updateNames(id, names, setNames)
}
func (r *layerStore) AddNames(id string, names []string) error {
return r.updateNames(id, names, addNames)
}
func (r *layerStore) RemoveNames(id string, names []string) error {
return r.updateNames(id, names, removeNames)
}
func (r *layerStore) updateNames(id string, names []string, op updateNameOperation) error {
if !r.IsReadWrite() {
return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to change layer name assignments at %q", r.layerspath())
}
names = dedupeNames(names)
if layer, ok := r.lookup(id); ok {
for _, name := range layer.Names {
delete(r.byname, name)
}
for _, name := range names {
if otherLayer, ok := r.byname[name]; ok {
r.removeName(otherLayer, name)
}
r.byname[name] = layer
}
layer.Names = names
return r.Save()
layer, ok := r.lookup(id)
if !ok {
return ErrLayerUnknown
}
return ErrLayerUnknown
oldNames := layer.Names
names, err := applyNameOperation(oldNames, names, op)
if err != nil {
return err
}
for _, name := range oldNames {
delete(r.byname, name)
}
for _, name := range names {
if otherLayer, ok := r.byname[name]; ok {
r.removeName(otherLayer, name)
}
r.byname[name] = layer
}
layer.Names = names
return r.Save()
}
func (r *layerStore) datadir(id string) string {

View File

@ -31,6 +31,14 @@ import (
"github.com/pkg/errors"
)
type updateNameOperation int
const (
setNames updateNameOperation = iota
addNames
removeNames
)
var (
stores []*store
storesLock sync.Mutex
@ -368,8 +376,17 @@ type Store interface {
// SetNames changes the list of names for a layer, image, or container.
// Duplicate names are removed from the list automatically.
// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
SetNames(id string, names []string) error
// AddNames adds the list of names for a layer, image, or container.
// Duplicate names are removed from the list automatically.
AddNames(id string, names []string) error
// RemoveNames removes the list of names for a layer, image, or container.
// Duplicate names are removed from the list automatically.
RemoveNames(id string, names []string) error
// ListImageBigData retrieves a list of the (possibly large) chunks of
// named data associated with an image.
ListImageBigData(id string) ([]string, error)
@ -2050,7 +2067,20 @@ func dedupeNames(names []string) []string {
return deduped
}
// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`.
func (s *store) SetNames(id string, names []string) error {
return s.updateNames(id, names, setNames)
}
func (s *store) AddNames(id string, names []string) error {
return s.updateNames(id, names, addNames)
}
func (s *store) RemoveNames(id string, names []string) error {
return s.updateNames(id, names, removeNames)
}
func (s *store) updateNames(id string, names []string, op updateNameOperation) error {
deduped := dedupeNames(names)
rlstore, err := s.LayerStore()
@ -2063,7 +2093,16 @@ func (s *store) SetNames(id string, names []string) error {
return err
}
if rlstore.Exists(id) {
return rlstore.SetNames(id, deduped)
switch op {
case setNames:
return rlstore.SetNames(id, deduped)
case removeNames:
return rlstore.RemoveNames(id, deduped)
case addNames:
return rlstore.AddNames(id, deduped)
default:
return errInvalidUpdateNameOperation
}
}
ristore, err := s.ImageStore()
@ -2076,7 +2115,16 @@ func (s *store) SetNames(id string, names []string) error {
return err
}
if ristore.Exists(id) {
return ristore.SetNames(id, deduped)
switch op {
case setNames:
return ristore.SetNames(id, deduped)
case removeNames:
return ristore.RemoveNames(id, deduped)
case addNames:
return ristore.AddNames(id, deduped)
default:
return errInvalidUpdateNameOperation
}
}
// Check is id refers to a RO Store
@ -2114,7 +2162,16 @@ func (s *store) SetNames(id string, names []string) error {
return err
}
if rcstore.Exists(id) {
return rcstore.SetNames(id, deduped)
switch op {
case setNames:
return rcstore.SetNames(id, deduped)
case removeNames:
return rcstore.RemoveNames(id, deduped)
case addNames:
return rcstore.AddNames(id, deduped)
default:
return errInvalidUpdateNameOperation
}
}
return ErrLayerUnknown
}

View File

@ -40,3 +40,35 @@ func validateMountOptions(mountOptions []string) error {
}
return nil
}
func applyNameOperation(oldNames []string, opParameters []string, op updateNameOperation) ([]string, error) {
result := make([]string, 0)
switch op {
case setNames:
// ignore all old names and just return new names
return dedupeNames(opParameters), nil
case removeNames:
// remove given names from old names
for _, name := range oldNames {
// only keep names in final result which do not intersect with input names
// basically `result = oldNames - opParameters`
nameShouldBeRemoved := false
for _, opName := range opParameters {
if name == opName {
nameShouldBeRemoved = true
}
}
if !nameShouldBeRemoved {
result = append(result, name)
}
}
return dedupeNames(result), nil
case addNames:
result = append(result, opParameters...)
result = append(result, oldNames...)
return dedupeNames(result), nil
default:
return result, errInvalidUpdateNameOperation
}
return dedupeNames(result), nil
}

4
vendor/modules.txt vendored
View File

@ -153,7 +153,7 @@ github.com/containers/common/version
# github.com/containers/conmon v2.0.20+incompatible
## explicit
github.com/containers/conmon/runner/config
# github.com/containers/image/v5 v5.19.2-0.20220224100137-1045fb70b094
# github.com/containers/image/v5 v5.19.2-0.20220302121925-9a9cd9322006
## explicit
github.com/containers/image/v5/copy
github.com/containers/image/v5/directory
@ -232,7 +232,7 @@ github.com/containers/psgo/internal/dev
github.com/containers/psgo/internal/host
github.com/containers/psgo/internal/proc
github.com/containers/psgo/internal/process
# github.com/containers/storage v1.38.3-0.20220228132533-ebc90aba7d29
# github.com/containers/storage v1.38.3-0.20220301151551-d06b0f81c0aa
## explicit
github.com/containers/storage
github.com/containers/storage/drivers