Do not print directly to stdout in Commit()

Buildah is being used as a library in Podman, and as such should
avoid printing directly to standard streams. Instead, return an
image ID which can be printed by the caller (if desired)

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #647
Approved by: rhatdan

cmd/buildah/commit.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"os"
 	"strings"
 	"time"
@@ -159,13 +160,16 @@ func commitCmd(c *cli.Context) error {
 	if !c.Bool("quiet") {
 		options.ReportWriter = os.Stderr
 	}
-	err = builder.Commit(ctx, dest, options)
+	id, err := builder.Commit(ctx, dest, options)
 	if err != nil {
 		return util.GetFailureCause(
 			err,
 			errors.Wrapf(err, "error committing container %q to %q", builder.Container, image),
 		)
 	}
+	if options.IIDFile == "" && id != "" {
+		fmt.Printf("%s\n", id)
+	}
 
 	if c.Bool("rm") {
 		return builder.Delete()

commit.go

@@ -78,16 +78,19 @@ type PushOptions struct {
 
 // Commit writes the contents of the container, along with its updated
 // configuration, to a new image in the specified location, and if we know how,
-// add any additional tags that were specified.
+// add any additional tags that were specified. Returns the ID of the new image
-func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options CommitOptions) error {
+// if commit was successful and the image destination was local
+func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options CommitOptions) (string, error) {
+	var imgID string
+
 	systemContext := getSystemContext(options.SystemContext, options.SignaturePolicyPath)
 	policy, err := signature.DefaultPolicy(systemContext)
 	if err != nil {
-		return errors.Wrapf(err, "error obtaining default signature policy")
+		return imgID, errors.Wrapf(err, "error obtaining default signature policy")
 	}
 	policyContext, err := signature.NewPolicyContext(policy)
 	if err != nil {
-		return errors.Wrapf(err, "error creating new signature policy context")
+		return imgID, errors.Wrapf(err, "error creating new signature policy context")
 	}
 	defer func() {
 		if err2 := policyContext.Destroy(); err2 != nil {
@@ -99,23 +102,23 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
 	exporting := !destIsStorage
 	src, err := b.makeImageRef(options.PreferredManifestType, exporting, options.Compression, options.HistoryTimestamp)
 	if err != nil {
-		return errors.Wrapf(err, "error computing layer digests and building metadata")
+		return imgID, errors.Wrapf(err, "error computing layer digests and building metadata")
 	}
 	// "Copy" our image to where it needs to be.
 	err = cp.Image(ctx, policyContext, dest, src, getCopyOptions(options.ReportWriter, nil, systemContext, ""))
 	if err != nil {
-		return errors.Wrapf(err, "error copying layers and metadata")
+		return imgID, errors.Wrapf(err, "error copying layers and metadata")
 	}
 	if len(options.AdditionalTags) > 0 {
 		switch dest.Transport().Name() {
 		case is.Transport.Name():
 			img, err := is.Transport.GetStoreImage(b.store, dest)
 			if err != nil {
-				return errors.Wrapf(err, "error locating just-written image %q", transports.ImageName(dest))
+				return imgID, errors.Wrapf(err, "error locating just-written image %q", transports.ImageName(dest))
 			}
 			err = util.AddImageNames(b.store, "", systemContext, img, options.AdditionalTags)
 			if err != nil {
-				return errors.Wrapf(err, "error setting image names to %v", append(img.Names, options.AdditionalTags...))
+				return imgID, errors.Wrapf(err, "error setting image names to %v", append(img.Names, options.AdditionalTags...))
 			}
 			logrus.Debugf("assigned names %v to image %q", img.Names, img.ID)
 		default:
@@ -124,16 +127,21 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
 	}
 
 	img, err := is.Transport.GetStoreImage(b.store, dest)
+	if err != nil && err != storage.ErrImageUnknown {
+		return imgID, err
+	}
+
 	if err == nil {
+		imgID = img.ID
+
 		if options.IIDFile != "" {
 			if err := ioutil.WriteFile(options.IIDFile, []byte(img.ID), 0644); err != nil {
-				return errors.Wrapf(err, "failed to write Image ID File %q", options.IIDFile)
+				return imgID, errors.Wrapf(err, "failed to write Image ID File %q", options.IIDFile)
 			}
-		} else {
-			fmt.Printf("%s\n", img.ID)
 		}
 	}
-	return nil
+
+	return imgID, nil
 }
 
 // Push copies the contents of the image to a new location.

imagebuildah/build.go

@@ -693,7 +693,14 @@ func (b *Executor) Commit(ctx context.Context, ib *imagebuilder.Builder) (err er
 		PreferredManifestType: b.outputFormat,
 		IIDFile:               b.iidfile,
 	}
-	return b.builder.Commit(ctx, imageRef, options)
+	imgID, err := b.builder.Commit(ctx, imageRef, options)
+	if err != nil {
+		return err
+	}
+	if options.IIDFile == "" && imgID != "" {
+		fmt.Printf("%s\n", imgID)
+	}
+	return nil
 }
 
 // Build takes care of the details of running Prepare/Execute/Commit/Delete