From 648933a1a995988d55a32ffbf525d22f87db0e2a Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 13 Feb 2024 13:43:16 +0100 Subject: [PATCH 1/6] [release-1.45] overlay: replace rmdir with rename instead of removing the "merged" directory, replace it with a fresh empty directory so that we can assume "merged" is always present and there is no need to recreate it. Signed-off-by: Giuseppe Scrivano --- drivers/overlay/overlay.go | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index ed67000a69..28c40ae795 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -1633,7 +1633,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 +1691,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 } From 48e0d66f5ccfaf67247d0d3fc855831465b433a7 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 14 Feb 2024 12:25:50 +0100 Subject: [PATCH 2/6] [release-1.45] overlay: ignore idtools.MkdirAllAs(diffDir) errors ignore errors creating and chowning the diffDiff if it is in an additional image store. Signed-off-by: Giuseppe Scrivano Signed-off-by: tomsweeneyredhat --- drivers/overlay/overlay.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 28c40ae795..d6abb7628c 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -1469,7 +1469,13 @@ 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") From c799ba9ab61762c882c888393224f770ca51887c Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 11 Mar 2024 13:55:02 +0100 Subject: [PATCH 3/6] [release-1.45] overlay: create the merged path only if it does not exist follow-up for ccb70a79a69a1a5137ff24720520534bfbcc2316 more information here: https://github.com/containers/storage/issues/1827#issuecomment-1988332922 Addresses: https://issues.redhat.com/browse/ACCELFIX-244 Signed-off-by: Giuseppe Scrivano Signed-off-by: tomsweeneyredhat --- drivers/overlay/overlay.go | 10 ++++++---- tests/overlay-recreate.bats | 5 ++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index d6abb7628c..15b343d156 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -1479,9 +1479,11 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO } 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 @@ -1639,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, _, inAdditionalStore := d.dir2(id) + dir, inAdditionalStore := d.dir2(id) if _, err := os.Stat(dir); err != nil { return err } diff --git a/tests/overlay-recreate.bats b/tests/overlay-recreate.bats index 038a48810a..c3ebbb3297 100644 --- a/tests/overlay-recreate.bats +++ b/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. From 85b598a6c1756b1608a799c56a84b852c24e9431 Mon Sep 17 00:00:00 2001 From: tomsweeneyredhat Date: Thu, 27 Jun 2024 14:49:16 -0400 Subject: [PATCH 4/6] [release-1.45] Bump lint and address lint issue Bump lint in the test area to 1.55.2 and then address an append that's causing lint heartache. Also change the install process for lint to be like the latest releases. Signed-off-by: tomsweeneyredhat --- drivers/aufs/aufs_test.go | 2 +- tests/tools/Makefile | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/aufs/aufs_test.go b/drivers/aufs/aufs_test.go index 415d07021b..36ee9c4206 100644 --- a/drivers/aufs/aufs_test.go +++ b/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 diff --git a/tests/tools/Makefile b/tests/tools/Makefile index 8da7d244d2..f9cd9eb23f 100644 --- a/tests/tools/Makefile +++ b/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) From 74a81ba69df3eb4b4fac26d66ea31d71bc3c6fe6 Mon Sep 17 00:00:00 2001 From: tomsweeneyredhat Date: Thu, 11 Jul 2024 10:07:18 -0400 Subject: [PATCH 5/6] [release-1.45] Remove lint from testing In https://github.com/containers/storage/pull/1992 there were a number of lint issues that were not solvable. After consulting with @edsantiago and @rhatdan, it was decided to remove the line processing from this branch. Signed-off-by: tomsweeneyredhat --- .cirrus.yml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 6f9048564c..ec59ea38bd 100644 --- a/.cirrus.yml +++ b/.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 From 6aedf42877134440afef1d7dbf096be7f9d567c8 Mon Sep 17 00:00:00 2001 From: tomsweeneyredhat Date: Fri, 28 Jun 2024 15:01:40 -0400 Subject: [PATCH 6/6] [release-1.45] Bump to v1.45.8 Signed-off-by: tomsweeneyredhat --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 238179f21e..e7ccb34250 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.45.7-dev +1.45.8