From 1fd31ccc282741c6dc2cfe825e35353e81830215 Mon Sep 17 00:00:00 2001
From: Feruzjon Muyassarov <feruzjon.muyassarov@intel.com>
Date: Tue, 26 Sep 2023 18:05:25 +0300
Subject: [PATCH] Refactor NRI activation for containerd and CRI-O (#10470)

Refactor NRI (Node Resource Interface) activation in CRI-O and
containerd. Introduce a shared variable, nri_enabled, to streamline
the process. Currently, enabling NRI requires a separate update of
defaults for each container runtime independently, without any
verification of NRI support for the specific version of containerd
or CRI-O in use.

With this commit, the previous approach is replaced. Now, a single
variable, nri_enabled, handles this functionality. Also, this commit
separates the responsibility of verifying NRI supported versions of
containerd and CRI-O from cluster administrators, and leaves it to
Ansible.

Signed-off-by: Feruzjon Muyassarov <feruzjon.muyassarov@intel.com>
---
 docs/containerd.md                                     | 10 ++++++++++
 docs/cri-o.md                                          | 10 ++++++++++
 roles/container-engine/containerd/defaults/main.yml    |  3 ---
 .../containerd/templates/config.toml.j2                |  4 +++-
 roles/container-engine/cri-o/defaults/main.yml         |  3 ---
 roles/container-engine/cri-o/templates/crio.conf.j2    |  5 +++--
 roles/kubespray-defaults/defaults/main.yaml            |  4 ++++
 7 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/docs/containerd.md b/docs/containerd.md
index c911a8df4..112c0fc1e 100644
--- a/docs/containerd.md
+++ b/docs/containerd.md
@@ -130,3 +130,13 @@ containerd_registries_mirrors:
 [RuntimeClass]: https://kubernetes.io/docs/concepts/containers/runtime-class/
 [runtime classes in containerd]: https://github.com/containerd/containerd/blob/main/docs/cri/config.md#runtime-classes
 [runtime-spec]: https://github.com/opencontainers/runtime-spec
+
+### Optional : NRI
+
+[Node Resource Interface](https://github.com/containerd/nri) (NRI) is disabled by default for the containerd. If you
+are using contained version v1.7.0 or above, then you can enable it with the
+following configuration:
+
+```yaml
+nri_enabled: true
+```
diff --git a/docs/cri-o.md b/docs/cri-o.md
index 213c39a01..2405697d3 100644
--- a/docs/cri-o.md
+++ b/docs/cri-o.md
@@ -62,3 +62,13 @@ The `allowed_annotations` configures `crio.conf` accordingly.
 
 The `crio_remap_enable` configures the `/etc/subuid` and `/etc/subgid` files to add an entry for the **containers** user.
 By default, 16M uids and gids are reserved for user namespaces (256 pods * 65536 uids/gids) at the end of the uid/gid space.
+
+## Optional : NRI
+
+[Node Resource Interface](https://github.com/containerd/nri) (NRI) is disabled by default for the CRI-O. If you
+are using CRI-O version v1.26.0 or above, then you can enable it with the
+following configuration:
+
+```yaml
+nri_enabled: true
+```
diff --git a/roles/container-engine/containerd/defaults/main.yml b/roles/container-engine/containerd/defaults/main.yml
index 9359e1aeb..05cfd95c1 100644
--- a/roles/container-engine/containerd/defaults/main.yml
+++ b/roles/container-engine/containerd/defaults/main.yml
@@ -64,9 +64,6 @@ containerd_enable_unprivileged_ports: false
 # If enabled it will allow non root users to use icmp sockets
 containerd_enable_unprivileged_icmp: false
 
-# If enabled, it will activate the NRI support in containerd
-containerd_nri_disable: true
-
 containerd_cfg_dir: /etc/containerd
 
 # Extra config to be put in {{ containerd_cfg_dir }}/config.toml literally
diff --git a/roles/container-engine/containerd/templates/config.toml.j2 b/roles/container-engine/containerd/templates/config.toml.j2
index ba107ee8b..fc3ea477f 100644
--- a/roles/container-engine/containerd/templates/config.toml.j2
+++ b/roles/container-engine/containerd/templates/config.toml.j2
@@ -78,8 +78,10 @@ oom_score = {{ containerd_oom_score }}
 {% endif %}
 {% endfor %}
 
+{% if nri_enabled and containerd_version >= 1.7.0 %}
   [plugins."io.containerd.nri.v1.nri"]
-    disable = {{ containerd_nri_disable | default(true) | lower }}
+    disable = false
+{% endif %}
 
 {% if containerd_extra_args is defined %}
 {{ containerd_extra_args }}
diff --git a/roles/container-engine/cri-o/defaults/main.yml b/roles/container-engine/cri-o/defaults/main.yml
index 053af20e6..949ed69ed 100644
--- a/roles/container-engine/cri-o/defaults/main.yml
+++ b/roles/container-engine/cri-o/defaults/main.yml
@@ -97,6 +97,3 @@ crio_man_files:
   8:
     - crio
     - crio-status
-
-# If set to true, it will enable the NRI support in cri-o
-crio_enable_nri: false
diff --git a/roles/container-engine/cri-o/templates/crio.conf.j2 b/roles/container-engine/cri-o/templates/crio.conf.j2
index f92206e32..f0455d093 100644
--- a/roles/container-engine/cri-o/templates/crio.conf.j2
+++ b/roles/container-engine/cri-o/templates/crio.conf.j2
@@ -377,7 +377,8 @@ enable_metrics = {{ crio_enable_metrics | bool | lower }}
 # The port on which the metrics server will listen.
 metrics_port = {{ crio_metrics_port }}
 
+{% if nri_enabled and crio_version >= v1.26.0 %}
 [crio.nri]
 
-# Enable or disable NRI (Node Resource Interface) support in CRI-O.
-enable_nri={{ crio_enable_nri | default(false) | lower }}
+enable_nri=true
+{% endif %}
diff --git a/roles/kubespray-defaults/defaults/main.yaml b/roles/kubespray-defaults/defaults/main.yaml
index 23c7a0e1b..7768ff323 100644
--- a/roles/kubespray-defaults/defaults/main.yaml
+++ b/roles/kubespray-defaults/defaults/main.yaml
@@ -270,6 +270,10 @@ deploy_container_engine: "{{ inventory_hostname in groups['k8s_cluster'] or etcd
 # Container for runtime
 container_manager: containerd
 
+# Enable Node Resource Interface in containerd or CRI-O. Requires crio_version >= v1.26.0
+# or containerd_version >= 1.7.0.
+nri_enabled: false
+
 # Enable Kata Containers as additional container runtime
 # When enabled, it requires `container_manager` different than Docker
 kata_containers_enabled: false
-- 
GitLab