From cfe4d46d897d48af1d0ae7e128c84d400d2d3f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Rod=C3=A1k?= Date: Thu, 7 Aug 2025 14:58:08 +0200 Subject: [PATCH] Optimize image loading for Podman machines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for loading images directly from machine paths to avoid unnecessary file transfers when the image archive is already accessible on the running machine through mounted directories. Changes include: - New /libpod/local/images/load API endpoint for direct machine loading - Machine detection and path mapping functionality - Fallback in tunnel mode to try optimized loading first This optimization significantly speeds up image loading operations when working with remote Podman machines by eliminating redundant file transfers for already-accessible image archives. Fixes: https://issues.redhat.com/browse/RUN-3249 Fixes: https://github.com/containers/podman/issues/26321 Signed-off-by: Jan Rodák --- cmd/podman/compose_machine.go | 47 +------- internal/localapi/types.go | 7 ++ internal/localapi/utils.go | 156 +++++++++++++++++++++++++ internal/localapi/utils_unsupported.go | 14 +++ pkg/api/handlers/libpod/images.go | 44 +++++++ pkg/api/server/register_images.go | 24 ++++ pkg/bindings/connection.go | 14 ++- pkg/bindings/images/images.go | 19 +++ pkg/domain/infra/tunnel/images.go | 19 +++ pkg/machine/config.go | 5 + test/apiv2/10-images.at | 28 +++++ 11 files changed, 333 insertions(+), 44 deletions(-) create mode 100644 internal/localapi/types.go create mode 100644 internal/localapi/utils.go create mode 100644 internal/localapi/utils_unsupported.go diff --git a/cmd/podman/compose_machine.go b/cmd/podman/compose_machine.go index aadbfe42fc..f705499d5b 100644 --- a/cmd/podman/compose_machine.go +++ b/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) } diff --git a/internal/localapi/types.go b/internal/localapi/types.go new file mode 100644 index 0000000000..1020d78b88 --- /dev/null +++ b/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"` +} diff --git a/internal/localapi/utils.go b/internal/localapi/utils.go new file mode 100644 index 0000000000..01e265318e --- /dev/null +++ b/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) +} diff --git a/internal/localapi/utils_unsupported.go b/internal/localapi/utils_unsupported.go new file mode 100644 index 0000000000..87ee9eafbe --- /dev/null +++ b/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 +} diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index 0356b6e145..ea4a38fd5f 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/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) diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index ab8c6f3748..d4851baebe 100644 --- a/pkg/api/server/register_images.go +++ b/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: diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go index db69072f2a..7f7572489b 100644 --- a/pkg/bindings/connection.go +++ b/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 } diff --git a/pkg/bindings/images/images.go b/pkg/bindings/images/images.go index 275e68a9ac..7f119ad20b 100644 --- a/pkg/bindings/images/images.go +++ b/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 { diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 3fdce4696f..de1ca45045 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/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 diff --git a/pkg/machine/config.go b/pkg/machine/config.go index 0ab162f501..aa237ccb2e 100644 --- a/pkg/machine/config.go +++ b/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 diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at index 6e25ec0287..8105f108b2 100644 --- a/test/apiv2/10-images.at +++ b/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