From a4efd401cd87d98dced4f4eda41a8a1b86e72379 Mon Sep 17 00:00:00 2001 From: Naoto Kobayashi Date: Thu, 11 Aug 2022 20:35:13 +0900 Subject: remote manifest push: show copy progress `podman-remote manifest push` has shown absolutely no progress at all. Fix that by doing the same as the remote-push code does. Like remote-push, `quiet` parameter is true by default for backwards compatibility. Signed-off-by: Naoto Kobayashi --- pkg/api/handlers/libpod/manifests.go | 70 +++++++++++++++++++++++++++++++++--- pkg/api/server/register_manifest.go | 5 +++ pkg/bindings/manifests/manifests.go | 43 ++++++++++++++++++++-- pkg/domain/entities/manifest.go | 12 +++++++ pkg/domain/infra/abi/manifest.go | 3 +- pkg/domain/infra/tunnel/manifest.go | 2 +- test/e2e/manifest_test.go | 5 +++ 7 files changed, 132 insertions(+), 8 deletions(-) diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index 2d6223e4e..b0c93f3b9 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -19,12 +19,14 @@ import ( "github.com/containers/podman/v4/pkg/api/handlers/utils" api "github.com/containers/podman/v4/pkg/api/types" "github.com/containers/podman/v4/pkg/auth" + "github.com/containers/podman/v4/pkg/channel" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/domain/infra/abi" "github.com/containers/podman/v4/pkg/errorhandling" "github.com/gorilla/mux" "github.com/gorilla/schema" "github.com/opencontainers/go-digest" + "github.com/sirupsen/logrus" ) func ManifestCreate(w http.ResponseWriter, r *http.Request) { @@ -311,9 +313,13 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) { Format string `schema:"format"` RemoveSignatures bool `schema:"removeSignatures"` TLSVerify bool `schema:"tlsVerify"` + Quiet bool `schema:"quiet"` }{ // Add defaults here once needed. TLSVerify: true, + // #15210: older versions did not sent *any* data, so we need + // to be quiet by default to remain backwards compatible + Quiet: true, } if err := decoder.Decode(&query, r.URL.Query()); err != nil { utils.Error(w, http.StatusBadRequest, @@ -344,6 +350,7 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) { CompressionFormat: query.CompressionFormat, Format: query.Format, Password: password, + Quiet: true, RemoveSignatures: query.RemoveSignatures, Username: username, } @@ -356,12 +363,67 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) { imageEngine := abi.ImageEngine{Libpod: runtime} source := utils.GetName(r) - digest, err := imageEngine.ManifestPush(context.Background(), source, destination, options) - if err != nil { - utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", destination, err)) + + // Let's keep thing simple when running in quiet mode and push directly. + if query.Quiet { + digest, err := imageEngine.ManifestPush(context.Background(), source, destination, options) + if err != nil { + utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", destination, err)) + return + } + utils.WriteResponse(w, http.StatusOK, entities.ManifestPushReport{ID: digest}) return } - utils.WriteResponse(w, http.StatusOK, entities.IDResponse{ID: digest}) + + writer := channel.NewWriter(make(chan []byte)) + defer writer.Close() + options.Writer = writer + + pushCtx, pushCancel := context.WithCancel(r.Context()) + var digest string + var pushError error + go func() { + defer pushCancel() + digest, pushError = imageEngine.ManifestPush(pushCtx, source, destination, options) + }() + + flush := func() { + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + } + + w.WriteHeader(http.StatusOK) + w.Header().Set("Content-Type", "application/json") + flush() + + enc := json.NewEncoder(w) + enc.SetEscapeHTML(true) + for { + var report entities.ManifestPushReport + select { + case s := <-writer.Chan(): + report.Stream = string(s) + if err := enc.Encode(report); err != nil { + logrus.Warnf("Failed to encode json: %v", err) + } + flush() + case <-pushCtx.Done(): + if pushError != nil { + report.Error = pushError.Error() + } else { + report.ID = digest + } + if err := enc.Encode(report); err != nil { + logrus.Warnf("Failed to encode json: %v", err) + } + flush() + return + case <-r.Context().Done(): + // Client has closed connection + return + } + } } // ManifestModify efficiently updates the named manifest list diff --git a/pkg/api/server/register_manifest.go b/pkg/api/server/register_manifest.go index 19b507047..c22479cf9 100644 --- a/pkg/api/server/register_manifest.go +++ b/pkg/api/server/register_manifest.go @@ -75,6 +75,11 @@ func (s *APIServer) registerManifestHandlers(r *mux.Router) error { // type: boolean // default: true // description: Require HTTPS and verify signatures when contacting registries. + // - in: query + // name: quiet + // description: "silences extra stream data on push" + // type: boolean + // default: true // responses: // 200: // schema: diff --git a/pkg/bindings/manifests/manifests.go b/pkg/bindings/manifests/manifests.go index 80153c4b4..49e4089f5 100644 --- a/pkg/bindings/manifests/manifests.go +++ b/pkg/bindings/manifests/manifests.go @@ -2,10 +2,13 @@ package manifests import ( "context" + "encoding/json" "errors" "fmt" + "io" "io/ioutil" "net/http" + "os" "strconv" "strings" @@ -142,7 +145,6 @@ func Delete(ctx context.Context, name string) (*entities.ManifestRemoveReport, e // the name will be used instead. If the optional all boolean is specified, all images specified // in the list will be pushed as well. func Push(ctx context.Context, name, destination string, options *images.PushOptions) (string, error) { - var idr entities.IDResponse if options == nil { options = new(images.PushOptions) } @@ -176,7 +178,44 @@ func Push(ctx context.Context, name, destination string, options *images.PushOpt } defer response.Body.Close() - return idr.ID, response.Process(&idr) + if !response.IsSuccess() { + return "", response.Process(err) + } + + // Historically push writes status to stderr + writer := io.Writer(os.Stderr) + if options.GetQuiet() { + writer = io.Discard + } else if progressWriter := options.GetProgressWriter(); progressWriter != nil { + writer = progressWriter + } + + dec := json.NewDecoder(response.Body) + for { + var report entities.ManifestPushReport + if err := dec.Decode(&report); err != nil { + return "", err + } + + select { + case <-response.Request.Context().Done(): + break + default: + // non-blocking select + } + + switch { + case report.ID != "": + return report.ID, nil + case report.Stream != "": + fmt.Fprint(writer, report.Stream) + case report.Error != "": + // There can only be one error. + return "", errors.New(report.Error) + default: + return "", fmt.Errorf("failed to parse push results stream, unexpected input: %v", report) + } + } } // Modify modifies the given manifest list using options and the optional list of images diff --git a/pkg/domain/entities/manifest.go b/pkg/domain/entities/manifest.go index e88c5f854..126b76c62 100644 --- a/pkg/domain/entities/manifest.go +++ b/pkg/domain/entities/manifest.go @@ -61,6 +61,18 @@ type ManifestModifyOptions struct { ManifestRemoveOptions } +// ManifestPushReport provides the model for the pushed manifest +// +// swagger:model +type ManifestPushReport struct { + // ID of the pushed manifest + ID string `json:"Id"` + // Stream used to provide push progress + Stream string `json:"stream,omitempty"` + // Error contains text of errors from pushing + Error string `json:"error,omitempty"` +} + // ManifestRemoveOptions provides the model for removing digests from a manifest // // swagger:model diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go index 4b10d9b18..e0c11267e 100644 --- a/pkg/domain/infra/abi/manifest.go +++ b/pkg/domain/infra/abi/manifest.go @@ -321,6 +321,7 @@ func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination strin pushOptions.SignBySigstorePrivateKeyFile = opts.SignBySigstorePrivateKeyFile pushOptions.SignSigstorePrivateKeyPassphrase = opts.SignSigstorePrivateKeyPassphrase pushOptions.InsecureSkipTLSVerify = opts.SkipTLSVerify + pushOptions.Writer = opts.Writer compressionFormat := opts.CompressionFormat if compressionFormat == "" { @@ -341,7 +342,7 @@ func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination strin if opts.All { pushOptions.ImageListSelection = cp.CopyAllImages } - if !opts.Quiet { + if !opts.Quiet && pushOptions.Writer == nil { pushOptions.Writer = os.Stderr } diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go index 00ecb3b59..2a514861d 100644 --- a/pkg/domain/infra/tunnel/manifest.go +++ b/pkg/domain/infra/tunnel/manifest.go @@ -99,7 +99,7 @@ func (ir *ImageEngine) ManifestRm(ctx context.Context, names []string) (*entitie // ManifestPush pushes a manifest list or image index to the destination func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination string, opts entities.ImagePushOptions) (string, error) { options := new(images.PushOptions) - options.WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile).WithRemoveSignatures(opts.RemoveSignatures).WithAll(opts.All).WithFormat(opts.Format).WithCompressionFormat(opts.CompressionFormat) + options.WithUsername(opts.Username).WithPassword(opts.Password).WithAuthfile(opts.Authfile).WithRemoveSignatures(opts.RemoveSignatures).WithAll(opts.All).WithFormat(opts.Format).WithCompressionFormat(opts.CompressionFormat).WithQuiet(opts.Quiet).WithProgressWriter(opts.Writer) if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined { if s == types.OptionalBoolTrue { diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go index e80772aed..ee954a1a4 100644 --- a/test/e2e/manifest_test.go +++ b/test/e2e/manifest_test.go @@ -379,6 +379,11 @@ var _ = Describe("Podman manifest", func() { push = podmanTest.Podman([]string{"manifest", "push", "--tls-verify=false", "--creds=" + registry.User + ":" + registry.Password, "foo", "localhost:" + registry.Port + "/credstest"}) push.WaitWithDefaultTimeout() Expect(push).Should(Exit(0)) + output := push.ErrorToString() + Expect(output).To(ContainSubstring("Copying blob ")) + Expect(output).To(ContainSubstring("Copying config ")) + Expect(output).To(ContainSubstring("Writing manifest to image destination")) + Expect(output).To(ContainSubstring("Storing signatures")) push = podmanTest.Podman([]string{"manifest", "push", "--tls-verify=false", "--creds=podmantest:wrongpasswd", "foo", "localhost:" + registry.Port + "/credstest"}) push.WaitWithDefaultTimeout() -- cgit v1.2.3-54-g00ecf