libpod: defer volume plugin init when loading volumes from DB

Add e2e regression tests for refresh and lazy plugin use paths.

Signed-off-by: Yoonseo Han <yooncer00@gmail.com>

Author: yoonseo-han

libpod/boltdb_state_internal.go

@@ -7,11 +7,13 @@ import (
 	"os"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/sirupsen/logrus"
 	bolt "go.etcd.io/bbolt"
 	"go.podman.io/common/libnetwork/types"
 	"go.podman.io/podman/v6/libpod/define"
+	"go.podman.io/podman/v6/libpod/plugin"
 )
 
 const (
@@ -297,21 +299,12 @@ func (s *BoltState) getVolumeFromDB(name []byte, volume *Volume, volBkt *bolt.Bu
 
 	// Need this for UsesVolumeDriver() so set it now.
 	volume.runtime = s.runtime
-
-	// Retrieve volume driver
-	if volume.UsesVolumeDriver() {
-		plugin, err := s.runtime.getVolumePlugin(volume.config)
-		if err != nil {
-			// We want to fail gracefully here, to ensure that we
-			// can still remove volumes even if their plugin is
-			// missing. Otherwise, we end up with volumes that
-			// cannot even be retrieved from the database and will
-			// cause things like `volume ls` to fail.
-			logrus.Errorf("Volume %s uses volume plugin %s, but it cannot be accessed - some functionality may not be available: %v", volume.Name(), volume.config.Driver, err)
-		} else {
-			volume.plugin = plugin
+	volume.volumePlugin = sync.OnceValues(func() (*plugin.VolumePlugin, error) {
+		if !volume.UsesVolumeDriver() {
+			return nil, nil
 		}
-	}
+		return volume.runtime.getVolumePlugin(volume.config)
+	})
 
 	// Get the lock
 	lock, err := s.runtime.lockManager.RetrieveLock(volume.config.LockID)

libpod/runtime_volume_common.go

@@ -70,13 +70,14 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
 		}
 	}
 
-	// Plugin can be nil if driver is local, but that's OK - superfluous
-	// assignment doesn't hurt much.
-	plugin, err := r.getVolumePlugin(volume.config)
-	if err != nil {
-		return nil, fmt.Errorf("volume %s uses volume plugin %s but it could not be retrieved: %w", volume.config.Name, volume.config.Driver, err)
+	var plugin *volplugin.VolumePlugin
+	if volume.UsesVolumeDriver() {
+		var err error
+		plugin, err = volume.volumePlugin()
+		if err != nil {
+			return nil, fmt.Errorf("volume %s uses volume plugin %s but it could not be retrieved: %w", volume.config.Name, volume.config.Driver, err)
+		}
 	}
-	volume.plugin = plugin
 
 	if volume.config.Driver == define.VolumeDriverLocal {
 		logrus.Debugf("Validating options for local driver")
@@ -150,13 +151,13 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
 
 	// Now we get conditional: we either need to make the volume in the
 	// volume plugin, or on disk if not using a plugin.
-	if volume.plugin != nil && !noCreatePluginVolume {
+	if volume.UsesVolumeDriver() && !noCreatePluginVolume {
 		// We can't chown, or relabel, or similar the path the volume is
 		// using, because it's not managed by us.
 		// TODO: reevaluate this once we actually have volume plugins in
 		// use in production - it may be safe, but I can't tell without
 		// knowing what the actual plugin does...
-		if err := makeVolumeInPluginIfNotExist(volume.config.Name, volume.config.Options, volume.plugin); err != nil {
+		if err := makeVolumeInPluginIfNotExist(volume.config.Name, volume.config.Options, plugin); err != nil {
 			return nil, err
 		}
 	} else {
@@ -437,27 +438,22 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool, timeo
 	if v.UsesVolumeDriver() && !ignoreVolumePlugin {
 		canRemove := true
 
-		// Do we have a volume driver?
-		if v.plugin == nil {
+		plugin, err := v.volumePlugin()
+		if err != nil {
 			canRemove = false
-			removalErr = fmt.Errorf("cannot remove volume %s from plugin %s, but it has been removed from Podman: %w", v.Name(), v.Driver(), define.ErrMissingPlugin)
+			removalErr = fmt.Errorf("cannot remove volume %s from plugin %s, but it has been removed from Podman: %w", v.Name(), v.Driver(), err)
 		} else {
-			// Ping the plugin first to verify the volume still
-			// exists.
-			// We're trying to be very tolerant of missing volumes
-			// in the backend, to avoid the problems we see with
-			// sync between c/storage and the Libpod DB.
 			getReq := new(pluginapi.GetRequest)
 			getReq.Name = v.Name()
-			if _, err := v.plugin.GetVolume(getReq); err != nil {
+			if _, err := plugin.GetVolume(getReq); err != nil {
 				canRemove = false
 				removalErr = fmt.Errorf("volume %s could not be retrieved from plugin %s, but it has been removed from Podman: %w", v.Name(), v.Driver(), err)
 			}
 		}
 		if canRemove {
 			req := new(pluginapi.RemoveRequest)
 			req.Name = v.Name()
-			if err := v.plugin.RemoveVolume(req); err != nil {
+			if err := plugin.RemoveVolume(req); err != nil {
 				return fmt.Errorf("volume %s could not be removed from plugin %s: %w", v.Name(), v.Driver(), err)
 			}
 		}

libpod/sqlite_state_internal.go

@@ -11,10 +11,12 @@ import (
 	"slices"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/sirupsen/logrus"
 	"go.podman.io/common/libnetwork/types"
 	"go.podman.io/podman/v6/libpod/define"
+	"go.podman.io/podman/v6/libpod/plugin"
 	"go.podman.io/storage/pkg/fileutils"
 
 	// SQLite backend for database/sql
@@ -350,22 +352,13 @@ func finalizeVolumeSqlite(vol *Volume) error {
 	}
 	vol.lock = lock
 
-	// Retrieve volume driver
-	if vol.UsesVolumeDriver() {
-		plugin, err := vol.runtime.getVolumePlugin(vol.config)
-		if err != nil {
-			// We want to fail gracefully here, to ensure that we
-			// can still remove volumes even if their plugin is
-			// missing. Otherwise, we end up with volumes that
-			// cannot even be retrieved from the database and will
-			// cause things like `volume ls` to fail.
-			logrus.Errorf("Volume %s uses volume plugin %s, but it cannot be accessed - some functionality may not be available: %v", vol.Name(), vol.config.Driver, err)
-		} else {
-			vol.plugin = plugin
-		}
-	}
-
 	vol.valid = true
+	vol.volumePlugin = sync.OnceValues(func() (*plugin.VolumePlugin, error) {
+		if !vol.UsesVolumeDriver() {
+			return nil, nil
+		}
+		return vol.runtime.getVolumePlugin(vol.config)
+	})
 
 	return nil
 }

libpod/volume.go

@@ -27,7 +27,7 @@ type Volume struct {
 
 	ignoreIfExists bool
 	valid          bool
-	plugin         *plugin.VolumePlugin
+	volumePlugin   func() (*plugin.VolumePlugin, error)
 	runtime        *Runtime
 	lock           lock.Locker
 }

libpod/volume_inspect.go

@@ -32,15 +32,16 @@ func (v *Volume) Inspect() (*define.InspectVolumeData, error) {
 		logrus.Debugf("Querying volume plugin %s for status", v.config.Driver)
 		data.Mountpoint = v.state.MountPoint
 
-		if v.plugin == nil {
-			return nil, fmt.Errorf("volume %s uses volume plugin %s but it is not available, cannot inspect: %w", v.Name(), v.config.Driver, define.ErrMissingPlugin)
+		plugin, err := v.volumePlugin()
+		if err != nil {
+			return nil, fmt.Errorf("volume %s uses volume plugin %s but it is not available, cannot inspect: %w", v.Name(), v.config.Driver, err)
 		}
 
 		// Retrieve status for the volume.
 		// Need to query the volume driver.
 		req := new(pluginapi.GetRequest)
 		req.Name = v.Name()
-		resp, err := v.plugin.GetVolume(req)
+		resp, err := plugin.GetVolume(req)
 		if err != nil {
 			return nil, fmt.Errorf("retrieving volume %s information from plugin %s: %w", v.Name(), v.Driver(), err)
 		}

libpod/volume_internal.go

@@ -6,8 +6,10 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sync"
 
 	"go.podman.io/podman/v6/libpod/define"
+	"go.podman.io/podman/v6/libpod/plugin"
 )
 
 // Creates a new volume
@@ -20,6 +22,12 @@ func newVolume(runtime *Runtime) *Volume {
 	volume.config.Options = make(map[string]string)
 	volume.state.NeedsCopyUp = true
 	volume.state.NeedsChown = true
+	volume.volumePlugin = sync.OnceValues(func() (*plugin.VolumePlugin, error) {
+		if !volume.UsesVolumeDriver() {
+			return nil, nil
+		}
+		return volume.runtime.getVolumePlugin(volume.config)
+	})
 	return volume
 }
 

libpod/volume_internal_common.go

@@ -50,14 +50,15 @@ func (v *Volume) mount() error {
 	// ours more. So hardcode container ID to something reasonable, and use
 	// the same one for everything.
 	if v.UsesVolumeDriver() {
-		if v.plugin == nil {
-			return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), define.ErrMissingPlugin)
+		plugin, err := v.volumePlugin()
+		if err != nil {
+			return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), err)
 		}
 
 		req := new(pluginapi.MountRequest)
 		req.Name = v.Name()
 		req.ID = pseudoCtrID
-		mountPoint, err := v.plugin.MountVolume(req)
+		mountPoint, err := plugin.MountVolume(req)
 		if err != nil {
 			return err
 		}
@@ -156,14 +157,15 @@ func (v *Volume) unmount(force bool) error {
 
 	if v.state.MountCount == 0 {
 		if v.UsesVolumeDriver() {
-			if v.plugin == nil {
-				return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), define.ErrMissingPlugin)
+			plugin, err := v.volumePlugin()
+			if err != nil {
+				return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), err)
 			}
 
 			req := new(pluginapi.UnmountRequest)
 			req.Name = v.Name()
 			req.ID = pseudoCtrID
-			if err := v.plugin.UnmountVolume(req); err != nil {
+			if err := plugin.UnmountVolume(req); err != nil {
 				return err
 			}
 

test/e2e/volume_plugin_test.go

@@ -301,4 +301,88 @@ Removed:
 		Expect(volInspect2).Should(ExitCleanly())
 		Expect(volInspect2.OutputToString()).To(ContainSubstring("3"))
 	})
+
+	It("unrelated command after refresh does not log plugin errors for plugin volumes", func() {
+		podmanTest.AddImageToRWStore(volumeTest)
+
+		pluginStatePath := filepath.Join(podmanTest.TempDir, "volumes")
+		err := os.Mkdir(pluginStatePath, 0o755)
+		Expect(err).ToNot(HaveOccurred())
+
+		pluginName := "testvol7"
+		ctrName := "pluginCtrRefresh"
+		podmanTest.PodmanExitCleanly(
+			"run", "--name", ctrName, "--security-opt", "label=disable",
+			"-v", "/run/docker/plugins:/run/docker/plugins",
+			"-v", fmt.Sprintf("%v:%v", pluginStatePath, pluginStatePath),
+			"-d", volumeTest, "--sock-name", pluginName, "--path", pluginStatePath,
+		)
+
+		err = WaitForFile(fmt.Sprintf("/run/docker/plugins/%s.sock", pluginName))
+		Expect(err).ToNot(HaveOccurred())
+
+		vol1 := "refreshVol1-" + stringid.GenerateRandomID()
+		vol2 := "refreshVol2-" + stringid.GenerateRandomID()
+		podmanTest.PodmanExitCleanly("volume", "create", "--driver", pluginName, vol1)
+		podmanTest.PodmanExitCleanly("volume", "create", "--driver", pluginName, vol2)
+
+		podmanTest.StopContainer(ctrName)
+
+		// Simulate post-reboot: tmp state is gone but RunRoot (volume DB) persists.
+		// Use a fresh tmpdir instead of deleting the alive file from setup.
+		refreshTmp := filepath.Join(podmanTest.TempDir, "refresh-tmp")
+		err = os.MkdirAll(refreshTmp, 0o755)
+		Expect(err).ToNot(HaveOccurred())
+
+		// Unrelated command triggers refresh; must succeed with no stderr.
+		podmanTest.PodmanExitCleanly("--log-level=error", "--tmpdir", refreshTmp, "version")
+
+		for _, vol := range []string{vol1, vol2} {
+			remove := podmanTest.Podman([]string{"volume", "rm", vol})
+			remove.WaitWithDefaultTimeout()
+			Expect(remove).To(ExitWithErrorRegex(125, "has been removed from Podman"))
+		}
+		ls := podmanTest.PodmanExitCleanly("volume", "ls", "-q")
+		Expect(ls.OutputToStringArray()).To(BeEmpty())
+	})
+
+	It("volume inspect and ls with stopped plugin fail without finalize spam", func() {
+		podmanTest.AddImageToRWStore(volumeTest)
+
+		pluginStatePath := filepath.Join(podmanTest.TempDir, "volumes")
+		err := os.Mkdir(pluginStatePath, 0o755)
+		Expect(err).ToNot(HaveOccurred())
+
+		pluginName := "testvol8"
+		ctrName := "pluginCtrInspect"
+		podmanTest.PodmanExitCleanly(
+			"run", "--name", ctrName, "--security-opt", "label=disable",
+			"-v", "/run/docker/plugins:/run/docker/plugins",
+			"-v", fmt.Sprintf("%v:%v", pluginStatePath, pluginStatePath),
+			"-d", volumeTest, "--sock-name", pluginName, "--path", pluginStatePath,
+		)
+
+		err = WaitForFile(fmt.Sprintf("/run/docker/plugins/%s.sock", pluginName))
+		Expect(err).ToNot(HaveOccurred())
+
+		volName := "inspectVol-" + stringid.GenerateRandomID()
+		podmanTest.PodmanExitCleanly("volume", "create", "--driver", pluginName, volName)
+
+		podmanTest.StopContainer(ctrName)
+
+		inspect := podmanTest.Podman([]string{"volume", "inspect", volName})
+		inspect.WaitWithDefaultTimeout()
+		Expect(inspect).To(ExitWithErrorRegex(125, "cannot inspect"))
+
+		ls := podmanTest.Podman([]string{"volume", "ls", "-q"})
+		ls.WaitWithDefaultTimeout()
+		Expect(ls).To(ExitWithErrorRegex(125, "cannot inspect"))
+
+		remove := podmanTest.Podman([]string{"volume", "rm", volName})
+		remove.WaitWithDefaultTimeout()
+		Expect(remove).To(ExitWithErrorRegex(125, "has been removed from Podman"))
+
+		lsAfter := podmanTest.PodmanExitCleanly("volume", "ls", "-q")
+		Expect(lsAfter.OutputToStringArray()).To(BeEmpty())
+	})
 })