From 76fec12274ebae33fee7180321545a79297bb823 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 11 Oct 2023 10:14:14 +0200 Subject: [PATCH 1/2] api: break out compat image pull Break out the code for pulling images via the compat API. The goal is to make this code shareable between the compat and libpod API to allow for a "compat mode" in the libpod pull endpoint. [NO NEW TESTS NEEDED] as it should not change behavior. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/compat/images.go | 102 +----------------------------- pkg/api/handlers/utils/images.go | 102 ++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 101 deletions(-) diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go index 04e59d58ff..0dc0d2bdad 100644 --- a/pkg/api/handlers/compat/images.go +++ b/pkg/api/handlers/compat/images.go @@ -15,7 +15,6 @@ import ( "github.com/containers/common/pkg/config" "github.com/containers/common/pkg/filters" "github.com/containers/image/v5/manifest" - "github.com/containers/image/v5/types" "github.com/containers/podman/v4/libpod" "github.com/containers/podman/v4/pkg/api/handlers" "github.com/containers/podman/v4/pkg/api/handlers/utils" @@ -25,10 +24,8 @@ import ( "github.com/containers/podman/v4/pkg/domain/infra/abi" "github.com/containers/podman/v4/pkg/util" "github.com/containers/storage" - "github.com/docker/distribution/registry/api/errcode" docker "github.com/docker/docker/api/types" dockerContainer "github.com/docker/docker/api/types/container" - "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/go-connections/nat" "github.com/opencontainers/go-digest" "github.com/sirupsen/logrus" @@ -253,11 +250,6 @@ func CreateImageFromSrc(w http.ResponseWriter, r *http.Request) { }) } -type pullResult struct { - images []*libimage.Image - err error -} - func CreateImageFromImage(w http.ResponseWriter, r *http.Request) { // 200 no error // 404 repo does not exist or no read access @@ -309,99 +301,7 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) { } } - progress := make(chan types.ProgressProperties) - pullOptions.Progress = progress - - pullResChan := make(chan pullResult) - go func() { - pulledImages, err := runtime.LibimageRuntime().Pull(r.Context(), possiblyNormalizedName, config.PullPolicyAlways, pullOptions) - pullResChan <- pullResult{images: pulledImages, err: err} - }() - - enc := json.NewEncoder(w) - enc.SetEscapeHTML(true) - - flush := func() { - if flusher, ok := w.(http.Flusher); ok { - flusher.Flush() - } - } - - statusWritten := false - writeStatusCode := func(code int) { - if !statusWritten { - w.WriteHeader(code) - w.Header().Set("Content-Type", "application/json") - flush() - statusWritten = true - } - } - -loop: // break out of for/select infinite loop - for { - report := jsonmessage.JSONMessage{} - report.Progress = &jsonmessage.JSONProgress{} - select { - case e := <-progress: - writeStatusCode(http.StatusOK) - switch e.Event { - case types.ProgressEventNewArtifact: - report.Status = "Pulling fs layer" - case types.ProgressEventRead: - report.Status = "Downloading" - report.Progress.Current = int64(e.Offset) - report.Progress.Total = e.Artifact.Size - report.ProgressMessage = report.Progress.String() - case types.ProgressEventSkipped: - report.Status = "Already exists" - case types.ProgressEventDone: - report.Status = "Download complete" - } - report.ID = e.Artifact.Digest.Encoded()[0:12] - if err := enc.Encode(report); err != nil { - logrus.Warnf("Failed to json encode error %q", err.Error()) - } - flush() - case pullRes := <-pullResChan: - err := pullRes.err - if err != nil { - var errcd errcode.ErrorCoder - if errors.As(err, &errcd) { - writeStatusCode(errcd.ErrorCode().Descriptor().HTTPStatusCode) - } else { - writeStatusCode(http.StatusInternalServerError) - } - msg := err.Error() - report.Error = &jsonmessage.JSONError{ - Message: msg, - } - report.ErrorMessage = msg - } else { - pulledImages := pullRes.images - if len(pulledImages) > 0 { - img := pulledImages[0].ID() - if utils.IsLibpodRequest(r) { - report.Status = "Pull complete" - } else { - report.Status = "Download complete" - } - report.ID = img[0:12] - } else { - msg := "internal error: no images pulled" - report.Error = &jsonmessage.JSONError{ - Message: msg, - } - report.ErrorMessage = msg - writeStatusCode(http.StatusInternalServerError) - } - } - if err := enc.Encode(report); err != nil { - logrus.Warnf("Failed to json encode error %q", err.Error()) - } - flush() - break loop // break out of for/select infinite loop - } - } + utils.CompatPull(r.Context(), w, runtime, possiblyNormalizedName, config.PullPolicyAlways, pullOptions) } func GetImage(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/api/handlers/utils/images.go b/pkg/api/handlers/utils/images.go index 7831718cdb..58cda0e01f 100644 --- a/pkg/api/handlers/utils/images.go +++ b/pkg/api/handlers/utils/images.go @@ -1,12 +1,14 @@ package utils import ( + "context" "errors" "fmt" "net/http" "strings" "github.com/containers/common/libimage" + "github.com/containers/common/pkg/config" "github.com/containers/image/v5/docker" "github.com/containers/image/v5/docker/reference" storageTransport "github.com/containers/image/v5/storage" @@ -16,6 +18,9 @@ import ( api "github.com/containers/podman/v4/pkg/api/types" "github.com/containers/podman/v4/pkg/util" "github.com/containers/storage" + "github.com/docker/distribution/registry/api/errcode" + "github.com/docker/docker/pkg/jsonmessage" + "github.com/sirupsen/logrus" ) // NormalizeToDockerHub normalizes the specified nameOrID to Docker Hub if the @@ -102,3 +107,100 @@ func GetImage(r *http.Request, name string) (*libimage.Image, error) { } return image, err } + +type pullResult struct { + images []*libimage.Image + err error +} + +func CompatPull(ctx context.Context, w http.ResponseWriter, runtime *libpod.Runtime, reference string, pullPolicy config.PullPolicy, pullOptions *libimage.PullOptions) { + progress := make(chan types.ProgressProperties) + pullOptions.Progress = progress + + pullResChan := make(chan pullResult) + go func() { + pulledImages, err := runtime.LibimageRuntime().Pull(ctx, reference, pullPolicy, pullOptions) + pullResChan <- pullResult{images: pulledImages, err: err} + }() + + enc := json.NewEncoder(w) + enc.SetEscapeHTML(true) + + flush := func() { + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + } + + statusWritten := false + writeStatusCode := func(code int) { + if !statusWritten { + w.WriteHeader(code) + w.Header().Set("Content-Type", "application/json") + flush() + statusWritten = true + } + } + +loop: // break out of for/select infinite loop + for { + report := jsonmessage.JSONMessage{} + report.Progress = &jsonmessage.JSONProgress{} + select { + case e := <-progress: + writeStatusCode(http.StatusOK) + switch e.Event { + case types.ProgressEventNewArtifact: + report.Status = "Pulling fs layer" + case types.ProgressEventRead: + report.Status = "Downloading" + report.Progress.Current = int64(e.Offset) + report.Progress.Total = e.Artifact.Size + report.ProgressMessage = report.Progress.String() + case types.ProgressEventSkipped: + report.Status = "Already exists" + case types.ProgressEventDone: + report.Status = "Download complete" + } + report.ID = e.Artifact.Digest.Encoded()[0:12] + if err := enc.Encode(report); err != nil { + logrus.Warnf("Failed to json encode error %q", err.Error()) + } + flush() + case pullRes := <-pullResChan: + err := pullRes.err + if err != nil { + var errcd errcode.ErrorCoder + if errors.As(err, &errcd) { + writeStatusCode(errcd.ErrorCode().Descriptor().HTTPStatusCode) + } else { + writeStatusCode(http.StatusInternalServerError) + } + msg := err.Error() + report.Error = &jsonmessage.JSONError{ + Message: msg, + } + report.ErrorMessage = msg + } else { + pulledImages := pullRes.images + if len(pulledImages) > 0 { + img := pulledImages[0].ID() + report.Status = "Download complete" + report.ID = img[0:12] + } else { + msg := "internal error: no images pulled" + report.Error = &jsonmessage.JSONError{ + Message: msg, + } + report.ErrorMessage = msg + writeStatusCode(http.StatusInternalServerError) + } + } + if err := enc.Encode(report); err != nil { + logrus.Warnf("Failed to json encode error %q", err.Error()) + } + flush() + break loop // break out of for/select infinite loop + } + } +} From 8b46e852ef41287bfdebcc0e1de379905c479470 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 11 Oct 2023 10:53:35 +0200 Subject: [PATCH 2/2] api: add `compatMode` paramenter to libpod's pull endpoint Add a new `compatMode` parameter to libpod's pull endpoint. If set, the streamed JSON payload is identical to the one of the Docker compat endpoint and allows for a smooth integration into existing tooling such as podman-py and Podman Desktop, some of which already have code for rendering the compat progress data. We may add a libpod-specific parameter in the future which will stream differnt progress data. Fixes: issues.redhat.com/browse/RUN-1936? Signed-off-by: Valentin Rothberg --- pkg/api/handlers/libpod/images_pull.go | 22 +++++++++++++++++----- pkg/api/server/register_images.go | 5 +++++ test/apiv2/10-images.at | 1 + 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/pkg/api/handlers/libpod/images_pull.go b/pkg/api/handlers/libpod/images_pull.go index 57b2e3a787..0f50b44a45 100644 --- a/pkg/api/handlers/libpod/images_pull.go +++ b/pkg/api/handlers/libpod/images_pull.go @@ -28,14 +28,16 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) query := struct { - Reference string `schema:"reference"` - OS string `schema:"OS"` - Arch string `schema:"Arch"` - Variant string `schema:"Variant"` - TLSVerify bool `schema:"tlsVerify"` AllTags bool `schema:"allTags"` + CompatMode bool `schema:"compatMode"` PullPolicy string `schema:"policy"` Quiet bool `schema:"quiet"` + Reference string `schema:"reference"` + TLSVerify bool `schema:"tlsVerify"` + // Platform fields below: + Arch string `schema:"Arch"` + OS string `schema:"OS"` + Variant string `schema:"Variant"` }{ TLSVerify: true, PullPolicy: "always", @@ -46,6 +48,11 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { return } + if query.Quiet && query.CompatMode { + utils.InternalServerError(w, errors.New("'quiet' and 'compatMode' cannot be used simultaneously")) + return + } + if len(query.Reference) == 0 { utils.InternalServerError(w, errors.New("reference parameter cannot be empty")) return @@ -104,6 +111,11 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { return } + if query.CompatMode { + utils.CompatPull(r.Context(), w, runtime, query.Reference, pullPolicy, pullOptions) + return + } + writer := channel.NewWriter(make(chan []byte)) defer writer.Close() pullOptions.Writer = writer diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index 5645052497..4c14f54140 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -1019,6 +1019,11 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // type: boolean // default: false // - in: query + // name: compatMode + // description: "Return the same JSON payload as the Docker-compat endpoint." + // type: boolean + // default: false + // - in: query // name: Arch // description: Pull image for the specified architecture. // type: string diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at index be742b01b9..d4b4c8ac35 100644 --- a/test/apiv2/10-images.at +++ b/test/apiv2/10-images.at @@ -49,6 +49,7 @@ t GET images/$iid/json 200 \ .RepoTags[0]=$IMAGE t POST "images/create?fromImage=alpine" 200 .error~null .status~".*Download complete.*" +t POST "libpod/images/pull?reference=alpine&compatMode=true" 200 .error~null .status~".*Download complete.*" t POST "images/create?fromImage=alpine&tag=latest" 200