fix: Ignore ENOENT error when calling unmount

[ninja-quokka]

If the file or directoy doesn't exist then we can assume it is unmounted.

JIRA: https://issues.redhat.com/browse/RHEL-99488

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ninja-quokka
Once this PR has been reviewed and has the lgtm label, please assign rhatdan for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[ninja-quokka]

Maybe I should handle this sooner, for example checking if d.home exists before calling mount.Unmount()

storage/drivers/overlay/overlay.go

Lines 859 to 865 in e1679c1

	func (d *Driver) Cleanup() error {
	anyPresent := d.pruneStagingDirectories()
	if anyPresent {
	return nil
	}
	return mount.Unmount(d.home)
	}

[mtrmac]

This looks like a fairly crude hammer, affecting many callers, and ENOENT might well be an indication of a serious problem.

Can you trace and write down how the ENOENT happens, please?

From a quick check, drivers/overlay.Init should always create home (e.g. as a side-effect of creating linkDir). And then, podman system reset invokes store.Shutdown, calling overlay.Driver.Cleanup — at that point I’d expect home to still exist. Only afterwards does podman system reset start actually indiscriminately removing things.

What am I missing? When does this ENOENT happen? Is it that the reset actually fully succeeds, and we fail only later when closing down the Podman process, and that triggers another store shutdown operation?

[mtrmac]

Looking a tiny bit further, libpod/Runtime.Reset shuts down c/storage at least twice: Once via direct r.store.Shutdown, once via r.Shutdown→r.libimageRuntime.Shutdown.

I’m not saying that definitely means that c/storage is not ultimately the best place to fix this — reset is rather a special case and fundamentally awkward to an extent — but it does, I think, strongly suggest that Podman’s idea of the states and lifetimes of Runtime and its components is not precisely modeled / thought out.

[ninja-quokka]

This looks like a fairly crude hammer, affecting many callers, and ENOENT might well be an indication of a serious problem.

Can you trace and write down how the ENOENT happens, please?

From a quick check, drivers/overlay.Init should always create home (e.g. as a side-effect of creating linkDir). And then, podman system reset invokes store.Shutdown, calling overlay.Driver.Cleanup — at that point I’d expect home to still exist. Only afterwards does podman system reset start actually indiscriminately removing things.

What am I missing? When does this ENOENT happen? Is it that the reset actually fully succeeds, and we fail only later when closing down the Podman process, and that triggers another store shutdown operation?

I agree it's a crude hammer, I couldn't think of when we would care that something doesn't exist when we're trying to unmount it anyway but I'm not happy other callers would be effected with this patch.

The ENOENT can be reproduced on a fresh system (RHEL or Fedora) where you run podman system reset as the root user before actually running any Podman commands. At that point in time, the store hasn't been initialized and the /var/lib/containers/storage/ directory has not been created yet.

In this situation, from the Podman side, we don't care if /var/lib/containers/storage/ doesn't exist as we're trying to reset it anyway.

What do you think about the following ideas?

1. Where the error prints in c/podman, ignore the error if it's ENOENT.
2. Where the error prints in c/podman, change the log message to debug, trace or warning level rather than error.
3. In c/storage, first check if the directory exists before attempting to unmount it.

Thank you and appreciate your expertise on this issue.

[mtrmac]

At that point in time, the store hasn't been initialized and the /var/lib/containers/storage/ directory has not been created yet.

Is the situation really that none of the directories were created yet? or is it that podman system reset initializes everything (e.g. the registry.ContainerEngine().Info call, or PodmanConfig.IsReset is certainly suggestive), shuts down and deletes everything, and then tries to shut things down the second time — all within a single process?

---

What do you think about the following ideas?

I’m afraid this might be infuriating, but at this point I don’t quite care how the message is suppressed, or this particular unmount is made conditional; if we decide that that is the right thing to do, we can figure out a way. (If we know nothing else, adding targeted conditionals to .Cleanup() — in all the 6 graph drivers — would be least disruptive.)

I’m much more concerned that, from a quick and very ignorant look at the reset code, the libpod.Runtime.Reset code seems to be confused about its internal state, cleaning up things after deleting them, and the like. And I think that if that is cleaned up to be precise, it is quite possible that no c/storage changes will be necessary.

That should probably be discussed in the Podman issue with the Podman experts — if my very cursory reading of the code is confirmed to be approximately correct.