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

Simplify ImagesPull for when Quiet flag is on

7 files changed, 77 insertions, 16 deletions

diff --git a/pkg/bindings/images/pull.go b/pkg/bindings/images/pull.go
index 1a4aa3038..109981c63 100644
--- a/pkg/bindings/images/pull.go
+++ b/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:
diff --git a/pkg/bindings/images/push.go b/pkg/bindings/images/push.go
index 5069dd780..f1e059f8c 100644
--- a/pkg/bindings/images/push.go
+++ b/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)
diff --git a/pkg/bindings/images/types.go b/pkg/bindings/images/types.go
index 7b28c499e..3ecfb9e09 100644
--- a/pkg/bindings/images/types.go
+++ b/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
diff --git a/pkg/bindings/images/types_pull_options.go b/pkg/bindings/images/types_pull_options.go
index 4cd525185..c1a88fd9e 100644
--- a/pkg/bindings/images/types_pull_options.go
+++ b/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
diff --git a/pkg/bindings/manifests/manifests.go b/pkg/bindings/manifests/manifests.go
index 49e4089f5..0163d21a0 100644
--- a/pkg/bindings/manifests/manifests.go
+++ b/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)
diff --git a/pkg/bindings/test/images_test.go b/pkg/bindings/test/images_test.go
index 9c9796661..53c5a1e83 100644
--- a/pkg/bindings/test/images_test.go
+++ b/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() {
diff --git a/pkg/bindings/test/manifests_test.go b/pkg/bindings/test/manifests_test.go
index 6a34ef5a6..d6749f920 100644
--- a/pkg/bindings/test/manifests_test.go
+++ b/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"))
 	})
 })