Merge pull request #15285 from YoitoFes/fix-15210

remote manifest push: show copy progress

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()