Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config option to disable hooks #871

Merged
merged 2 commits into from
Sep 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions deployment/components/worker-config/nfd-worker.conf.example
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
# - "class"
# - "vendor"
# - "device"
# local:
# hooksEnabled: true
# custom:
# # The following feature demonstrates the capabilities of the matchFeatures
# - name: "my custom rule"
Expand Down
2 changes: 2 additions & 0 deletions deployment/helm/node-feature-discovery/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ worker:
# - "class"
# - "vendor"
# - "device"
# local:
# hooksEnabled: true
# custom:
# # The following feature demonstrates the capabilities of the matchFeatures
# - name: "my custom rule"
Expand Down
10 changes: 10 additions & 0 deletions docs/advanced/customization-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,16 @@ should be placed in a separate directory in order 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:** Hooks are being DEPRECATED and will be removed in a future release.
For backward compatibility, currently hooks are enabled by default and can be
disabled 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.
Expand Down
23 changes: 23 additions & 0 deletions docs/advanced/worker-configuration-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,29 @@ sources:
configOpts: [NO_HZ, X86, DMI]
```

### sources.local

### sources.local.hooksEnabled

Configuration option to disable/enable hooks execution. Enabled by default.
Hooks are DEPRECATED since v0.12.0 release and support will be removed in a
future release. Use [feature files](./customization-guide.md#feature-files)
fmuyassarov marked this conversation as resolved.
Show resolved Hide resolved
instead.

Related tracking issues:

1. Config option to disable hooks [#859](https://github.com/kubernetes-sigs/node-feature-discovery/issues/859).
1. Disable hook support by default [#855](https://github.com/kubernetes-sigs/node-feature-discovery/issues/855).
1. Drop support for hooks [#856](https://github.com/kubernetes-sigs/node-feature-discovery/issues/856).

Example:

```yaml
sources:
local:
hooksEnabled: true # true by default
```

### soures.pci

#### soures.pci.deviceClassWhitelist
Expand Down
67 changes: 53 additions & 14 deletions source/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,40 @@ var (
// localSource implements the FeatureSource and LabelSource interfaces.
type localSource struct {
features *feature.DomainFeatures
config *Config
}

type Config struct {
HooksEnabled bool `json:"hooksEnabled,omitempty"`
}

// Singleton source instance
var (
src localSource
_ source.FeatureSource = &src
_ source.LabelSource = &src
src = localSource{config: newDefaultConfig()}
_ source.FeatureSource = &src
_ source.LabelSource = &src
_ source.ConfigurableSource = &src
)

// Name method of the LabelSource interface
func (s *localSource) Name() string { return Name }

// NewConfig method of the LabelSource interface
func (s *localSource) NewConfig() source.Config { return newDefaultConfig() }

// GetConfig method of the LabelSource interface
func (s *localSource) GetConfig() source.Config { return s.config }

// SetConfig method of the LabelSource interface
func (s *localSource) SetConfig(conf source.Config) {
switch v := conf.(type) {
case *Config:
s.config = v
default:
klog.Fatalf("invalid config type: %T", conf)
}
}

// Priority method of the LabelSource interface
func (s *localSource) Priority() int { return 20 }

Expand All @@ -73,28 +95,41 @@ 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: true,
}
}

// Discover method of the FeatureSource interface
func (s *localSource) Discover() error {
s.features = feature.NewDomainFeatures()

featuresFromHooks, err := getFeaturesFromHooks()
if err != nil {
klog.Error(err)
}

featuresFromFiles, err := getFeaturesFromFiles()
if err != nil {
klog.Error(err)
}

// Merge features from hooks and files
for k, v := range featuresFromHooks {
if old, ok := featuresFromFiles[k]; ok {
klog.Warningf("overriding '%s': value changed from '%s' to '%s'",
k, old, v)
if s.config.HooksEnabled {

klog.Info("starting hooks...")

featuresFromHooks, err := getFeaturesFromHooks()
if err != nil {
klog.Error(err)
}

// Merge features from hooks and files
for k, v := range featuresFromHooks {
if old, ok := featuresFromFiles[k]; ok {
klog.Warningf("overriding '%s': value changed from '%s' to '%s'",
k, old, v)
}
featuresFromFiles[k] = v
}
featuresFromFiles[k] = v
}

s.features.Values[LabelFeature] = feature.NewValueFeatures(featuresFromFiles)

utils.KlogDump(3, "discovered local features:", " ", s.features)
Expand Down Expand Up @@ -133,6 +168,7 @@ func parseFeatures(lines [][]byte) map[string]string {

// Run all hooks and get features
func getFeaturesFromHooks() (map[string]string, error) {

features := make(map[string]string)

files, err := ioutil.ReadDir(hookDir)
Expand All @@ -143,6 +179,9 @@ func getFeaturesFromHooks() (map[string]string, error) {
}
return features, fmt.Errorf("unable to access %v: %v", hookDir, err)
}
if len(files) > 0 {
klog.Warning("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()
Expand Down