Podman info add support for status of standard available cgroup controllers #10387

flouthoc · 2021-05-18T19:54:53Z

Following PR adds support for reflecting availability status of cgroup controllers to info. Patch attempts to resolve #10306

rhatdan · 2021-05-18T20:03:15Z

libpod/define/info.go

@@ -27,6 +27,9 @@ type HostInfo struct {
 	BuildahVersion string           `json:"buildahVersion"`
 	CgroupManager  string           `json:"cgroupManager"`
 	CGroupsVersion string           `json:"cgroupVersion"`
+	MemoryLimit    bool             `json:"memoryLimit"`


I think this would be better in a subgroup,

CGroupInfo { CPUShares... MemoryLimit ... PidsLimit ... }

And Alphabetized.

@rhatdan Sure makes sense, much cleaner i'll do the required changes.

@rhatdan This is resolved. But format might change as there is a discussion going below for CgroupControllers []string

@rhatdan Made some changes in latest commit , added CgroupControllers []string instead. Could you please review that.

pkg/cgroups/cgroups.go

AkihiroSuda · 2021-05-19T07:04:15Z

libpod/info.go

 	info := define.HostInfo{
 		Arch:           runtime.GOARCH,
 		BuildahVersion: buildah.Version,
 		CgroupManager:  r.config.Engine.CgroupManager,
+		MemoryLimit:    availableControllers["memory"],


Wondering it may make more sense to just return CgroupControllers []string here.

(Docker-compatible REST API should emulate Docker REST API, though)

@AkihiroSuda docker info spits boolean for separate controllers so i tried keeping it same. But i guess a common string for all available controllers would be better. WDYT ?

Yes, for Podman CLI, returning the common string slice SGTM.
Docker-compatible REST API should retain Docker-compatible booleans.

@AkihiroSuda I tried this but dumping all the available controllers is making info too noisy, for instance on my machine even single liner has too much content. I guess docker only prints standard controllers maybe because this is too verbose but i am cool with this.
On my machine info shows this

cgroupControllers: - blkio - cpu - cpu,cpuacct - cpuacct - cpuset - devices - freezer - hugetlb - memory - net_cls - net_cls,net_prio - net_prio - perf_event - pids - rdma - systemd - unified

For cgroup v1, the right way is to parse /proc/cgroups

@AkihiroSuda Would still give the same output as i have all controllers enabled on v1

$ cat /proc/cgroups #subsys_name hierarchy num_cgroups enabled cpuset 0 230 1 cpu 0 230 1 cpuacct 0 230 1 blkio 0 230 1 memory 0 230 1 devices 0 230 1 freezer 0 230 1 net_cls 0 230 1 perf_event 0 230 1 net_prio 0 230 1 hugetlb 0 230 1 pids 0 230 1 rdma 0 230 1

not "same" :)

@AkihiroSuda ah i get it. So i guess logic for existing getAvailableControllers https://github.com/containers/podman/blob/master/pkg/cgroups/cgroups.go#L129 for cgroupv1 needs to be changed or i can add a new function but dont think redundant code for similar task is a good idea. @rhatdan @giuseppe @AkihiroSuda If we can mutually agree upon having CgroupControllers []string in info then i think we can proceed with removing old logic in master and use /proc/cgroups for cgroupv1

cgroupv1 is not really supported for rootless, even if manually chowned as the delegation is unsafe.

For root, we can parse /proc/cgroups or /proc/self/cgroup but for rootless we can just force every controller to be disabled.

@giuseppe @AkihiroSuda Thanks a lot . I have added changes, this is resolved in latest commit

rhatdan · 2021-05-19T20:25:48Z

Please update the man page with the new output.

flouthoc · 2021-05-20T04:10:32Z

@rhatdan Updated Man pages.

AkihiroSuda · 2021-05-20T07:10:55Z

pkg/cgroups/cgroups.go

+		// rootless cgroupv2: check available controllers for current user , systemd or servicescope will inherit
+		if rootless.IsRootless() {
+			uid := rootless.GetRootlessUID()
+			subtreeControl = fmt.Sprintf("%s/user.slice/user-%d.slice/cgroup.subtree_control", cgroupRoot, uid)


IIUC the controllers there are not always available for rootless users.
Reading the cgorup.subtree_control of Podman process itself might be better (it is still not robust, either, but at least better...)

cc @giuseppe @kolyshkin

We cant use subtree created by podman process as it wouldn't exist , if podman info is called prior to creating any container.

also i think podman cleans up its entries as soon as running container exits. Not sure.

yes the problem is that some controllers are not enabled for the rootless user (and we don't really know what controllers are enabled until we do the equivalent of systemd-run --scope --user with systemd). So I agree with @AkihiroSuda that reading the current enabled controllers is a best effort way of doing it, which should be fine in most cases

@giuseppe @AkihiroSuda Thanks a lot . I have added changes, this is resolved in latest commit

flouthoc · 2021-05-22T09:25:44Z

@giuseppe @AkihiroSuda @rhatdan I have addressed all the comments in latest commit. Could you guys please take a look.

flouthoc · 2021-05-22T09:26:06Z

Missing test is a flake i think.

flouthoc · 2021-05-22T15:26:04Z

It was a flake force push solved it. 😃

flouthoc · 2021-05-24T05:37:41Z

yay!!! thanks @AkihiroSuda waiting for @giuseppe and @rhatdan 's approval.

pkg/cgroups/cgroups.go

Signed-off-by: flouthoc <[email protected]>

mheon · 2021-05-24T13:24:36Z

Restarted a single flake.
/approve
LGTM

flouthoc · 2021-05-24T13:28:47Z

@mheon Thanks 😄

flouthoc · 2021-05-24T13:31:28Z

@mheon How do we restart single flake ? There is still one check which has a flake.

giuseppe

LGTM

openshift-ci · 2021-05-24T14:26:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AkihiroSuda, flouthoc, giuseppe, mheon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [giuseppe,mheon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

giuseppe · 2021-05-24T14:26:22Z

/retest

mheon · 2021-05-24T15:14:44Z

/lgtm

@flouthoc I click the "Details" link on the test in question. The Github summary page it takes you to should have a "Retest" button if the test is red.

flouthoc mentioned this pull request May 18, 2021

podman: make Info() robust kubernetes-sigs/kind#2256

Merged

rhatdan reviewed May 18, 2021

View reviewed changes

flouthoc force-pushed the cgroupv1-v2-info branch 3 times, most recently from 0bbbf15 to 2bbdfcc Compare May 18, 2021 20:27

AkihiroSuda suggested changes May 19, 2021

View reviewed changes

pkg/cgroups/cgroups.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed May 19, 2021

View reviewed changes

flouthoc force-pushed the cgroupv1-v2-info branch 2 times, most recently from 192a979 to 4dbfed7 Compare May 19, 2021 14:31

flouthoc requested review from AkihiroSuda, rhatdan and giuseppe May 19, 2021 15:19

flouthoc force-pushed the cgroupv1-v2-info branch 2 times, most recently from 3cb56a7 to 440c098 Compare May 20, 2021 04:05

AkihiroSuda reviewed May 20, 2021

View reviewed changes

flouthoc force-pushed the cgroupv1-v2-info branch 2 times, most recently from ebaaefa to d44d049 Compare May 22, 2021 07:24

flouthoc requested a review from AkihiroSuda May 22, 2021 09:26

flouthoc force-pushed the cgroupv1-v2-info branch from d44d049 to 0bf92a0 Compare May 22, 2021 09:47

AkihiroSuda approved these changes May 24, 2021

View reviewed changes

giuseppe reviewed May 24, 2021

View reviewed changes

pkg/cgroups/cgroups.go Outdated Show resolved Hide resolved

pkg/cgroups/cgroups.go Outdated Show resolved Hide resolved

flouthoc force-pushed the cgroupv1-v2-info branch 2 times, most recently from 60e28ab to 7e70b33 Compare May 24, 2021 10:05

Podman info add support for status of cgroup controllers

2f5552c

Signed-off-by: flouthoc <[email protected]>

flouthoc force-pushed the cgroupv1-v2-info branch from 7e70b33 to 2f5552c Compare May 24, 2021 11:25

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 24, 2021

giuseppe approved these changes May 24, 2021

View reviewed changes

openshift-ci bot assigned mheon May 24, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 24, 2021

openshift-merge-robot merged commit 4d6b66a into containers:master May 24, 2021

This was referenced May 24, 2021

rootless: pods: unlinkat, EBUSY #7139

Closed

podman rm, with more than one arg: error removing container, unlinkat, EBUSY #10454

Closed

flouthoc mentioned this pull request Oct 24, 2021

podman-provider: get actually available cgroupControllers kubernetes-sigs/kind#2511

Merged

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Podman info add support for status of standard available cgroup controllers #10387

Podman info add support for status of standard available cgroup controllers #10387

flouthoc commented May 18, 2021 •

edited

Loading

rhatdan May 18, 2021 •

edited

Loading

flouthoc May 18, 2021

flouthoc May 19, 2021

flouthoc May 22, 2021

AkihiroSuda May 19, 2021

flouthoc May 19, 2021

AkihiroSuda May 19, 2021

flouthoc May 19, 2021

AkihiroSuda May 20, 2021

flouthoc May 20, 2021

AkihiroSuda May 20, 2021

flouthoc May 20, 2021

giuseppe May 21, 2021 •

edited

Loading

flouthoc May 22, 2021 •

edited

Loading

rhatdan commented May 19, 2021

flouthoc commented May 20, 2021

AkihiroSuda May 20, 2021

AkihiroSuda May 20, 2021

flouthoc May 20, 2021

flouthoc May 20, 2021 •

edited

Loading

giuseppe May 21, 2021

flouthoc May 22, 2021 •

edited

Loading

flouthoc commented May 22, 2021

flouthoc commented May 22, 2021

flouthoc commented May 22, 2021

flouthoc commented May 24, 2021

mheon commented May 24, 2021

flouthoc commented May 24, 2021

flouthoc commented May 24, 2021

giuseppe left a comment

openshift-ci bot commented May 24, 2021

giuseppe commented May 24, 2021

mheon commented May 24, 2021

Podman info add support for status of standard available cgroup controllers #10387

Podman info add support for status of standard available cgroup controllers #10387

Conversation

flouthoc commented May 18, 2021 • edited Loading

rhatdan May 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giuseppe May 21, 2021 • edited Loading

Choose a reason for hiding this comment

flouthoc May 22, 2021 • edited Loading

Choose a reason for hiding this comment

rhatdan commented May 19, 2021

flouthoc commented May 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flouthoc May 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flouthoc May 22, 2021 • edited Loading

Choose a reason for hiding this comment

flouthoc commented May 22, 2021

flouthoc commented May 22, 2021

flouthoc commented May 22, 2021

flouthoc commented May 24, 2021

mheon commented May 24, 2021

flouthoc commented May 24, 2021

flouthoc commented May 24, 2021

giuseppe left a comment

Choose a reason for hiding this comment

openshift-ci bot commented May 24, 2021

giuseppe commented May 24, 2021

mheon commented May 24, 2021

flouthoc commented May 18, 2021 •

edited

Loading

rhatdan May 18, 2021 •

edited

Loading

giuseppe May 21, 2021 •

edited

Loading

flouthoc May 22, 2021 •

edited

Loading

flouthoc May 20, 2021 •

edited

Loading

flouthoc May 22, 2021 •

edited

Loading