Skip to content

Commit

Permalink
fix pod volume passing and alter infra inheritance
Browse files Browse the repository at this point in the history
the infra Inherit function was not properly passing pod volume information to new containers
alter the inherit function and struct to use the new `ConfigToSpec` function used in clone
pick and choose the proper entities from a temp spec and validate them on the spegen side rather
than passing directly to a config

resolves containers#13548

Signed-off-by: cdoern <cbdoer23@g.holycross.edu>
  • Loading branch information
cdoern committed Mar 24, 2022
1 parent b4b8b8b commit aa5cac8
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 45 deletions.
23 changes: 15 additions & 8 deletions libpod/container_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/containers/common/pkg/secrets"
"github.com/containers/image/v5/manifest"
"github.com/containers/podman/v4/pkg/namespaces"
"github.com/containers/podman/v4/pkg/specgen"
"github.com/containers/storage"
spec "github.com/opencontainers/runtime-spec/specs-go"
)
Expand Down Expand Up @@ -405,13 +406,19 @@ type ContainerMiscConfig struct {
InitContainerType string `json:"init_container_type,omitempty"`
}

// InfraInherit contains the compatible options inheritable from the infra container
type InfraInherit struct {
InfraSecurity ContainerSecurityConfig
InfraLabels []string `json:"labelopts,omitempty"`
InfraVolumes []*ContainerNamedVolume `json:"namedVolumes,omitempty"`
InfraOverlay []*ContainerOverlayVolume `json:"overlayVolumes,omitempty"`
InfraImageVolumes []*ContainerImageVolume `json:"ctrImageVolumes,omitempty"`
InfraUserVolumes []string `json:"userVolumes,omitempty"`
InfraResources *spec.LinuxResources `json:"resources,omitempty"`
InfraDevices []spec.LinuxDevice `json:"device_host_src,omitempty"`
ApparmorProfile string `json:"apparmor_profile,omitempty"`
CapAdd []string `json:"cap_add,omitempty"`
CapDrop []string `json:"cap_drop,omitempty"`
HostDeviceList []spec.LinuxDevice `json:"host_device_list,omitempty"`
ImageVolumes []*specgen.ImageVolume `json:"image_volumes,omitempty"`
InfraResources *spec.LinuxResources `json:"resource_limits,omitempty"`
Mounts []spec.Mount `json:"mounts,omitempty"`
NoNewPrivileges bool `json:"no_new_privileges,omitempty"`
OverlayVolumes []*specgen.OverlayVolume `json:"overlay_volumes,omitempty"`
SeccompPolicy string `json:"seccomp_policy,omitempty"`
SeccompProfilePath string `json:"seccomp_profile_path,omitempty"`
SelinuxOpts []string `json:"selinux_opts,omitempty"`
Volumes []*specgen.NamedVolume `json:"volumes,omitempty"`
}
2 changes: 1 addition & 1 deletion libpod/container_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
}
}

namedVolumes, mounts := c.sortUserVolumes(ctrSpec)
namedVolumes, mounts := c.SortUserVolumes(ctrSpec)
inspectMounts, err := c.GetInspectMounts(namedVolumes, c.config.ImageVolumes, mounts)
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions libpod/container_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -2242,9 +2242,9 @@ func (c *Container) prepareCheckpointExport() error {
return nil
}

// sortUserVolumes sorts the volumes specified for a container
// SortUserVolumes sorts the volumes specified for a container
// between named and normal volumes
func (c *Container) sortUserVolumes(ctrSpec *spec.Spec) ([]*ContainerNamedVolume, []spec.Mount) {
func (c *Container) SortUserVolumes(ctrSpec *spec.Spec) ([]*ContainerNamedVolume, []spec.Mount) {
namedUserVolumes := []*ContainerNamedVolume{}
userMounts := []spec.Mount{}

Expand Down
2 changes: 1 addition & 1 deletion libpod/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ func libpodEnvVarsToKubeEnvVars(envs []string, imageEnvs []string) ([]v1.EnvVar,

// libpodMountsToKubeVolumeMounts converts the containers mounts to a struct kube understands
func libpodMountsToKubeVolumeMounts(c *Container) ([]v1.VolumeMount, []v1.Volume, map[string]string, error) {
namedVolumes, mounts := c.sortUserVolumes(c.config.Spec)
namedVolumes, mounts := c.SortUserVolumes(c.config.Spec)
vms := make([]v1.VolumeMount, 0, len(mounts))
vos := make([]v1.Volume, 0, len(mounts))
annotations := make(map[string]string)
Expand Down
2 changes: 1 addition & 1 deletion libpod/pod_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) {
infraConfig.CPUSetCPUs = p.ResourceLim().CPU.Cpus
infraConfig.PidNS = p.PidMode()
infraConfig.UserNS = p.UserNSMode()
namedVolumes, mounts := infra.sortUserVolumes(infra.config.Spec)
namedVolumes, mounts := infra.SortUserVolumes(infra.config.Spec)
inspectMounts, err = infra.GetInspectMounts(namedVolumes, infra.config.ImageVolumes, mounts)
infraSecurity = infra.GetSecurityOptions()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/domain/infra/abi/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1496,7 +1496,7 @@ func (ic *ContainerEngine) ContainerRename(ctx context.Context, nameOrID string,
func (ic *ContainerEngine) ContainerClone(ctx context.Context, ctrCloneOpts entities.ContainerCloneOptions) (*entities.ContainerCreateReport, error) {
spec := specgen.NewSpecGenerator(ctrCloneOpts.Image, ctrCloneOpts.CreateOpts.RootFS)
var c *libpod.Container
c, err := generate.ConfigToSpec(ic.Libpod, spec, ctrCloneOpts.ID)
c, _, err := generate.ConfigToSpec(ic.Libpod, spec, ctrCloneOpts.ID)
if err != nil {
return nil, err
}
Expand Down
81 changes: 75 additions & 6 deletions pkg/specgen/generate/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package generate
import (
"context"
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"
"time"

Expand All @@ -15,6 +17,7 @@ import (
envLib "github.com/containers/podman/v4/pkg/env"
"github.com/containers/podman/v4/pkg/signal"
"github.com/containers/podman/v4/pkg/specgen"
"github.com/containers/podman/v4/pkg/specgenutil"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -338,10 +341,10 @@ func FinishThrottleDevices(s *specgen.SpecGenerator) error {
}

// ConfigToSpec takes a completed container config and converts it back into a specgenerator for purposes of cloning an exisiting container
func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, contaierID string) (*libpod.Container, error) {
func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, contaierID string) (*libpod.Container, *libpod.InfraInherit, error) {
c, err := rt.LookupContainer(contaierID)
if err != nil {
return nil, err
return nil, nil, err
}
conf := c.Config()

Expand All @@ -351,17 +354,22 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, contaierID s
conf.Systemd = nil
conf.Mounts = []string{}

if specg == nil {
specg = &specgen.SpecGenerator{}
}

specg.Pod = conf.Pod

matching, err := json.Marshal(conf)
if err != nil {
return nil, err
return nil, nil, err
}

err = json.Unmarshal(matching, specg)
if err != nil {
return nil, err
return nil, nil, err
}

conf.Systemd = tmpSystemd
conf.Mounts = tmpMounts

Expand Down Expand Up @@ -481,7 +489,68 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, contaierID s
}
}
specg.OverlayVolumes = overlay
specg.Mounts = conf.Spec.Mounts
name, mounts := c.SortUserVolumes(c.Spec())
tempInspect, err := c.GetInspectMounts(name, conf.ImageVolumes, mounts)
if err != nil {
return nil, nil, err
}
mountsToPass := []string{}
for _, val := range tempInspect {
mountsToPass = append(mountsToPass, fmt.Sprintf("type=%v,src=%v,target=%v,%v", val.Type, val.Source, val.Destination, strings.Join(val.Options, ",")))
}
m, _, _, err := specgenutil.GetMounts(mountsToPass)

// a lot of options get added later and are hard to translate.
// weed those out here
for err != nil && strings.Contains(err.Error(), "invalid mount option") {
errString := err.Error()
errString = errString[:strings.LastIndex(errString, ":")]
for i, val := range mountsToPass {
if strings.Contains(val, errString) {
ind := strings.Index(mountsToPass[i], errString) - 1
firstH := mountsToPass[i][0:ind]
secondH := mountsToPass[i][ind+len(errString)+1:]
mountsToPass[i] = firstH + secondH
}
}
m, _, _, err = specgenutil.GetMounts(mountsToPass)
}
if err != nil {
return nil, nil, err
}
newMounts := make([]spec.Mount, 0, len(m))
for _, mount := range m {
if mount.Type == define.TypeBind {
absSrc, err := filepath.Abs(mount.Source)
if err != nil {
return nil, nil, errors.Wrapf(err, "could not get path of %s", mount.Source)
}
mount.Source = absSrc
}
newMounts = append(newMounts, mount)
}
specg.Mounts = newMounts
specg.HostDeviceList = conf.DeviceHostSrc
return c, nil
mapSecurityConfig(conf, specg)

if c.IsInfra() { // if we are creating this spec for a pod's infra ctr, map the compatible options
spec, err := json.Marshal(specg)
if err != nil {
return nil, nil, err
}
infraInherit := &libpod.InfraInherit{}
err = json.Unmarshal(spec, infraInherit)
return c, infraInherit, err
}
// else just return the container
return c, nil, nil
}

// mapSecurityConfig takes a libpod.ContainerSecurityConfig and converts it to a specgen.ContinerSecurityConfig
func mapSecurityConfig(c *libpod.ContainerConfig, s *specgen.SpecGenerator) {
s.Privileged = c.Privileged
s.SelinuxOpts = append(s.SelinuxOpts, c.LabelOpts...)
s.User = c.User
s.Groups = c.Groups
s.HostUsers = c.HostUsers
}
34 changes: 13 additions & 21 deletions pkg/specgen/generate/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener
compatibleOptions := &libpod.InfraInherit{}
var infraSpec *spec.Spec
if infra != nil {
options, infraSpec, compatibleOptions, err = Inherit(*infra)
options, infraSpec, compatibleOptions, err = Inherit(*infra, s, rt)
if err != nil {
return nil, nil, nil, err
}
Expand Down Expand Up @@ -152,8 +152,8 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener
return nil, nil, nil, err
}

infraVolumes := (len(compatibleOptions.InfraVolumes) > 0 || len(compatibleOptions.InfraUserVolumes) > 0 || len(compatibleOptions.InfraImageVolumes) > 0)
opts, err := createContainerOptions(ctx, rt, s, pod, finalVolumes, finalOverlays, imageData, command, infraVolumes, *compatibleOptions)
infraVol := (len(compatibleOptions.Mounts) > 0 || len(compatibleOptions.Volumes) > 0 || len(compatibleOptions.ImageVolumes) > 0 || len(compatibleOptions.OverlayVolumes) > 0)
opts, err := createContainerOptions(ctx, rt, s, pod, finalVolumes, finalOverlays, imageData, command, infraVol, *compatibleOptions)
if err != nil {
return nil, nil, nil, err
}
Expand Down Expand Up @@ -437,7 +437,7 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen.
if len(s.SelinuxOpts) > 0 {
options = append(options, libpod.WithSecLabels(s.SelinuxOpts))
} else {
if pod != nil && len(compatibleOptions.InfraLabels) == 0 {
if pod != nil && len(compatibleOptions.SelinuxOpts) == 0 {
// duplicate the security options from the pod
processLabel, err := pod.ProcessLabel()
if err != nil {
Expand Down Expand Up @@ -535,32 +535,24 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen.
return options, nil
}

func Inherit(infra libpod.Container) (opts []libpod.CtrCreateOption, infraS *spec.Spec, compat *libpod.InfraInherit, err error) {
func Inherit(infra libpod.Container, s *specgen.SpecGenerator, rt *libpod.Runtime) (opts []libpod.CtrCreateOption, infraS *spec.Spec, compat *libpod.InfraInherit, err error) {
inheritSpec := &specgen.SpecGenerator{}
_, compatibleOptions, err := ConfigToSpec(rt, inheritSpec, infra.ID())
if err != nil {
return nil, nil, nil, err
}
options := []libpod.CtrCreateOption{}
compatibleOptions := &libpod.InfraInherit{}
infraConf := infra.Config()
infraSpec := infraConf.Spec

config, err := json.Marshal(infraConf)
var compatByte []byte
compatByte, err = json.Marshal(compatibleOptions)
if err != nil {
return nil, nil, nil, err
}
err = json.Unmarshal(config, compatibleOptions)
err = json.Unmarshal(compatByte, s)
if err != nil {
return nil, nil, nil, err
}
if infraSpec.Linux != nil && infraSpec.Linux.Resources != nil {
resources, err := json.Marshal(infraSpec.Linux.Resources)
if err != nil {
return nil, nil, nil, err
}
err = json.Unmarshal(resources, &compatibleOptions.InfraResources)
if err != nil {
return nil, nil, nil, err
}
}
if compatibleOptions != nil {
options = append(options, libpod.WithInfraConfig(*compatibleOptions))
}
return options, infraSpec, compatibleOptions, nil
}
4 changes: 2 additions & 2 deletions pkg/specgen/generate/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
return nil, err
}
}
if len(compatibleOptions.InfraDevices) > 0 && len(s.Devices) == 0 {
userDevices = compatibleOptions.InfraDevices
if len(compatibleOptions.HostDeviceList) > 0 && len(s.Devices) == 0 {
userDevices = compatibleOptions.HostDeviceList
} else {
userDevices = s.Devices
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/specgenutil/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ var (
// TODO: handle options parsing/processing via containers/storage/pkg/mount
func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bool) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, []*specgen.ImageVolume, error) {
// Get mounts from the --mounts flag.
unifiedMounts, unifiedVolumes, unifiedImageVolumes, err := getMounts(mountFlag)
unifiedMounts, unifiedVolumes, unifiedImageVolumes, err := GetMounts(mountFlag)
if err != nil {
return nil, nil, nil, nil, err
}
Expand Down Expand Up @@ -172,7 +172,7 @@ func findMountType(input string) (mountType string, tokens []string, err error)
// podman run --mount type=bind,src=/etc/resolv.conf,target=/etc/resolv.conf ...
// podman run --mount type=tmpfs,target=/dev/shm ...
// podman run --mount type=volume,source=test-volume, ...
func getMounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.NamedVolume, map[string]*specgen.ImageVolume, error) {
func GetMounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.NamedVolume, map[string]*specgen.ImageVolume, error) {
finalMounts := make(map[string]spec.Mount)
finalNamedVolumes := make(map[string]*specgen.NamedVolume)
finalImageVolumes := make(map[string]*specgen.ImageVolume)
Expand Down Expand Up @@ -466,6 +466,7 @@ func getTmpfsMount(args []string) (spec.Mount, error) {
// Since Docker ignores this option so shall we.
continue
default:
//fmt.Println()
return newMount, errors.Wrapf(util.ErrBadMntOption, "%s", kv[0])
}
}
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/pod_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,10 @@ ENTRYPOINT ["sleep","99999"]
ctr3 := podmanTest.Podman([]string{"run", "--pod", podName, ALPINE, "cat", "/tmp1/test"})
ctr3.WaitWithDefaultTimeout()
Expect(ctr3.OutputToString()).To(ContainSubstring("hello"))

ctr4 := podmanTest.Podman([]string{"run", "--pod", podName, ALPINE, "touch", "/tmp1/testing.txt"})
ctr4.WaitWithDefaultTimeout()
Expect(ctr4).Should(Exit(0))
})

It("podman pod create --device", func() {
Expand Down

0 comments on commit aa5cac8

Please sign in to comment.