Revert commit that breaks podman backwards compatibility

[Luap99]

Revert "overlay: do not recreate "merged" dir"

This reverts commit de38d80.

Podman upgrade tests are faiing because of this[1]. A start of container
created with a previous version is now failing because of the missing
merged dir.

A very simple reproducer is to run a container with a older versin then
update to one with this commit and try to start the container again:
podman run --name test quay.io/libpod/testimage:20240123 echo hi
<-- update podman to a storage version with the reverted commit
podman start -a test
<-- now failing while trying to mount the container

[1] https://api.cirrus-ci.com/v1/artifact/task/5918496639418368/html/upgrade_test-podman-fedora-39-root-host.log.html

[Luap99]

@giuseppe @mtrmac @rhatdan PTAL

I haven't looked closely at #1827 but this likely means we need to reopen that one. Breaking backwards compatibility by no longer creating the merged dir seems to be not an option.

If you want to propose a better fix instead of reverting this is fine for me but I like to add that we a short one time for the vendor dance.

[giuseppe]

let me take a look

[giuseppe]

the failure I see is something broken in the Podman test:

    # umount, and make sure files are gone
    run_podman umount $external_cid
    if [ -d "$mount_path" ]; then
        # Under VFS, mountpoint always exists even despite umount
        if [[ "$(podman_storage_driver)" != "vfs" ]]; then
            die "'podman umount' did not umount $mount_path"
        fi
    fi

The directory can be present, and not be a mountpoint

[giuseppe]

a better test would be findmnt "$mount_path" >/dev/null; if it succeeds the path is a mountpoint

[giuseppe]

then we don't even need to worry about the vfs special case

diff --git a/test/system/060-mount.bats b/test/system/060-mount.bats
index fbc925a47..0a7123c40 100644
--- a/test/system/060-mount.bats
+++ b/test/system/060-mount.bats
@@ -251,11 +251,8 @@ EOF
 
     # umount, and make sure files are gone
     run_podman umount $external_cid
-    if [ -d "$mount_path" ]; then
-        # Under VFS, mountpoint always exists even despite umount
-        if [[ "$(podman_storage_driver)" != "vfs" ]]; then
-            die "'podman umount' did not umount $mount_path"
-        fi
+    if test \! findmnt "$mount_path" >/dev/null ; then
+        die "'podman umount' did not umount $mount_path"
     fi
     buildah rm $external_cid
 }

[Luap99]

Yeah the first failure did indeed seems a bit odd to me as well but I didn't took to closely at the test.
We should certainly fix the test for this then, however the second one is a real problem as it breaks all old podman containers.

[edsantiago]

There is nothing special about that "Under VFS" code: it's only a few months old, from #20161, and it was my (ignorant) way of dealing with a difference between VFS and overlay. IOW I wrote that code as a way to deal with the situation as I saw it, not because I knew what I was doing.

[giuseppe]

we can probably revert 12dd077, it should be a no-op if the directory exists

[Luap99]

Ack, let me drop the fist revert then.

[giuseppe]

There is nothing special about that "Under VFS" code: it's only a few months old, from #20161, and it was my (ignorant) way of dealing with a difference between VFS and overlay. IOW I wrote that code as a way to deal with the situation as I saw it, not because I knew what I was doing.

to be fair, that was confusing in c/storage. We are still treating vfs differently, and the lack of a mountpoint for vfs is the root cause for most/(all?) the flakes we see in the Podman CI when vfs is used.

[giuseppe]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Luap99,giuseppe]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Luap99]

I created containers/podman#21875 in podman to fix the test.

[rhatdan]

LGTM