Add support for containers.conf volume timeouts

Also, do a general cleanup of all the timeout code. Changes
include:
- Convert from int to *uint where possible. Timeouts cannot be
  negative, hence the uint change; and a timeout of 0 is valid,
  so we need a new way to detect that the user set a timeout
  (hence, pointer).
- Change name in the database to avoid conflicts between new data
  type and old one. This will cause timeouts set with 4.2.0 to be
  lost, but considering nobody is using the feature at present
  (and the lack of validation means we could have invalid,
  negative timeouts in the DB) this feels safe.
- Ensure volume plugin timeouts can only be used with volumes
  created using a plugin. Timeouts on the local driver are
  nonsensical.
- Remove the existing test, as it did not use a volume plugin.
  Write a new test that does.

The actual plumbing of the containers.conf timeout in is one line
in volume_api.go; the remainder are the above-described cleanups.

Signed-off-by: Matthew Heon <mheon@redhat.com>

7 files changed, 29 insertions, 19 deletions

diff --git a/libpod/define/volume_inspect.go b/libpod/define/volume_inspect.go
index 9279812da..76120647c 100644
--- a/libpod/define/volume_inspect.go
+++ b/libpod/define/volume_inspect.go
@@ -57,7 +57,7 @@ type InspectVolumeData struct {
 	// UID/GID.
 	NeedsChown bool `json:"NeedsChown,omitempty"`
 	// Timeout is the specified driver timeout if given
-	Timeout int `json:"Timeout,omitempty"`
+	Timeout uint `json:"Timeout,omitempty"`
 }
 
 type VolumeReload struct {
diff --git a/libpod/options.go b/libpod/options.go
index 43ed1ff78..d31741094 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -1695,14 +1695,22 @@ func withSetAnon() VolumeCreateOption {
 	}
 }
 
-// WithVolumeDriverTimeout sets the volume creation timeout period
-func WithVolumeDriverTimeout(timeout int) VolumeCreateOption {
+// WithVolumeDriverTimeout sets the volume creation timeout period.
+// Only usable if a non-local volume driver is in use.
+func WithVolumeDriverTimeout(timeout uint) VolumeCreateOption {
 	return func(volume *Volume) error {
 		if volume.valid {
 			return define.ErrVolumeFinalized
 		}
 
-		volume.config.Timeout = timeout
+		if volume.config.Driver == "" || volume.config.Driver == define.VolumeDriverLocal {
+			return fmt.Errorf("Volume driver timeout can only be used with non-local volume drivers: %w", define.ErrInvalidArg)
+		}
+
+		tm := timeout
+
+		volume.config.Timeout = &tm
+
 		return nil
 	}
 }
diff --git a/libpod/plugin/volume_api.go b/libpod/plugin/volume_api.go
index 0a5eaae53..b13578388 100644
--- a/libpod/plugin/volume_api.go
+++ b/libpod/plugin/volume_api.go
@@ -3,6 +3,7 @@ package plugin
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -13,8 +14,7 @@ import (
 	"sync"
 	"time"
 
-	"errors"
-
+	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/docker/go-plugins-helpers/sdk"
 	"github.com/docker/go-plugins-helpers/volume"
@@ -40,7 +40,6 @@ var (
 )
 
 const (
-	defaultTimeout   = 5 * time.Second
 	volumePluginType = "VolumeDriver"
 )
 
@@ -129,7 +128,7 @@ func validatePlugin(newPlugin *VolumePlugin) error {
 
 // GetVolumePlugin gets a single volume plugin, with the given name, at the
 // given path.
-func GetVolumePlugin(name string, path string, timeout int) (*VolumePlugin, error) {
+func GetVolumePlugin(name string, path string, timeout *uint, cfg *config.Config) (*VolumePlugin, error) {
 	pluginsLock.Lock()
 	defer pluginsLock.Unlock()
 
@@ -152,13 +151,11 @@ func GetVolumePlugin(name string, path string, timeout int) (*VolumePlugin, erro
 	// Need an HTTP client to force a Unix connection.
 	// And since we can reuse it, might as well cache it.
 	client := new(http.Client)
-	client.Timeout = defaultTimeout
-	// if the user specified a non-zero timeout, use their value. Else, keep the default.
-	if timeout != 0 {
-		if time.Duration(timeout)*time.Second < defaultTimeout {
-			logrus.Warnf("the default timeout for volume creation is %d seconds, setting a time less than that may break this feature.", defaultTimeout)
-		}
-		client.Timeout = time.Duration(timeout) * time.Second
+	client.Timeout = 5 * time.Second
+	if timeout != nil {
+		client.Timeout = time.Duration(*timeout) * time.Second
+	} else if cfg != nil {
+		client.Timeout = time.Duration(cfg.Engine.VolumePluginTimeout) * time.Second
 	}
 	// This bit borrowed from pkg/bindings/connection.go
 	client.Transport = &http.Transport{
diff --git a/libpod/runtime.go b/libpod/runtime.go
index 684f4abd7..9b97fd724 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -1097,7 +1097,7 @@ func (r *Runtime) getVolumePlugin(volConfig *VolumeConfig) (*plugin.VolumePlugin
 		return nil, fmt.Errorf("no volume plugin with name %s available: %w", name, define.ErrMissingPlugin)
 	}
 
-	return plugin.GetVolumePlugin(name, pluginPath, timeout)
+	return plugin.GetVolumePlugin(name, pluginPath, timeout, r.config)
 }
 
 // GetSecretsStorageDir returns the directory that the secrets manager should take
diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go
index 1f354e41b..65f2a1005 100644
--- a/libpod/runtime_volume_linux.go
+++ b/libpod/runtime_volume_linux.go
@@ -184,7 +184,7 @@ func (r *Runtime) UpdateVolumePlugins(ctx context.Context) *define.VolumeReload
 	)
 
 	for driverName, socket := range r.config.Engine.VolumePlugins {
-		driver, err := volplugin.GetVolumePlugin(driverName, socket, 0)
+		driver, err := volplugin.GetVolumePlugin(driverName, socket, nil, r.config)
 		if err != nil {
 			errs = append(errs, err)
 			continue
diff --git a/libpod/volume.go b/libpod/volume.go
index 2e8cd77a5..a054e4032 100644
--- a/libpod/volume.go
+++ b/libpod/volume.go
@@ -56,7 +56,7 @@ type VolumeConfig struct {
 	// quota tracking.
 	DisableQuota bool `json:"disableQuota,omitempty"`
 	// Timeout allows users to override the default driver timeout of 5 seconds
-	Timeout int
+	Timeout *uint `json:"timeout,omitempty"`
 }
 
 // VolumeState holds the volume's mutable state.
diff --git a/libpod/volume_inspect.go b/libpod/volume_inspect.go
index dd2f3fd01..c3872bca7 100644
--- a/libpod/volume_inspect.go
+++ b/libpod/volume_inspect.go
@@ -64,7 +64,12 @@ func (v *Volume) Inspect() (*define.InspectVolumeData, error) {
 	data.MountCount = v.state.MountCount
 	data.NeedsCopyUp = v.state.NeedsCopyUp
 	data.NeedsChown = v.state.NeedsChown
-	data.Timeout = v.config.Timeout
+
+	if v.config.Timeout != nil {
+		data.Timeout = *v.config.Timeout
+	} else if v.UsesVolumeDriver() {
+		data.Timeout = v.runtime.config.Engine.VolumePluginTimeout
+	}
 
 	return data, nil
 }