From 62f4eddce67afdcb80cac6999672cf2ac67d97f0 Mon Sep 17 00:00:00 2001 From: Carlos Eduardo Arango Gutierrez Date: Mon, 4 Nov 2024 10:25:47 +0100 Subject: [PATCH] Drop support for hooks Signed-off-by: Carlos Eduardo Arango Gutierrez --- .../components/common/worker-mounts.yaml | 6 - .../worker-config/nfd-worker.conf.example | 2 - .../templates/worker.yaml | 6 - .../helm/node-feature-discovery/values.yaml | 2 - docs/deployment/image-variants.md | 4 +- docs/get-started/introduction.md | 2 +- .../worker-configuration-reference.md | 26 ---- docs/usage/customization-guide.md | 70 ++------- pkg/nfd-worker/nfd-worker.go | 2 +- source/local/local.go | 133 +----------------- 10 files changed, 19 insertions(+), 234 deletions(-) diff --git a/deployment/components/common/worker-mounts.yaml b/deployment/components/common/worker-mounts.yaml index 274b0374d..8b64f271a 100644 --- a/deployment/components/common/worker-mounts.yaml +++ b/deployment/components/common/worker-mounts.yaml @@ -19,9 +19,6 @@ - name: host-lib hostPath: path: "/lib" - - name: source-d - hostPath: - path: "/etc/kubernetes/node-feature-discovery/source.d/" - name: features-d hostPath: path: "/etc/kubernetes/node-feature-discovery/features.d/" @@ -50,9 +47,6 @@ - name: host-lib mountPath: "/host-lib" readOnly: true - - name: source-d - mountPath: "/etc/kubernetes/node-feature-discovery/source.d/" - readOnly: true - name: features-d mountPath: "/etc/kubernetes/node-feature-discovery/features.d/" readOnly: true diff --git a/deployment/components/worker-config/nfd-worker.conf.example b/deployment/components/worker-config/nfd-worker.conf.example index af6ea7ca8..c608a72da 100644 --- a/deployment/components/worker-config/nfd-worker.conf.example +++ b/deployment/components/worker-config/nfd-worker.conf.example @@ -77,8 +77,6 @@ # - "class" # - "vendor" # - "device" -# local: -# hooksEnabled: false # custom: # # The following feature demonstrates the capabilities of the matchFeatures # - name: "my custom rule" diff --git a/deployment/helm/node-feature-discovery/templates/worker.yaml b/deployment/helm/node-feature-discovery/templates/worker.yaml index fbbc741e5..ea5c5be62 100644 --- a/deployment/helm/node-feature-discovery/templates/worker.yaml +++ b/deployment/helm/node-feature-discovery/templates/worker.yaml @@ -141,9 +141,6 @@ spec: mountPath: "/host-usr/src" readOnly: true {{- end }} - - name: source-d - mountPath: "/etc/kubernetes/node-feature-discovery/source.d/" - readOnly: true - name: features-d mountPath: "/etc/kubernetes/node-feature-discovery/features.d/" readOnly: true @@ -174,9 +171,6 @@ spec: hostPath: path: "/usr/src" {{- end }} - - name: source-d - hostPath: - path: "/etc/kubernetes/node-feature-discovery/source.d/" - name: features-d hostPath: path: "/etc/kubernetes/node-feature-discovery/features.d/" diff --git a/deployment/helm/node-feature-discovery/values.yaml b/deployment/helm/node-feature-discovery/values.yaml index 5ff0df68b..6c3c6f595 100644 --- a/deployment/helm/node-feature-discovery/values.yaml +++ b/deployment/helm/node-feature-discovery/values.yaml @@ -255,8 +255,6 @@ worker: # - "class" # - "vendor" # - "device" - # local: - # hooksEnabled: false # custom: # # The following feature demonstrates the capabilities of the matchFeatures # - name: "my custom rule" diff --git a/docs/deployment/image-variants.md b/docs/deployment/image-variants.md index 54826df0e..970563135 100644 --- a/docs/deployment/image-variants.md +++ b/docs/deployment/image-variants.md @@ -24,8 +24,8 @@ For backwards compatibility a container image tag with suffix `-minimal` ## Full This image is based on [debian:bookworm-slim](https://hub.docker.com/_/debian) -and contains a full Linux system for running shell-based nfd-worker hooks and -doing live debugging and diagnosis of the NFD images. +and contains a full Linux system for doing live debugging and diagnosis +of the NFD images. The container image tag has suffix `-full` (e.g. `{{ site.container_image }}-full`). diff --git a/docs/get-started/introduction.md b/docs/get-started/introduction.md index 5b6f6288d..73b7b8e45 100644 --- a/docs/get-started/introduction.md +++ b/docs/get-started/introduction.md @@ -71,7 +71,7 @@ Feature discovery is divided into domain-specific feature sources: - System - USB - Custom (rule-based custom features) -- Local (hooks for user-specific features) +- Local (features files) Each feature source is responsible for detecting a set of features which. in turn, are turned into node feature labels. Feature labels are prefixed with diff --git a/docs/reference/worker-configuration-reference.md b/docs/reference/worker-configuration-reference.md index 709f2eb35..c8a900799 100644 --- a/docs/reference/worker-configuration-reference.md +++ b/docs/reference/worker-configuration-reference.md @@ -304,32 +304,6 @@ sources: ### sources.local -### sources.local.hooksEnabled - -**DEPRECATED**: Hooks are DEPRECATED since v0.12.0 release and support (and -this configuration option) will be removed in NFD v0.17. Use -[feature files](../usage//customization-guide.md#feature-files) instead. - -Configuration option to disable/enable hooks execution. Disabled by default. - -> **NOTE:** The default NFD container image only supports statically linked -> binaries. Use the [full](../deployment/image-variants.md#full) image variant -> for a slightly more extensive environment that additionally supports bash and -> perl runtimes. - -GitHub tracking issue: -[Drop support for hooks (#856)](https://github.com/kubernetes-sigs/node-feature-discovery/issues/856). - -Default: false - -Example: - -```yaml -sources: - local: - hooksEnabled: true -``` - ### sources.pci #### sources.pci.deviceClassWhitelist diff --git a/docs/usage/customization-guide.md b/docs/usage/customization-guide.md index 7d55f3619..3fca6578e 100644 --- a/docs/usage/customization-guide.md +++ b/docs/usage/customization-guide.md @@ -26,7 +26,7 @@ labeling: - [`NodeFeatureRule`](#nodefeaturerule-custom-resource) objects provide a way to deploy custom labeling rules via the Kubernetes API. - [`local`](#local-feature-source) feature source of nfd-worker creates - labels by reading text files and executing hooks. + labels by reading text files. - [`custom`](#custom-feature-source) feature source of nfd-worker creates labels based on user-specified rules. @@ -232,13 +232,12 @@ point for external feature detectors. It provides a mechanism for pluggable extensions, allowing the creation of new user-specific features and even overriding built-in labels. -The `local` feature source has two methods for detecting features, feature -files and hooks (hooks are deprecated and slated for removal in NFD v0.17). The -features discovered by the `local` source can further be used in label rules -specified in [`NodeFeatureRule`](#nodefeaturerule-custom-resource) objects and +The `local` feature source uses feature files. The features discovered by the +`local` source can further be used in label rules specified in +[`NodeFeatureRule`](#nodefeaturerule-custom-resource) objects and the [`custom`](#custom-feature-source) feature source. -> **NOTE:** Be careful when creating and/or updating hook or feature files +> **NOTE:** Be careful when creating and/or updating feature files > while NFD is running. To avoid race conditions you should write > into a temporary file, and atomically create/update the original file by > doing a file rename operation. NFD ignores dot files, @@ -250,9 +249,7 @@ the [`custom`](#custom-feature-source) feature source. Consider a plaintext file `/etc/kubernetes/node-feature-discovery/features.d/my-features` -having the following contents (or alternatively a shell script -`/etc/kubernetes/node-feature-discovery/source.d/my-hook.sh` having the -following stdout output): +having the following contents: ```plaintext feature.node.kubernetes.io/my-feature.1 @@ -274,47 +271,9 @@ The `local` source reads files found in `/etc/kubernetes/node-feature-discovery/features.d/`. File content is parsed and translated into node labels, see the [input format below](#input-format). -### Hooks - -**DEPRECATED** Hooks are deprecated and will be completely removed in NFD -v0.17. - -The `local` source executes hooks found in -`/etc/kubernetes/node-feature-discovery/source.d/`. The hook files must be -executable and they are supposed to print all discovered features in `stdout`. -Since NFD v0.13 the default container image only supports statically linked ELF -binaries. - -`stderr` output of hooks is propagated to NFD log so it can be used for -debugging and logging. - -NFD tries to execute any regular files found from the hooks directory. -Any additional data files the hook might need (e.g. a configuration file) -should be placed in a separate directory to avoid NFD unnecessarily -trying to execute them. A subdirectory under the hooks directory can be used, -for example `/etc/kubernetes/node-feature-discovery/source.d/conf/`. - -> **NOTE:** Starting from release v0.14 hooks are disabled by default and can -> be enabled via `sources.local.hooksEnabled` field in the worker -> configuration. - -```yaml -sources: - local: - hooksEnabled: true # true by default at this point -``` - -> **NOTE:** NFD will blindly run any executables placed/mounted in the hooks -> directory. It is the user's responsibility to review the hooks for e.g. -> possible security implications. -> -> **NOTE:** The [full](../deployment/image-variants.md#full) image variant -> provides backwards-compatibility with older NFD versions by including a more -> expanded environment, supporting bash and perl runtimes. - ### Input format -The hook stdout and feature files are expected to contain features in simple +The feature files are expected to contain features in simple key-value pairs, separated by newlines: ```plaintext @@ -410,19 +369,16 @@ vendor.io/my-feature=value ### Mounts The standard NFD deployments contain `hostPath` mounts for -`/etc/kubernetes/node-feature-discovery/source.d/` and `/etc/kubernetes/node-feature-discovery/features.d/`, making these directories from the host available inside the nfd-worker container. #### Injecting labels from other pods -One use case for the feature files and hooks is detecting features in other +One use case for the feature files is detecting features in other Pods outside NFD, e.g. in Kubernetes device plugins. By using the same -`hostPath` mounts for `/etc/kubernetes/node-feature-discovery/source.d/` and -`/etc/kubernetes/node-feature-discovery/features.d/` in the side-car (e.g. -device plugin) creates a shared area for deploying feature files and hooks to -NFD. NFD periodically scans the directories and reads any feature files and -runs any hooks it finds. +`hostPath` mounts `/etc/kubernetes/node-feature-discovery/features.d/` +in the side-car (e.g. device plugin) creates a shared area for +deploying feature files to NFD. ## Custom feature source @@ -1000,8 +956,8 @@ The following features are available for matching: | | | **`major`** | int | First component of the kernel version (e.g. ‘4') | | | | **`minor`** | int | Second component of the kernel version (e.g. ‘5') | | | | **`revision`** | int | Third component of the kernel version (e.g. ‘6') | -| **`local.label`** | attribute | | | Labels from feature files and hooks, i.e. labels from the [*local* feature source](#local-feature-source) | -| **`local.feature`** | attribute | | | Features from feature files and hooks, i.e. features from the [*local* feature source](#local-feature-source) | +| **`local.label`** | attribute | | | Labels from feature files, i.e. labels from the [*local* feature source](#local-feature-source) | +| **`local.feature`** | attribute | | | Features from feature files, i.e. features from the [*local* feature source](#local-feature-source) | | | | **``** | string | Label `` created by the local feature source, value equals the value of the label | | **`memory.nv`** | instance | | | NVDIMM devices present in the system | | | | **``** | string | Value of the sysfs device attribute, available attributes: `devtype`, `mode` | diff --git a/pkg/nfd-worker/nfd-worker.go b/pkg/nfd-worker/nfd-worker.go index 0c65a59f2..7b5ce4d16 100644 --- a/pkg/nfd-worker/nfd-worker.go +++ b/pkg/nfd-worker/nfd-worker.go @@ -572,7 +572,7 @@ func getFeatureLabels(source source.LabelSource, labelWhiteList regexp.Regexp) ( name := k switch sourceName := source.Name(); sourceName { case "local", "custom": - // No mangling of labels from the custom rules, hooks or feature files + // No mangling of labels from the custom rules or feature files default: // Prefix for labels from other sources if !strings.Contains(name, "/") { diff --git a/source/local/local.go b/source/local/local.go index dcb93d927..ebad5a1cc 100644 --- a/source/local/local.go +++ b/source/local/local.go @@ -20,7 +20,6 @@ import ( "bytes" "fmt" "os" - "os/exec" "path/filepath" "strings" "time" @@ -64,7 +63,6 @@ const MaxFeatureFileSize = 65536 // Config var ( featureFilesDir = "/etc/kubernetes/node-feature-discovery/features.d/" - hookDir = "/etc/kubernetes/node-feature-discovery/source.d/" ) // localSource implements the FeatureSource and LabelSource interfaces. @@ -74,7 +72,6 @@ type localSource struct { } type Config struct { - HooksEnabled bool `json:"hooksEnabled,omitempty"` } // parsingOpts contains options used for directives parsing @@ -86,7 +83,7 @@ type parsingOpts struct { // Singleton source instance var ( - src = localSource{config: newDefaultConfig()} + src = localSource{} _ source.FeatureSource = &src _ source.LabelSource = &src _ source.ConfigurableSource = &src @@ -96,7 +93,7 @@ var ( func (s *localSource) Name() string { return Name } // NewConfig method of the LabelSource interface -func (s *localSource) NewConfig() source.Config { return newDefaultConfig() } +func (s *localSource) NewConfig() source.Config { return &Config{} } // GetConfig method of the LabelSource interface func (s *localSource) GetConfig() source.Config { return s.config } @@ -125,13 +122,6 @@ func (s *localSource) GetLabels() (source.FeatureLabels, error) { return labels, nil } -// newDefaultConfig returns a new config with pre-populated defaults -func newDefaultConfig() *Config { - return &Config{ - HooksEnabled: false, - } -} - // Discover method of the FeatureSource interface func (s *localSource) Discover() error { s.features = nfdv1alpha1.NewFeatures() @@ -141,33 +131,6 @@ func (s *localSource) Discover() error { klog.ErrorS(err, "failed to read feature files") } - if s.config.HooksEnabled { - - klog.InfoS("starting hooks...") - klog.InfoS("NOTE: hooks are deprecated and will be completely removed in a future release.") - - featuresFromHooks, labelsFromHooks, err := getFeaturesFromHooks() - if err != nil { - klog.ErrorS(err, "failed to run hooks") - } - - // Merge features from hooks and files - for k, v := range featuresFromHooks { - if old, ok := featuresFromFiles[k]; ok { - klog.InfoS("overriding feature value", "featureKey", k, "oldValue", old, "newValue", v) - } - featuresFromFiles[k] = v - } - - // Merge labels from hooks and files - for k, v := range labelsFromHooks { - if old, ok := labelsFromFiles[k]; ok { - klog.InfoS("overriding label value", "labelKey", k, "oldValue", old, "newValue", v) - } - labelsFromFiles[k] = v - } - } - s.features.Attributes[LabelFeature] = nfdv1alpha1.NewAttributeFeatures(labelsFromFiles) s.features.Attributes[RawFeature] = nfdv1alpha1.NewAttributeFeatures(featuresFromFiles) @@ -279,98 +242,6 @@ func updateFeatures(m map[string]string, lineSplit []string) { } } -// Run all hooks and get features -func getFeaturesFromHooks() (map[string]string, map[string]string, error) { - - features := make(map[string]string) - labels := make(map[string]string) - - files, err := os.ReadDir(hookDir) - if err != nil { - if os.IsNotExist(err) { - klog.InfoS("hook directory does not exist", "path", hookDir) - return features, labels, nil - } - return features, labels, fmt.Errorf("unable to access %v: %w", hookDir, err) - } - if len(files) > 0 { - klog.InfoS("hooks are DEPRECATED since v0.12.0 and support will be removed in a future release; use feature files instead") - } - - for _, file := range files { - fileName := file.Name() - // ignore hidden feature file - if strings.HasPrefix(fileName, ".") { - continue - } - lines, err := runHook(fileName) - if err != nil { - klog.ErrorS(err, "failed to run hook", "fileName", fileName) - continue - } - - // Append features - fileFeatures, fileLabels := parseFeatureFile(lines, fileName) - klog.V(4).InfoS("hook executed", "fileName", fileName, "features", utils.DelayedDumper(fileFeatures), "labels", utils.DelayedDumper(fileLabels)) - for k, v := range fileFeatures { - if old, ok := features[k]; ok { - klog.InfoS("overriding feature value from another hook", "featureKey", k, "oldValue", old, "newValue", v, "fileName", fileName) - } - features[k] = v - } - - for k, v := range fileLabels { - if old, ok := labels[k]; ok { - klog.InfoS("overriding label value from another hook", "labelKey", k, "oldValue", old, "newValue", v, "fileName", fileName) - } - labels[k] = v - } - } - - return features, labels, nil -} - -// Run one hook -func runHook(file string) ([][]byte, error) { - var lines [][]byte - - path := filepath.Join(hookDir, file) - filestat, err := os.Stat(path) - if err != nil { - klog.ErrorS(err, "failed to get filestat, skipping hook", "path", path) - return lines, err - } - - if filestat.Mode().IsRegular() { - cmd := exec.Command(path) - var stdout bytes.Buffer - var stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - // Run hook - err = cmd.Run() - - // Forward stderr to our logger - errLines := bytes.Split(stderr.Bytes(), []byte("\n")) - for i, line := range errLines { - if i == len(errLines)-1 && len(line) == 0 { - // Don't print the last empty string - break - } - klog.InfoS(fmt.Sprintf("%s: %s", file, line)) - } - - // Do not return any lines if an error occurred - if err != nil { - return lines, err - } - lines = bytes.Split(stdout.Bytes(), []byte("\n")) - } - - return lines, nil -} - // Read all files to get features func getFeaturesFromFiles() (map[string]string, map[string]string, error) { features := make(map[string]string)