Skip to content

Commit

Permalink
Merge pull request #871 from fmuyassarov/disable-hook
Browse files Browse the repository at this point in the history
Config option to disable hooks
  • Loading branch information
k8s-ci-robot authored Sep 26, 2022
2 parents 0abae92 + e7af8d0 commit 8662d17
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 14 deletions.
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)
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 @@ -46,18 +46,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 @@ -72,28 +94,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 @@ -132,6 +167,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 := os.ReadDir(hookDir)
Expand All @@ -142,6 +178,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

0 comments on commit 8662d17

Please sign in to comment.