Merge pull request #13594 from cdoern/podVolumes

fix pod volume passing and alter infra inheritance

libpod/container_config.go

@@ -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"
 )
@@ -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
+	ApparmorProfile    string                   `json:"apparmor_profile,omitempty"`
-	InfraLabels       []string                  `json:"labelopts,omitempty"`
+	CapAdd             []string                 `json:"cap_add,omitempty"`
-	InfraVolumes      []*ContainerNamedVolume   `json:"namedVolumes,omitempty"`
+	CapDrop            []string                 `json:"cap_drop,omitempty"`
-	InfraOverlay      []*ContainerOverlayVolume `json:"overlayVolumes,omitempty"`
+	HostDeviceList     []spec.LinuxDevice       `json:"host_device_list,omitempty"`
-	InfraImageVolumes []*ContainerImageVolume   `json:"ctrImageVolumes,omitempty"`
+	ImageVolumes       []*specgen.ImageVolume   `json:"image_volumes,omitempty"`
-	InfraUserVolumes  []string                  `json:"userVolumes,omitempty"`
+	InfraResources     *spec.LinuxResources     `json:"resource_limits,omitempty"`
-	InfraResources    *spec.LinuxResources      `json:"resources,omitempty"`
+	Mounts             []spec.Mount             `json:"mounts,omitempty"`
-	InfraDevices      []spec.LinuxDevice        `json:"device_host_src,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"`
 }

libpod/container_inspect.go

@@ -103,8 +103,8 @@ 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)
+	inspectMounts, err := c.GetMounts(namedVolumes, c.config.ImageVolumes, mounts)
 	if err != nil {
 		return nil, err
 	}
@@ -222,7 +222,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
 // Get inspect-formatted mounts list.
 // Only includes user-specified mounts. Only includes bind mounts and named
 // volumes, not tmpfs volumes.
-func (c *Container) GetInspectMounts(namedVolumes []*ContainerNamedVolume, imageVolumes []*ContainerImageVolume, mounts []spec.Mount) ([]define.InspectMount, error) {
+func (c *Container) GetMounts(namedVolumes []*ContainerNamedVolume, imageVolumes []*ContainerImageVolume, mounts []spec.Mount) ([]define.InspectMount, error) {
 	inspectMounts := []define.InspectMount{}
 
 	// No mounts, return early

libpod/container_internal.go

@@ -2235,9 +2235,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{}
 

libpod/kube.go

@@ -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)

libpod/pod_api.go

@@ -602,8 +602,8 @@ 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)
+		inspectMounts, err = infra.GetMounts(namedVolumes, infra.config.ImageVolumes, mounts)
 		infraSecurity = infra.GetSecurityOptions()
 		if err != nil {
 			return nil, err

pkg/domain/infra/abi/containers.go

@@ -1491,7 +1491,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
 	}

pkg/specgen/generate/container.go

@@ -337,11 +337,11 @@ func FinishThrottleDevices(s *specgen.SpecGenerator) error {
 	return nil
 }
 
-// ConfigToSpec takes a completed container config and converts it back into a specgenerator for purposes of cloning an existing container
+// 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, containerID string) (*libpod.Container, error) {
+func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, contaierID string) (*libpod.Container, *libpod.InfraInherit, error) {
-	c, err := rt.LookupContainer(containerID)
+	c, err := rt.LookupContainer(contaierID)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	conf := c.Config()
 
@@ -351,17 +351,22 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, containerID
 	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
 
@@ -481,7 +486,29 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, containerID
 		}
 	}
 	specg.OverlayVolumes = overlay
-	specg.Mounts = conf.Spec.Mounts
+	_, mounts := c.SortUserVolumes(c.Spec())
+	specg.Mounts = mounts
 	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
 }

pkg/specgen/generate/container_create.go

@@ -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
 		}
@@ -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)
+	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, infraVolumes, *compatibleOptions)
+	opts, err := createContainerOptions(ctx, rt, s, pod, finalVolumes, finalOverlays, imageData, command, infraVol, *compatibleOptions)
 	if err != nil {
 		return nil, nil, nil, err
 	}
@@ -446,7 +446,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 {
@@ -544,32 +544,23 @@ 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)
+	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
 }

pkg/specgen/generate/oci.go

@@ -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 {
+		if len(compatibleOptions.HostDeviceList) > 0 && len(s.Devices) == 0 {
-			userDevices = compatibleOptions.InfraDevices
+			userDevices = compatibleOptions.HostDeviceList
 		} else {
 			userDevices = s.Devices
 		}

pkg/specgenutil/volumes.go

@@ -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 := Mounts(mountFlag)
 	if err != nil {
 		return nil, nil, nil, nil, err
 	}
@@ -167,12 +167,12 @@ func findMountType(input string) (mountType string, tokens []string, err error)
 	return
 }
 
-// getMounts takes user-provided input from the --mount flag and creates OCI
+// Mounts takes user-provided input from the --mount flag and creates OCI
 // spec mounts and Libpod named volumes.
 // 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 Mounts(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)

test/e2e/pod_create_test.go

@@ -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() {