Merge pull request #1992 from TomSweeneyRedHat/dev/tsweeney/acel244-1.45

 [release-1.45] Backport ignore chown errors in additionalimagestore

.cirrus.yml

@@ -56,8 +56,6 @@ gce_instance:
 fedora_testing_task: &fedora_testing
     alias: fedora_testing
     name: &std_test_name "${OS_NAME} ${TEST_DRIVER}"
-    depends_on:
-        - lint
 
     gce_instance:  # Only need to specify differences from defaults (above)
         image_name: "${VM_IMAGE}"
@@ -104,21 +102,6 @@ ubuntu_testing_task: &ubuntu_testing
             TEST_DRIVER: "overlay"
 
 
-lint_task:
-    env:
-        CIRRUS_WORKING_DIR: "/go/src/github.com/containers/storage"
-    container:
-        image: golang:1.17
-    modules_cache:
-        fingerprint_script: cat go.sum
-        folder: $GOPATH/pkg/mod
-    build_script: |
-      echo "deb http://deb.debian.org/debian stretch-backports main" > /etc/apt/sources.list.d/backports.list
-      apt-get update
-      apt-get install -y libbtrfs-dev libdevmapper-dev
-    test_script: make TAGS=regex_precompile local-validate && make lint && make clean
-
-
 # Update metadata on VM images referenced by this repository state
 meta_task:
 
@@ -155,7 +138,6 @@ vendor_task:
 # Represent overall pass/fail status from required dependent tasks
 success_task:
     depends_on:
-        - lint
         - fedora_testing
         - ubuntu_testing
         - meta

VERSION

@@ -1 +1 @@
-1.45.7-dev
+1.45.8

drivers/aufs/aufs_test.go

@@ -744,7 +744,7 @@ func BenchmarkConcurrentAccess(b *testing.B) {
 	}
 
 	parent := ids[1]
-	ids = append(ids[2:])
+	ids = ids[2:]
 
 	chErr := make(chan error, numConcurrent)
 	var outerGroup sync.WaitGroup

drivers/overlay/overlay.go

@@ -1469,13 +1469,21 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
 	}
 	diffDir := path.Join(dir, "diff")
 	if err := idtools.MkdirAllAs(diffDir, perms, rootUID, rootGID); err != nil {
-		return "", err
+		if !inAdditionalStore {
+			return "", err
+		}
+		// if it is in an additional store, do not fail if the directory already exists
+		if _, err2 := os.Stat(diffDir); err2 != nil {
+			return "", err
+		}
 	}
 
 	mergedDir := path.Join(dir, "merged")
-	// Create the driver merged dir
-	if err := idtools.MkdirAs(mergedDir, 0700, rootUID, rootGID); err != nil && !os.IsExist(err) {
-		return "", err
+	// Attempt to create the merged dir only if it doesn't exist.
+	if _, err := os.Stat(mergedDir); err != nil && os.IsNotExist(err) {
+		if err := idtools.MkdirAs(mergedDir, 0o700, rootUID, rootGID); err != nil && !os.IsExist(err) {
+			return "", err
+		}
 	}
 	if count := d.ctr.Increment(mergedDir); count > 1 {
 		return mergedDir, nil
@@ -1633,7 +1641,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
 
 // Put unmounts the mount path created for the give id.
 func (d *Driver) Put(id string) error {
-	dir := d.dir(id)
+	dir, inAdditionalStore := d.dir2(id)
 	if _, err := os.Stat(dir); err != nil {
 		return err
 	}
@@ -1691,10 +1699,27 @@ func (d *Driver) Put(id string) error {
 		}
 	}
 
-	if err := unix.Rmdir(mountpoint); err != nil && !os.IsNotExist(err) {
-		logrus.Debugf("Failed to remove mountpoint %s overlay: %s - %v", id, mountpoint, err)
-	}
+	if !inAdditionalStore {
+		uid, gid := int(0), int(0)
+		fi, err := os.Stat(mountpoint)
+		if err != nil {
+			return err
+		}
+		if stat, ok := fi.Sys().(*syscall.Stat_t); ok {
+			uid, gid = int(stat.Uid), int(stat.Gid)
+		}
 
+		tmpMountpoint := path.Join(dir, "merged.1")
+		if err := idtools.MkdirAs(tmpMountpoint, 0o700, uid, gid); err != nil && !errors.Is(err, os.ErrExist) {
+			return err
+		}
+		// rename(2) can be used on an empty directory, as it is the mountpoint after umount, and it retains
+		// its atomic semantic.  In this way the "merged" directory is never removed.
+		if err := unix.Rename(tmpMountpoint, mountpoint); err != nil {
+			logrus.Debugf("Failed to replace mountpoint %s overlay: %s - %v", id, mountpoint, err)
+			return fmt.Errorf("replacing mount point %q: %w", mountpoint, err)
+		}
+	}
 	return nil
 }
 

tests/overlay-recreate.bats

@@ -18,7 +18,8 @@ load helpers
 	storage unmount "$lowerlayer"
 	storage mount "$midlayer"
 	storage unmount "$midlayer"
-	storage mount "$upperlayer"
+	run storage --debug=false mount "$upperlayer"
+	merged_dir="$output"
 	storage unmount "$upperlayer"
 	# okay, but how about this?
 	rm -v ${TESTDIR}/root/overlay/*/link
@@ -27,6 +28,8 @@ load helpers
 	storage unmount "$lowerlayer"
 	storage mount "$midlayer"
 	storage unmount "$midlayer"
+	# check it works if we delete the merged directory.
+	rmdir "$merged_dir"
 	storage mount "$upperlayer"
 	storage unmount "$upperlayer"
 	# okay, not bad, kid.

tests/tools/Makefile

@@ -33,9 +33,6 @@ $(BUILDDIR)/git-validation:
 $(BUILDDIR)/go-md2man:
 	$(call go-build,./vendor/github.com/cpuguy83/go-md2man)
 
+$(BUILDDIR)/golangci-lint: VERSION=v1.55.2
 $(BUILDDIR)/golangci-lint:
-	export \
-		VERSION=v1.24.0 \
-		URL=https://raw.githubusercontent.com/golangci/golangci-lint \
-		BINDIR=$(BUILDDIR) && \
-	curl -sfL $$URL/$$VERSION/install.sh | sh -s $$VERSION
+	curl -fsSL https://raw.githubusercontent.com/golangci/golangci-lint/$(VERSION)/install.sh | sh -s -- -b ./$(BUILDDIR) $(VERSION)