Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions cmd/podman/pods/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -52,6 +53,7 @@ var (
podIDFile string
replace bool
share string
shareParent bool
)

func init() {
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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")
Expand Down
8 changes: 7 additions & 1 deletion docs/source/markdown/podman-pod-create.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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”)
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion libpod/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/handlers/libpod/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "error filling out specgen"))
Expand Down
2 changes: 2 additions & 0 deletions pkg/domain/entities/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion pkg/specgen/generate/namespaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions pkg/specgen/generate/pod_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/specgen/namespaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/specgen/podspecgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions test/e2e/pod_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

})

})
2 changes: 1 addition & 1 deletion test/system/200-pod.bats
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,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"
}
Expand Down