Merge pull request #3312 from mheon/podman_inspect_fixes_cont

Further fixes for podman inspect

2 files changed, 155 insertions, 27 deletions

diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go
index 8e34e7088..0a62ceb7c 100644
--- a/libpod/container_inspect.go
+++ b/libpod/container_inspect.go
@@ -1,12 +1,11 @@
 package libpod
 
 import (
-	"strings"
 	"time"
 
 	"github.com/containers/libpod/libpod/driver"
 	"github.com/cri-o/ocicni/pkg/ocicni"
-	specs "github.com/opencontainers/runtime-spec/specs-go"
+	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -29,6 +28,7 @@ type InspectContainerData struct {
 	HostnamePath    string                  `json:"HostnamePath"`
 	HostsPath       string                  `json:"HostsPath"`
 	StaticDir       string                  `json:"StaticDir"`
+	OCIConfigPath   string                  `json:"OCIConfigPath,omitempty"`
 	LogPath         string                  `json:"LogPath"`
 	ConmonPidFile   string                  `json:"ConmonPidFile"`
 	Name            string                  `json:"Name"`
@@ -43,7 +43,7 @@ type InspectContainerData struct {
 	GraphDriver     *driver.Data            `json:"GraphDriver"`
 	SizeRw          int64                   `json:"SizeRw,omitempty"`
 	SizeRootFs      int64                   `json:"SizeRootFs,omitempty"`
-	Mounts          []specs.Mount           `json:"Mounts"`
+	Mounts          []*InspectMount         `json:"Mounts"`
 	Dependencies    []string                `json:"Dependencies"`
 	NetworkSettings *InspectNetworkSettings `json:"NetworkSettings"` //TODO
 	ExitCommand     []string                `json:"ExitCommand"`
@@ -51,6 +51,35 @@ type InspectContainerData struct {
 	IsInfra         bool                    `json:"IsInfra"`
 }
 
+// InspectMount provides a record of a single mount in a container. It contains
+// fields for both named and normal volumes. Only user-specified volumes will be
+// included, and tmpfs volumes are not included even if the user specified them.
+type InspectMount struct {
+	// Whether the mount is a volume or bind mount. Allowed values are
+	// "volume" and "bind".
+	Type string `json:"Type"`
+	// The name of the volume. Empty for bind mounts.
+	Name string `json:"Name,omptempty"`
+	// The source directory for the volume.
+	Src string `json:"Source"`
+	// The destination directory for the volume. Specified as a path within
+	// the container, as it would be passed into the OCI runtime.
+	Dst string `json:"Destination"`
+	// The driver used for the named volume. Empty for bind mounts.
+	Driver string `json:"Driver"`
+	// Contains SELinux :z/:Z mount options. Unclear what, if anything, else
+	// goes in here.
+	Mode string `json:"Mode"`
+	// All remaining mount options. Additional data, not present in the
+	// original output.
+	Options []string `json:"Options"`
+	// Whether the volume is read-write
+	RW bool `json:"RW"`
+	// Mount propagation for the mount. Can be empty if not specified, but
+	// is always printed - no omitempty.
+	Propagation string `json:"Propagation"`
+}
+
 // InspectContainerState provides a detailed record of a container's current
 // state. It is returned as part of InspectContainerData.
 // As with InspectContainerData, many portions of this struct are matched to
@@ -148,34 +177,24 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
 		execIDs = append(execIDs, id)
 	}
 
-	if c.state.BindMounts == nil {
-		c.state.BindMounts = make(map[string]string)
-	}
-
 	resolvPath := ""
-	if getPath, ok := c.state.BindMounts["/etc/resolv.conf"]; ok {
-		resolvPath = getPath
-	}
-
 	hostsPath := ""
-	if getPath, ok := c.state.BindMounts["/etc/hosts"]; ok {
-		hostsPath = getPath
-	}
-
 	hostnamePath := ""
-	if getPath, ok := c.state.BindMounts["/etc/hostname"]; ok {
-		hostnamePath = getPath
+	if c.state.BindMounts != nil {
+		if getPath, ok := c.state.BindMounts["/etc/resolv.conf"]; ok {
+			resolvPath = getPath
+		}
+		if getPath, ok := c.state.BindMounts["/etc/hosts"]; ok {
+			hostsPath = getPath
+		}
+		if getPath, ok := c.state.BindMounts["/etc/hostname"]; ok {
+			hostnamePath = getPath
+		}
 	}
 
-	var mounts []specs.Mount
-	for i, mnt := range spec.Mounts {
-		mounts = append(mounts, mnt)
-		// We only want to show the name of the named volume in the inspect
-		// output, so split the path and get the name out of it.
-		if strings.Contains(mnt.Source, c.runtime.config.VolumePath) {
-			split := strings.Split(mnt.Source[len(c.runtime.config.VolumePath)+1:], "/")
-			mounts[i].Source = split[0]
-		}
+	mounts, err := c.getInspectMounts()
+	if err != nil {
+		return nil, err
 	}
 
 	data := &InspectContainerData{
@@ -242,8 +261,12 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
 		IsInfra: c.IsInfra(),
 	}
 
+	if c.state.ConfigPath != "" {
+		data.OCIConfigPath = c.state.ConfigPath
+	}
+
 	if c.config.HealthCheckConfig != nil {
-		//	This container has a healthcheck defined in it; we need to add it's state
+		// This container has a healthcheck defined in it; we need to add it's state
 		healthCheckState, err := c.GetHealthCheckLog()
 		if err != nil {
 			// An error here is not considered fatal; no health state will be displayed
@@ -275,3 +298,106 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
 	}
 	return data, nil
 }
+
+// Get inspect-formatted mounts list.
+// Only includes user-specified mounts. Only includes bind mounts and named
+// volumes, not tmpfs volumes.
+func (c *Container) getInspectMounts() ([]*InspectMount, error) {
+	inspectMounts := []*InspectMount{}
+
+	// No mounts, return early
+	if len(c.config.UserVolumes) == 0 {
+		return inspectMounts, nil
+	}
+
+	// We need to parse all named volumes and mounts into maps, so we don't
+	// end up with repeated lookups for each user volume.
+	// Map destination to struct, as destination is what is stored in
+	// UserVolumes.
+	namedVolumes := make(map[string]*ContainerNamedVolume)
+	mounts := make(map[string]spec.Mount)
+	for _, namedVol := range c.config.NamedVolumes {
+		namedVolumes[namedVol.Dest] = namedVol
+	}
+	for _, mount := range c.config.Spec.Mounts {
+		mounts[mount.Destination] = mount
+	}
+
+	for _, vol := range c.config.UserVolumes {
+		// We need to look up the volumes.
+		// First: is it a named volume?
+		if volume, ok := namedVolumes[vol]; ok {
+			mountStruct := new(InspectMount)
+			mountStruct.Type = "volume"
+			mountStruct.Dst = volume.Dest
+			mountStruct.Name = volume.Name
+
+			// For src and driver, we need to look up the named
+			// volume.
+			volFromDB, err := c.runtime.state.Volume(volume.Name)
+			if err != nil {
+				return nil, errors.Wrapf(err, "error looking up volume %s in container %s config", volume.Name, c.ID())
+			}
+			mountStruct.Driver = volFromDB.Driver()
+			mountStruct.Src = volFromDB.MountPoint()
+
+			parseMountOptionsForInspect(volume.Options, mountStruct)
+
+			inspectMounts = append(inspectMounts, mountStruct)
+		} else if mount, ok := mounts[vol]; ok {
+			// It's a mount.
+			// Is it a tmpfs? If so, discard.
+			if mount.Type == "tmpfs" {
+				continue
+			}
+
+			mountStruct := new(InspectMount)
+			mountStruct.Type = "bind"
+			mountStruct.Src = mount.Source
+			mountStruct.Dst = mount.Destination
+
+			parseMountOptionsForInspect(mount.Options, mountStruct)
+
+			inspectMounts = append(inspectMounts, mountStruct)
+		}
+		// We couldn't find a mount. Log a warning.
+		logrus.Warnf("Could not find mount at destination %q when building inspect output for container %s", vol, c.ID())
+	}
+
+	return inspectMounts, nil
+}
+
+// Parse mount options so we can populate them in the mount structure.
+// The mount passed in will be modified.
+func parseMountOptionsForInspect(options []string, mount *InspectMount) {
+	isRW := true
+	mountProp := ""
+	zZ := ""
+	otherOpts := []string{}
+
+	// Some of these may be overwritten if the user passes us garbage opts
+	// (for example, [ro,rw])
+	// We catch these on the Podman side, so not a problem there, but other
+	// users of libpod who do not properly validate mount options may see
+	// this.
+	// Not really worth dealing with on our end - garbage in, garbage out.
+	for _, opt := range options {
+		switch opt {
+		case "ro":
+			isRW = false
+		case "rw":
+			// Do nothing, silently discard
+		case "shared", "slave", "private", "rshared", "rslave", "rprivate":
+			mountProp = opt
+		case "z", "Z":
+			zZ = opt
+		default:
+			otherOpts = append(otherOpts, opt)
+		}
+	}
+
+	mount.RW = isRW
+	mount.Propagation = mountProp
+	mount.Mode = zZ
+	mount.Options = otherOpts
+}
diff --git a/libpod/options.go b/libpod/options.go
index 20aa51981..cdac09654 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -1127,6 +1127,8 @@ func WithGroups(groups []string) CtrCreateOption {
 // 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.
+// Furthermore, they are used in the output of inspect, to filter volumes -
+// only volumes included in this list will be included in the output.
 // Unless explicitly set, committed images will have no volumes.
 // The given volumes slice must not be nil.
 func WithUserVolumes(volumes []string) CtrCreateOption {