From 7a5342804944472246ed0b977e9088e0b01be87b Mon Sep 17 00:00:00 2001 From: cdoern Date: Mon, 21 Mar 2022 22:52:50 -0400 Subject: fix pod volume passing and alter infra inheritance 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 #13548 Signed-off-by: cdoern Signed-off-by: cdoern Signed-off-by: cdoern --- libpod/container_config.go | 23 +++++++++++------ libpod/container_inspect.go | 6 ++--- libpod/container_internal.go | 4 +-- libpod/kube.go | 2 +- libpod/pod_api.go | 4 +-- pkg/domain/infra/abi/containers.go | 2 +- pkg/specgen/generate/container.go | 43 ++++++++++++++++++++++++++------ pkg/specgen/generate/container_create.go | 33 +++++++++--------------- pkg/specgen/generate/oci.go | 4 +-- pkg/specgenutil/volumes.go | 6 ++--- test/e2e/pod_create_test.go | 4 +++ 11 files changed, 80 insertions(+), 51 deletions(-) diff --git a/libpod/container_config.go b/libpod/container_config.go index 0d9cd5723..ea644764c 100644 --- a/libpod/container_config.go +++ b/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 - 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"` } diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 5fb32bd90..f2a2c2d16 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -103,8 +103,8 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver } } - namedVolumes, mounts := c.sortUserVolumes(ctrSpec) - inspectMounts, err := c.GetInspectMounts(namedVolumes, c.config.ImageVolumes, mounts) + namedVolumes, mounts := c.SortUserVolumes(ctrSpec) + 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 diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 0db59f2fe..f1f467879 100644 --- a/libpod/container_internal.go +++ b/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{} diff --git a/libpod/kube.go b/libpod/kube.go index a193df2cb..22fbb5f9f 100644 --- a/libpod/kube.go +++ b/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) diff --git a/libpod/pod_api.go b/libpod/pod_api.go index be726d8d1..48049798b 100644 --- a/libpod/pod_api.go +++ b/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) - inspectMounts, err = infra.GetInspectMounts(namedVolumes, infra.config.ImageVolumes, mounts) + namedVolumes, mounts := infra.SortUserVolumes(infra.config.Spec) + inspectMounts, err = infra.GetMounts(namedVolumes, infra.config.ImageVolumes, mounts) infraSecurity = infra.GetSecurityOptions() if err != nil { return nil, err diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index f45bdeba5..a2933a267 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/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 } diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index 118d80e2c..b38b0e695 100644 --- a/pkg/specgen/generate/container.go +++ b/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 -func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, containerID string) (*libpod.Container, error) { - c, err := rt.LookupContainer(containerID) +// 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, *libpod.InfraInherit, error) { + 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 } diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index a014f5047..6a611e854 100644 --- a/pkg/specgen/generate/container_create.go +++ b/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) - 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 } @@ -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 } diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go index 1cc3a463f..961cea933 100644 --- a/pkg/specgen/generate/oci.go +++ b/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 { - userDevices = compatibleOptions.InfraDevices + if len(compatibleOptions.HostDeviceList) > 0 && len(s.Devices) == 0 { + userDevices = compatibleOptions.HostDeviceList } else { userDevices = s.Devices } diff --git a/pkg/specgenutil/volumes.go b/pkg/specgenutil/volumes.go index 2bd79b186..dd7eed2fd 100644 --- a/pkg/specgenutil/volumes.go +++ b/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) diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go index 04e8cfd07..8def80213 100644 --- a/test/e2e/pod_create_test.go +++ b/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() { -- cgit v1.2.3-54-g00ecf