Merge pull request #3081 from baude/remotecommit

podman remote-client commit

10 files changed, 151 insertions, 70 deletions

diff --git a/API.md b/API.md
index 976899f3d..b6fdea695 100755
--- a/API.md
+++ b/API.md
@@ -11,7 +11,7 @@ in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in
 
 [func BuildImageHierarchyMap(name: string) string](#BuildImageHierarchyMap)
 
-[func Commit(name: string, image_name: string, changes: []string, author: string, message: string, pause: bool, manifestType: string) string](#Commit)
+[func Commit(name: string, image_name: string, changes: []string, author: string, message: string, pause: bool, manifestType: string) MoreResponse](#Commit)
 
 [func ContainerArtifacts(name: string, artifactName: string) string](#ContainerArtifacts)
 
@@ -308,14 +308,14 @@ BuildImageHierarchyMap is for the development of Podman and should not be used.
 ### <a name="Commit"></a>func Commit
 <div style="background-color: #E8E8E8; padding: 15px; margin: 10px; border-radius: 10px;">
 
-method Commit(name: [string](https://godoc.org/builtin#string), image_name: [string](https://godoc.org/builtin#string), changes: [[]string](#[]string), author: [string](https://godoc.org/builtin#string), message: [string](https://godoc.org/builtin#string), pause: [bool](https://godoc.org/builtin#bool), manifestType: [string](https://godoc.org/builtin#string)) [string](https://godoc.org/builtin#string)</div>
+method Commit(name: [string](https://godoc.org/builtin#string), image_name: [string](https://godoc.org/builtin#string), changes: [[]string](#[]string), author: [string](https://godoc.org/builtin#string), message: [string](https://godoc.org/builtin#string), pause: [bool](https://godoc.org/builtin#bool), manifestType: [string](https://godoc.org/builtin#string)) [MoreResponse](#MoreResponse)</div>
 Commit, creates an image from an existing container. It requires the name or
 ID of the container as well as the resulting image name.  Optionally, you can define an author and message
 to be added to the resulting image.  You can also define changes to the resulting image for the following
 attributes: _CMD, ENTRYPOINT, ENV, EXPOSE, LABEL, ONBUILD, STOPSIGNAL, USER, VOLUME, and WORKDIR_.  To pause the
 container while it is being committed, pass a _true_ bool for the pause argument.  If the container cannot
 be found by the ID or name provided, a (ContainerNotFound)[#ContainerNotFound] error will be returned; otherwise,
-the resulting image's ID will be returned as a string.
+the resulting image's ID will be returned as a string inside a MoreResponse.
 ### <a name="ContainerArtifacts"></a>func ContainerArtifacts
 <div style="background-color: #E8E8E8; padding: 15px; margin: 10px; border-radius: 10px;">
 
diff --git a/cmd/podman/commands.go b/cmd/podman/commands.go
index 2ac465b9d..18b0b7857 100644
--- a/cmd/podman/commands.go
+++ b/cmd/podman/commands.go
@@ -11,7 +11,6 @@ const remoteclient = false
 // Commands that the local client implements
 func getMainCommands() []*cobra.Command {
 	rootCommands := []*cobra.Command{
-		_commitCommand,
 		_execCommand,
 		_playCommand,
 		_loginCommand,
@@ -41,7 +40,6 @@ func getContainerSubCommands() []*cobra.Command {
 
 	return []*cobra.Command{
 		_cleanupCommand,
-		_commitCommand,
 		_execCommand,
 		_mountCommand,
 		_refreshCommand,
diff --git a/cmd/podman/commit.go b/cmd/podman/commit.go
index 2b38bab35..01e2ec701 100644
--- a/cmd/podman/commit.go
+++ b/cmd/podman/commit.go
@@ -2,16 +2,11 @@ package main
 
 import (
 	"fmt"
-	"io"
-	"os"
 	"strings"
 
-	"github.com/containers/buildah"
-	"github.com/containers/image/manifest"
 	"github.com/containers/libpod/cmd/podman/cliconfig"
-	"github.com/containers/libpod/cmd/podman/libpodruntime"
 	"github.com/containers/libpod/libpod"
-	"github.com/containers/libpod/libpod/image"
+	"github.com/containers/libpod/pkg/adapter"
 	"github.com/containers/libpod/pkg/util"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -52,32 +47,17 @@ func init() {
 }
 
 func commitCmd(c *cliconfig.CommitValues) error {
-	runtime, err := libpodruntime.GetRuntime(getContext(), &c.PodmanCommand)
+	runtime, err := adapter.GetRuntime(getContext(), &c.PodmanCommand)
 	if err != nil {
 		return errors.Wrapf(err, "could not get runtime")
 	}
 	defer runtime.Shutdown(false)
 
-	var (
-		writer   io.Writer
-		mimeType string
-	)
 	args := c.InputArgs
 	if len(args) != 2 {
 		return errors.Errorf("you must provide a container name or ID and a target image name")
 	}
 
-	switch c.Format {
-	case "oci":
-		mimeType = buildah.OCIv1ImageManifest
-		if c.Flag("message").Changed {
-			return errors.Errorf("messages are only compatible with the docker image format (-f docker)")
-		}
-	case "docker":
-		mimeType = manifest.DockerV2Schema2MediaType
-	default:
-		return errors.Errorf("unrecognized image format %q", c.Format)
-	}
 	container := args[0]
 	reference := args[1]
 	if c.Flag("change").Changed {
@@ -92,38 +72,10 @@ func commitCmd(c *cliconfig.CommitValues) error {
 		}
 	}
 
-	if !c.Quiet {
-		writer = os.Stderr
-	}
-	ctr, err := runtime.LookupContainer(container)
-	if err != nil {
-		return errors.Wrapf(err, "error looking up container %q", container)
-	}
-
-	rtc, err := runtime.GetConfig()
-	if err != nil {
-		return err
-	}
-
-	sc := image.GetSystemContext(rtc.SignaturePolicyPath, "", false)
-	coptions := buildah.CommitOptions{
-		SignaturePolicyPath:   rtc.SignaturePolicyPath,
-		ReportWriter:          writer,
-		SystemContext:         sc,
-		PreferredManifestType: mimeType,
-	}
-	options := libpod.ContainerCommitOptions{
-		CommitOptions:  coptions,
-		Pause:          c.Pause,
-		IncludeVolumes: c.IncludeVolumes,
-		Message:        c.Message,
-		Changes:        c.Change,
-		Author:         c.Author,
-	}
-	newImage, err := ctr.Commit(getContext(), reference, options)
+	iid, err := runtime.Commit(getContext(), c, container, reference)
 	if err != nil {
 		return err
 	}
-	fmt.Println(newImage.ID())
+	fmt.Println(iid)
 	return nil
 }
diff --git a/cmd/podman/container.go b/cmd/podman/container.go
index 839ae3a0e..cb54317c0 100644
--- a/cmd/podman/container.go
+++ b/cmd/podman/container.go
@@ -52,6 +52,7 @@ var (
 	containerCommands = []*cobra.Command{
 		_attachCommand,
 		_checkpointCommand,
+		_commitCommand,
 		_containerExistsCommand,
 		_contInspectSubCommand,
 		_cpCommand,
diff --git a/cmd/podman/main.go b/cmd/podman/main.go
index 787dd55c0..a149a47f9 100644
--- a/cmd/podman/main.go
+++ b/cmd/podman/main.go
@@ -30,6 +30,7 @@ var (
 var mainCommands = []*cobra.Command{
 	_attachCommand,
 	_buildCommand,
+	_commitCommand,
 	_diffCommand,
 	_createCommand,
 	_eventsCommand,
diff --git a/cmd/podman/varlink/io.podman.varlink b/cmd/podman/varlink/io.podman.varlink
index ed7b49c68..5b3d5ae4c 100644
--- a/cmd/podman/varlink/io.podman.varlink
+++ b/cmd/podman/varlink/io.podman.varlink
@@ -802,8 +802,8 @@ method DeleteUnusedImages() -> (images: []string)
 # attributes: _CMD, ENTRYPOINT, ENV, EXPOSE, LABEL, ONBUILD, STOPSIGNAL, USER, VOLUME, and WORKDIR_.  To pause the
 # container while it is being committed, pass a _true_ bool for the pause argument.  If the container cannot
 # be found by the ID or name provided, a (ContainerNotFound)[#ContainerNotFound] error will be returned; otherwise,
-# the resulting image's ID will be returned as a string.
-method Commit(name: string, image_name: string, changes: []string, author: string, message: string, pause: bool, manifestType: string) -> (image: string)
+# the resulting image's ID will be returned as a string inside a MoreResponse.
+method Commit(name: string, image_name: string, changes: []string, author: string, message: string, pause: bool, manifestType: string) -> (reply: MoreResponse)
 
 # ImportImage imports an image from a source (like tarball) into local storage.  The image can have additional
 # descriptions added to it using the message and changes options. See also [ExportImage](ExportImage).
diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index ff7b6377a..34ee70d3d 100644
--- a/pkg/adapter/containers.go
+++ b/pkg/adapter/containers.go
@@ -6,6 +6,7 @@ import (
 	"bufio"
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -15,9 +16,12 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/containers/buildah"
+	"github.com/containers/image/manifest"
 	"github.com/containers/libpod/cmd/podman/cliconfig"
 	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/pkg/adapter/shortcuts"
 	"github.com/containers/libpod/pkg/systemdgen"
 	"github.com/containers/psgo"
@@ -1030,3 +1034,55 @@ func (r *LocalRuntime) GenerateSystemd(c *cliconfig.GenerateSystemdValues) (stri
 func (r *LocalRuntime) GetNamespaces(container shared.PsContainerOutput) *shared.Namespace {
 	return shared.GetNamespaces(container.Pid)
 }
+
+// Commit creates a local image from a container
+func (r *LocalRuntime) Commit(ctx context.Context, c *cliconfig.CommitValues, container, imageName string) (string, error) {
+	var (
+		writer   io.Writer
+		mimeType string
+	)
+	switch c.Format {
+	case "oci":
+		mimeType = buildah.OCIv1ImageManifest
+		if c.Flag("message").Changed {
+			return "", errors.Errorf("messages are only compatible with the docker image format (-f docker)")
+		}
+	case "docker":
+		mimeType = manifest.DockerV2Schema2MediaType
+	default:
+		return "", errors.Errorf("unrecognized image format %q", c.Format)
+	}
+	if !c.Quiet {
+		writer = os.Stderr
+	}
+	ctr, err := r.Runtime.LookupContainer(container)
+	if err != nil {
+		return "", errors.Wrapf(err, "error looking up container %q", container)
+	}
+
+	rtc, err := r.Runtime.GetConfig()
+	if err != nil {
+		return "", err
+	}
+
+	sc := image.GetSystemContext(rtc.SignaturePolicyPath, "", false)
+	coptions := buildah.CommitOptions{
+		SignaturePolicyPath:   rtc.SignaturePolicyPath,
+		ReportWriter:          writer,
+		SystemContext:         sc,
+		PreferredManifestType: mimeType,
+	}
+	options := libpod.ContainerCommitOptions{
+		CommitOptions:  coptions,
+		Pause:          c.Pause,
+		IncludeVolumes: c.IncludeVolumes,
+		Message:        c.Message,
+		Changes:        c.Change,
+		Author:         c.Author,
+	}
+	newImage, err := ctr.Commit(ctx, imageName, options)
+	if err != nil {
+		return "", err
+	}
+	return newImage.ID(), nil
+}
diff --git a/pkg/adapter/containers_remote.go b/pkg/adapter/containers_remote.go
index 0c0bce813..bc6a9cfcd 100644
--- a/pkg/adapter/containers_remote.go
+++ b/pkg/adapter/containers_remote.go
@@ -994,3 +994,26 @@ func (r *LocalRuntime) GetNamespaces(container shared.PsContainerOutput) *shared
 	}
 	return &ns
 }
+
+// Commit creates a local image from a container
+func (r *LocalRuntime) Commit(ctx context.Context, c *cliconfig.CommitValues, container, imageName string) (string, error) {
+	var iid string
+	reply, err := iopodman.Commit().Send(r.Conn, varlink.More, container, imageName, c.Change, c.Author, c.Message, c.Pause, c.Format)
+	if err != nil {
+		return "", err
+	}
+	for {
+		responses, flags, err := reply()
+		if err != nil {
+			return "", err
+		}
+		for _, line := range responses.Logs {
+			fmt.Fprintln(os.Stderr, line)
+		}
+		iid = responses.Id
+		if flags&varlink.Continues == 0 {
+			break
+		}
+	}
+	return iid, nil
+}
diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go
index fa1a0a109..1abc4f086 100644
--- a/pkg/varlinkapi/images.go
+++ b/pkg/varlinkapi/images.go
@@ -371,7 +371,6 @@ func (i *LibpodAPI) PushImage(call iopodman.VarlinkCall, name, tag string, compr
 				done = true
 			default:
 				if !call.WantsMore() {
-					time.Sleep(1 * time.Second)
 					break
 				}
 				br := iopodman.MoreResponse{
@@ -495,6 +494,9 @@ func (i *LibpodAPI) DeleteUnusedImages(call iopodman.VarlinkCall) error {
 
 // Commit ...
 func (i *LibpodAPI) Commit(call iopodman.VarlinkCall, name, imageName string, changes []string, author, message string, pause bool, manifestType string) error {
+	var newImage *image.Image
+
+	output := bytes.NewBuffer([]byte{})
 	ctr, err := i.Runtime.LookupContainer(name)
 	if err != nil {
 		return call.ReplyContainerNotFound(name, err.Error())
@@ -515,7 +517,7 @@ func (i *LibpodAPI) Commit(call iopodman.VarlinkCall, name, imageName string, ch
 	}
 	coptions := buildah.CommitOptions{
 		SignaturePolicyPath:   rtc.SignaturePolicyPath,
-		ReportWriter:          nil,
+		ReportWriter:          output,
 		SystemContext:         sc,
 		PreferredManifestType: mimeType,
 	}
@@ -527,11 +529,61 @@ func (i *LibpodAPI) Commit(call iopodman.VarlinkCall, name, imageName string, ch
 		Author:        author,
 	}
 
-	newImage, err := ctr.Commit(getContext(), imageName, options)
-	if err != nil {
-		return call.ReplyErrorOccurred(err.Error())
+	if call.WantsMore() {
+		call.Continues = true
 	}
-	return call.ReplyCommit(newImage.ID())
+
+	c := make(chan error)
+
+	go func() {
+		newImage, err = ctr.Commit(getContext(), imageName, options)
+		if err != nil {
+			c <- err
+		}
+		c <- nil
+		close(c)
+	}()
+
+	var log []string
+	done := false
+	for {
+		line, err := output.ReadString('\n')
+		if err == nil {
+			log = append(log, line)
+			continue
+		} else if err == io.EOF {
+			select {
+			case err := <-c:
+				if err != nil {
+					logrus.Errorf("reading of output during commit failed for %s", name)
+					return call.ReplyErrorOccurred(err.Error())
+				}
+				done = true
+			default:
+				if !call.WantsMore() {
+					break
+				}
+				br := iopodman.MoreResponse{
+					Logs: log,
+				}
+				call.ReplyCommit(br)
+				log = []string{}
+			}
+		} else {
+			return call.ReplyErrorOccurred(err.Error())
+		}
+		if done {
+			break
+		}
+	}
+	call.Continues = false
+
+	br := iopodman.MoreResponse{
+		Logs: log,
+		Id:   newImage.ID(),
+	}
+
+	return call.ReplyCommit(br)
 }
 
 // ImportImage imports an image from a tarball to the image store
@@ -633,7 +685,6 @@ func (i *LibpodAPI) PullImage(call iopodman.VarlinkCall, name string) error {
 				done = true
 			default:
 				if !call.WantsMore() {
-					time.Sleep(1 * time.Second)
 					break
 				}
 				br := iopodman.MoreResponse{
@@ -764,7 +815,6 @@ func (i *LibpodAPI) ImageSave(call iopodman.VarlinkCall, options iopodman.ImageS
 				done = true
 			default:
 				if !call.WantsMore() {
-					time.Sleep(1 * time.Second)
 					break
 				}
 				br := iopodman.MoreResponse{
@@ -844,7 +894,6 @@ func (i *LibpodAPI) LoadImage(call iopodman.VarlinkCall, name, inputFile string,
 				done = true
 			default:
 				if !call.WantsMore() {
-					time.Sleep(1 * time.Second)
 					break
 				}
 				br := iopodman.MoreResponse{
diff --git a/test/e2e/commit_test.go b/test/e2e/commit_test.go
index 50577fdfb..e9d274649 100644
--- a/test/e2e/commit_test.go
+++ b/test/e2e/commit_test.go
@@ -1,5 +1,3 @@
-// +build !remoteclient
-
 package integration
 
 import (
@@ -174,6 +172,9 @@ var _ = Describe("Podman commit", func() {
 	})
 
 	It("podman commit with volume mounts and --include-volumes", func() {
+		// We need to figure out how volumes are going to work correctly with the remote
+		// client.  This does not currently work.
+		SkipIfRemote()
 		s := podmanTest.Podman([]string{"run", "--name", "test1", "-v", "/tmp:/foo", "alpine", "date"})
 		s.WaitWithDefaultTimeout()
 		Expect(s.ExitCode()).To(Equal(0))