Move the HostConfig portion of Inspect inside libpod

When we first began writing Podman, we ran into a major issue
when implementing Inspect. Libpod deliberately does not tie its
internal data structures to Docker, and stores most information
about containers encoded within the OCI spec. However, Podman
must present a CLI compatible with Docker, which means it must
expose all the information in 'docker inspect' - most of which is
not contained in the OCI spec or libpod's Config struct.

Our solution at the time was the create artifact. We JSON'd the
complete CreateConfig (a parsed form of the CLI arguments to
'podman run') and stored it with the container, restoring it when
we needed to run commands that required the extra info.

Over the past month, I've been looking more at Inspect, and
refactored large portions of it into Libpod - generating them
from what we know about the OCI config and libpod's (now much
expanded, versus previously) container configuration. This path
comes close to completing the process, moving the last part of
inspect into libpod and removing the need for the create
artifact.

This improves libpod's compatability with non-Podman containers.
We no longer require an arbitrarily-formatted JSON blob to be
present to run inspect.

Fixes: #3500

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

cmd/podman/inspect.go

@@ -6,9 +6,7 @@ import (
 
 	"github.com/containers/buildah/pkg/formats"
 	"github.com/containers/libpod/cmd/podman/cliconfig"
-	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/pkg/adapter"
-	cc "github.com/containers/libpod/pkg/spec"
 	"github.com/containers/libpod/pkg/util"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -148,19 +146,9 @@ func iterateInput(ctx context.Context, size bool, args []string, runtime *adapte
 				inspectError = errors.Wrapf(err, "error looking up container %q", input)
 				break
 			}
-			libpodInspectData, err := ctr.Inspect(size)
+			data, err = ctr.Inspect(size)
 			if err != nil {
-				inspectError = errors.Wrapf(err, "error getting libpod container inspect data %s", ctr.ID())
-				break
-			}
-			artifact, err := getArtifact(ctr)
-			if inspectError != nil {
-				inspectError = err
-				break
-			}
-			data, err = shared.GetCtrInspectInfo(ctr.Config(), libpodInspectData, artifact)
-			if err != nil {
-				inspectError = errors.Wrapf(err, "error parsing container data %q", ctr.ID())
+				inspectError = errors.Wrapf(err, "error inspecting container %s", ctr.ID())
 				break
 			}
 		case inspectTypeImage:
@@ -188,19 +176,9 @@ func iterateInput(ctx context.Context, size bool, args []string, runtime *adapte
 					break
 				}
 			} else {
-				libpodInspectData, err := ctr.Inspect(size)
+				data, err = ctr.Inspect(size)
 				if err != nil {
-					inspectError = errors.Wrapf(err, "error getting libpod container inspect data %s", ctr.ID())
-					break
-				}
-				artifact, err := getArtifact(ctr)
-				if err != nil {
-					inspectError = err
-					break
-				}
-				data, err = shared.GetCtrInspectInfo(ctr.Config(), libpodInspectData, artifact)
-				if err != nil {
-					inspectError = errors.Wrapf(err, "error parsing container data %s", ctr.ID())
+					inspectError = errors.Wrapf(err, "error inspecting container %s", ctr.ID())
 					break
 				}
 			}
@@ -211,15 +189,3 @@ func iterateInput(ctx context.Context, size bool, args []string, runtime *adapte
 	}
 	return inspectedItems, inspectError
 }
-
-func getArtifact(ctr *adapter.Container) (*cc.CreateConfig, error) {
-	var createArtifact cc.CreateConfig
-	artifact, err := ctr.GetArtifact("create-config")
-	if err != nil {
-		return nil, err
-	}
-	if err := json.Unmarshal(artifact, &createArtifact); err != nil {
-		return nil, err
-	}
-	return &createArtifact, nil
-}

cmd/podman/shared/container_inspect.go

@@ -1,211 +0,0 @@
-package shared
-
-import (
-	"github.com/containers/libpod/libpod"
-	cc "github.com/containers/libpod/pkg/spec"
-	"github.com/docker/go-connections/nat"
-	"github.com/opencontainers/runtime-spec/specs-go"
-)
-
-// InspectContainer holds all inspect data for a container.
-// The format of individual components is fixed so the overall structure, when
-// JSON encoded, matches the output of `docker inspect`.
-// It combines Libpod-source inspect data with Podman-specific inspect data.
-type InspectContainer struct {
-	*libpod.InspectContainerData
-	HostConfig *InspectContainerHostConfig `json:"HostConfig"`
-}
-
-// InspectContainerHostConfig holds Container configuration that is not specific
-// to Libpod. This information is (mostly) stored by Podman as an artifact.
-// This struct is matched to the output of `docker inspect`.
-type InspectContainerHostConfig struct {
-	ContainerIDFile      string                      `json:"ContainerIDFile"`
-	LogConfig            *InspectLogConfig           `json:"LogConfig"` //TODO
-	NetworkMode          string                      `json:"NetworkMode"`
-	PortBindings         nat.PortMap                 `json:"PortBindings"` //TODO
-	AutoRemove           bool                        `json:"AutoRemove"`
-	CapAdd               []string                    `json:"CapAdd"`
-	CapDrop              []string                    `json:"CapDrop"`
-	DNS                  []string                    `json:"DNS"`
-	DNSOptions           []string                    `json:"DNSOptions"`
-	DNSSearch            []string                    `json:"DNSSearch"`
-	ExtraHosts           []string                    `json:"ExtraHosts"`
-	GroupAdd             []uint32                    `json:"GroupAdd"`
-	IpcMode              string                      `json:"IpcMode"`
-	Cgroup               string                      `json:"Cgroup"`
-	OomScoreAdj          *int                        `json:"OomScoreAdj"`
-	PidMode              string                      `json:"PidMode"`
-	Privileged           bool                        `json:"Privileged"`
-	PublishAllPorts      bool                        `json:"PublishAllPorts"` //TODO
-	ReadOnlyRootfs       bool                        `json:"ReadonlyRootfs"`
-	ReadOnlyTmpfs        bool                        `json:"ReadonlyTmpfs"`
-	SecurityOpt          []string                    `json:"SecurityOpt"`
-	UTSMode              string                      `json:"UTSMode"`
-	UsernsMode           string                      `json:"UsernsMode"`
-	ShmSize              int64                       `json:"ShmSize"`
-	Runtime              string                      `json:"Runtime"`
-	ConsoleSize          *specs.Box                  `json:"ConsoleSize"`
-	CPUShares            *uint64                     `json:"CpuShares"`
-	Memory               int64                       `json:"Memory"`
-	NanoCPUs             int                         `json:"NanoCpus"`
-	CgroupParent         string                      `json:"CgroupParent"`
-	BlkioWeight          *uint16                     `json:"BlkioWeight"`
-	BlkioWeightDevice    []specs.LinuxWeightDevice   `json:"BlkioWeightDevice"`
-	BlkioDeviceReadBps   []specs.LinuxThrottleDevice `json:"BlkioDeviceReadBps"`
-	BlkioDeviceWriteBps  []specs.LinuxThrottleDevice `json:"BlkioDeviceWriteBps"`
-	BlkioDeviceReadIOps  []specs.LinuxThrottleDevice `json:"BlkioDeviceReadIOps"`
-	BlkioDeviceWriteIOps []specs.LinuxThrottleDevice `json:"BlkioDeviceWriteIOps"`
-	CPUPeriod            *uint64                     `json:"CpuPeriod"`
-	CPUQuota             *int64                      `json:"CpuQuota"`
-	CPURealtimePeriod    *uint64                     `json:"CpuRealtimePeriod"`
-	CPURealtimeRuntime   *int64                      `json:"CpuRealtimeRuntime"`
-	CPUSetCPUs           string                      `json:"CpuSetCpus"`
-	CPUSetMems           string                      `json:"CpuSetMems"`
-	Devices              []specs.LinuxDevice         `json:"Devices"`
-	DiskQuota            int                         `json:"DiskQuota"` //check type, TODO
-	KernelMemory         *int64                      `json:"KernelMemory"`
-	MemoryReservation    *int64                      `json:"MemoryReservation"`
-	MemorySwap           *int64                      `json:"MemorySwap"`
-	MemorySwappiness     *uint64                     `json:"MemorySwappiness"`
-	OomKillDisable       *bool                       `json:"OomKillDisable"`
-	PidsLimit            *int64                      `json:"PidsLimit"`
-	Ulimits              []string                    `json:"Ulimits"`
-	CPUCount             int                         `json:"CpuCount"`
-	CPUPercent           int                         `json:"CpuPercent"`
-	IOMaximumIOps        int                         `json:"IOMaximumIOps"`      //check type, TODO
-	IOMaximumBandwidth   int                         `json:"IOMaximumBandwidth"` //check type, TODO
-	Tmpfs                []string                    `json:"Tmpfs"`
-}
-
-// InspectLogConfig holds information about a container's configured log driver
-// and is presently unused. It is retained for Docker compatibility.
-type InspectLogConfig struct {
-	Type   string            `json:"Type"`
-	Config map[string]string `json:"Config"` //idk type, TODO
-}
-
-// GetCtrInspectInfo inspects a container, combining Libpod inspect information
-// with other information not stored in Libpod and returning a struct that, when
-// formatted for JSON output, is compatible with `docker inspect`.
-func GetCtrInspectInfo(config *libpod.ContainerConfig, ctrInspectData *libpod.InspectContainerData, createArtifact *cc.CreateConfig) (*InspectContainer, error) {
-	spec := config.Spec
-
-	cpus, mems, period, quota, realtimePeriod, realtimeRuntime, shares := getCPUInfo(spec)
-	blkioWeight, blkioWeightDevice, blkioReadBps, blkioWriteBps, blkioReadIOPS, blkioeWriteIOPS := getBLKIOInfo(spec)
-	memKernel, memReservation, memSwap, memSwappiness, memDisableOOMKiller := getMemoryInfo(spec)
-	pidsLimit := getPidsInfo(spec)
-	cgroup := getCgroup(spec)
-	logConfig := InspectLogConfig{
-		config.LogDriver,
-		make(map[string]string),
-	}
-
-	data := &InspectContainer{
-		ctrInspectData,
-		&InspectContainerHostConfig{
-			ConsoleSize:          spec.Process.ConsoleSize,
-			OomScoreAdj:          spec.Process.OOMScoreAdj,
-			CPUShares:            shares,
-			BlkioWeight:          blkioWeight,
-			BlkioWeightDevice:    blkioWeightDevice,
-			BlkioDeviceReadBps:   blkioReadBps,
-			BlkioDeviceWriteBps:  blkioWriteBps,
-			BlkioDeviceReadIOps:  blkioReadIOPS,
-			BlkioDeviceWriteIOps: blkioeWriteIOPS,
-			CPUPeriod:            period,
-			CPUQuota:             quota,
-			CPURealtimePeriod:    realtimePeriod,
-			CPURealtimeRuntime:   realtimeRuntime,
-			CPUSetCPUs:           cpus,
-			CPUSetMems:           mems,
-			Devices:              spec.Linux.Devices,
-			KernelMemory:         memKernel,
-			LogConfig:            &logConfig,
-			MemoryReservation:    memReservation,
-			MemorySwap:           memSwap,
-			MemorySwappiness:     memSwappiness,
-			OomKillDisable:       memDisableOOMKiller,
-			PidsLimit:            pidsLimit,
-			Privileged:           config.Privileged,
-			ReadOnlyRootfs:       spec.Root.Readonly,
-			ReadOnlyTmpfs:        createArtifact.ReadOnlyTmpfs,
-			Runtime:              config.OCIRuntime,
-			NetworkMode:          string(createArtifact.NetMode),
-			IpcMode:              string(createArtifact.IpcMode),
-			Cgroup:               cgroup,
-			UTSMode:              string(createArtifact.UtsMode),
-			UsernsMode:           string(createArtifact.UsernsMode),
-			GroupAdd:             spec.Process.User.AdditionalGids,
-			ContainerIDFile:      createArtifact.CidFile,
-			AutoRemove:           createArtifact.Rm,
-			CapAdd:               createArtifact.CapAdd,
-			CapDrop:              createArtifact.CapDrop,
-			DNS:                  createArtifact.DNSServers,
-			DNSOptions:           createArtifact.DNSOpt,
-			DNSSearch:            createArtifact.DNSSearch,
-			PidMode:              string(createArtifact.PidMode),
-			CgroupParent:         createArtifact.CgroupParent,
-			ShmSize:              createArtifact.Resources.ShmSize,
-			Memory:               createArtifact.Resources.Memory,
-			Ulimits:              createArtifact.Resources.Ulimit,
-			SecurityOpt:          createArtifact.SecurityOpts,
-			Tmpfs:                createArtifact.Tmpfs,
-		},
-	}
-	return data, nil
-}
-
-func getCPUInfo(spec *specs.Spec) (string, string, *uint64, *int64, *uint64, *int64, *uint64) {
-	if spec.Linux.Resources == nil {
-		return "", "", nil, nil, nil, nil, nil
-	}
-	cpu := spec.Linux.Resources.CPU
-	if cpu == nil {
-		return "", "", nil, nil, nil, nil, nil
-	}
-	return cpu.Cpus, cpu.Mems, cpu.Period, cpu.Quota, cpu.RealtimePeriod, cpu.RealtimeRuntime, cpu.Shares
-}
-
-func getBLKIOInfo(spec *specs.Spec) (*uint16, []specs.LinuxWeightDevice, []specs.LinuxThrottleDevice, []specs.LinuxThrottleDevice, []specs.LinuxThrottleDevice, []specs.LinuxThrottleDevice) {
-	if spec.Linux.Resources == nil {
-		return nil, nil, nil, nil, nil, nil
-	}
-	blkio := spec.Linux.Resources.BlockIO
-	if blkio == nil {
-		return nil, nil, nil, nil, nil, nil
-	}
-	return blkio.Weight, blkio.WeightDevice, blkio.ThrottleReadBpsDevice, blkio.ThrottleWriteBpsDevice, blkio.ThrottleReadIOPSDevice, blkio.ThrottleWriteIOPSDevice
-}
-
-func getMemoryInfo(spec *specs.Spec) (*int64, *int64, *int64, *uint64, *bool) {
-	if spec.Linux.Resources == nil {
-		return nil, nil, nil, nil, nil
-	}
-	memory := spec.Linux.Resources.Memory
-	if memory == nil {
-		return nil, nil, nil, nil, nil
-	}
-	return memory.Kernel, memory.Reservation, memory.Swap, memory.Swappiness, memory.DisableOOMKiller
-}
-
-func getPidsInfo(spec *specs.Spec) *int64 {
-	if spec.Linux.Resources == nil {
-		return nil
-	}
-	pids := spec.Linux.Resources.Pids
-	if pids == nil {
-		return nil
-	}
-	return &pids.Limit
-}
-
-func getCgroup(spec *specs.Spec) string {
-	cgroup := "host"
-	for _, ns := range spec.Linux.Namespaces {
-		if ns.Type == specs.CgroupNamespace && ns.Path != "" {
-			cgroup = "container"
-		}
-	}
-	return cgroup
-}

cmd/podman/shared/create.go

@@ -651,6 +651,7 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		ImageVolumeType:   c.String("image-volume"),
 		CapAdd:            c.StringSlice("cap-add"),
 		CapDrop:           c.StringSlice("cap-drop"),
+		CidFile:           c.String("cidfile"),
 		CgroupParent:      c.String("cgroup-parent"),
 		Command:           command,
 		Detach:            c.Bool("detach"),
@@ -766,14 +767,6 @@ func CreateContainerFromCreateConfig(r *libpod.Runtime, createConfig *cc.CreateC
 	if err != nil {
 		return nil, err
 	}
-
-	createConfigJSON, err := json.Marshal(createConfig)
-	if err != nil {
-		return nil, err
-	}
-	if err := ctr.AddArtifact("create-config", createConfigJSON); err != nil {
-		return nil, err
-	}
 	return ctr, nil
 }
 

go.mod

@@ -101,7 +101,7 @@ require (
 	github.com/spf13/pflag v1.0.3
 	github.com/spf13/viper v1.4.0 // indirect
 	github.com/stretchr/testify v1.3.0
-	github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2 // indirect
+	github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2
 	github.com/tchap/go-patricia v2.3.0+incompatible // indirect
 	github.com/uber/jaeger-client-go v2.16.0+incompatible
 	github.com/uber/jaeger-lib v0.0.0-20190122222657-d036253de8f5 // indirect

pkg/spec/spec.go

@@ -418,6 +418,43 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM
 		}
 	}
 
+	// Add annotations
+	if configSpec.Annotations == nil {
+		configSpec.Annotations = make(map[string]string)
+	}
+
+	if config.CidFile != "" {
+		configSpec.Annotations[libpod.InspectAnnotationCIDFile] = config.CidFile
+	}
+
+	if config.Rm {
+		configSpec.Annotations[libpod.InspectAnnotationAutoremove] = libpod.InspectResponseTrue
+	} else {
+		configSpec.Annotations[libpod.InspectAnnotationAutoremove] = libpod.InspectResponseFalse
+	}
+
+	if len(config.VolumesFrom) > 0 {
+		configSpec.Annotations[libpod.InspectAnnotationVolumesFrom] = strings.Join(config.VolumesFrom, ",")
+	}
+
+	if config.Privileged {
+		configSpec.Annotations[libpod.InspectAnnotationPrivileged] = libpod.InspectResponseTrue
+	} else {
+		configSpec.Annotations[libpod.InspectAnnotationPrivileged] = libpod.InspectResponseFalse
+	}
+
+	if config.PublishAll {
+		configSpec.Annotations[libpod.InspectAnnotationPublishAll] = libpod.InspectResponseTrue
+	} else {
+		configSpec.Annotations[libpod.InspectAnnotationPublishAll] = libpod.InspectResponseFalse
+	}
+
+	if config.Init {
+		configSpec.Annotations[libpod.InspectAnnotationInit] = libpod.InspectResponseTrue
+	} else {
+		configSpec.Annotations[libpod.InspectAnnotationInit] = libpod.InspectResponseFalse
+	}
+
 	return configSpec, nil
 }
 

pkg/util/utils_linux.go

@@ -1,7 +1,14 @@
 package util
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+
 	"github.com/containers/psgo"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // GetContainerPidInformationDescriptors returns a string slice of all supported
@@ -9,3 +16,39 @@ import (
 func GetContainerPidInformationDescriptors() ([]string, error) {
 	return psgo.ListDescriptors(), nil
 }
+
+// FindDeviceNodes parses /dev/ into a set of major:minor -> path, where
+// [major:minor] is the device's major and minor numbers formatted as, for
+// example, 2:0 and path is the path to the device node.
+// Symlinks to nodes are ignored.
+func FindDeviceNodes() (map[string]string, error) {
+	nodes := make(map[string]string)
+	err := filepath.Walk("/dev", func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			logrus.Warnf("Error descending into path %s: %v", path, err)
+			return filepath.SkipDir
+		}
+
+		// If we aren't a device node, do nothing.
+		if info.Mode()&(os.ModeDevice|os.ModeCharDevice) == 0 {
+			return nil
+		}
+
+		// We are a device node. Get major/minor.
+		sysstat, ok := info.Sys().(*syscall.Stat_t)
+		if !ok {
+			return errors.Errorf("Could not convert stat output for use")
+		}
+		major := uint64(sysstat.Rdev / 256)
+		minor := uint64(sysstat.Rdev % 256)
+
+		nodes[fmt.Sprintf("%d:%d", major, minor)] = path
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return nodes, nil
+}

pkg/util/utils_unsupported.go

@@ -0,0 +1,12 @@
+// +build darwin windows
+
+package util
+
+import (
+	"github.com/pkg/errors"
+)
+
+// FindDeviceNodes is not implemented anywhere except Linux.
+func FindDeviceNodes() (map[string]string, error) {
+	return nil, errors.Errorf("not supported on non-Linux OSes")
+}

pkg/varlinkapi/containers.go

@@ -19,7 +19,6 @@ import (
 	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/libpod/logs"
 	"github.com/containers/libpod/pkg/adapter/shortcuts"
-	cc "github.com/containers/libpod/pkg/spec"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/pkg/errors"
 )
@@ -170,16 +169,7 @@ func (i *LibpodAPI) InspectContainer(call iopodman.VarlinkCall, name string) err
 	if err != nil {
 		return call.ReplyContainerNotFound(name, err.Error())
 	}
-	inspectInfo, err := ctr.Inspect(true)
-	if err != nil {
-		return call.ReplyErrorOccurred(err.Error())
-	}
-	artifact, err := getArtifact(ctr)
-	if err != nil {
-		return call.ReplyErrorOccurred(err.Error())
-	}
-
-	data, err := shared.GetCtrInspectInfo(ctr.Config(), inspectInfo, artifact)
+	data, err := ctr.Inspect(true)
 	if err != nil {
 		return call.ReplyErrorOccurred(err.Error())
 	}
@@ -587,18 +577,6 @@ func (i *LibpodAPI) ContainerRestore(call iopodman.VarlinkCall, name string, kee
 	return call.ReplyContainerRestore(ctr.ID())
 }
 
-func getArtifact(ctr *libpod.Container) (*cc.CreateConfig, error) {
-	var createArtifact cc.CreateConfig
-	artifact, err := ctr.GetArtifact("create-config")
-	if err != nil {
-		return nil, err
-	}
-	if err := json.Unmarshal(artifact, &createArtifact); err != nil {
-		return nil, err
-	}
-	return &createArtifact, nil
-}
-
 // ContainerConfig returns just the container.config struct
 func (i *LibpodAPI) ContainerConfig(call iopodman.VarlinkCall, name string) error {
 	ctr, err := i.Runtime.LookupContainer(name)

test/e2e/common_test.go

@@ -12,7 +12,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/inspect"
 	"github.com/containers/libpod/pkg/rootless"
@@ -322,7 +321,7 @@ func (s *PodmanSessionIntegration) InspectImageJSON() []inspect.ImageData {
 }
 
 // InspectContainer returns a container's inspect data in JSON format
-func (p *PodmanTestIntegration) InspectContainer(name string) []shared.InspectContainer {
+func (p *PodmanTestIntegration) InspectContainer(name string) []libpod.InspectContainerData {
 	cmd := []string{"inspect", name}
 	session := p.Podman(cmd)
 	session.WaitWithDefaultTimeout()
@@ -481,8 +480,8 @@ func (p *PodmanTestIntegration) PullImage(image string) error {
 
 // InspectContainerToJSON takes the session output of an inspect
 // container and returns json
-func (s *PodmanSessionIntegration) InspectContainerToJSON() []shared.InspectContainer {
-	var i []shared.InspectContainer
+func (s *PodmanSessionIntegration) InspectContainerToJSON() []libpod.InspectContainerData {
+	var i []libpod.InspectContainerData
 	err := json.Unmarshal(s.Out.Contents(), &i)
 	Expect(err).To(BeNil())
 	return i

vendor/modules.txt

@@ -355,8 +355,8 @@ github.com/opencontainers/runc/libcontainer/system
 github.com/opencontainers/runtime-spec/specs-go
 # github.com/opencontainers/runtime-tools v0.9.0
 github.com/opencontainers/runtime-tools/generate
-github.com/opencontainers/runtime-tools/generate/seccomp
 github.com/opencontainers/runtime-tools/validate
+github.com/opencontainers/runtime-tools/generate/seccomp
 github.com/opencontainers/runtime-tools/filepath
 github.com/opencontainers/runtime-tools/specerror
 github.com/opencontainers/runtime-tools/error