cgroup: add cgroup v2 support

[giuseppe]

allow users to specify cgroup v2 resources.

Each element in the map refers to a file in the cgroup v2 hierarchy and the element value has its content.

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

[giuseppe]

@AkihiroSuda @mrunalp @kolyshkin @vbatts @rhatdan

[giuseppe]

@filbranden

[mrunalp]

I think we need a way to tell the container runtime whether to use v1 or v2. It could be an optional field cgroupsVersion. One possible way to interpret:

• If not set, assume v1 and convert to v2 values when running on a v2 system.
• If set to v2, then don't perform any conversions and directly set the v2 values.

[h-vetinari]

@mrunalp: I think we need a way to tell the container runtime whether to use v1 or v2. It could be an optional field cgroupsVersion. [...]

I think that's a good idea (see similar thoughts in #1002)

[giuseppe]

I think we need a way to tell the container runtime whether to use v1 or v2. It could be an optional field cgroupsVersion. One possible way to interpret:

* If not set, assume v1 and convert to v2 values when running on a v2 system.

* If set to v2, then don't perform any conversions and directly set the v2 values.

I was convinced about having a way to tell whether it is cgroup v1 or cgroup v2, but since also runc has adopted implicit conversions now, there wouldn't be any difference in what the OCI runtime does.

The conversions are performed for properties that are only on cgroup v1, e.g. shares. On cgroup v2 that should not be used, but instead both the OCI runtime and the container engine should use cpu.weight.

[h-vetinari]

@giuseppe: I was convinced about having a way to tell whether it is cgroup v1 or cgroup v2, but since also runc has adopted implicit conversions now, there wouldn't be any difference in what the OCI runtime does.

Isn't that putting the cart before the horse? Yes, runc has already implemented cgroups v2 in the absence of more details from the specification, but it seems like the runtime-spec should make the "right" choice independently from what runc has done (and of course: runc would be able to follow what the spec decides to mandate).

[giuseppe]

Isn't that putting the cart before the horse? Yes, runc has already implemented cgroups v2 in the absence of more details from the specification, but it seems like the runtime-spec should make the "right" choice independently from what runc has done (and of course: runc would be able to follow what the spec decides to mandate).

yes, I am for the "right" choice. Just what would you do differently once you know what mode to use? It is like duplicating the information we already have, if you use share on cgroup v2, then it is better to do the conversion when running on cgroup v2, differently if weight is used then that value is used without any conversion

[h-vetinari]

@giuseppe: Just what would you do differently once you know what mode to use?

Well, for one thing, runtimes (other than runc, where the established behaviour creates compatibility expectations) could choose to warn/error if the specified fields are not compatible with the given cgroupsVersion (in other words, your share-vs-cpu.weight example, IIUC).

Something very roughly like:

possible spec-formulation: [...] runtimes MAY choose to raise an error if any of the given fields doesn't match the specified cgroupsVersion. Alternatively, runtimes MAY warn only and proceed by converting the values as appropriate according to the following table [...]

Runtimes might even choose to warn/error if using a v2-specified container on a v1-host (or vice versa) - however, this approach would likely be too harsh, as then containers with prespecified limits wouldn't be compatible across hosts with different cgroup versions anymore...

In any case, I think cgroupsVersion would be a good piece of information to have in a specification (whatever the runtime chooses to do).

[crosbymichael]

Since v2 will be the future, should we map fields better to the v2 naming more than what we are doing here? "low|high|max" for memory, etc?

[mrunalp]

Since v2 will be the future, should we map fields better to the v2 naming more than what we are doing here? "low|high|max" for memory, etc?

Yeah, another option is creating new fields that map better to v2 and then root the cgroup heirarchy settings at v1 or v2.

[giuseppe]

In any case, I think cgroupsVersion would be a good piece of information to have in a specification (whatever the runtime chooses to do).

one thing to keep in mind is that v1 vs v2 is really about each controller. The kernel allows to use different controllers (what is known as the hybrid mode: https://systemd.io/CGROUP_DELEGATION/#three-different-tree-setups-). While both runc and crun decided to not support it, so it is either cgroup v1 or cgroup v2, I don't think the OCI specs should deny this configuration.

So for OCI conformance, I think it should be fine to choose either cpu.shares or cpu.weight (but not both), then it is left to the OCI runtime what to do in this case. Perhaps we can expect that OCI runtimes MUST support when all controllers are used on the same cgroup v1 version and MAY support controllers on different versions?

[giuseppe]

Since v2 will be the future, should we map fields better to the v2 naming more than what we are doing here? "low|high|max" for memory, etc?

sure, should we also replace existing fields (e.g. the existing limit has the same semantic as max)?

[crosbymichael]

Instead of replacing fields, could we maybe make a new struct in the resources field, maybe name it "unified"?

"resources": {
   "devices": [
    {
     "allow": false,
     "access": "rwm"
    }
   ],
   "memory": {
    "limit": 12582912000
   },
   "cpu": {
    "cpus": "4-7"
   },
  "unified": {
    "memory.high": 234,
    "memory.max": 555
    "cpu.max": 1
  }
  },

This makes it super simple and backwards compatible. I feel like if we didn't do something that maps well to v2, in a year we will just be frustrated.

What do you all think?

[AkihiroSuda]

Will the "unified" field contain devices?

[kolyshkin]

Instead of replacing fields, could we maybe make a new struct in the resources field, maybe name it "unified"?
What do you all think?

I like it. Perhaps "v2" would be a better name.

[crosbymichael]

idk, v2 seems out of place within the spec as a type name

[mrunalp]

This makes it super simple and backwards compatible. I feel like if we didn't do something that maps well to v2, in a year we will just be frustrated.

What do you all think?

I like this 👍

[mrunalp]

Will the "unified" field contain devices?

Yes, I think it should, so unified is self-contained.

[AkihiroSuda]

 "unified": {
   "memory.high": 234,
...

I think we should avoid using . symbol in the JSON key names. We could use _ instead, but I feel it is better to keep using separate memory{} cpu{} objects.

Yes, I think it should, so unified is self-contained.

Then we will probably end up duplicating huge devices objects in the both places for compatibility 🤔

[giuseppe]

should we try to avoid using an extra level? Since cgroup v2 is going to be the default, we will end up with something like:

"resources": {
    "unified": {
        "memory.high": 234,
        "memory.max": 555
        "cpu.max": 1
    }
},

or:

"resources": {
    "unified": {
        "memory" : {
            "high": 234,
            "max": 555
        }
    }
}

Could it be a new field at the same level? Perhaps define a new field cgroups?

"resources": {
...
},
"cgroups": {
...
},

@AkihiroSuda could we just add in the specs that if cgroups.devices is missing to look into resources.devices? That should be fine for new implementations

[crosbymichael]

I'm leaning towards a flat structure since it is unified on disk as well.

[kolyshkin]

It might be beneficial to quote kernel docs, saying this is the main mechanism to control memory usage. If usage goes over the high boundary, the processes of the container are throttled and put under heavy reclaim pressure or something like this.

Otherwise people will still use memory.limit which is not the best mechanism for v2.

[kolyshkin]

What if we describe cgroups v1 and v2 in a separate sections? Otherwise it's kinda hard to read, since a reader has to always bear in mind which setting applies to which cgroup version. This will also help to retire cgroupv1 in the [hopefully not-so-distant] future.

In cgroup v1 section we can say that in case v2 is used on the host, the runtime SHOULD make effort to transform the settings to v2.