From e187b9711d92d6229d139df123143eeaf282fa2f Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Thu, 29 Sep 2022 18:11:30 +0100 Subject: [PATCH 1/5] libpod: Factor out cgroups handling from (*Pod).refresh This moves the cgroup code to pod_internal_linux.go and adds a no-op stub for FreeBSD. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- libpod/pod_internal.go | 24 ++---------------------- libpod/pod_internal_freebsd.go | 5 +++++ libpod/pod_internal_linux.go | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 22 deletions(-) create mode 100644 libpod/pod_internal_freebsd.go create mode 100644 libpod/pod_internal_linux.go diff --git a/libpod/pod_internal.go b/libpod/pod_internal.go index d63bff2c99..743280f49f 100644 --- a/libpod/pod_internal.go +++ b/libpod/pod_internal.go @@ -2,14 +2,10 @@ package libpod import ( "fmt" - "path/filepath" "time" - "github.com/containers/common/pkg/config" "github.com/containers/podman/v4/libpod/define" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/storage/pkg/stringid" - "github.com/sirupsen/logrus" ) // Creates a new, empty pod @@ -64,24 +60,8 @@ func (p *Pod) refresh() error { } p.lock = lock - // We need to recreate the pod's cgroup - if p.config.UsePodCgroup { - switch p.runtime.config.Engine.CgroupManager { - case config.SystemdCgroupsManager: - cgroupPath, err := systemdSliceFromPath(p.config.CgroupParent, fmt.Sprintf("libpod_pod_%s", p.ID()), p.ResourceLim()) - if err != nil { - logrus.Errorf("Creating Cgroup for pod %s: %v", p.ID(), err) - } - p.state.CgroupPath = cgroupPath - case config.CgroupfsCgroupsManager: - if rootless.IsRootless() && isRootlessCgroupSet(p.config.CgroupParent) { - p.state.CgroupPath = filepath.Join(p.config.CgroupParent, p.ID()) - - logrus.Debugf("setting pod cgroup to %s", p.state.CgroupPath) - } - default: - return fmt.Errorf("unknown cgroups manager %s specified: %w", p.runtime.config.Engine.CgroupManager, define.ErrInvalidArg) - } + if err := p.platformRefresh(); err != nil { + return err } // Save changes diff --git a/libpod/pod_internal_freebsd.go b/libpod/pod_internal_freebsd.go new file mode 100644 index 0000000000..48f6e2bcf8 --- /dev/null +++ b/libpod/pod_internal_freebsd.go @@ -0,0 +1,5 @@ +package libpod + +func (p *Pod) platformRefresh() error { + return nil +} diff --git a/libpod/pod_internal_linux.go b/libpod/pod_internal_linux.go new file mode 100644 index 0000000000..fa85666ca1 --- /dev/null +++ b/libpod/pod_internal_linux.go @@ -0,0 +1,34 @@ +package libpod + +import ( + "fmt" + "path/filepath" + + "github.com/containers/common/pkg/config" + "github.com/containers/podman/v4/libpod/define" + "github.com/containers/podman/v4/pkg/rootless" + "github.com/sirupsen/logrus" +) + +func (p *Pod) platformRefresh() error { + // We need to recreate the pod's cgroup + if p.config.UsePodCgroup { + switch p.runtime.config.Engine.CgroupManager { + case config.SystemdCgroupsManager: + cgroupPath, err := systemdSliceFromPath(p.config.CgroupParent, fmt.Sprintf("libpod_pod_%s", p.ID()), p.ResourceLim()) + if err != nil { + logrus.Errorf("Creating Cgroup for pod %s: %v", p.ID(), err) + } + p.state.CgroupPath = cgroupPath + case config.CgroupfsCgroupsManager: + if rootless.IsRootless() && isRootlessCgroupSet(p.config.CgroupParent) { + p.state.CgroupPath = filepath.Join(p.config.CgroupParent, p.ID()) + + logrus.Debugf("setting pod cgroup to %s", p.state.CgroupPath) + } + default: + return fmt.Errorf("unknown cgroups manager %s specified: %w", p.runtime.config.Engine.CgroupManager, define.ErrInvalidArg) + } + } + return nil +} From c35a70d21196f43eea84a1ea7da3e92d56479d8b Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Sun, 2 Oct 2022 08:13:19 +0100 Subject: [PATCH 2/5] specgen/generate: Avoid a nil dereference in MakePod The value of p.PodSpecGen.InfraContainerSpec.ResourceLimits can be nil on FreeBSD. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- pkg/specgen/generate/pod_create.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 14d390e49f..b99a5457ca 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -49,10 +49,10 @@ func MakePod(p *entities.PodSpec, rt *libpod.Runtime) (*libpod.Pod, error) { if err != nil { return nil, err } - if p.PodSpecGen.InfraContainerSpec.ResourceLimits.BlockIO != nil { + if p.PodSpecGen.InfraContainerSpec.ResourceLimits != nil && + p.PodSpecGen.InfraContainerSpec.ResourceLimits.BlockIO != nil { p.PodSpecGen.ResourceLimits.BlockIO = p.PodSpecGen.InfraContainerSpec.ResourceLimits.BlockIO } - err = specgen.WeightDevices(p.PodSpecGen.InfraContainerSpec) if err != nil { return nil, err From d71160539d8c2978676dcb539425c36057dbf9e8 Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Sun, 2 Oct 2022 08:23:37 +0100 Subject: [PATCH 3/5] libpod: Move runtime_pod_linux.go to runtime_pod_common.go Most of the code can be shared with other unix-like platforms. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- libpod/{runtime_pod_linux.go => runtime_pod_common.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename libpod/{runtime_pod_linux.go => runtime_pod_common.go} (100%) diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_common.go similarity index 100% rename from libpod/runtime_pod_linux.go rename to libpod/runtime_pod_common.go From 7f8964a78f90aca610dfca5b962b6839acbd745e Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Sun, 2 Oct 2022 14:24:31 +0100 Subject: [PATCH 4/5] libpod: Factor out cgroup validation from (*Runtime).NewPod This moves the code to runtime_pod_linux.go since cgroups are platform-specific. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- libpod/runtime_pod_common.go | 74 +---------------------------- libpod/runtime_pod_linux.go | 90 ++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 72 deletions(-) create mode 100644 libpod/runtime_pod_linux.go diff --git a/libpod/runtime_pod_common.go b/libpod/runtime_pod_common.go index 24e9f3da71..b108d00906 100644 --- a/libpod/runtime_pod_common.go +++ b/libpod/runtime_pod_common.go @@ -7,15 +7,12 @@ import ( "context" "errors" "fmt" - "path" "path/filepath" - "strings" "github.com/containers/common/pkg/cgroups" "github.com/containers/common/pkg/config" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/specgen" "github.com/hashicorp/go-multierror" "github.com/sirupsen/logrus" @@ -59,75 +56,8 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option pod.valid = true - // Check Cgroup parent sanity, and set it if it was not set - if r.config.Cgroups() != "disabled" { - switch r.config.Engine.CgroupManager { - case config.CgroupfsCgroupsManager: - canUseCgroup := !rootless.IsRootless() || isRootlessCgroupSet(pod.config.CgroupParent) - if canUseCgroup { - // need to actually create parent here - if pod.config.CgroupParent == "" { - pod.config.CgroupParent = CgroupfsDefaultCgroupParent - } else if strings.HasSuffix(path.Base(pod.config.CgroupParent), ".slice") { - return nil, fmt.Errorf("systemd slice received as cgroup parent when using cgroupfs: %w", define.ErrInvalidArg) - } - // If we are set to use pod cgroups, set the cgroup parent that - // all containers in the pod will share - if pod.config.UsePodCgroup { - pod.state.CgroupPath = filepath.Join(pod.config.CgroupParent, pod.ID()) - if p.InfraContainerSpec != nil { - p.InfraContainerSpec.CgroupParent = pod.state.CgroupPath - // cgroupfs + rootless = permission denied when creating the cgroup. - if !rootless.IsRootless() { - res, err := GetLimits(p.ResourceLimits) - if err != nil { - return nil, err - } - // Need to both create and update the cgroup - // rather than create a new path in c/common for pod cgroup creation - // just create as if it is a ctr and then update figures out that we need to - // populate the resource limits on the pod level - cgc, err := cgroups.New(pod.state.CgroupPath, &res) - if err != nil { - return nil, err - } - err = cgc.Update(&res) - if err != nil { - return nil, err - } - } - } - } - } - case config.SystemdCgroupsManager: - if pod.config.CgroupParent == "" { - if rootless.IsRootless() { - pod.config.CgroupParent = SystemdDefaultRootlessCgroupParent - } else { - pod.config.CgroupParent = SystemdDefaultCgroupParent - } - } else if len(pod.config.CgroupParent) < 6 || !strings.HasSuffix(path.Base(pod.config.CgroupParent), ".slice") { - return nil, fmt.Errorf("did not receive systemd slice as cgroup parent when using systemd to manage cgroups: %w", define.ErrInvalidArg) - } - // If we are set to use pod cgroups, set the cgroup parent that - // all containers in the pod will share - if pod.config.UsePodCgroup { - cgroupPath, err := systemdSliceFromPath(pod.config.CgroupParent, fmt.Sprintf("libpod_pod_%s", pod.ID()), p.ResourceLimits) - if err != nil { - return nil, fmt.Errorf("unable to create pod cgroup for pod %s: %w", pod.ID(), err) - } - pod.state.CgroupPath = cgroupPath - if p.InfraContainerSpec != nil { - p.InfraContainerSpec.CgroupParent = pod.state.CgroupPath - } - } - default: - return nil, fmt.Errorf("unsupported Cgroup manager: %s - cannot validate cgroup parent: %w", r.config.Engine.CgroupManager, define.ErrInvalidArg) - } - } - - if pod.config.UsePodCgroup { - logrus.Debugf("Got pod cgroup as %s", pod.state.CgroupPath) + if err := r.platformMakePod(pod, p); err != nil { + return nil, err } if !pod.HasInfraContainer() && pod.SharesNamespaces() { diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go new file mode 100644 index 0000000000..830d9e4ef4 --- /dev/null +++ b/libpod/runtime_pod_linux.go @@ -0,0 +1,90 @@ +package libpod + +import ( + "fmt" + "path" + "path/filepath" + "strings" + + "github.com/containers/common/pkg/cgroups" + "github.com/containers/common/pkg/config" + "github.com/containers/podman/v4/libpod/define" + "github.com/containers/podman/v4/pkg/rootless" + "github.com/containers/podman/v4/pkg/specgen" + "github.com/sirupsen/logrus" +) + +func (r *Runtime) platformMakePod(pod *Pod, p specgen.PodSpecGenerator) error { + // Check Cgroup parent sanity, and set it if it was not set + if r.config.Cgroups() != "disabled" { + switch r.config.Engine.CgroupManager { + case config.CgroupfsCgroupsManager: + canUseCgroup := !rootless.IsRootless() || isRootlessCgroupSet(pod.config.CgroupParent) + if canUseCgroup { + // need to actually create parent here + if pod.config.CgroupParent == "" { + pod.config.CgroupParent = CgroupfsDefaultCgroupParent + } else if strings.HasSuffix(path.Base(pod.config.CgroupParent), ".slice") { + return fmt.Errorf("systemd slice received as cgroup parent when using cgroupfs: %w", define.ErrInvalidArg) + } + // If we are set to use pod cgroups, set the cgroup parent that + // all containers in the pod will share + if pod.config.UsePodCgroup { + pod.state.CgroupPath = filepath.Join(pod.config.CgroupParent, pod.ID()) + if p.InfraContainerSpec != nil { + p.InfraContainerSpec.CgroupParent = pod.state.CgroupPath + // cgroupfs + rootless = permission denied when creating the cgroup. + if !rootless.IsRootless() { + res, err := GetLimits(p.ResourceLimits) + if err != nil { + return err + } + // Need to both create and update the cgroup + // rather than create a new path in c/common for pod cgroup creation + // just create as if it is a ctr and then update figures out that we need to + // populate the resource limits on the pod level + cgc, err := cgroups.New(pod.state.CgroupPath, &res) + if err != nil { + return err + } + err = cgc.Update(&res) + if err != nil { + return err + } + } + } + } + } + case config.SystemdCgroupsManager: + if pod.config.CgroupParent == "" { + if rootless.IsRootless() { + pod.config.CgroupParent = SystemdDefaultRootlessCgroupParent + } else { + pod.config.CgroupParent = SystemdDefaultCgroupParent + } + } else if len(pod.config.CgroupParent) < 6 || !strings.HasSuffix(path.Base(pod.config.CgroupParent), ".slice") { + return fmt.Errorf("did not receive systemd slice as cgroup parent when using systemd to manage cgroups: %w", define.ErrInvalidArg) + } + // If we are set to use pod cgroups, set the cgroup parent that + // all containers in the pod will share + if pod.config.UsePodCgroup { + cgroupPath, err := systemdSliceFromPath(pod.config.CgroupParent, fmt.Sprintf("libpod_pod_%s", pod.ID()), p.ResourceLimits) + if err != nil { + return fmt.Errorf("unable to create pod cgroup for pod %s: %w", pod.ID(), err) + } + pod.state.CgroupPath = cgroupPath + if p.InfraContainerSpec != nil { + p.InfraContainerSpec.CgroupParent = pod.state.CgroupPath + } + } + default: + return fmt.Errorf("unsupported Cgroup manager: %s - cannot validate cgroup parent: %w", r.config.Engine.CgroupManager, define.ErrInvalidArg) + } + } + + if pod.config.UsePodCgroup { + logrus.Debugf("Got pod cgroup as %s", pod.state.CgroupPath) + } + + return nil +} From b4b701139280fe0575c3a03497bb035c1a45064e Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Sun, 2 Oct 2022 14:25:57 +0100 Subject: [PATCH 5/5] libpod: Add support for 'podman pod' on FreeBSD [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- libpod/runtime_pod_common.go | 4 ++-- libpod/runtime_pod_freebsd.go | 9 +++++++++ libpod/runtime_pod_unsupported.go | 4 ++-- libpod/util_freebsd.go | 5 +++++ 4 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 libpod/runtime_pod_freebsd.go diff --git a/libpod/runtime_pod_common.go b/libpod/runtime_pod_common.go index b108d00906..9ca85d9a74 100644 --- a/libpod/runtime_pod_common.go +++ b/libpod/runtime_pod_common.go @@ -1,5 +1,5 @@ -//go:build linux -// +build linux +//go:build linux || freebsd +// +build linux freebsd package libpod diff --git a/libpod/runtime_pod_freebsd.go b/libpod/runtime_pod_freebsd.go new file mode 100644 index 0000000000..eb5315fc1a --- /dev/null +++ b/libpod/runtime_pod_freebsd.go @@ -0,0 +1,9 @@ +package libpod + +import ( + "github.com/containers/podman/v4/pkg/specgen" +) + +func (r *Runtime) platformMakePod(pod *Pod, p specgen.PodSpecGenerator) error { + return nil +} diff --git a/libpod/runtime_pod_unsupported.go b/libpod/runtime_pod_unsupported.go index 0c7ff8655a..1e32d3d513 100644 --- a/libpod/runtime_pod_unsupported.go +++ b/libpod/runtime_pod_unsupported.go @@ -1,5 +1,5 @@ -//go:build !linux -// +build !linux +//go:build !linux && !freebsd +// +build !linux,!freebsd package libpod diff --git a/libpod/util_freebsd.go b/libpod/util_freebsd.go index 72019743c0..894fa502b4 100644 --- a/libpod/util_freebsd.go +++ b/libpod/util_freebsd.go @@ -19,6 +19,11 @@ func systemdSliceFromPath(parent, name string, resources *spec.LinuxResources) ( return "", errors.New("not implemented systemdSliceFromPath") } +// deleteSystemdCgroup deletes the systemd cgroup at the given location +func deleteSystemdCgroup(path string, resources *spec.LinuxResources) error { + return nil +} + // No equivalent on FreeBSD? func LabelVolumePath(path string) error { return nil