From 0a9d5ca75da562c15d7d55eb2705af8be91b612d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Rod=C3=A1k?= Date: Mon, 4 Aug 2025 17:00:31 +0200 Subject: [PATCH 1/2] Skip JSON parsing for non-JSON error responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check Content-Type header before unmarshaling errors to avoid unnecessary JSON parsing overhead for plain text responses. Signed-off-by: Jan Rodák --- pkg/bindings/errors.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/bindings/errors.go b/pkg/bindings/errors.go index 0b68c04db86..74cf616ebc8 100644 --- a/pkg/bindings/errors.go +++ b/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) { 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 2/2] 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 aadbfe42fcd..f705499d5b8 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) - if err != nil { - return "", 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()) + podmanSocket, podmanPipe, err := mc.ConnectionInfo(machineProvider.VMType()) 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 "", err } - 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 00000000000..1020d78b88b --- /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 00000000000..01e265318e2 --- /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 00000000000..87ee9eafbed --- /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 0356b6e1455..ea4a38fd5f5 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 ab8c6f37485..d4851baebe4 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 db69072f2a5..7f7572489b3 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 275e68a9ac2..7f119ad20bc 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 3fdce4696ff..de1ca450457 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 0ab162f5019..aa237ccb2ea 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 6e25ec02876..8105f108b22 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