search

k3s-io / k3s

Lightweight Kubernetes
https://k3s.io
Apache License 2.0
27.76k stars 2.33k forks source link

QoS-class resource containerd config files #8720

Closed LarssonOliver closed 10 months ago

LarssonOliver commented 11 months ago

Is your feature request related to a problem? Please describe.

Recent versions of containerd (and cri-o) support QoS-class resource control through container annotations and config files.

The config files are specified in the containerd config.toml file as such:

[plugins."io.containerd.service.v1.tasks-service"]
  blockio_config_file = ""
  rdt_config_file = ""

(see https://github.com/containerd/containerd/blob/main/docs/man/containerd-config.toml.5.md)

The config files specified follow the formats specified by https://github.com/intel/goresctrl.

Describe the solution you'd like

It would have been nice if there was some way of configuring these without creating a custom config.toml.tmpl, but rather having this integrated into the default template.

Describe alternatives you've considered

Writing a custom config.toml.tmpl file.

Additional context

KEP 3008 describes the planned Kubernetes integration. Meanwhile, this is controlled through pod annotations.

brandond commented 11 months ago

It would have been nice if there was some way of configuring these without creating a custom config.toml.tmpl, but rather having this integrated into the default template.

We've not seen anyone ask for this yet, or open a PR to add support for it. Probably due to it not being used by Kubernetes yet.

We're reluctant to add any additional CLI flags, so this would probably look a lot like our Nvidia container runtime support - we can check for existence of user-provided config files in a predefined location, and if found, add the path to the appropriate section of the config.

LarssonOliver commented 11 months ago

I think checking for files in predefined locations would be sufficient. It is always possible to provide a custom config.toml.tmpl if other file locations are desired.

I would have use of this and could work on an implementation. Do you have any suggestions of what would be appropriate locations? I'm not too familiar with the k3s config structure, but my initial idea would be somewhere in /var/lib/rancher/k3s/agent/etc/...

brandond commented 11 months ago

In the same directory as the containerd config file would be my suggestion

fmoral2 commented 10 months ago

hey! @LarssonOliver i am testing it but even though i am creating an empty file and its being loaded on the config 

 cat /var/lib/rancher/k3s/agent/etc/containerd/config.toml 

# File generated by k3s. DO NOT EDIT. Use config.toml.tmpl instead.
version = 2

[plugins."io.containerd.internal.v1.opt"]
  path = "/var/lib/rancher/k3s/agent/containerd"
[plugins."io.containerd.grpc.v1.cri"]
  stream_server_address = "127.0.0.1"
  stream_server_port = "10010"
  enable_selinux = false
  enable_unprivileged_ports = true
  enable_unprivileged_icmp = true
  sandbox_image = "rancher/mirrored-pause:3.6"

[plugins."io.containerd.grpc.v1.cri".containerd]
  snapshotter = "overlayfs"
  disable_snapshot_annotations = true

[plugins."io.containerd.grpc.v1.cri".cni]
  bin_dir = "/var/lib/rancher/k3s/data/267e6f7f3c53e8144faf1646c85ab5d30a21262ebd4ae4b963ed4c7a6358bdfa/bin"
  conf_dir = "/var/lib/rancher/k3s/agent/etc/cni/net.d"

[plugins."io.containerd.service.v1.tasks-service"]

  rdt_config_file = "/var/lib/rancher/k3s/agent/etc/containerd/rdt_config.yaml"

[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc]
  runtime_type = "io.containerd.runc.v2"

[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options]
  SystemdCgroup = true

i can restart normally 

sudo systemctl restart k3s
sudo systemctl status k3s
● k3s.service - Lightweight Kubernetes
     Loaded: loaded (/etc/systemd/system/k3s.service; enabled; vendor preset: enabled)
     Active: active (running) since Thu 2023-11-16 21:27:13 UTC; 6min ago
       Docs: https://k3s.io

any suggestion here ?

brandond commented 10 months ago

@fmoral2 even though i am creating an empty file and its being loaded on the config i can restart normally

were you expecting an error when the file is empty? As per the docs I don't see that an empty file would constitute an error, I suspect you just won't get any new partitions or classes, as you haven't defined any...

LarssonOliver commented 10 months ago

Hi,

This happens when running on a system without hardware support for QoS-class resource management. I just tested it, and contained spits out this line:

$ sudo less /var/lib/rancher/k3s/agent/containerd/containerd.log
...
time="2023-11-17T07:20:46.136084780+01:00" level=error msg="RDT initialization failed" error="RDT not enabled: failed to detect resctrl mount point: resctrl not found in /proc/mounts"
...

At which point it just ignores the specified config and continues normally.

I'm not sure if there is a need for any logic on k3s' side here. We're just passing the config to containerd after all...

LarssonOliver commented 10 months ago

@brandond were you expecting an error when the file is empty? As per the docs I don't see that an empty file would constitute an error, I suspect you just won't get any new partitions or classes, as you haven't defined any...

Yes, I also mentioned it in the PR (#8726) but containerd exits with an error if the config file is invalid, which an empty one is considered to be.

fmoral2 commented 10 months ago

Validated on Version:

-$ k3s version v1.28.3+k3s-fa4c1806 (fa4c1806)

Environment Details

Infrastructure Cloud EC2 instance

Node(s) CPU architecture, OS, and Version: PRETTY_NAME="Ubuntu 22.04.1 LTS" NAME="Ubuntu" VERSION_ID="22.04"

Cluster Configuration: 1 node server

Steps to validate the fix

  1. Install k3s
  2. Create a example config file for blockio_config_file and rdt_config_file
  3. Restart k3s
  4. Validate if config is now present on config.toml
  5. Validate nodes and pods are up and ok

Reproduction Issue:

``` #$ k3s -v k3s version v1.28.3+k3s-0011eb5e (0011eb5e) #$ vim /var/lib/rancher/k3s/agent/etc/containerd/rdt_config.yaml l3CacheSchema: "L3:0=f;1=f" #$ vim /var/lib/rancher/k3s/agent/etc/containerd/blockio_config.yaml weight: 500 #$ sudo systemctl restart k3s #$ cat /var/lib/rancher/k3s/agent/etc/containerd/config.toml # File generated by k3s. DO NOT EDIT. Use config.toml.tmpl instead. version = 2 [plugins."io.containerd.internal.v1.opt"] path = "/var/lib/rancher/k3s/agent/containerd" [plugins."io.containerd.grpc.v1.cri"] stream_server_address = "127.0.0.1" stream_server_port = "10010" enable_selinux = false enable_unprivileged_ports = true enable_unprivileged_icmp = true sandbox_image = "rancher/mirrored-pause:3.6" [plugins."io.containerd.grpc.v1.cri".containerd] snapshotter = "overlayfs" disable_snapshot_annotations = true [plugins."io.containerd.grpc.v1.cri".cni] bin_dir = "/var/lib/rancher/k3s/data/ec64c5df3003a34901347952f56929af0a02b8af70c1111359c2bbc3315b1c14/bin" conf_dir = "/var/lib/rancher/k3s/agent/etc/cni/net.d" [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc] runtime_type = "io.containerd.runc.v2" [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options] SystemdCgroup = true ``` ## **Validation Results:**
``` #$ k3s -v k3s version v1.28.3+k3s-fa4c1806 (fa4c1806) #$ vim /var/lib/rancher/k3s/agent/etc/containerd/rdt_config.yaml l3CacheSchema: "L3:0=f;1=f" #$ vim /var/lib/rancher/k3s/agent/etc/containerd/blockio_config.yaml weight: 500 #$ sudo systemctl restart k3s #$ cat /var/lib/rancher/k3s/agent/etc/containerd/config.toml # File generated by k3s. DO NOT EDIT. Use config.toml.tmpl instead. version = 2 [plugins."io.containerd.internal.v1.opt"] path = "/var/lib/rancher/k3s/agent/containerd" [plugins."io.containerd.grpc.v1.cri"] stream_server_address = "127.0.0.1" stream_server_port = "10010" enable_selinux = false enable_unprivileged_ports = true enable_unprivileged_icmp = true sandbox_image = "rancher/mirrored-pause:3.6" [plugins."io.containerd.grpc.v1.cri".containerd] snapshotter = "overlayfs" disable_snapshot_annotations = true [plugins."io.containerd.grpc.v1.cri".cni] bin_dir = "/var/lib/rancher/k3s/data/267e6f7f3c53e8144faf1646c85ab5d30a21262ebd4ae4b963ed4c7a6358bdfa/bin" conf_dir = "/var/lib/rancher/k3s/agent/etc/cni/net.d" [plugins."io.containerd.service.v1.tasks-service"] blockio_config_file = "/var/lib/rancher/k3s/agent/etc/containerd/blockio_config.yaml" rdt_config_file = "/var/lib/rancher/k3s/agent/etc/containerd/rdt_config.yaml" [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc] runtime_type = "io.containerd.runc.v2" [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.runc.options] SystemdCgroup = true kubectl get nodes,pods -A NAME STATUS ROLES AGE VERSION node/ip- Ready control-plane,etcd,master 16m v1.28.3+k3s-fa4c1806 NAMESPACE NAME READY STATUS RESTARTS AGE kube-system pod/coredns-6799fbcd5-wrjx8 1/1 Running 0 16m kube-system pod/helm-install-traefik-crd-69cpf 0/1 Completed 0 16m kube-system pod/helm-install-traefik-x58hd 0/1 Completed 1 16m kube-system pod/local-path-provisioner-84db5d44d9-9rds4 1/1 Running 0 16m kube-system pod/metrics-server-67c658944b-42zbh 1/1 Running 0 16m kube-system pod/svclb-traefik-f1d6e03e-l8lqk 2/2 Running 0 15m kube-system pod/traefik-f4564c4f4-lnsnf 1/1 Running 0 15m ```
fmoral2 commented 10 months ago

i feel that we should have added unit test for this on the PR, since this is a separated method called in get() could be easily tested only on this layer

3 points:

whats your thoughts @brandond ?

brandond commented 10 months ago

I don't think this needs a ton of testing. It is not complicated logic; it is just checking for the presence of files, and if they exist, adding a line to the config to point containerd at them. We don't need to validate that any of the containerd support for this feature works, all we need to do is confirm that the config is added if the files exist.

Maybe just a basic test like we have for nvidia, that uses a mock filesystem and confirms that the correct config struct is generated?

fmoral2 commented 10 months ago

I don't think this needs a ton of testing. It is not complicated logic; it is just checking for the presence of files, and if they exist, adding a line to the config to point containerd at them. We don't need to validate that any of the containerd support for this feature works, all we need to do is confirm that the config is added if the files exist.

Maybe just a basic test like we have for nvidia, that uses a mock filesystem and confirms that the correct config struct is generated?

LarssonOliver commented 10 months ago

I can take care of this within a few days if you'd like!