Merge pull request #15351 from marshall-lee/images-pull-simple

Simplify ImagesPull for when Quiet flag is on

cmd/podman/images/pull.go

@@ -155,6 +155,11 @@ func imagePull(cmd *cobra.Command, args []string) error {
 		pullOptions.Username = creds.Username
 		pullOptions.Password = creds.Password
 	}
+
+	if !pullOptions.Quiet {
+		pullOptions.Writer = os.Stderr
+	}
+
 	// Let's do all the remaining Yoga in the API to prevent us from
 	// scattering logic across (too) many parts of the code.
 	var errs utils.OutputErrors

cmd/podman/images/push.go

@@ -164,6 +164,10 @@ func imagePush(cmd *cobra.Command, args []string) error {
 		pushOptions.Password = creds.Password
 	}
 
+	if !pushOptions.Quiet {
+		pushOptions.Writer = os.Stderr
+	}
+
 	if err := common.PrepareSigningPassphrase(&pushOptions.ImagePushOptions, pushOptions.SignPassphraseFileCLI); err != nil {
 		return err
 	}

cmd/podman/manifest/push.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"os"
 
 	"github.com/containers/common/pkg/auth"
 	"github.com/containers/common/pkg/completion"
@@ -122,6 +123,10 @@ func push(cmd *cobra.Command, args []string) error {
 		manifestPushOpts.Password = creds.Password
 	}
 
+	if !manifestPushOpts.Quiet {
+		manifestPushOpts.Writer = os.Stderr
+	}
+
 	if err := common.PrepareSigningPassphrase(&manifestPushOpts.ImagePushOptions, manifestPushOpts.SignPassphraseFileCLI); err != nil {
 		return err
 	}

pkg/api/handlers/compat/auth.go

@@ -1,7 +1,6 @@
 package compat
 
 import (
-	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -44,7 +43,7 @@ func Auth(w http.ResponseWriter, r *http.Request) {
 
 	fmt.Println("Authenticating with existing credentials...")
 	registry := stripAddressOfScheme(authConfig.ServerAddress)
-	if err := DockerClient.CheckAuth(context.Background(), sysCtx, authConfig.Username, authConfig.Password, registry); err == nil {
+	if err := DockerClient.CheckAuth(r.Context(), sysCtx, authConfig.Username, authConfig.Password, registry); err == nil {
 		utils.WriteResponse(w, http.StatusOK, entities.AuthReport{
 			IdentityToken: "",
 			Status:        "Login Succeeded",

pkg/api/handlers/compat/images_build.go

@@ -694,7 +694,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		success bool
 	)
 
-	runCtx, cancel := context.WithCancel(context.Background())
+	runCtx, cancel := context.WithCancel(r.Context())
 	go func() {
 		defer cancel()
 		imageID, _, err = runtime.Build(r.Context(), buildOptions, containerFiles...)

pkg/api/handlers/libpod/containers_create.go

@@ -1,7 +1,6 @@
 package libpod
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -63,12 +62,12 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 		utils.InternalServerError(w, err)
 		return
 	}
-	rtSpec, spec, opts, err := generate.MakeContainer(context.Background(), runtime, &sg, false, nil)
+	rtSpec, spec, opts, err := generate.MakeContainer(r.Context(), runtime, &sg, false, nil)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
-	ctr, err := generate.ExecuteCreate(context.Background(), runtime, rtSpec, spec, false, opts...)
+	ctr, err := generate.ExecuteCreate(r.Context(), runtime, rtSpec, spec, false, opts...)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return

pkg/api/handlers/libpod/images_pull.go

@@ -82,17 +82,32 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) {
 		pullOptions.IdentityToken = authConf.IdentityToken
 	}
 
-	writer := channel.NewWriter(make(chan []byte))
-	defer writer.Close()
-
-	pullOptions.Writer = writer
-
 	pullPolicy, err := config.ParsePullPolicy(query.PullPolicy)
 	if err != nil {
 		utils.Error(w, http.StatusBadRequest, err)
 		return
 	}
 
+	// Let's keep thing simple when running in quiet mode and pull directly.
+	if query.Quiet {
+		images, err := runtime.LibimageRuntime().Pull(r.Context(), query.Reference, pullPolicy, pullOptions)
+		var report entities.ImagePullReport
+		if err != nil {
+			report.Error = err.Error()
+		}
+		for _, image := range images {
+			report.Images = append(report.Images, image.ID())
+			// Pull last ID from list and publish in 'id' stanza.  This maintains previous API contract
+			report.ID = image.ID()
+		}
+		utils.WriteResponse(w, http.StatusOK, report)
+		return
+	}
+
+	writer := channel.NewWriter(make(chan []byte))
+	defer writer.Close()
+	pullOptions.Writer = writer
+
 	var pulledImages []*libimage.Image
 	var pullError error
 	runCtx, cancel := context.WithCancel(r.Context())
@@ -118,10 +133,8 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) {
 		select {
 		case s := <-writer.Chan():
 			report.Stream = string(s)
-			if !query.Quiet {
-				if err := enc.Encode(report); err != nil {
-					logrus.Warnf("Failed to encode json: %v", err)
-				}
+			if err := enc.Encode(report); err != nil {
+				logrus.Warnf("Failed to encode json: %v", err)
 			}
 			flush()
 		case <-runCtx.Done():

pkg/api/handlers/libpod/images_push.go

@@ -90,7 +90,7 @@ func PushImage(w http.ResponseWriter, r *http.Request) {
 
 	// Let's keep thing simple when running in quiet mode and push directly.
 	if query.Quiet {
-		if err := imageEngine.Push(context.Background(), source, destination, options); err != nil {
+		if err := imageEngine.Push(r.Context(), source, destination, options); err != nil {
 			utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", destination, err))
 			return
 		}

pkg/api/handlers/libpod/manifests.go

@@ -293,7 +293,7 @@ func ManifestPushV3(w http.ResponseWriter, r *http.Request) {
 		options.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify)
 	}
 	imageEngine := abi.ImageEngine{Libpod: runtime}
-	digest, err := imageEngine.ManifestPush(context.Background(), source, query.Destination, options)
+	digest, err := imageEngine.ManifestPush(r.Context(), source, query.Destination, options)
 	if err != nil {
 		utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", query.Destination, err))
 		return
@@ -367,7 +367,7 @@ func ManifestPush(w http.ResponseWriter, r *http.Request) {
 
 	// 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)
+		digest, err := imageEngine.ManifestPush(r.Context(), source, destination, options)
 		if err != nil {
 			utils.Error(w, http.StatusBadRequest, fmt.Errorf("error pushing image %q: %w", destination, err))
 			return

pkg/api/handlers/types.go

@@ -162,7 +162,7 @@ type ExecStartConfig struct {
 
 func ImageDataToImageInspect(ctx context.Context, l *libimage.Image) (*ImageInspect, error) {
 	options := &libimage.InspectOptions{WithParent: true, WithSize: true}
-	info, err := l.Inspect(context.Background(), options)
+	info, err := l.Inspect(ctx, options)
 	if err != nil {
 		return nil, err
 	}

pkg/bindings/images/pull.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"strconv"
@@ -57,10 +56,14 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string,
 		return nil, response.Process(err)
 	}
 
-	// Historically pull writes status to stderr
-	stderr := io.Writer(os.Stderr)
+	var writer io.Writer
 	if options.GetQuiet() {
-		stderr = ioutil.Discard
+		writer = io.Discard
+	} else if progressWriter := options.GetProgressWriter(); progressWriter != nil {
+		writer = progressWriter
+	} else {
+		// Historically push writes status to stderr
+		writer = os.Stderr
 	}
 
 	dec := json.NewDecoder(response.Body)
@@ -84,7 +87,7 @@ func Pull(ctx context.Context, rawImage string, options *PullOptions) ([]string,
 
 		switch {
 		case report.Stream != "":
-			fmt.Fprint(stderr, report.Stream)
+			fmt.Fprint(writer, report.Stream)
 		case report.Error != "":
 			pullErrors = append(pullErrors, errors.New(report.Error))
 		case len(report.Images) > 0:

pkg/bindings/images/push.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"strconv"
@@ -58,12 +57,14 @@ func Push(ctx context.Context, source string, destination string, options *PushO
 		return response.Process(err)
 	}
 
-	// Historically push writes status to stderr
-	writer := io.Writer(os.Stderr)
+	var writer io.Writer
 	if options.GetQuiet() {
-		writer = ioutil.Discard
+		writer = io.Discard
 	} else if progressWriter := options.GetProgressWriter(); progressWriter != nil {
 		writer = progressWriter
+	} else {
+		// Historically push writes status to stderr
+		writer = os.Stderr
 	}
 
 	dec := json.NewDecoder(response.Body)

pkg/bindings/images/types.go

@@ -182,6 +182,8 @@ type PullOptions struct {
 	Policy *string
 	// Password for authenticating against the registry.
 	Password *string
+	// ProgressWriter is a writer where pull progress are sent.
+	ProgressWriter *io.Writer
 	// Quiet can be specified to suppress pull progress when pulling.  Ignored
 	// for remote calls.
 	Quiet *bool

pkg/bindings/images/types_pull_options.go

@@ -2,6 +2,7 @@
 package images
 
 import (
+	"io"
 	"net/url"
 
 	"github.com/containers/podman/v4/pkg/bindings/internal/util"
@@ -107,6 +108,21 @@ func (o *PullOptions) GetPassword() string {
 	return *o.Password
 }
 
+// WithProgressWriter set field ProgressWriter to given value
+func (o *PullOptions) WithProgressWriter(value io.Writer) *PullOptions {
+	o.ProgressWriter = &value
+	return o
+}
+
+// GetProgressWriter returns value of field ProgressWriter
+func (o *PullOptions) GetProgressWriter() io.Writer {
+	if o.ProgressWriter == nil {
+		var z io.Writer
+		return z
+	}
+	return *o.ProgressWriter
+}
+
 // WithQuiet set field Quiet to given value
 func (o *PullOptions) WithQuiet(value bool) *PullOptions {
 	o.Quiet = &value

pkg/bindings/manifests/manifests.go

@@ -182,12 +182,14 @@ func Push(ctx context.Context, name, destination string, options *images.PushOpt
 		return "", response.Process(err)
 	}
 
-	// Historically push writes status to stderr
-	writer := io.Writer(os.Stderr)
+	var writer io.Writer
 	if options.GetQuiet() {
 		writer = io.Discard
 	} else if progressWriter := options.GetProgressWriter(); progressWriter != nil {
 		writer = progressWriter
+	} else {
+		// Historically push writes status to stderr
+		writer = os.Stderr
 	}
 
 	dec := json.NewDecoder(response.Body)

pkg/bindings/test/images_test.go

@@ -1,11 +1,14 @@
 package bindings_test
 
 import (
+	"bytes"
+	"fmt"
 	"net/http"
 	"os"
 	"path/filepath"
 	"time"
 
+	podmanRegistry "github.com/containers/podman/v4/hack/podman-registry-go"
 	"github.com/containers/podman/v4/pkg/bindings"
 	"github.com/containers/podman/v4/pkg/bindings/containers"
 	"github.com/containers/podman/v4/pkg/bindings/images"
@@ -362,9 +365,14 @@ var _ = Describe("Podman images", func() {
 	It("Image Pull", func() {
 		rawImage := "docker.io/library/busybox:latest"
 
-		pulledImages, err := images.Pull(bt.conn, rawImage, nil)
+		var writer bytes.Buffer
+		pullOpts := new(images.PullOptions).WithProgressWriter(&writer)
+		pulledImages, err := images.Pull(bt.conn, rawImage, pullOpts)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(pulledImages)).To(Equal(1))
+		output := writer.String()
+		Expect(output).To(ContainSubstring("Trying to pull "))
+		Expect(output).To(ContainSubstring("Getting image source signatures"))
 
 		exists, err := images.Exists(bt.conn, rawImage, nil)
 		Expect(err).NotTo(HaveOccurred())
@@ -380,7 +388,19 @@ var _ = Describe("Podman images", func() {
 	})
 
 	It("Image Push", func() {
-		Skip("TODO: implement test for image push to registry")
+		registry, err := podmanRegistry.Start()
+		Expect(err).To(BeNil())
+
+		var writer bytes.Buffer
+		pushOpts := new(images.PushOptions).WithUsername(registry.User).WithPassword(registry.Password).WithSkipTLSVerify(true).WithProgressWriter(&writer).WithQuiet(false)
+		err = images.Push(bt.conn, alpine.name, fmt.Sprintf("localhost:%s/test:latest", registry.Port), pushOpts)
+		Expect(err).ToNot(HaveOccurred())
+
+		output := writer.String()
+		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"))
 	})
 
 	It("Build no options", func() {

pkg/bindings/test/manifests_test.go

@@ -1,9 +1,12 @@
 package bindings_test
 
 import (
+	"bytes"
+	"fmt"
 	"net/http"
 	"time"
 
+	podmanRegistry "github.com/containers/podman/v4/hack/podman-registry-go"
 	"github.com/containers/podman/v4/pkg/bindings"
 	"github.com/containers/podman/v4/pkg/bindings/images"
 	"github.com/containers/podman/v4/pkg/bindings/manifests"
@@ -12,7 +15,7 @@ import (
 	"github.com/onsi/gomega/gexec"
 )
 
-var _ = Describe("podman manifest", func() {
+var _ = Describe("Podman manifests", func() {
 	var (
 		bt *bindingTest
 		s  *gexec.Session
@@ -172,7 +175,21 @@ var _ = Describe("podman manifest", func() {
 		Expect(list.Manifests[0].Platform.OS).To(Equal("foo"))
 	})
 
-	It("push manifest", func() {
-		Skip("TODO: implement test for manifest push to registry")
+	It("Manifest Push", func() {
+		registry, err := podmanRegistry.Start()
+		Expect(err).To(BeNil())
+
+		name := "quay.io/libpod/foobar:latest"
+		_, err = manifests.Create(bt.conn, name, []string{alpine.name}, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		var writer bytes.Buffer
+		pushOpts := new(images.PushOptions).WithUsername(registry.User).WithPassword(registry.Password).WithAll(true).WithSkipTLSVerify(true).WithProgressWriter(&writer).WithQuiet(false)
+		_, err = manifests.Push(bt.conn, name, fmt.Sprintf("localhost:%s/test:latest", registry.Port), pushOpts)
+		Expect(err).ToNot(HaveOccurred())
+
+		output := writer.String()
+		Expect(output).To(ContainSubstring("Writing manifest list to image destination"))
+		Expect(output).To(ContainSubstring("Storing list signatures"))
 	})
 })

pkg/domain/entities/images.go

@@ -156,6 +156,8 @@ type ImagePullOptions struct {
 	SkipTLSVerify types.OptionalBool
 	// PullPolicy whether to pull new image
 	PullPolicy config.PullPolicy
+	// Writer is used to display copy information including progress bars.
+	Writer io.Writer
 }
 
 // ImagePullReport is the response from pulling one or more images.

pkg/domain/infra/abi/images.go

@@ -237,8 +237,9 @@ func (ir *ImageEngine) Pull(ctx context.Context, rawImage string, options entiti
 	pullOptions.Variant = options.Variant
 	pullOptions.SignaturePolicyPath = options.SignaturePolicy
 	pullOptions.InsecureSkipTLSVerify = options.SkipTLSVerify
+	pullOptions.Writer = options.Writer
 
-	if !options.Quiet {
+	if !options.Quiet && pullOptions.Writer == nil {
 		pullOptions.Writer = os.Stderr
 	}
 

pkg/domain/infra/tunnel/images.go

@@ -110,6 +110,7 @@ func (ir *ImageEngine) Pull(ctx context.Context, rawImage string, opts entities.
 	options.WithAllTags(opts.AllTags).WithAuthfile(opts.Authfile).WithArch(opts.Arch).WithOS(opts.OS)
 	options.WithVariant(opts.Variant).WithPassword(opts.Password)
 	options.WithQuiet(opts.Quiet).WithUsername(opts.Username).WithPolicy(opts.PullPolicy.String())
+	options.WithProgressWriter(opts.Writer)
 	if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined {
 		if s == types.OptionalBoolTrue {
 			options.WithSkipTLSVerify(true)

test/e2e/manifest_test.go

@@ -350,6 +350,33 @@ var _ = Describe("Podman manifest", func() {
 		Expect(foundZstdFile).To(BeTrue())
 	})
 
+	It("push progress", func() {
+		SkipIfRemote("manifest push to dir not supported in remote mode")
+
+		session := podmanTest.Podman([]string{"manifest", "create", "foo", imageList})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		dest := filepath.Join(podmanTest.TempDir, "pushed")
+		err := os.MkdirAll(dest, os.ModePerm)
+		Expect(err).To(BeNil())
+		defer func() {
+			os.RemoveAll(dest)
+		}()
+
+		session = podmanTest.Podman([]string{"push", "foo", "-q", "dir:" + dest})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(session.ErrorToString()).To(BeEmpty())
+
+		session = podmanTest.Podman([]string{"push", "foo", "dir:" + dest})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		output := session.ErrorToString()
+		Expect(output).To(ContainSubstring("Writing manifest list to image destination"))
+		Expect(output).To(ContainSubstring("Storing list signatures"))
+	})
+
 	It("authenticated push", func() {
 		registryOptions := &podmanRegistry.Options{
 			Image: "docker-archive:" + imageTarPath(REGISTRY_IMAGE),

test/e2e/pull_test.go

@@ -545,4 +545,18 @@ var _ = Describe("Podman pull", func() {
 		Expect(data[0]).To(HaveField("Os", runtime.GOOS))
 		Expect(data[0]).To(HaveField("Architecture", "arm64"))
 	})
+
+	It("podman pull progress", func() {
+		session := podmanTest.Podman([]string{"pull", ALPINE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		output := session.ErrorToString()
+		Expect(output).To(ContainSubstring("Getting image source signatures"))
+		Expect(output).To(ContainSubstring("Copying blob "))
+
+		session = podmanTest.Podman([]string{"pull", "-q", ALPINE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(session.ErrorToString()).To(BeEmpty())
+	})
 })