Skip to content

Commit 4b6febc

Browse files
committed
libpod: defer volume plugin init when loading volumes from DB
Do not contact volume plugins in finalizeVolume or when loading from the DB. Initialize lazily on inspect/mount/unmount/remove; fail eagerly on create. Fixes #28110 Signed-off-by: Yoonseo Han <yooncer00@gmail.com>
1 parent 9e13b9e commit 4b6febc

6 files changed

Lines changed: 49 additions & 58 deletions

File tree

libpod/boltdb_state_internal.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -298,21 +298,6 @@ func (s *BoltState) getVolumeFromDB(name []byte, volume *Volume, volBkt *bolt.Bu
298298
// Need this for UsesVolumeDriver() so set it now.
299299
volume.runtime = s.runtime
300300

301-
// Retrieve volume driver
302-
if volume.UsesVolumeDriver() {
303-
plugin, err := s.runtime.getVolumePlugin(volume.config)
304-
if err != nil {
305-
// We want to fail gracefully here, to ensure that we
306-
// can still remove volumes even if their plugin is
307-
// missing. Otherwise, we end up with volumes that
308-
// cannot even be retrieved from the database and will
309-
// cause things like `volume ls` to fail.
310-
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)
311-
} else {
312-
volume.plugin = plugin
313-
}
314-
}
315-
316301
// Get the lock
317302
lock, err := s.runtime.lockManager.RetrieveLock(volume.config.LockID)
318303
if err != nil {

libpod/runtime_volume_common.go

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,15 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
7070
}
7171
}
7272

73-
// Plugin can be nil if driver is local, but that's OK - superfluous
74-
// assignment doesn't hurt much.
75-
plugin, err := r.getVolumePlugin(volume.config)
76-
if err != nil {
77-
return nil, fmt.Errorf("volume %s uses volume plugin %s but it could not be retrieved: %w", volume.config.Name, volume.config.Driver, err)
73+
var plugin *volplugin.VolumePlugin
74+
if volume.UsesVolumeDriver() {
75+
var err error
76+
plugin, err = r.getVolumePlugin(volume.config)
77+
if err != nil {
78+
return nil, fmt.Errorf("volume %s uses volume plugin %s but it could not be retrieved: %w", volume.config.Name, volume.config.Driver, err)
79+
}
80+
volume.cacheVolumePlugin(plugin)
7881
}
79-
volume.plugin = plugin
8082

8183
if volume.config.Driver == define.VolumeDriverLocal {
8284
logrus.Debugf("Validating options for local driver")
@@ -150,13 +152,13 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
150152

151153
// Now we get conditional: we either need to make the volume in the
152154
// volume plugin, or on disk if not using a plugin.
153-
if volume.plugin != nil && !noCreatePluginVolume {
155+
if volume.UsesVolumeDriver() && !noCreatePluginVolume {
154156
// We can't chown, or relabel, or similar the path the volume is
155157
// using, because it's not managed by us.
156158
// TODO: reevaluate this once we actually have volume plugins in
157159
// use in production - it may be safe, but I can't tell without
158160
// knowing what the actual plugin does...
159-
if err := makeVolumeInPluginIfNotExist(volume.config.Name, volume.config.Options, volume.plugin); err != nil {
161+
if err := makeVolumeInPluginIfNotExist(volume.config.Name, volume.config.Options, plugin); err != nil {
160162
return nil, err
161163
}
162164
} else {
@@ -437,27 +439,22 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool, timeo
437439
if v.UsesVolumeDriver() && !ignoreVolumePlugin {
438440
canRemove := true
439441

440-
// Do we have a volume driver?
441-
if v.plugin == nil {
442+
plugin, err := v.getPlugin()
443+
if err != nil {
442444
canRemove = false
443-
removalErr = fmt.Errorf("cannot remove volume %s from plugin %s, but it has been removed from Podman: %w", v.Name(), v.Driver(), define.ErrMissingPlugin)
445+
removalErr = fmt.Errorf("cannot remove volume %s from plugin %s, but it has been removed from Podman: %w", v.Name(), v.Driver(), err)
444446
} else {
445-
// Ping the plugin first to verify the volume still
446-
// exists.
447-
// We're trying to be very tolerant of missing volumes
448-
// in the backend, to avoid the problems we see with
449-
// sync between c/storage and the Libpod DB.
450447
getReq := new(pluginapi.GetRequest)
451448
getReq.Name = v.Name()
452-
if _, err := v.plugin.GetVolume(getReq); err != nil {
449+
if _, err := plugin.GetVolume(getReq); err != nil {
453450
canRemove = false
454451
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)
455452
}
456453
}
457454
if canRemove {
458455
req := new(pluginapi.RemoveRequest)
459456
req.Name = v.Name()
460-
if err := v.plugin.RemoveVolume(req); err != nil {
457+
if err := plugin.RemoveVolume(req); err != nil {
461458
return fmt.Errorf("volume %s could not be removed from plugin %s: %w", v.Name(), v.Driver(), err)
462459
}
463460
}

libpod/sqlite_state_internal.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -350,21 +350,6 @@ func finalizeVolumeSqlite(vol *Volume) error {
350350
}
351351
vol.lock = lock
352352

353-
// Retrieve volume driver
354-
if vol.UsesVolumeDriver() {
355-
plugin, err := vol.runtime.getVolumePlugin(vol.config)
356-
if err != nil {
357-
// We want to fail gracefully here, to ensure that we
358-
// can still remove volumes even if their plugin is
359-
// missing. Otherwise, we end up with volumes that
360-
// cannot even be retrieved from the database and will
361-
// cause things like `volume ls` to fail.
362-
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)
363-
} else {
364-
vol.plugin = plugin
365-
}
366-
}
367-
368353
vol.valid = true
369354

370355
return nil

libpod/volume.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"io"
88
"maps"
9+
"sync"
910
"time"
1011

1112
"github.com/sirupsen/logrus"
@@ -27,7 +28,7 @@ type Volume struct {
2728

2829
ignoreIfExists bool
2930
valid bool
30-
plugin *plugin.VolumePlugin
31+
volumePlugin func() (*plugin.VolumePlugin, error)
3132
runtime *Runtime
3233
lock lock.Locker
3334
}
@@ -130,6 +131,26 @@ func (v *Volume) Driver() string {
130131
return v.config.Driver
131132
}
132133

134+
// getPlugin returns the volume plugin driver, initializing on first use.
135+
func (v *Volume) getPlugin() (*plugin.VolumePlugin, error) {
136+
if !v.UsesVolumeDriver() {
137+
return nil, nil
138+
}
139+
if v.volumePlugin == nil {
140+
v.volumePlugin = sync.OnceValues(func() (*plugin.VolumePlugin, error) {
141+
return v.runtime.getVolumePlugin(v.config)
142+
})
143+
}
144+
return v.volumePlugin()
145+
}
146+
147+
// cacheVolumePlugin records a plugin that was already validated (e.g. volume create).
148+
func (v *Volume) cacheVolumePlugin(p *plugin.VolumePlugin) {
149+
v.volumePlugin = sync.OnceValues(func() (*plugin.VolumePlugin, error) {
150+
return p, nil
151+
})
152+
}
153+
133154
// Scope retrieves the volume's scope.
134155
// Libpod does not implement volume scoping, and this is provided solely for
135156
// Docker compatibility. It returns only "local".

libpod/volume_inspect.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,16 @@ func (v *Volume) Inspect() (*define.InspectVolumeData, error) {
3232
logrus.Debugf("Querying volume plugin %s for status", v.config.Driver)
3333
data.Mountpoint = v.state.MountPoint
3434

35-
if v.plugin == nil {
36-
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)
35+
plugin, err := v.getPlugin()
36+
if err != nil {
37+
return nil, fmt.Errorf("volume %s uses volume plugin %s but it is not available, cannot inspect: %w", v.Name(), v.config.Driver, err)
3738
}
3839

3940
// Retrieve status for the volume.
4041
// Need to query the volume driver.
4142
req := new(pluginapi.GetRequest)
4243
req.Name = v.Name()
43-
resp, err := v.plugin.GetVolume(req)
44+
resp, err := plugin.GetVolume(req)
4445
if err != nil {
4546
return nil, fmt.Errorf("retrieving volume %s information from plugin %s: %w", v.Name(), v.Driver(), err)
4647
}

libpod/volume_internal_common.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,15 @@ func (v *Volume) mount() error {
5050
// ours more. So hardcode container ID to something reasonable, and use
5151
// the same one for everything.
5252
if v.UsesVolumeDriver() {
53-
if v.plugin == nil {
54-
return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), define.ErrMissingPlugin)
53+
plugin, err := v.getPlugin()
54+
if err != nil {
55+
return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), err)
5556
}
5657

5758
req := new(pluginapi.MountRequest)
5859
req.Name = v.Name()
5960
req.ID = pseudoCtrID
60-
mountPoint, err := v.plugin.MountVolume(req)
61+
mountPoint, err := plugin.MountVolume(req)
6162
if err != nil {
6263
return err
6364
}
@@ -156,14 +157,15 @@ func (v *Volume) unmount(force bool) error {
156157

157158
if v.state.MountCount == 0 {
158159
if v.UsesVolumeDriver() {
159-
if v.plugin == nil {
160-
return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), define.ErrMissingPlugin)
160+
plugin, err := v.getPlugin()
161+
if err != nil {
162+
return fmt.Errorf("volume plugin %s (needed by volume %s) missing: %w", v.Driver(), v.Name(), err)
161163
}
162164

163165
req := new(pluginapi.UnmountRequest)
164166
req.Name = v.Name()
165167
req.ID = pseudoCtrID
166-
if err := v.plugin.UnmountVolume(req); err != nil {
168+
if err := plugin.UnmountVolume(req); err != nil {
167169
return err
168170
}
169171

0 commit comments

Comments
 (0)