Drop dynamic run-time reconfiguration

marquiz · marquiz · commit 02b6b7395c13 · 2024-08-21T12:46:36.000+03:00
Simplify the code and reduce possible error scenarios by dropping
fsnotify-based reconfiguration from nfd-master and nfd-worker. Also
eliminates repeated re-configuration in scenarios where kubelet
continuosly touches the (every minute) mounted file (configmap) on the
filesystem.

Also modifies the Helm and kustomize deployments so that nfd-master,
nfd-worker and nfd-topology-updater pods are restarted on configmap
updates. In kustomize, the slght downside of this is the name of the
config map(s) depends on the content, so every time a user customizes
the config data, the old unused configmap will be left and must be
garbage-collected manually.
diff --git a/deployment/components/master-config/kustomization.yaml b/deployment/components/master-config/kustomization.yaml
@@ -1,9 +1,6 @@
 apiVersion: kustomize.config.k8s.io/v1alpha1
 kind: Component
 
-generatorOptions:
-  disableNameSuffixHash: true
-
 configMapGenerator:
 - files:
   - nfd-master.conf=nfd-master.conf.example
diff --git a/deployment/components/topology-updater-config/kustomization.yaml b/deployment/components/topology-updater-config/kustomization.yaml
@@ -1,9 +1,6 @@
 apiVersion: kustomize.config.k8s.io/v1alpha1
 kind: Component
 
-generatorOptions:
-  disableNameSuffixHash: true
-
 configMapGenerator:
 - files:
   - nfd-topology-updater.conf=nfd-topology-updater.conf.example
diff --git a/deployment/components/worker-config/kustomization.yaml b/deployment/components/worker-config/kustomization.yaml
@@ -1,9 +1,6 @@
 apiVersion: kustomize.config.k8s.io/v1alpha1
 kind: Component
 
-generatorOptions:
-  disableNameSuffixHash: true
-
 configMapGenerator:
 - files:
   - nfd-worker.conf=nfd-worker.conf.example
diff --git a/deployment/helm/node-feature-discovery/templates/master.yaml b/deployment/helm/node-feature-discovery/templates/master.yaml
@@ -23,10 +23,11 @@ spec:
       labels:
         {{- include "node-feature-discovery.selectorLabels" . | nindent 8 }}
         role: master
-      {{- with .Values.master.annotations }}
       annotations:
+        checksum/config: {{ include (print $.Template.BasePath "/nfd-master-conf.yaml") . | sha256sum }}
+        {{- with .Values.master.annotations }}
         {{- toYaml . | nindent 8 }}
-      {{- end }}
+        {{- end }}
     spec:
     {{- with .Values.priorityClassName }}
       priorityClassName: {{ . }}
diff --git a/deployment/helm/node-feature-discovery/templates/topologyupdater.yaml b/deployment/helm/node-feature-discovery/templates/topologyupdater.yaml
@@ -22,10 +22,11 @@ spec:
       labels:
         {{- include "node-feature-discovery.selectorLabels" . | nindent 8 }}
         role: topology-updater
-      {{- with .Values.topologyUpdater.annotations }}
       annotations:
+        checksum/config: {{ include (print $.Template.BasePath "/nfd-topologyupdater-conf.yaml") . | sha256sum }}
+        {{- with .Values.topologyUpdater.annotations }}
         {{- toYaml . | nindent 8 }}
-      {{- end }}
+        {{- end }}
     spec:
       serviceAccountName: {{ include "node-feature-discovery.topologyUpdater.serviceAccountName" . }}
       dnsPolicy: ClusterFirstWithHostNet
diff --git a/deployment/helm/node-feature-discovery/templates/worker.yaml b/deployment/helm/node-feature-discovery/templates/worker.yaml
@@ -22,10 +22,11 @@ spec:
       labels:
         {{- include "node-feature-discovery.selectorLabels" . | nindent 8 }}
         role: worker
-      {{- with .Values.worker.annotations }}
       annotations:
+        checksum/config: {{ include (print $.Template.BasePath "/nfd-worker-conf.yaml") . | sha256sum }}
+        {{- with .Values.worker.annotations }}
         {{- toYaml . | nindent 8 }}
-      {{- end }}
+        {{- end }}
     spec:
       dnsPolicy: ClusterFirstWithHostNet
     {{- with .Values.priorityClassName }}
diff --git a/docs/reference/worker-configuration-reference.md b/docs/reference/worker-configuration-reference.md
@@ -151,8 +151,7 @@ core:
 
 ### core.klog
 
-The following options specify the logger configuration. Most of which can be
-dynamically adjusted at run-time.
+The following options specify the logger configuration.
 
 > **NOTE:** The logger options can also be specified via command line flags
 > which take precedence over any corresponding config file options.
@@ -163,95 +162,71 @@ If true, adds the file directory to the header of the log messages.
 
 Default: `false`
 
-Run-time configurable: yes
-
 #### core.klog.alsologtostderr
 
 Log to standard error as well as files.
 
 Default: `false`
 
-Run-time configurable: yes
-
 #### core.klog.logBacktraceAt
 
 When logging hits line file:N, emit a stack trace.
 
 Default: *empty*
 
-Run-time configurable: yes
-
 #### core.klog.logDir
 
 If non-empty, write log files in this directory.
 
 Default: *empty*
 
-Run-time configurable: no
-
 #### core.klog.logFile
 
 If non-empty, use this log file.
 
 Default: *empty*
 
-Run-time configurable: no
-
 #### core.klog.logFileMaxSize
 
 Defines the maximum size a log file can grow to. Unit is megabytes. If the
 value is 0, the maximum file size is unlimited.
 
 Default: `1800`
 
-Run-time configurable: no
-
 #### core.klog.logtostderr
 
 Log to standard error instead of files
 
 Default: `true`
 
-Run-time configurable: yes
-
 #### core.klog.skipHeaders
 
 If true, avoid header prefixes in the log messages.
 
 Default: `false`
 
-Run-time configurable: yes
-
 #### core.klog.skipLogHeaders
 
 If true, avoid headers when opening log files.
 
 Default: `false`
 
-Run-time configurable: no
-
 #### core.klog.stderrthreshold
 
 Logs at or above this threshold go to stderr (default 2)
 
-Run-time configurable: yes
-
 #### core.klog.v
 
 Number for the log level verbosity.
 
 Default: `0`
 
-Run-time configurable: yes
-
 #### core.klog.vmodule
 
 Comma-separated list of `pattern=N` settings for file-filtered logging.
 
 Default: *empty*
 
-Run-time configurable: yes
-
 ## sources
 
 The `sources` section contains feature source specific configuration parameters.
diff --git a/docs/usage/nfd-master.md b/docs/usage/nfd-master.md
@@ -32,29 +32,28 @@ received from nfd-worker instances through
 
 ## Master configuration
 
-NFD-Master supports dynamic configuration through a configuration file. The
+NFD-Master supports configuration through a configuration file. The
 default location is `/etc/kubernetes/node-feature-discovery/nfd-master.conf`,
 but, this can be changed by specifying the`-config` command line flag.
-Configuration file is re-read whenever it is modified which makes run-time
-re-configuration of nfd-master straightforward.
 
 Master configuration file is read inside the container, and thus, Volumes and
 VolumeMounts are needed to make your configuration available for NFD. The
 preferred method is to use a ConfigMap which provides easy deployment and
 re-configurability.
 
-The provided nfd-master deployment templates create an empty configmap and
-mount it inside the nfd-master containers. In kustomize deployments,
-configuration can be edited with:
-
-```bash
-kubectl -n ${NFD_NS} edit configmap nfd-master-conf
-```
+The provided deployment methods (Helm and Kustomize) create an empty configmap
+and mount it inside the nfd-master containers.
 
 In Helm deployments,
 [Master pod parameter](../deployment/helm.md#master-pod-parameters)
 `master.config` can be used to edit the respective configuration.
 
+In Kustomize deployments, modify the `nfd-master-conf` ConfigMap with a custom
+overlay.
+
+> **NOTE:** dynamic run-time reconfiguration was dropped in NFD v0.17.
+> Re-configuration is handled by pod restarts.
+
 See
 [nfd-master configuration file reference](../reference/master-configuration-reference.md)
 for more details.
diff --git a/docs/usage/nfd-topology-updater.md b/docs/usage/nfd-topology-updater.md
@@ -62,27 +62,22 @@ NFD-Topology-Updater supports configuration through a configuration file. The
 default location is `/etc/kubernetes/node-feature-discovery/topology-updater.conf`,
 but, this can be changed by specifying the`-config` command line flag.
 
-> **NOTE:** unlike nfd-worker, dynamic configuration updates are not supported.
-
 Topology-Updater configuration file is read inside the container,
 and thus, Volumes and VolumeMounts are needed
 to make your configuration available for NFD.
 The preferred method is to use a ConfigMap
 which provides easy deployment and re-configurability.
 
-The provided nfd-topology-updater deployment templates
-create an empty configmap
+The provided deployment templates create an empty configmap
 and mount it inside the nfd-topology-updater containers.
-In kustomize deployments, configuration can be edited with:
-
-```bash
-kubectl -n ${NFD_NS} edit configmap nfd-topology-updater-conf
-```
 
 In Helm deployments,
 [Topology Updater parameters](../deployment/helm.md#topology-updater-parameters)
 `toplogyUpdater.config` can be used to edit the respective configuration.
 
+In Kustomize deployments, modify the `nfd-worker-conf` ConfigMap with a custom
+overlay.
+
 See
 [nfd-topology-updater configuration file reference](../reference/topology-updater-configuration-reference.md)
 for more details.
diff --git a/docs/usage/nfd-worker.md b/docs/usage/nfd-worker.md
@@ -19,13 +19,9 @@ This can be changed by using the
 [`core.sleepInterval`](../reference/worker-configuration-reference.md#coresleepinterval)
 config option.
 
-The worker configuration file is watched and re-read on every change which
-provides a mechanism of dynamic run-time reconfiguration. See
-[worker configuration](#worker-configuration) for more details.
-
 ## Worker configuration
 
-NFD-Worker supports dynamic configuration through a configuration file. The
+NFD-Worker supports configuration through a configuration file. The
 default location is `/etc/kubernetes/node-feature-discovery/nfd-worker.conf`,
 but, this can be changed by specifying the`-config` command line flag.
 Configuration file is re-read whenever it is modified which makes run-time
@@ -36,18 +32,19 @@ VolumeMounts are needed to make your configuration available for NFD. The
 preferred method is to use a ConfigMap which provides easy deployment and
 re-configurability.
 
-The provided nfd-worker deployment templates create an empty configmap and
-mount it inside the nfd-worker containers. In kustomize deployments,
-configuration can be edited with:
-
-```bash
-kubectl -n ${NFD_NS} edit configmap nfd-worker-conf
-```
+The provided deployment methods (Helm and Kustomize) create an empty configmap
+and mount it inside the nfd-master containers.
 
 In Helm deployments,
 [Worker pod parameter](../deployment/helm.md#worker-pod-parameters)
 `worker.config` can be used to edit the respective configuration.
 
+In Kustomize deployments, modify the `nfd-worker-conf` ConfigMap with a custom
+overlay.
+
+> **NOTE:** dynamic run-time reconfiguration was dropped in NFD v0.17.
+> Re-configuration is handled by pod restarts.
+
 See
 [nfd-worker configuration file reference](../reference/worker-configuration-reference)
 for more details.
diff --git a/pkg/nfd-master/nfd-master-internal_test.go b/pkg/nfd-master/nfd-master-internal_test.go
@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"maps"
 	"os"
-	"path/filepath"
 	"sort"
 	"strings"
 	"testing"
@@ -665,96 +664,6 @@ leaderElection:
 	})
 }
 
-func TestDynamicConfig(t *testing.T) {
-	Convey("When running nfd-master", t, func() {
-		// Add feature gates as running nfd-master depends on that
-		err := features.NFDMutableFeatureGate.Add(features.DefaultNFDFeatureGates)
-		So(err, ShouldBeNil)
-
-		tmpDir, err := os.MkdirTemp("", "*.nfd-test")
-		So(err, ShouldBeNil)
-		defer os.RemoveAll(tmpDir)
-
-		// Create (temporary) dir for config
-		configDir := filepath.Join(tmpDir, "subdir-1", "subdir-2", "master.conf")
-		err = os.MkdirAll(configDir, 0755)
-		So(err, ShouldBeNil)
-
-		// Create config file
-		configFile := filepath.Clean(filepath.Join(configDir, "master.conf"))
-
-		writeConfig := func(data string) {
-			f, err := os.Create(configFile)
-			So(err, ShouldBeNil)
-			_, err = f.WriteString(data)
-			So(err, ShouldBeNil)
-			err = f.Close()
-			So(err, ShouldBeNil)
-		}
-		writeConfig(`
-klog:
-  v: "4"
-extraLabelNs: ["added.ns.io"]
-`)
-
-		master := newFakeMaster(
-			WithArgs(&Args{ConfigFile: configFile}),
-			WithKubernetesClient(fakeclient.NewSimpleClientset(newTestNode())))
-
-		Convey("config file updates should take effect", func() {
-			go func() {
-				Convey("nfd-master should exit gracefully", t, func() {
-					err = master.Run()
-					So(err, ShouldBeNil)
-				})
-			}()
-			defer master.Stop()
-			// Check initial config
-			time.Sleep(10 * time.Second)
-			So(func() interface{} { return master.config.ExtraLabelNs },
-				withTimeout, 2*time.Second, ShouldResemble, utils.StringSetVal{"added.ns.io": struct{}{}})
-
-			// Update config and verify the effect
-			writeConfig(`
-extraLabelNs: ["override.ns.io"]
-resyncPeriod: '2h'
-nfdApiParallelism: 300
-`)
-			So(func() interface{} { return master.config.ExtraLabelNs },
-				withTimeout, 2*time.Second, ShouldResemble, utils.StringSetVal{"override.ns.io": struct{}{}})
-			So(func() interface{} { return master.config.ResyncPeriod.Duration },
-				withTimeout, 2*time.Second, ShouldResemble, time.Duration(2)*time.Hour)
-			So(func() interface{} { return master.config.NfdApiParallelism },
-				withTimeout, 2*time.Second, ShouldResemble, 300)
-
-			// Removing config file should get back our defaults
-			err = os.RemoveAll(tmpDir)
-			So(err, ShouldBeNil)
-			So(func() interface{} { return master.config.ExtraLabelNs },
-				withTimeout, 2*time.Second, ShouldResemble, utils.StringSetVal{})
-			So(func() interface{} { return master.config.ResyncPeriod.Duration },
-				withTimeout, 2*time.Second, ShouldResemble, time.Duration(1)*time.Hour)
-			So(func() interface{} { return master.config.NfdApiParallelism },
-				withTimeout, 2*time.Second, ShouldResemble, 10)
-
-			// Re-creating config dir and file should change the config
-			err = os.MkdirAll(configDir, 0755)
-			So(err, ShouldBeNil)
-			writeConfig(`
-extraLabelNs: ["another.override.ns"]
-resyncPeriod: '3m'
-nfdApiParallelism: 100
-`)
-			So(func() interface{} { return master.config.ExtraLabelNs },
-				withTimeout, 2*time.Second, ShouldResemble, utils.StringSetVal{"another.override.ns": struct{}{}})
-			So(func() interface{} { return master.config.ResyncPeriod.Duration },
-				withTimeout, 2*time.Second, ShouldResemble, time.Duration(3)*time.Minute)
-			So(func() interface{} { return master.config.NfdApiParallelism },
-				withTimeout, 2*time.Second, ShouldResemble, 100)
-		})
-	})
-}
-
 func newTestNodeList() *corev1.NodeList {
 	l := corev1.NodeList{}
 
diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go
diff --git a/pkg/nfd-worker/nfd-worker-internal_test.go b/pkg/nfd-worker/nfd-worker-internal_test.go
diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go
diff --git a/test/e2e/node_feature_discovery_test.go b/test/e2e/node_feature_discovery_test.go
