nri icon indicating copy to clipboard operation
nri copied to clipboard

Support pid cgroup control in plugin

Open yylt opened this issue 1 year ago • 5 comments

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

yylt avatar Mar 29 '24 06:03 yylt

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.

bitmingw avatar Apr 04 '24 16:04 bitmingw

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.

yylt avatar Apr 07 '24 02:04 yylt

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.

bitmingw avatar Apr 07 '24 05:04 bitmingw

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 .

yylt avatar Apr 07 '24 08:04 yylt

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 ?

klihub avatar Apr 08 '24 10:04 klihub