From 38209ef49d2f334ce37bf8179789e540e0024c85 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 6 Sep 2023 15:41:20 +0200 Subject: [PATCH] libpod: refactor platformMakePod signature accept only the resources to be used by the pod, so that the function can more easily be used by a successive patch. Signed-off-by: Giuseppe Scrivano --- libpod/runtime_pod_common.go | 6 +++- libpod/runtime_pod_freebsd.go | 6 ++-- libpod/runtime_pod_linux.go | 59 +++++++++++++++++------------------ 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/libpod/runtime_pod_common.go b/libpod/runtime_pod_common.go index d9f1f5276b..4e1b1b7e71 100644 --- a/libpod/runtime_pod_common.go +++ b/libpod/runtime_pod_common.go @@ -58,9 +58,13 @@ func (r *Runtime) NewPod(ctx context.Context, p specgen.PodSpecGenerator, option pod.valid = true - if err := r.platformMakePod(pod, p); err != nil { + parentCgroup, err := r.platformMakePod(pod, p.ResourceLimits) + if err != nil { return nil, err } + if p.InfraContainerSpec != nil { + p.InfraContainerSpec.CgroupParent = parentCgroup + } if !pod.HasInfraContainer() && pod.SharesNamespaces() { return nil, errors.New("Pods must have an infra container to share namespaces") diff --git a/libpod/runtime_pod_freebsd.go b/libpod/runtime_pod_freebsd.go index eb5315fc1a..7ec9bf026a 100644 --- a/libpod/runtime_pod_freebsd.go +++ b/libpod/runtime_pod_freebsd.go @@ -1,9 +1,9 @@ package libpod import ( - "github.com/containers/podman/v4/pkg/specgen" + spec "github.com/opencontainers/runtime-spec/specs-go" ) -func (r *Runtime) platformMakePod(pod *Pod, p specgen.PodSpecGenerator) error { - return nil +func (r *Runtime) platformMakePod(pod *Pod, resourceLimits *spec.LinuxResources) (string, error) { + return "", nil } diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index 830d9e4ef4..9d4a8641a3 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -10,11 +10,12 @@ import ( "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" + spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) -func (r *Runtime) platformMakePod(pod *Pod, p specgen.PodSpecGenerator) error { +func (r *Runtime) platformMakePod(pod *Pod, resourceLimits *spec.LinuxResources) (string, error) { + cgroupParent := "" // Check Cgroup parent sanity, and set it if it was not set if r.config.Cgroups() != "disabled" { switch r.config.Engine.CgroupManager { @@ -25,32 +26,30 @@ func (r *Runtime) platformMakePod(pod *Pod, p specgen.PodSpecGenerator) error { 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) + 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 - } + cgroupParent = pod.state.CgroupPath + // cgroupfs + rootless = permission denied when creating the cgroup. + if !rootless.IsRootless() { + res, err := GetLimits(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 } } } @@ -63,22 +62,20 @@ func (r *Runtime) platformMakePod(pod *Pod, p specgen.PodSpecGenerator) error { 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) + 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) + cgroupPath, err := systemdSliceFromPath(pod.config.CgroupParent, fmt.Sprintf("libpod_pod_%s", pod.ID()), resourceLimits) if err != nil { - return fmt.Errorf("unable to create pod cgroup for pod %s: %w", pod.ID(), err) + 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 - } + cgroupParent = pod.state.CgroupPath } default: - return fmt.Errorf("unsupported Cgroup manager: %s - cannot validate cgroup parent: %w", r.config.Engine.CgroupManager, define.ErrInvalidArg) + return "", fmt.Errorf("unsupported Cgroup manager: %s - cannot validate cgroup parent: %w", r.config.Engine.CgroupManager, define.ErrInvalidArg) } } @@ -86,5 +83,5 @@ func (r *Runtime) platformMakePod(pod *Pod, p specgen.PodSpecGenerator) error { logrus.Debugf("Got pod cgroup as %s", pod.state.CgroupPath) } - return nil + return cgroupParent, nil }