Store user Volumes, Entrypoint, Command in database

We need these for commit, and they cannot be properly deduced
from just the OCI spec, so save them in the database so we can
retrieve them for commit.

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

Closes: #700
Approved by: rhatdan

7 files changed, 119 insertions, 42 deletions

diff --git a/cmd/podman/commit.go b/cmd/podman/commit.go
index 105ab2d99..14b7ddace 100644
--- a/cmd/podman/commit.go
+++ b/cmd/podman/commit.go
@@ -1,7 +1,6 @@
 package main
 
 import (
-	"encoding/json"
 	"fmt"
 	"io"
 	"os"
@@ -121,17 +120,7 @@ func commitCmd(c *cli.Context) error {
 		Changes:       c.StringSlice("change"),
 		Author:        c.String("author"),
 	}
-	var createArtifact createConfig
-	artifact, err := ctr.GetArtifact("create-config")
-	if err == nil {
-		if err := json.Unmarshal(artifact, &createArtifact); err != nil {
-			return err
-		}
-	}
-	mounts := getMounts(createArtifact.Volumes, true)
-	command := createArtifact.Command
-	entryPoint := createArtifact.Entrypoint
-	newImage, err := ctr.Commit(getContext(), reference, options, strings.Split(mounts, ","), command, entryPoint)
+	newImage, err := ctr.Commit(getContext(), reference, options)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/podman/spec.go b/cmd/podman/spec.go
index fbe611cc3..e807ab642 100644
--- a/cmd/podman/spec.go
+++ b/cmd/podman/spec.go
@@ -653,9 +653,18 @@ func (c *createConfig) GetContainerCreateOptions() ([]libpod.CtrCreateOption, er
 	}
 
 	if len(c.Volumes) != 0 {
-		options = append(options, libpod.WithUserVolumes())
+		options = append(options, libpod.WithUserVolumes(c.Volumes))
 	}
 
+	if len(c.Command) != 0 {
+		options = append(options, libpod.WithCommand(c.Command))
+	}
+
+	// Add entrypoint unconditionally
+	// If it's empty it's because it was explicitly set to "" or the image
+	// does not have one
+	options = append(options, libpod.WithEntrypoint(c.Entrypoint))
+
 	if c.NetMode.IsContainer() {
 		connectedCtr, err := c.Runtime.LookupContainer(c.NetMode.ConnectedContainer())
 		if err != nil {
diff --git a/libpod/container.go b/libpod/container.go
index 67aa086fc..f70856d67 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -174,28 +174,26 @@ type ContainerConfig struct {
 	// TODO consider breaking these subsections up into smaller structs
 
 	// Storage Config
-	// Information on the image used for the root filesystem
+
+	// Information on the image used for the root filesystem/
 	RootfsImageID   string `json:"rootfsImageID,omitempty"`
 	RootfsImageName string `json:"rootfsImageName,omitempty"`
-	// Whether to mount volumes specified in the image
+	// Whether to mount volumes specified in the image.
 	ImageVolumes bool `json:"imageVolumes"`
-	// Src path to be mounted on /dev/shm in container
+	// Src path to be mounted on /dev/shm in container.
 	ShmDir string `json:"ShmDir,omitempty"`
-	// Size of the container's SHM
+	// Size of the container's SHM.
 	ShmSize int64 `json:"shmSize"`
 	// Static directory for container content that will persist across
-	// reboot
+	// reboot.
 	StaticDir string `json:"staticDir"`
-	// Mounts list contains all additional mounts into the container rootfs
-	// These include the SHM mount
-	// These must be unmounted before the container's rootfs is unmounted
+	// Mounts list contains all additional mounts into the container rootfs.
+	// These include the SHM mount.
+	// These must be unmounted before the container's rootfs is unmounted.
 	Mounts []string `json:"mounts,omitempty"`
-	// UserVolumes indicates that the container has user-added volume mounts
-	// It is used to trigger OCI hooks that rely on the presence of user
-	// volumes
-	UserVolumes bool `json:"userVolumes, omitempty"`
 
 	// Security Config
+
 	// Whether the container is privileged
 	Privileged bool `json:"privileged"`
 	// SELinux process label for container
@@ -221,11 +219,12 @@ type ContainerConfig struct {
 	UTSNsCtr    string `json:"utsNsCtr,omitempty"`
 	CgroupNsCtr string `json:"cgroupNsCtr,omitempty"`
 
-	// IDs of dependency containers
-	// These containers must be started before this container is started
+	// IDs of dependency containers.
+	// These containers must be started before this container is started.
 	Dependencies []string
 
 	// Network Config
+
 	// CreateNetNS indicates that libpod should create and configure a new
 	// network namespace for the container
 	// This cannot be set if NetNsCtr is also set
@@ -247,7 +246,26 @@ type ContainerConfig struct {
 	// Will be appended to host's host file
 	HostAdd []string `json:"hostsAdd,omitempty"`
 
+	// Image Config
+
+	// UserVolumes contains user-added volume mounts in the container.
+	// These will not be added to the container's spec, as it is assumed
+	// they are already present in the spec given to Libpod. Instead, it is
+	// used when committing containers to generate the VOLUMES field of the
+	// image that is created, and for triggering some OCI hooks which do not
+	// fire unless user-added volume mounts are present.
+	UserVolumes []string `json:"userVolumes,omitempty"`
+	// Entrypoint is the container's entrypoint.
+	// It is not used in spec generation, but will be used when the
+	// container is committed to populate the entrypoint of the new image.
+	Entrypoint []string `json:"entrypoint,omitempty"`
+	// Command is the container's command.
+	// It is not used in spec generation, but will be used when the
+	// container is committed to populate the command of the new image.
+	Command []string `json:"command,omitempty"`
+
 	// Misc Options
+
 	// Whether to keep container STDIN open
 	Stdin bool `json:"stdin,omitempty"`
 	// Labels is a set of key-value pairs providing additional information
diff --git a/libpod/container_commit.go b/libpod/container_commit.go
index 354aff8b9..568fad9f7 100644
--- a/libpod/container_commit.go
+++ b/libpod/container_commit.go
@@ -26,7 +26,7 @@ type ContainerCommitOptions struct {
 
 // Commit commits the changes between a container and its image, creating a new
 // image
-func (c *Container) Commit(ctx context.Context, destImage string, options ContainerCommitOptions, mounts, command, entryPoint []string) (*image.Image, error) {
+func (c *Container) Commit(ctx context.Context, destImage string, options ContainerCommitOptions) (*image.Image, error) {
 	if !c.batched {
 		c.lock.Lock()
 		defer c.lock.Unlock()
@@ -74,14 +74,10 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
 	// add it to the resulting image.
 
 	// Entrypoint - always set this first or cmd will get wiped out
-	if len(entryPoint) > 0 {
-		importBuilder.SetEntrypoint(entryPoint)
-	}
+	importBuilder.SetEntrypoint(c.config.Entrypoint)
 
 	// Cmd
-	if len(command) > 0 {
-		importBuilder.SetCmd(command)
-	}
+	importBuilder.SetCmd(c.config.Command)
 
 	// Env
 	for _, e := range c.config.Spec.Process.Env {
@@ -100,7 +96,7 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
 	// User
 	importBuilder.SetUser(c.User())
 	// Volumes
-	for _, v := range mounts {
+	for _, v := range c.config.UserVolumes {
 		if v != "" {
 			importBuilder.AddVolume(v)
 		}
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 7631b80e8..8897b3cbf 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -198,6 +198,10 @@ func (c *Container) setupStorage(ctx context.Context) error {
 	c.config.StaticDir = containerInfo.Dir
 	c.state.RunDir = containerInfo.RunDir
 
+	// Set the default Entrypoint and Command
+	c.config.Entrypoint = containerInfo.Config.Config.Entrypoint
+	c.config.Command = containerInfo.Config.Config.Cmd
+
 	artifacts := filepath.Join(c.config.StaticDir, artifactsDir)
 	if err := os.MkdirAll(artifacts, 0755); err != nil {
 		return errors.Wrapf(err, "error creating artifacts directory %q", artifacts)
@@ -1144,7 +1148,7 @@ func (c *Container) setupOCIHooks(g *generate.Generator) error {
 	}
 	for _, hook := range ocihooks {
 		logrus.Debugf("SetupOCIHooks", hook)
-		if hook.HasBindMounts && c.config.UserVolumes {
+		if hook.HasBindMounts && len(c.config.UserVolumes) > 0 {
 			if err := addHook(hook); err != nil {
 				return err
 			}
diff --git a/libpod/options.go b/libpod/options.go
index 968507906..101ff9833 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -821,15 +821,67 @@ func WithGroups(groups []string) CtrCreateOption {
 	}
 }
 
-// WithUserVolumes informs libpod that the container has user-added volumes.
-// It is used to for triggering hooks that check for the presence of volume
-// mounts.
-func WithUserVolumes() CtrCreateOption {
+// WithUserVolumes sets the user-added volumes of the container.
+// These are not added to the container's spec, but will instead be used during
+// commit to populate the volumes of the new image, and to trigger some OCI
+// hooks that are only added if volume mounts are present.
+// Unless explicitly set, committed images will have no volumes.
+// The given volumes slice must not be nil.
+func WithUserVolumes(volumes []string) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
 			return ErrCtrFinalized
 		}
-		ctr.config.UserVolumes = true
+
+		if volumes == nil {
+			return ErrInvalidArg
+		}
+
+		ctr.config.UserVolumes = make([]string, 0, len(volumes))
+		for _, vol := range volumes {
+			ctr.config.UserVolumes = append(ctr.config.UserVolumes, vol)
+		}
+
+		return nil
+	}
+}
+
+// WithEntrypoint sets the entrypoint of the container.
+// This is not used to change the container's spec, but will instead be used
+// during commit to populate the entrypoint of the new image.
+// If not explicitly set it will default to the image's entrypoint.
+// A nil entrypoint is allowed, and will clear entrypoint on the created image.
+func WithEntrypoint(entrypoint []string) CtrCreateOption {
+	return func(ctr *Container) error {
+		if ctr.valid {
+			return ErrCtrFinalized
+		}
+
+		ctr.config.Entrypoint = make([]string, 0, len(entrypoint))
+		for _, str := range entrypoint {
+			ctr.config.Entrypoint = append(ctr.config.Entrypoint, str)
+		}
+
+		return nil
+	}
+}
+
+// WithCommand sets the command of the container.
+// This is not used to change the container's spec, but will instead be used
+// during commit to populate the command of the new image.
+// If not explicitly set it will default to the image's command.
+// A nil command is allowed, and will clear command on the created image.
+func WithCommand(command []string) CtrCreateOption {
+	return func(ctr *Container) error {
+		if ctr.valid {
+			return ErrCtrFinalized
+		}
+
+		ctr.config.Command = make([]string, 0, len(command))
+		for _, str := range command {
+			ctr.config.Command = append(ctr.config.Command, str)
+		}
+
 		return nil
 	}
 }
diff --git a/libpod/storage.go b/libpod/storage.go
index ee9ecbda4..910db1970 100644
--- a/libpod/storage.go
+++ b/libpod/storage.go
@@ -8,6 +8,7 @@ import (
 	istorage "github.com/containers/image/storage"
 	"github.com/containers/image/types"
 	"github.com/containers/storage"
+	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -29,6 +30,7 @@ func getStorageService(store storage.Store) (*storageService, error) {
 type ContainerInfo struct {
 	Dir    string
 	RunDir string
+	Config *v1.Image
 }
 
 // RuntimeContainerMetadata is the structure that we encode as JSON and store
@@ -65,7 +67,7 @@ func (r *storageService) CreateContainerStorage(ctx context.Context, systemConte
 	if containerName == "" {
 		return ContainerInfo{}, ErrEmptyID
 	}
-	//// Check if we have the specified image.
+	// Check if we have the specified image.
 	ref, err := istorage.Transport.ParseStoreReference(r.store, imageID)
 	if err != nil {
 		return ContainerInfo{}, err
@@ -81,6 +83,12 @@ func (r *storageService) CreateContainerStorage(ctx context.Context, systemConte
 	}
 	defer image.Close()
 
+	// Get OCI configuration of image
+	imageConfig, err := image.OCIConfig(ctx)
+	if err != nil {
+		return ContainerInfo{}, err
+	}
+
 	// Update the image name and ID.
 	if imageName == "" && len(img.Names) > 0 {
 		imageName = img.Names[0]
@@ -159,6 +167,7 @@ func (r *storageService) CreateContainerStorage(ctx context.Context, systemConte
 	return ContainerInfo{
 		Dir:    containerDir,
 		RunDir: containerRunDir,
+		Config: imageConfig,
 	}, nil
 }