Merge pull request #9509 from mlegenovic/master

Correct compat images/create?fromImage response

12 files changed, 125 insertions, 45 deletions

diff --git a/libpod/image/image.go b/libpod/image/image.go
index 7c760a79a..265178ad5 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -143,7 +143,7 @@ func (ir *Runtime) NewFromLocal(name string) (*Image, error) {
 
 // New creates a new image object where the image could be local
 // or remote
-func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile string, writer io.Writer, dockeroptions *DockerRegistryOptions, signingoptions SigningOptions, label *string, pullType util.PullType) (*Image, error) {
+func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile string, writer io.Writer, dockeroptions *DockerRegistryOptions, signingoptions SigningOptions, label *string, pullType util.PullType, progress chan types.ProgressProperties) (*Image, error) {
 	span, _ := opentracing.StartSpanFromContext(ctx, "newImage")
 	span.SetTag("type", "runtime")
 	defer span.Finish()
@@ -162,7 +162,7 @@ func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile
 	if signaturePolicyPath == "" {
 		signaturePolicyPath = ir.SignaturePolicyPath
 	}
-	imageName, err := ir.pullImageFromHeuristicSource(ctx, name, writer, authfile, signaturePolicyPath, signingoptions, dockeroptions, &retry.RetryOptions{MaxRetry: maxRetry}, label)
+	imageName, err := ir.pullImageFromHeuristicSource(ctx, name, writer, authfile, signaturePolicyPath, signingoptions, dockeroptions, &retry.RetryOptions{MaxRetry: maxRetry}, label, progress)
 	if err != nil {
 		return nil, err
 	}
@@ -323,7 +323,7 @@ func (ir *Runtime) LoadAllImagesFromDockerArchive(ctx context.Context, fileName
 	}
 
 	defer goal.cleanUp()
-	imageNames, err := ir.doPullImage(ctx, sc, goal, writer, SigningOptions{}, &DockerRegistryOptions{}, &retry.RetryOptions{}, nil)
+	imageNames, err := ir.doPullImage(ctx, sc, goal, writer, SigningOptions{}, &DockerRegistryOptions{}, &retry.RetryOptions{}, nil, nil)
 	if err != nil {
 		return nil, err
 	}
diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go
index 1ea4f6c11..3e6e7b9db 100644
--- a/libpod/image/image_test.go
+++ b/libpod/image/image_test.go
@@ -94,9 +94,9 @@ func TestImage_NewFromLocal(t *testing.T) {
 	ir, err := NewImageRuntimeFromOptions(so)
 	assert.NoError(t, err)
 	ir.Eventer = events.NewNullEventer()
-	bb, err := ir.New(context.Background(), "docker.io/library/busybox:latest", "", "", writer, nil, SigningOptions{}, nil, util.PullImageMissing)
+	bb, err := ir.New(context.Background(), "docker.io/library/busybox:latest", "", "", writer, nil, SigningOptions{}, nil, util.PullImageMissing, nil)
 	assert.NoError(t, err)
-	bbglibc, err := ir.New(context.Background(), "docker.io/library/busybox:glibc", "", "", writer, nil, SigningOptions{}, nil, util.PullImageMissing)
+	bbglibc, err := ir.New(context.Background(), "docker.io/library/busybox:glibc", "", "", writer, nil, SigningOptions{}, nil, util.PullImageMissing, nil)
 	assert.NoError(t, err)
 
 	tm := makeLocalMatrix(bb, bbglibc)
@@ -140,7 +140,7 @@ func TestImage_New(t *testing.T) {
 	// Iterate over the names and delete the image
 	// after the pull
 	for _, img := range names {
-		newImage, err := ir.New(context.Background(), img, "", "", writer, nil, SigningOptions{}, nil, util.PullImageMissing)
+		newImage, err := ir.New(context.Background(), img, "", "", writer, nil, SigningOptions{}, nil, util.PullImageMissing, nil)
 		assert.NoError(t, err)
 		assert.NotEqual(t, newImage.ID(), "")
 		err = newImage.Remove(context.Background(), false)
@@ -169,7 +169,7 @@ func TestImage_MatchRepoTag(t *testing.T) {
 	ir, err := NewImageRuntimeFromOptions(so)
 	assert.NoError(t, err)
 	ir.Eventer = events.NewNullEventer()
-	newImage, err := ir.New(context.Background(), "busybox", "", "", os.Stdout, nil, SigningOptions{}, nil, util.PullImageMissing)
+	newImage, err := ir.New(context.Background(), "busybox", "", "", os.Stdout, nil, SigningOptions{}, nil, util.PullImageMissing, nil)
 	assert.NoError(t, err)
 	err = newImage.TagImage("foo:latest")
 	assert.NoError(t, err)
diff --git a/libpod/image/pull.go b/libpod/image/pull.go
index 3cb1e57c7..c5fafc25d 100644
--- a/libpod/image/pull.go
+++ b/libpod/image/pull.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/containers/common/pkg/retry"
 	cp "github.com/containers/image/v5/copy"
@@ -241,7 +242,7 @@ func toLocalImageName(imageName string) string {
 
 // pullImageFromHeuristicSource pulls an image based on inputName, which is heuristically parsed and may involve configured registries.
 // Use pullImageFromReference if the source is known precisely.
-func (ir *Runtime) pullImageFromHeuristicSource(ctx context.Context, inputName string, writer io.Writer, authfile, signaturePolicyPath string, signingOptions SigningOptions, dockerOptions *DockerRegistryOptions, retryOptions *retry.RetryOptions, label *string) ([]string, error) {
+func (ir *Runtime) pullImageFromHeuristicSource(ctx context.Context, inputName string, writer io.Writer, authfile, signaturePolicyPath string, signingOptions SigningOptions, dockerOptions *DockerRegistryOptions, retryOptions *retry.RetryOptions, label *string, progress chan types.ProgressProperties) ([]string, error) {
 	span, _ := opentracing.StartSpanFromContext(ctx, "pullImageFromHeuristicSource")
 	defer span.Finish()
 
@@ -275,7 +276,7 @@ func (ir *Runtime) pullImageFromHeuristicSource(ctx context.Context, inputName s
 		}
 	}
 	defer goal.cleanUp()
-	return ir.doPullImage(ctx, sc, *goal, writer, signingOptions, dockerOptions, retryOptions, label)
+	return ir.doPullImage(ctx, sc, *goal, writer, signingOptions, dockerOptions, retryOptions, label, progress)
 }
 
 // pullImageFromReference pulls an image from a types.imageReference.
@@ -294,7 +295,7 @@ func (ir *Runtime) pullImageFromReference(ctx context.Context, srcRef types.Imag
 		return nil, errors.Wrapf(err, "error determining pull goal for image %q", transports.ImageName(srcRef))
 	}
 	defer goal.cleanUp()
-	return ir.doPullImage(ctx, sc, *goal, writer, signingOptions, dockerOptions, retryOptions, nil)
+	return ir.doPullImage(ctx, sc, *goal, writer, signingOptions, dockerOptions, retryOptions, nil, nil)
 }
 
 func cleanErrorMessage(err error) string {
@@ -304,7 +305,7 @@ func cleanErrorMessage(err error) string {
 }
 
 // doPullImage is an internal helper interpreting pullGoal. Almost everyone should call one of the callers of doPullImage instead.
-func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goal pullGoal, writer io.Writer, signingOptions SigningOptions, dockerOptions *DockerRegistryOptions, retryOptions *retry.RetryOptions, label *string) ([]string, error) {
+func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goal pullGoal, writer io.Writer, signingOptions SigningOptions, dockerOptions *DockerRegistryOptions, retryOptions *retry.RetryOptions, label *string, progress chan types.ProgressProperties) ([]string, error) {
 	span, _ := opentracing.StartSpanFromContext(ctx, "doPullImage")
 	defer span.Finish()
 
@@ -328,6 +329,10 @@ func (ir *Runtime) doPullImage(ctx context.Context, sc *types.SystemContext, goa
 	for _, imageInfo := range goal.refPairs {
 		copyOptions := getCopyOptions(sc, writer, dockerOptions, nil, signingOptions, "", nil)
 		copyOptions.SourceCtx.SystemRegistriesConfPath = systemRegistriesConfPath // FIXME: Set this more globally.  Probably no reason not to have it in every types.SystemContext, and to compute the value just once in one place.
+		if progress != nil {
+			copyOptions.Progress = progress
+			copyOptions.ProgressInterval = time.Second
+		}
 		// Print the following statement only when pulling from a docker or atomic registry
 		if writer != nil && (imageInfo.srcRef.Transport().Name() == DockerTransport || imageInfo.srcRef.Transport().Name() == AtomicTransport) {
 			if _, err := io.WriteString(writer, fmt.Sprintf("Trying to pull %s...\n", imageInfo.image)); err != nil {
diff --git a/libpod/rootless_cni_linux.go b/libpod/rootless_cni_linux.go
index e97985180..df690e914 100644
--- a/libpod/rootless_cni_linux.go
+++ b/libpod/rootless_cni_linux.go
@@ -265,7 +265,7 @@ func startRootlessCNIInfraContainer(ctx context.Context, r *Runtime) (*Container
 	}
 	logrus.Debugf("rootless CNI: ensuring image %q to exist", imageName)
 	newImage, err := r.ImageRuntime().New(ctx, imageName, "", "", nil, nil,
-		image.SigningOptions{}, nil, util.PullImageMissing)
+		image.SigningOptions{}, nil, util.PullImageMissing, nil)
 	if err != nil {
 		return nil, err
 	}
diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go
index 000029fa4..0a09e40ea 100644
--- a/libpod/runtime_pod_infra_linux.go
+++ b/libpod/runtime_pod_infra_linux.go
@@ -216,7 +216,7 @@ func (r *Runtime) createInfraContainer(ctx context.Context, p *Pod) (*Container,
 	if img == "" {
 		img = r.config.Engine.InfraImage
 	}
-	newImage, err := r.ImageRuntime().New(ctx, img, "", "", nil, nil, image.SigningOptions{}, nil, util.PullImageMissing)
+	newImage, err := r.ImageRuntime().New(ctx, img, "", "", nil, nil, image.SigningOptions{}, nil, util.PullImageMissing, nil)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go
index 1a4dd939e..e5caa9ea5 100644
--- a/pkg/api/handlers/compat/images.go
+++ b/pkg/api/handlers/compat/images.go
@@ -1,6 +1,7 @@
 package compat
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -11,11 +12,13 @@ import (
 
 	"github.com/containers/buildah"
 	"github.com/containers/image/v5/manifest"
+	"github.com/containers/image/v5/types"
 	"github.com/containers/podman/v3/libpod"
 	image2 "github.com/containers/podman/v3/libpod/image"
 	"github.com/containers/podman/v3/pkg/api/handlers"
 	"github.com/containers/podman/v3/pkg/api/handlers/utils"
 	"github.com/containers/podman/v3/pkg/auth"
+	"github.com/containers/podman/v3/pkg/channel"
 	"github.com/containers/podman/v3/pkg/domain/entities"
 	"github.com/containers/podman/v3/pkg/util"
 	"github.com/gorilla/schema"
@@ -236,33 +239,103 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) {
 	if sys := runtime.SystemContext(); sys != nil {
 		registryOpts.DockerCertPath = sys.DockerCertPath
 	}
-	img, err := runtime.ImageRuntime().New(r.Context(),
-		fromImage,
-		"", // signature policy
-		authfile,
-		nil, // writer
-		&registryOpts,
-		image2.SigningOptions{},
-		nil, // label
-		util.PullImageAlways,
-	)
-	if err != nil {
-		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
-		return
+
+	stderr := channel.NewWriter(make(chan []byte))
+	defer stderr.Close()
+
+	progress := make(chan types.ProgressProperties)
+
+	var img string
+	runCtx, cancel := context.WithCancel(context.Background())
+	go func() {
+		defer cancel()
+
+		newImage, err := runtime.ImageRuntime().New(
+			runCtx,
+			fromImage,
+			"", // signature policy
+			authfile,
+			nil, // writer
+			&registryOpts,
+			image2.SigningOptions{},
+			nil, // label
+			util.PullImageAlways,
+			progress)
+		if err != nil {
+			stderr.Write([]byte(err.Error() + "\n"))
+		} else {
+			img = newImage.ID()
+		}
+	}()
+
+	flush := func() {
+		if flusher, ok := w.(http.Flusher); ok {
+			flusher.Flush()
+		}
 	}
 
-	// Success
-	utils.WriteResponse(w, http.StatusOK, struct {
-		Status         string            `json:"status"`
-		Error          string            `json:"error,omitempty"`
-		Progress       string            `json:"progress"`
-		ProgressDetail map[string]string `json:"progressDetail"`
-		Id             string            `json:"id"` // nolint
-	}{
-		Status:         fmt.Sprintf("pulling image (%s) from %s (Download complete)", img.Tag, strings.Join(img.Names(), ", ")),
-		ProgressDetail: map[string]string{},
-		Id:             img.ID(),
-	})
+	w.WriteHeader(http.StatusOK)
+	w.Header().Add("Content-Type", "application/json")
+	flush()
+
+	enc := json.NewEncoder(w)
+	enc.SetEscapeHTML(true)
+	var failed bool
+
+loop: // break out of for/select infinite loop
+	for {
+		var report struct {
+			Stream   string `json:"stream,omitempty"`
+			Status   string `json:"status,omitempty"`
+			Progress struct {
+				Current uint64 `json:"current,omitempty"`
+				Total   int64  `json:"total,omitempty"`
+			} `json:"progressDetail,omitempty"`
+			Error string `json:"error,omitempty"`
+			Id    string `json:"id,omitempty"` // nolint
+		}
+
+		select {
+		case e := <-progress:
+			switch e.Event {
+			case types.ProgressEventNewArtifact:
+				report.Status = "Pulling fs layer"
+			case types.ProgressEventRead:
+				report.Status = "Downloading"
+				report.Progress.Current = e.Offset
+				report.Progress.Total = e.Artifact.Size
+			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 {
+				stderr.Write([]byte(err.Error()))
+			}
+			flush()
+		case e := <-stderr.Chan():
+			failed = true
+			report.Error = string(e)
+			if err := enc.Encode(report); err != nil {
+				logrus.Warnf("Failed to json encode error %q", err.Error())
+			}
+			flush()
+		case <-runCtx.Done():
+			if !failed {
+				report.Status = "Pull complete"
+				report.Id = img[0:12]
+				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
+		case <-r.Context().Done():
+			// Client has closed connection
+			break loop // break out of for/select infinite loop
+		}
+	}
 }
 
 func GetImage(w http.ResponseWriter, r *http.Request) {
diff --git a/pkg/api/handlers/libpod/images_pull.go b/pkg/api/handlers/libpod/images_pull.go
index c8b777be4..e2e4b53b4 100644
--- a/pkg/api/handlers/libpod/images_pull.go
+++ b/pkg/api/handlers/libpod/images_pull.go
@@ -136,7 +136,8 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) {
 				&dockerRegistryOptions,
 				image.SigningOptions{},
 				nil,
-				util.PullImageAlways)
+				util.PullImageAlways,
+				nil)
 			if err != nil {
 				stderr.Write([]byte(err.Error() + "\n"))
 			} else {
diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 0cf51e5a6..53095c295 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -304,6 +304,7 @@ func updateImage(runtime *libpod.Runtime, name string, options Options) (*image.
 		image.SigningOptions{},
 		nil,
 		util.PullImageAlways,
+		nil,
 	)
 	if err != nil {
 		return nil, err
diff --git a/pkg/checkpoint/checkpoint_restore.go b/pkg/checkpoint/checkpoint_restore.go
index a608762b5..6285680c0 100644
--- a/pkg/checkpoint/checkpoint_restore.go
+++ b/pkg/checkpoint/checkpoint_restore.go
@@ -121,7 +121,7 @@ func CRImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, restoreOpt
 		return nil, err
 	}
 
-	_, err = runtime.ImageRuntime().New(ctx, config.RootfsImageName, rtc.Engine.SignaturePolicyPath, "", writer, nil, image.SigningOptions{}, nil, util.PullImageMissing)
+	_, err = runtime.ImageRuntime().New(ctx, config.RootfsImageName, rtc.Engine.SignaturePolicyPath, "", writer, nil, image.SigningOptions{}, nil, util.PullImageMissing, nil)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go
index 46d967789..562653403 100644
--- a/pkg/domain/infra/abi/images.go
+++ b/pkg/domain/infra/abi/images.go
@@ -247,7 +247,7 @@ func pull(ctx context.Context, runtime *image.Runtime, rawImage string, options
 	}
 
 	if !options.AllTags {
-		newImage, err := runtime.New(ctx, rawImage, options.SignaturePolicy, options.Authfile, writer, &dockerRegistryOptions, image.SigningOptions{}, label, options.PullPolicy)
+		newImage, err := runtime.New(ctx, rawImage, options.SignaturePolicy, options.Authfile, writer, &dockerRegistryOptions, image.SigningOptions{}, label, options.PullPolicy, nil)
 		if err != nil {
 			return nil, err
 		}
@@ -280,7 +280,7 @@ func pull(ctx context.Context, runtime *image.Runtime, rawImage string, options
 	foundIDs := []string{}
 	for _, tag := range tags {
 		name := rawImage + ":" + tag
-		newImage, err := runtime.New(ctx, name, options.SignaturePolicy, options.Authfile, writer, &dockerRegistryOptions, image.SigningOptions{}, nil, util.PullImageAlways)
+		newImage, err := runtime.New(ctx, name, options.SignaturePolicy, options.Authfile, writer, &dockerRegistryOptions, image.SigningOptions{}, nil, util.PullImageAlways, nil)
 		if err != nil {
 			logrus.Errorf("error pulling image %q", name)
 			continue
diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go
index c5e20a607..b7ca69281 100644
--- a/pkg/domain/infra/abi/play.go
+++ b/pkg/domain/infra/abi/play.go
@@ -221,7 +221,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 		}
 
 		// This ensures the image is the image store
-		newImage, err := ic.Libpod.ImageRuntime().New(ctx, container.Image, options.SignaturePolicy, options.Authfile, writer, &dockerRegistryOptions, image.SigningOptions{}, nil, pullPolicy)
+		newImage, err := ic.Libpod.ImageRuntime().New(ctx, container.Image, options.SignaturePolicy, options.Authfile, writer, &dockerRegistryOptions, image.SigningOptions{}, nil, pullPolicy, nil)
 		if err != nil {
 			return nil, err
 		}
diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at
index a650cf958..f866422e2 100644
--- a/test/apiv2/10-images.at
+++ b/test/apiv2/10-images.at
@@ -41,7 +41,7 @@ t GET images/$iid/json 200 \
   .Id=sha256:$iid \
   .RepoTags[0]=$IMAGE
 
-t POST "images/create?fromImage=alpine" '' 200 .error=null .status~".*Download complete.*"
+t POST "images/create?fromImage=alpine" '' 200 .error~null .status~".*Download complete.*"
 
 t POST "images/create?fromImage=alpine&tag=latest" '' 200
 
@@ -49,7 +49,7 @@ t POST "images/create?fromImage=alpine&tag=latest" '' 200
 old_iid=$(podman image inspect --format "{{.ID}}" docker.io/library/alpine:latest)
 podman rmi -f docker.io/library/alpine:latest
 podman tag $IMAGE docker.io/library/alpine:latest
-t POST "images/create?fromImage=alpine" '' 200 .error=null .status~".*$old_iid.*"
+t POST "images/create?fromImage=alpine" '' 200 .error~null .status~".*$old_iid.*"
 podman untag $IMAGE docker.io/library/alpine:latest
 
 t POST "images/create?fromImage=quay.io/libpod/alpine&tag=sha256:fa93b01658e3a5a1686dc3ae55f170d8de487006fb53a28efcd12ab0710a2e5f" '' 200