podman: add option --cgroup-conf #7372
Conversation
openshift-ci-robot
added
the
approved
giuseppe
force-pushed
the
add-unified-configuration
branch
2 times, most recently
from
cf81b3d to
666612b
Compare
|
Should we look into exposing any commonly-set versions of this as specific CLI options? This doesn't seem like a very easy/intuitive flag to use, but some of the advanced settings are going to be useful and should probably be documented independently. |
yes definitely. We will need to expose some of them in a more user friendly way. This is just to make sure we have full control on the cgroup if required, since the OCI specs permit it |
|
I would prefer this to be hidden for now. since we have no idea what would be useful or not, or I guess we could state it as experimental. We also need to know if the oci runtime supports it. |
I am fine with hiding the feature for now, or marking it experimental but the idea is that we don't want to know what each option really means. The OCI runtime will take care of that. |
|
@giuseppe Why don't we add a option for that then? I can understand future proofing the tool, but if there is a primary feature of cgroups available in the kernel, we should expose an option for that controller. That would greatly increase the number of people who would use the option. Do we set sane defaults for Hugetlb. |
hugetlb requires a system wide configuration before it can be used, but I agree we need a way to set it up (for cgroup v1 as well). I still would like a way to have full control on the cgroup, also for knobs that we don't want to expose. cgroupv2 makes it easier as all the files are in the same directory so it is enough to have a map |
giuseppe
force-pushed
the
add-unified-configuration
branch
from
666612b to
3b68abf
Compare
giuseppe
changed the title
giuseppe
force-pushed
the
add-unified-configuration
branch
from
3b68abf to
3706353
Compare
giuseppe
changed the title
cmd/podman/common/specgen.go
Outdated
| for _, unified := range c.CgroupConf { | ||
| splitUnified := strings.SplitN(unified, "=", 2) | ||
| if len(splitUnified) < 2 { | ||
| return errors.Errorf("Unifieds must be formatted KEY=VALUE") |
There was a problem hiding this comment.
--cgroup-conf must be formatted ...
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, rhatdan 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:
Approvers can indicate their approval by writing |
Signed-off-by: Giuseppe Scrivano <[email protected]>
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --cgroup-conf mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
add-unified-configuration
branch
from
3706353 to
d856210
Compare
|
addressed the comments |
| @@ -89,6 +89,10 @@ The *split* option splits the current cgroup in two sub-cgroups: one for conmon | |||
|
|
|||
| Path to cgroups under which the cgroup for the container will be created. If the path is not absolute, the path is considered to be relative to the cgroups path of the init process. Cgroups will be created if they do not already exist. | |||
|
|
|||
| **--cgroup-conf**=*KEY=VALUE* | |||
|
|
|||
| When running on cgroup v2, specify the cgroup file to write to and its value. For example **--cgroup-conf=memory.high=1073741824** sets the memory.high limit to 1GB. | |||
There was a problem hiding this comment.
is it file or option?
| When running on cgroup v2, specify the cgroup file to write to and its value. For example **--cgroup-conf=memory.high=1073741824** sets the memory.high limit to 1GB. | |
| When running on cgroup v2, specify the cgroup option to write to and its value. For example **--cgroup-conf=memory.high=1073741824** sets the memory.high limit to 1GB. |
There was a problem hiding this comment.
File is correct, the key is the file name in the cgroup directory
| @@ -104,6 +104,10 @@ The **split** option splits the current cgroup in two sub-cgroups: one for conmo | |||
|
|
|||
| Path to cgroups under which the cgroup for the container will be created. If the path is not absolute, the path is considered to be relative to the cgroups path of the init process. Cgroups will be created if they do not already exist. | |||
|
|
|||
| **--cgroup-conf**=*KEY=VALUE* | |||
|
|
|||
| When running on cgroup v2, specify the cgroup file to write to and its value. For example **--cgroup-conf=memory.high=1073741824** sets the memory.high limit to 1GB. | |||
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
it allows to manually tweak the configuration for cgroup v2.
we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic
--cgroup-conf mechanism for maximum control on the cgroup configuration.
OCI specs change: opencontainers/runtime-spec#1040
Requires: containers/crun#459
Signed-off-by: Giuseppe Scrivano [email protected]