containerd / nri

Node Resource Interface
Apache License 2.0
220 stars 58 forks source link

Support pid cgroup control in plugin #75

Open yylt opened 3 months ago

yylt commented 3 months ago

In the current implementation, it is not possible to control pid cgroup in nri plugin. Could pid be included?

// Container (linux) resources.
type LinuxResources struct {
    state         protoimpl.MessageState
    sizeCache     protoimpl.SizeCache
    unknownFields protoimpl.UnknownFields

    Memory         *LinuxMemory         `protobuf:"bytes,1,opt,name=memory,proto3" json:"memory,omitempty"`
    Cpu            *LinuxCPU            `protobuf:"bytes,2,opt,name=cpu,proto3" json:"cpu,omitempty"`
    HugepageLimits []*HugepageLimit     `protobuf:"bytes,3,rep,name=hugepage_limits,json=hugepageLimits,proto3" json:"hugepage_limits,omitempty"`
    BlockioClass   *OptionalString      `protobuf:"bytes,4,opt,name=blockio_class,json=blockioClass,proto3" json:"blockio_class,omitempty"`
    RdtClass       *OptionalString      `protobuf:"bytes,5,opt,name=rdt_class,json=rdtClass,proto3" json:"rdt_class,omitempty"`
    Unified        map[string]string    `protobuf:"bytes,6,rep,name=unified,proto3" json:"unified,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
    Devices        []*LinuxDeviceCgroup `protobuf:"bytes,7,rep,name=devices,proto3" json:"devices,omitempty"` // for NRI v1 emulation
}

Reference: https://github.com/opencontainers/runtime-spec/blob/main/config-linux.md#pids

bitmingw commented 2 months ago

pid is here https://github.com/containerd/nri/blob/53d3371559b3aedf4f491c33dc638fe535cc37ea/pkg/api/api.proto#L248

But it says "for NRI v1 emulation". I don't know if it is actually provided.

yylt commented 2 months ago

pid is here

https://github.com/containerd/nri/blob/53d3371559b3aedf4f491c33dc638fe535cc37ea/pkg/api/api.proto#L248

But it says "for NRI v1 emulation". I don't know if it is actually provided.

If I'm not mistaken, this seems to be metadata for a sandbox or container, it cannot be used for ContainerAdjustment.

bitmingw commented 2 months ago

This value, if provided, should be read only. I don't think there is a way to control which pid to assign to a process.

yylt commented 2 months ago

Yes, so what I mean is to add pids in the LinuxResources for controlling the pids cgroup.

Additionally, the pids cgroup is to control the number of PIDs inside the container, which is a good restriction limit for the issue of unhandled process exit signals or starting too many processes .

klihub commented 2 months ago

In the current implementation, it is not possible to control pid cgroup in nri plugin. Could pid be included?

I think for the cgroup v2 version of the controller we already support setting the max allowed pids using LinuxResources.Unified["pids.max"]. Do you have a use case for this cgroup v1-specific addition ?