Podman pod create --share-parent vs --share=cgroup

separated cgroupNS sharing from setting the pod as the cgroup parent, made a new flag --share-parent which sets the pod as the cgroup parent for all containers entering the pod remove cgroup from the default kernel namespaces since we want the same default behavior as before which is just the cgroup parent. resolves containers#12765 Signed-off-by: cdoern <[email protected]> Signed-off-by: cdoern <[email protected]> Signed-off-by: cdoern <[email protected]>
mheon · Feb 10, 2022 · b1bf91a · b1bf91a
1 parent 85c0fe7
commit b1bf91a
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 5 deletions.
diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go
@@ -17,6 +17,7 @@ import (
 	"github.com/containers/podman/v4/cmd/podman/parse"
 	"github.com/containers/podman/v4/cmd/podman/registry"
 	"github.com/containers/podman/v4/cmd/podman/validate"
+	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/pkg/domain/entities"
 	"github.com/containers/podman/v4/pkg/errorhandling"
 	"github.com/containers/podman/v4/pkg/specgen"
@@ -52,6 +53,7 @@ var (
 	podIDFile         string
 	replace           bool
 	share             string
+	shareParent       bool
 )
 
 func init() {
@@ -88,6 +90,9 @@ func init() {
 	flags.StringVar(&share, shareFlagName, specgen.DefaultKernelNamespaces, "A comma delimited list of kernel namespaces the pod will share")
 	_ = createCommand.RegisterFlagCompletionFunc(shareFlagName, common.AutocompletePodShareNamespace)
 
+	shareParentFlagName := "share-parent"
+	flags.BoolVar(&shareParent, shareParentFlagName, true, "Set the pod's cgroup as the cgroup parent for all containers joining the pod")
+
 	flags.SetNormalizeFunc(aliasNetworkFlag)
 }
 
@@ -147,7 +152,11 @@ func create(cmd *cobra.Command, args []string) error {
 		if err != nil {
 			return err
 		}
+		if strings.Contains(share, "cgroup") && shareParent {
+			return errors.Wrapf(define.ErrInvalidArg, "cannot define the pod as the cgroup parent at the same time as joining the infra container's cgroupNS")
+		}
 		createOptions.Share = strings.Split(share, ",")
+		createOptions.ShareParent = &shareParent
 		if cmd.Flag("infra-command").Changed {
 			// Only send content to server side if user changed defaults
 			cmdIn, err := cmd.Flags().GetString("infra-command")

diff --git a/docs/source/markdown/podman-pod-create.1.md b/docs/source/markdown/podman-pod-create.1.md
@@ -265,7 +265,7 @@ Note: Labeling can be disabled for all containers by setting label=false in the
 
 #### **--share**=*namespace*
 
-A comma-separated list of kernel namespaces to share. If none or "" is specified, no namespaces will be shared. The namespaces to choose from are ipc, net, pid, uts.
+A comma-separated list of kernel namespaces to share. If none or "" is specified, no namespaces will be shared. The namespaces to choose from are cgroup, ipc, net, pid, uts.
 
 The operator can identify a pod in three ways:
 UUID long identifier (“f78375b1c487e03c9438c729345e54db9d20cfa2ac1fc3494b6eb60872e74778”)
@@ -276,6 +276,12 @@ podman generates a UUID for each pod, and if a name is not assigned
 to the container with **--name** then a random string name will be generated
 for it. The name is useful any place you need to identify a pod.
 
+#### **--share-parent**
+
+This boolean determines whether or not all containers entering the pod will use the pod as their cgroup parent. The default value of this flag is true. If you are looking to share the cgroup namespace rather than a cgroup parent in a pod, use **--share**
+
+Note: This options conflict with **--share=cgroup** since that would set the pod as the cgroup parent but enter the container into the same cgroupNS as the infra container.
+
 #### **--sysctl**=_name_=_value_
 
 Configure namespace kernel parameters for all containers in the pod.

diff --git a/libpod/options.go b/libpod/options.go
@@ -1865,7 +1865,7 @@ func WithPodCgroupParent(path string) PodCreateOption {
 // this pod.
 // This can still be overridden at the container level by explicitly specifying
 // a Cgroup parent.
-func WithPodCgroups() PodCreateOption {
+func WithPodParent() PodCreateOption {
 	return func(pod *Pod) error {
 		if pod.valid {
 			return define.ErrPodFinalized

diff --git a/pkg/api/handlers/libpod/pods.go b/pkg/api/handlers/libpod/pods.go
@@ -45,6 +45,10 @@ func PodCreate(w http.ResponseWriter, r *http.Request) {
 		infraOptions.Net = &entities.NetOptions{}
 		infraOptions.Devices = psg.Devices
 		infraOptions.SecurityOpt = psg.SecurityOpt
+		if psg.ShareParent == nil {
+			t := true
+			psg.ShareParent = &t
+		}
 		err = specgenutil.FillOutSpecGen(psg.InfraContainerSpec, &infraOptions, []string{}) // necessary for default values in many cases (userns, idmappings)
 		if err != nil {
 			utils.Error(w, http.StatusInternalServerError, errors.Wrap(err, "error filling out specgen"))

diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go
@@ -132,6 +132,7 @@ type PodCreateOptions struct {
 	Name               string            `json:"name,omitempty"`
 	Net                *NetOptions       `json:"net,omitempty"`
 	Share              []string          `json:"share,omitempty"`
+	ShareParent        *bool             `json:"share_parent,omitempty"`
 	Pid                string            `json:"pid,omitempty"`
 	Cpus               float64           `json:"cpus,omitempty"`
 	CpusetCpus         string            `json:"cpuset_cpus,omitempty"`
@@ -324,6 +325,7 @@ func ToPodSpecGen(s specgen.PodSpecGenerator, p *PodCreateOptions) (*specgen.Pod
 	}
 	s.InfraImage = p.InfraImage
 	s.SharedNamespaces = p.Share
+	s.ShareParent = p.ShareParent
 	s.PodCreateCommand = p.CreateCommand
 	s.VolumesFrom = p.VolumesFrom
 

diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go
@@ -482,7 +482,7 @@ func GetNamespaceOptions(ns []string, netnsIsHost bool) ([]libpod.PodCreateOptio
 	for _, toShare := range ns {
 		switch toShare {
 		case "cgroup":
-			options = append(options, libpod.WithPodCgroups())
+			options = append(options, libpod.WithPodCgroup())
 		case "net":
 			// share the netns setting with other containers in the pod only when it is not set to host
 			if !netnsIsHost {

diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go
@@ -166,6 +166,9 @@ func createPodOptions(p *specgen.PodSpecGenerator, rt *libpod.Runtime, infraSpec
 	)
 	if !p.NoInfra { //&& infraSpec != nil {
 		options = append(options, libpod.WithInfraContainer())
+		if p.ShareParent == nil || (p.ShareParent != nil && *p.ShareParent) {
+			options = append(options, libpod.WithPodParent())
+		}
 		nsOptions, err := GetNamespaceOptions(p.SharedNamespaces, p.InfraContainerSpec.NetNS.IsHost())
 		if err != nil {
 			return nil, err

diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go
@@ -57,7 +57,7 @@ const (
 
 	// DefaultKernelNamespaces is a comma-separated list of default kernel
 	// namespaces.
-	DefaultKernelNamespaces = "cgroup,ipc,net,uts"
+	DefaultKernelNamespaces = "ipc,net,uts"
 )
 
 // Namespace describes the namespace

diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go
@@ -63,6 +63,8 @@ type PodBasicConfig struct {
 	// also be used by some tools that wish to recreate the pod
 	// (e.g. `podman generate systemd --new`).
 	// Optional.
+	// ShareParent determines if all containers in the pod will share the pod's cgroup as the cgroup parent
+	ShareParent      *bool    `json:"share_parent,omitempty"`
 	PodCreateCommand []string `json:"pod_create_command,omitempty"`
 	// Pid sets the process id namespace of the pod
 	// Optional (defaults to private if unset). This sets the PID namespace of the infra container

diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go
@@ -1068,4 +1068,47 @@ ENTRYPOINT ["sleep","99999"]
 
 	})
 
+	It("podman pod create --share-parent test", func() {
+		SkipIfRootlessCgroupsV1("rootless cannot use cgroups with cgroupsv1")
+		podCreate := podmanTest.Podman([]string{"pod", "create", "--share-parent=false"})
+		podCreate.WaitWithDefaultTimeout()
+		Expect(podCreate).Should(Exit(0))
+
+		ctrCreate := podmanTest.Podman([]string{"run", "-dt", "--pod", podCreate.OutputToString(), ALPINE})
+		ctrCreate.WaitWithDefaultTimeout()
+		Expect(ctrCreate).Should(Exit(0))
+
+		inspectPod := podmanTest.Podman([]string{"pod", "inspect", podCreate.OutputToString()})
+		inspectPod.WaitWithDefaultTimeout()
+		Expect(inspectPod).Should(Exit(0))
+		data := inspectPod.InspectPodToJSON()
+
+		inspect := podmanTest.InspectContainer(ctrCreate.OutputToString())
+		Expect(data.CgroupPath).To(HaveLen(0))
+		if podmanTest.CgroupManager == "cgroupfs" || !rootless.IsRootless() {
+			Expect(inspect[0].HostConfig.CgroupParent).To(HaveLen(0))
+		} else if podmanTest.CgroupManager == "systemd" {
+			Expect(inspect[0].HostConfig.CgroupParent).To(Equal("user.slice"))
+		}
+
+		podCreate2 := podmanTest.Podman([]string{"pod", "create", "--share", "cgroup,ipc,net,uts", "--share-parent=false", "--infra-name", "cgroupCtr"})
+		podCreate2.WaitWithDefaultTimeout()
+		Expect(podCreate2).Should(Exit(0))
+
+		ctrCreate2 := podmanTest.Podman([]string{"run", "-dt", "--pod", podCreate2.OutputToString(), ALPINE})
+		ctrCreate2.WaitWithDefaultTimeout()
+		Expect(ctrCreate2).Should(Exit(0))
+
+		inspectInfra := podmanTest.InspectContainer("cgroupCtr")
+
+		inspect2 := podmanTest.InspectContainer(ctrCreate2.OutputToString())
+
+		Expect(inspect2[0].HostConfig.CgroupMode).To(ContainSubstring(inspectInfra[0].ID))
+
+		podCreate3 := podmanTest.Podman([]string{"pod", "create", "--share", "cgroup"})
+		podCreate3.WaitWithDefaultTimeout()
+		Expect(podCreate3).ShouldNot(Exit(0))
+
+	})
+
 })
diff --git a/test/system/200-pod.bats b/test/system/200-pod.bats
@@ -340,7 +340,7 @@ EOF
     run_podman 125 pod create --share bogus --name $pod_name
     is "$output" ".*Invalid kernel namespace to share: bogus. Options are: cgroup, ipc, net, pid, uts or none" \
        "pod test for bogus --share option"
-    run_podman pod create --share cgroup,ipc --name $pod_name
+    run_podman pod create --share ipc --name $pod_name
     run_podman run --rm --pod $pod_name --hostname foobar $IMAGE hostname
     is "$output" "foobar" "--hostname should work with non share UTS namespace"
 }