Merge pull request #26660 from Honny1/speed-up-load

Optimize image loading for Podman machines

cmd/podman/compose_machine.go

@@ -3,57 +3,20 @@
 package main
 
 import (
-	"fmt"
 	"net/url"
-	"strconv"
 
-	"github.com/containers/podman/v5/pkg/machine/define"
-	"github.com/containers/podman/v5/pkg/machine/env"
-	"github.com/containers/podman/v5/pkg/machine/provider"
-	"github.com/containers/podman/v5/pkg/machine/vmconfigs"
+	"github.com/containers/podman/v5/internal/localapi"
 )
 
 func getMachineConn(connectionURI string, parsedConnection *url.URL) (string, error) {
-	machineProvider, err := provider.Get()
-	if err != nil {
-		return "", fmt.Errorf("getting machine provider: %w", err)
-	}
-	dirs, err := env.GetMachineDirs(machineProvider.VMType())
+	mc, machineProvider, err := localapi.FindMachineByPort(connectionURI, parsedConnection)
 	if err != nil {
 		return "", err
 	}
 
-	machineList, err := vmconfigs.LoadMachinesInDir(dirs)
+	podmanSocket, podmanPipe, err := mc.ConnectionInfo(machineProvider.VMType())
 	if err != nil {
-		return "", fmt.Errorf("listing machines: %w", err)
+		return "", err
 	}
-
-	// Now we know that the connection points to a machine and we
-	// can find the machine by looking for the one with the
-	// matching port.
-	connectionPort, err := strconv.Atoi(parsedConnection.Port())
-	if err != nil {
-		return "", fmt.Errorf("parsing connection port: %w", err)
-	}
-	for _, mc := range machineList {
-		if connectionPort != mc.SSH.Port {
-			continue
-		}
-
-		state, err := machineProvider.State(mc, false)
-		if err != nil {
-			return "", err
-		}
-
-		if state != define.Running {
-			return "", fmt.Errorf("machine %s is not running but in state %s", mc.Name, state)
-		}
-
-		podmanSocket, podmanPipe, err := mc.ConnectionInfo(machineProvider.VMType())
-		if err != nil {
-			return "", err
-		}
-		return extractConnectionString(podmanSocket, podmanPipe)
-	}
-	return "", fmt.Errorf("could not find a matching machine for connection %q", connectionURI)
+	return extractConnectionString(podmanSocket, podmanPipe)
 }

internal/localapi/types.go

@@ -0,0 +1,7 @@
+package localapi
+
+// LocalAPIMap is a map of local paths to their target paths in the VM
+type LocalAPIMap struct {
+	ClientPath string `json:"ClientPath,omitempty"`
+	RemotePath string `json:"RemotePath,omitempty"`
+}

internal/localapi/utils.go

@@ -0,0 +1,156 @@
+//go:build amd64 || arm64
+
+package localapi
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io/fs"
+	"net/url"
+	"path/filepath"
+	"strconv"
+	"strings"
+
+	"github.com/containers/podman/v5/pkg/bindings"
+	"github.com/containers/podman/v5/pkg/machine/define"
+	"github.com/containers/podman/v5/pkg/machine/env"
+	"github.com/containers/podman/v5/pkg/machine/provider"
+	"github.com/containers/podman/v5/pkg/machine/vmconfigs"
+	"github.com/containers/podman/v5/pkg/specgen"
+	"github.com/containers/storage/pkg/fileutils"
+	"github.com/sirupsen/logrus"
+)
+
+// FindMachineByPort finds a running machine that matches the given connection port.
+// It returns the machine configuration and provider, or an error if not found.
+func FindMachineByPort(connectionURI string, parsedConnection *url.URL) (*vmconfigs.MachineConfig, vmconfigs.VMProvider, error) {
+	machineProvider, err := provider.Get()
+	if err != nil {
+		return nil, nil, fmt.Errorf("getting machine provider: %w", err)
+	}
+
+	dirs, err := env.GetMachineDirs(machineProvider.VMType())
+	if err != nil {
+		return nil, nil, err
+	}
+
+	machineList, err := vmconfigs.LoadMachinesInDir(dirs)
+	if err != nil {
+		return nil, nil, fmt.Errorf("listing machines: %w", err)
+	}
+
+	// Now we know that the connection points to a machine and we
+	// can find the machine by looking for the one with the
+	// matching port.
+	connectionPort, err := strconv.Atoi(parsedConnection.Port())
+	if err != nil {
+		return nil, nil, fmt.Errorf("parsing connection port: %w", err)
+	}
+
+	for _, mc := range machineList {
+		if connectionPort != mc.SSH.Port {
+			continue
+		}
+
+		state, err := machineProvider.State(mc, false)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		if state != define.Running {
+			return nil, nil, fmt.Errorf("machine %s is not running but in state %s", mc.Name, state)
+		}
+
+		return mc, machineProvider, nil
+	}
+
+	return nil, nil, fmt.Errorf("could not find a matching machine for connection %q", connectionURI)
+}
+
+// getMachineMountsAndVMType retrieves the mounts and VM type of a machine based on the connection URI and parsed URL.
+// It returns a slice of mounts, the VM type, or an error if the machine cannot be found or is not running.
+func getMachineMountsAndVMType(connectionURI string, parsedConnection *url.URL) ([]*vmconfigs.Mount, define.VMType, error) {
+	mc, machineProvider, err := FindMachineByPort(connectionURI, parsedConnection)
+	if err != nil {
+		return nil, define.UnknownVirt, err
+	}
+	return mc.Mounts, machineProvider.VMType(), nil
+}
+
+// isPathAvailableOnMachine checks if a local path is available on the machine through mounted directories.
+// If the path is available, it returns a LocalAPIMap with the corresponding remote path.
+func isPathAvailableOnMachine(mounts []*vmconfigs.Mount, vmType define.VMType, path string) (*LocalAPIMap, bool) {
+	pathABS, err := filepath.Abs(path)
+	if err != nil {
+		logrus.Debugf("Failed to get absolute path for %s: %v", path, err)
+		return nil, false
+	}
+
+	// WSLVirt is a special case where there is no real concept of doing a mount in WSL,
+	// WSL by default mounts the drives to /mnt/c, /mnt/d, etc...
+	if vmType == define.WSLVirt {
+		converted_path, err := specgen.ConvertWinMountPath(pathABS)
+		if err != nil {
+			logrus.Debugf("Failed to convert Windows mount path: %v", err)
+			return nil, false
+		}
+
+		return &LocalAPIMap{
+			ClientPath: pathABS,
+			RemotePath: converted_path,
+		}, true
+	}
+
+	for _, mount := range mounts {
+		mountSource := filepath.Clean(mount.Source)
+		relPath, err := filepath.Rel(mountSource, pathABS)
+		if err != nil {
+			logrus.Debugf("Failed to get relative path: %v", err)
+			continue
+		}
+		// If relPath starts with ".." or is absolute, pathABS is not under mountSource
+		if relPath == "." || (!strings.HasPrefix(relPath, "..") && !filepath.IsAbs(relPath)) {
+			target := filepath.Join(mount.Target, relPath)
+			converted_path, err := specgen.ConvertWinMountPath(target)
+			if err != nil {
+				logrus.Debugf("Failed to convert Windows mount path: %v", err)
+				return nil, false
+			}
+			logrus.Debugf("Converted client path: %q", converted_path)
+			return &LocalAPIMap{
+				ClientPath: pathABS,
+				RemotePath: converted_path,
+			}, true
+		}
+	}
+	return nil, false
+}
+
+// CheckPathOnRunningMachine is a convenience function that checks if a path is available
+// on any currently running machine. It combines machine inspection and path checking.
+func CheckPathOnRunningMachine(ctx context.Context, path string) (*LocalAPIMap, bool) {
+	if err := fileutils.Exists(path); errors.Is(err, fs.ErrNotExist) {
+		logrus.Debugf("Path %s does not exist locally, skipping machine check", path)
+		return nil, false
+	}
+
+	if machineMode := bindings.GetMachineMode(ctx); !machineMode {
+		logrus.Debug("Machine mode is not enabled, skipping machine check")
+		return nil, false
+	}
+
+	conn, err := bindings.GetClient(ctx)
+	if err != nil {
+		logrus.Debugf("Failed to get client connection: %v", err)
+		return nil, false
+	}
+
+	mounts, vmType, err := getMachineMountsAndVMType(conn.URI.String(), conn.URI)
+	if err != nil {
+		logrus.Debugf("Failed to get machine mounts: %v", err)
+		return nil, false
+	}
+
+	return isPathAvailableOnMachine(mounts, vmType, path)
+}

internal/localapi/utils_unsupported.go

@@ -0,0 +1,14 @@
+//go:build !amd64 && !arm64
+
+package localapi
+
+import (
+	"context"
+
+	"github.com/sirupsen/logrus"
+)
+
+func CheckPathOnRunningMachine(ctx context.Context, path string) (*LocalAPIMap, bool) {
+	logrus.Debug("CheckPathOnRunningMachine is not supported")
+	return nil, false
+}

pkg/api/handlers/libpod/images.go

@@ -8,8 +8,10 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net/http"
 	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -36,6 +38,7 @@ import (
 	"github.com/containers/storage"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/containers/storage/pkg/chrootarchive"
+	"github.com/containers/storage/pkg/fileutils"
 	"github.com/containers/storage/pkg/idtools"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/gorilla/schema"
@@ -374,6 +377,47 @@ func ImagesLoad(w http.ResponseWriter, r *http.Request) {
 	utils.WriteResponse(w, http.StatusOK, loadReport)
 }
 
+func ImagesLocalLoad(w http.ResponseWriter, r *http.Request) {
+	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
+	decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
+	query := struct {
+		Path string `schema:"path"`
+	}{}
+
+	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
+		utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err))
+		return
+	}
+
+	if query.Path == "" {
+		utils.Error(w, http.StatusBadRequest, fmt.Errorf("path query parameter is required"))
+		return
+	}
+
+	cleanPath := filepath.Clean(query.Path)
+	// Check if the path exists on server side.
+	// Note: fileutils.Exists returns nil if the file exists, not an error.
+	switch err := fileutils.Exists(cleanPath); {
+	case err == nil:
+		// no error -> continue
+	case errors.Is(err, fs.ErrNotExist):
+		utils.Error(w, http.StatusNotFound, fmt.Errorf("file does not exist: %q", cleanPath))
+		return
+	default:
+		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to access file: %w", err))
+		return
+	}
+
+	imageEngine := abi.ImageEngine{Libpod: runtime}
+	loadOptions := entities.ImageLoadOptions{Input: cleanPath}
+	loadReport, err := imageEngine.Load(r.Context(), loadOptions)
+	if err != nil {
+		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("unable to load image: %w", err))
+		return
+	}
+	utils.WriteResponse(w, http.StatusOK, loadReport)
+}
+
 func ImagesImport(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
 	decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)

pkg/api/server/register_images.go

@@ -941,6 +941,30 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
 	//   500:
 	//     $ref: '#/responses/internalError'
 	r.Handle(VersionedPath("/libpod/images/load"), s.APIHandler(libpod.ImagesLoad)).Methods(http.MethodPost)
+	// swagger:operation POST /libpod/local/images/load libpod LocalImagesLibpod
+	// ---
+	// tags:
+	//  - images
+	// summary: Load image from local path
+	// description: Load an image (oci-archive or docker-archive) from a file path accessible on the server.
+	// parameters:
+	//   - in: query
+	//     name: path
+	//     type: string
+	//     required: true
+	//     description: Path to the image archive file on the server filesystem
+	// produces:
+	// - application/json
+	// responses:
+	//   200:
+	//     $ref: "#/responses/imagesLoadResponseLibpod"
+	//   400:
+	//     $ref: "#/responses/badParamError"
+	//   404:
+	//     $ref: "#/responses/imageNotFound"
+	//   500:
+	//     $ref: '#/responses/internalError'
+	r.Handle(VersionedPath("/libpod/local/images/load"), s.APIHandler(libpod.ImagesLocalLoad)).Methods(http.MethodPost)
 	// swagger:operation POST /libpod/images/import libpod ImageImportLibpod
 	// ---
 	// tags:

pkg/bindings/connection.go

@@ -38,8 +38,9 @@ type Connection struct {
 type valueKey string
 
 const (
-	clientKey  = valueKey("Client")
-	versionKey = valueKey("ServiceVersion")
+	clientKey      = valueKey("Client")
+	versionKey     = valueKey("ServiceVersion")
+	machineModeKey = valueKey("MachineMode")
 )
 
 type ConnectError struct {
@@ -66,6 +67,13 @@ func GetClient(ctx context.Context) (*Connection, error) {
 	return nil, fmt.Errorf("%s not set in context", clientKey)
 }
 
+func GetMachineMode(ctx context.Context) bool {
+	if v, ok := ctx.Value(machineModeKey).(bool); ok {
+		return v
+	}
+	return false
+}
+
 // ServiceVersion from context build by NewConnection()
 func ServiceVersion(ctx context.Context) *semver.Version {
 	if v, ok := ctx.Value(versionKey).(*semver.Version); ok {
@@ -142,6 +150,8 @@ func NewConnectionWithIdentity(ctx context.Context, uri string, identity string,
 		return nil, newConnectError(err)
 	}
 	ctx = context.WithValue(ctx, versionKey, serviceVersion)
+
+	ctx = context.WithValue(ctx, machineModeKey, machine)
 	return ctx, nil
 }
 

pkg/bindings/errors.go

@@ -47,9 +47,13 @@ func (h *APIResponse) ProcessWithError(unmarshalInto interface{}, unmarshalError
 	if h.IsConflictError() {
 		return handleError(data, unmarshalErrorInto)
 	}
-
-	// TODO should we add a debug here with the response code?
-	return handleError(data, &errorhandling.ErrorModel{})
+	if h.Response.Header.Get("Content-Type") == "application/json" {
+		return handleError(data, &errorhandling.ErrorModel{})
+	}
+	return &errorhandling.ErrorModel{
+		Message:      string(data),
+		ResponseCode: h.Response.StatusCode,
+	}
 }
 
 func CheckResponseCode(inError error) (int, error) {

pkg/bindings/images/images.go

@@ -139,6 +139,25 @@ func Load(ctx context.Context, r io.Reader) (*types.ImageLoadReport, error) {
 	return &report, response.Process(&report)
 }
 
+func LoadLocal(ctx context.Context, path string) (*types.ImageLoadReport, error) {
+	var report types.ImageLoadReport
+	conn, err := bindings.GetClient(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	params := url.Values{}
+	params.Set("path", path)
+
+	response, err := conn.DoRequest(ctx, nil, http.MethodPost, "/local/images/load", params, nil)
+	if err != nil {
+		return nil, err
+	}
+	defer response.Body.Close()
+
+	return &report, response.Process(&report)
+}
+
 // Export saves images from local storage as a tarball or image archive.  The optional format
 // parameter is used to change the format of the output.
 func Export(ctx context.Context, nameOrIDs []string, w io.Writer, options *ExportOptions) error {

pkg/domain/infra/tunnel/images.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"os"
 	"strconv"
 	"strings"
@@ -14,6 +15,7 @@ import (
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/docker/reference"
 	"github.com/containers/image/v5/types"
+	"github.com/containers/podman/v5/internal/localapi"
 	"github.com/containers/podman/v5/libpod/define"
 	"github.com/containers/podman/v5/pkg/bindings/images"
 	"github.com/containers/podman/v5/pkg/domain/entities"
@@ -221,6 +223,23 @@ func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts en
 }
 
 func (ir *ImageEngine) Load(ctx context.Context, opts entities.ImageLoadOptions) (*entities.ImageLoadReport, error) {
+	if localMap, ok := localapi.CheckPathOnRunningMachine(ir.ClientCtx, opts.Input); ok {
+		report, err := images.LoadLocal(ir.ClientCtx, localMap.RemotePath)
+		if err == nil {
+			return report, nil
+		}
+		var errModel *errorhandling.ErrorModel
+		if errors.As(err, &errModel) {
+			switch errModel.ResponseCode {
+			case http.StatusNotFound, http.StatusMethodNotAllowed:
+			default:
+				return nil, err
+			}
+		} else {
+			return nil, err
+		}
+	}
+
 	f, err := os.Open(opts.Input)
 	if err != nil {
 		return nil, err

pkg/machine/config.go

@@ -116,6 +116,11 @@ type InspectInfo struct {
 	Rosetta            bool
 }
 
+type InternalInspectInfo struct {
+	InspectInfo
+	Mounts []*vmconfigs.Mount
+}
+
 // ImageConfig describes the bootable image for the VM
 type ImageConfig struct {
 	// IgnitionFile is the path to the filesystem where the

test/apiv2/10-images.at

@@ -478,4 +478,32 @@ t GET images/json 200 \
 t GET images/json?shared-size=true 200 \
     .[0].SharedSize=0
 
+TMPD=$(mktemp -d podman-apiv2-test.build.XXXXXXXX)
+function cleanLoad() {
+    podman rmi -a -f
+    rm -rf "${TMPD}" &> /dev/null
+}
+
+podman pull quay.io/libpod/alpine:latest quay.io/libpod/busybox:latest
+podman save -o ${TMPD}/test.tar quay.io/libpod/alpine:latest quay.io/libpod/busybox:latest
+podman rmi quay.io/libpod/alpine:latest quay.io/libpod/busybox:latest
+ABS_PATH=$( realpath "${TMPD}/test.tar" )
+t POST libpod/local/images/load?path="${ABS_PATH}" 200
+t GET libpod/images/quay.io/libpod/alpine:latest/exists  204
+t GET libpod/images/quay.io/libpod/busybox:latest/exists  204
+
+# Test with directory instead of file
+mkdir -p ${TMPD}/testdir
+t POST libpod/local/images/load?path="${TMPD}/testdir" 500
+
+cleanLoad
+
+t POST libpod/local/images/load?path="/tmp/notexisting.tar" 404
+
+t POST libpod/local/images/load?invalid=arg 400
+
+t POST libpod/local/images/load?path="" 400
+
+t POST libpod/local/images/load?path="../../../etc/passwd" 404
+
 # vim: filetype=sh