go.d nvme: drop using `nvme` directly (#17386)

8 files changed, 37 insertions, 138 deletions

diff --git a/src/go/collectors/go.d.plugin/config/go.d/zfspool.conf b/src/go/collectors/go.d.plugin/config/go.d/zfspool.conf
index 587c9b772a..f18ff54e14 100644
--- a/src/go/collectors/go.d.plugin/config/go.d/zfspool.conf
+++ b/src/go/collectors/go.d.plugin/config/go.d/zfspool.conf
@@ -1,5 +1,5 @@
 ## All available configuration options, their descriptions and default values:
-## https://github.com/netdata/netdata/tree/master/src/go/collectors/go.d.plugin/modules/zpool#readme
+## https://github.com/netdata/netdata/tree/master/src/go/collectors/go.d.plugin/modules/zfspool#readme
 
 jobs:
   - name: zfspool
diff --git a/src/go/collectors/go.d.plugin/modules/intelgpu/metadata.yaml b/src/go/collectors/go.d.plugin/modules/intelgpu/metadata.yaml
index 0aabcb6ceb..1ff16ed785 100644
--- a/src/go/collectors/go.d.plugin/modules/intelgpu/metadata.yaml
+++ b/src/go/collectors/go.d.plugin/modules/intelgpu/metadata.yaml
@@ -64,7 +64,13 @@ modules:
           folding:
             title: Config
             enabled: true
-          list: []
+          list:
+            - name: Custom update_every
+              description: Allows you to override the default data collection interval.
+              config: |
+                jobs:
+                  - name: intelgpu
+                    update_every: 5  # Collect Intel iGPU metrics every 5 seconds
     troubleshooting:
       problems:
         list: []
diff --git a/src/go/collectors/go.d.plugin/modules/nvme/config_schema.json b/src/go/collectors/go.d.plugin/modules/nvme/config_schema.json
index 001d2a3e7d..179a24ab17 100644
--- a/src/go/collectors/go.d.plugin/modules/nvme/config_schema.json
+++ b/src/go/collectors/go.d.plugin/modules/nvme/config_schema.json
@@ -11,23 +11,15 @@
         "minimum": 1,
         "default": 10
       },
-      "binary_path": {
-        "title": "Binary path",
-        "description": "Path to the `nvme` binary.",
-        "type": "string",
-        "default": "nvme"
-      },
       "timeout": {
         "title": "Timeout",
-        "description": "Timeout for executing the binary, specified in seconds.",
+        "description": "Timeout for executing the `nvme`, specified in seconds.",
         "type": "number",
         "minimum": 0.5,
-        "default": 10
+        "default": 2
       }
     },
-    "required": [
-      "binary_path"
-    ],
+    "required": [],
     "additionalProperties": false,
     "patternProperties": {
       "^name$": {}
@@ -37,9 +29,6 @@
     "uiOptions": {
       "fullPage": true
     },
-    "binary_path": {
-      "ui:help": "If an absolute path is provided, the collector will use it directly; otherwise, it will search for the binary in directories specified in the PATH environment variable."
-    },
     "timeout": {
       "ui:help": "Accepts decimals for precise control (e.g., type 1.5 for 1.5 seconds)."
     }
diff --git a/src/go/collectors/go.d.plugin/modules/nvme/exec.go b/src/go/collectors/go.d.plugin/modules/nvme/exec.go
index 2e7687a87e..8c1281a2f8 100644
--- a/src/go/collectors/go.d.plugin/modules/nvme/exec.go
+++ b/src/go/collectors/go.d.plugin/modules/nvme/exec.go
@@ -54,27 +54,18 @@ func (n *nvmeNumber) UnmarshalJSON(b []byte) error {
 }
 
 type nvmeCLIExec struct {
-	sudoPath   string
-	nvmePath   string
 	ndsudoPath string
 	timeout    time.Duration
 }
 
 func (n *nvmeCLIExec) list() (*nvmeDeviceList, error) {
-	var data []byte
-	var err error
-
-	if n.ndsudoPath != "" {
-		data, err = n.executeNdSudo("nvme-list")
-	} else {
-		data, err = n.execute("list", "--output-format=json")
-	}
+	bs, err := n.execute("nvme-list")
 	if err != nil {
 		return nil, err
 	}
 
 	var v nvmeDeviceList
-	if err := json.Unmarshal(data, &v); err != nil {
+	if err := json.Unmarshal(bs, &v); err != nil {
 		return nil, err
 	}
 
@@ -82,20 +73,13 @@ func (n *nvmeCLIExec) list() (*nvmeDeviceList, error) {
 }
 
 func (n *nvmeCLIExec) smartLog(devicePath string) (*nvmeDeviceSmartLog, error) {
-	var data []byte
-	var err error
-
-	if n.ndsudoPath != "" {
-		data, err = n.executeNdSudo("nvme-smart-log", "--device", devicePath)
-	} else {
-		data, err = n.execute("smart-log", devicePath, "--output-format=json")
-	}
+	bs, err := n.execute("nvme-smart-log", "--device", devicePath)
 	if err != nil {
 		return nil, err
 	}
 
 	var v nvmeDeviceSmartLog
-	if err := json.Unmarshal(data, &v); err != nil {
+	if err := json.Unmarshal(bs, &v); err != nil {
 		return nil, err
 	}
 
@@ -106,17 +90,5 @@ func (n *nvmeCLIExec) execute(arg ...string) ([]byte, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), n.timeout)
 	defer cancel()
 
-	if n.sudoPath != "" {
-		args := append([]string{"-n", n.nvmePath}, arg...)
-		return exec.CommandContext(ctx, n.sudoPath, args...).Output()
-	}
-
-	return exec.CommandContext(ctx, n.nvmePath, arg...).Output()
-}
-
-func (n *nvmeCLIExec) executeNdSudo(arg ...string) ([]byte, error) {
-	ctx, cancel := context.WithTimeout(context.Background(), n.timeout)
-	defer cancel()
-
 	return exec.CommandContext(ctx, n.ndsudoPath, arg...).Output()
 }
diff --git a/src/go/collectors/go.d.plugin/modules/nvme/init.go b/src/go/collectors/go.d.plugin/modules/nvme/init.go
index 44ff90f4e2..51f1400a01 100644
--- a/src/go/collectors/go.d.plugin/modules/nvme/init.go
+++ b/src/go/collectors/go.d.plugin/modules/nvme/init.go
@@ -3,72 +3,24 @@
 package nvme
 
 import (
-	"context"
-	"errors"
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
-)
-
-func (n *NVMe) validateConfig() error {
-	if n.BinaryPath == "" {
-		return errors.New("'binary_path' can not be empty")
-	}
 
-	return nil
-}
+	"github.com/netdata/netdata/go/go.d.plugin/agent/executable"
+)
 
 func (n *NVMe) initNVMeCLIExec() (nvmeCLI, error) {
-	if exePath, err := os.Executable(); err == nil {
-		ndsudoPath := filepath.Join(filepath.Dir(exePath), "ndsudo")
-
-		if fi, err := os.Stat(ndsudoPath); err == nil {
-			// executable by owner or group
-			if fi.Mode().Perm()&0110 != 0 {
-				n.Debug("using ndsudo")
-				return &nvmeCLIExec{
-					ndsudoPath: ndsudoPath,
-					timeout:    n.Timeout.Duration(),
-				}, nil
-			}
-		}
-	}
-
-	// TODO: remove after next minor release of Netdata (latest is v1.44.0)
-	// can't remove now because it will break "from source + stable channel" installations
-	nvmePath, err := exec.LookPath(n.BinaryPath)
-	if err != nil {
-		return nil, err
-	}
+	ndsudoPath := filepath.Join(executable.Directory, "ndsudo")
 
-	var sudoPath string
-	if os.Getuid() != 0 {
-		sudoPath, err = exec.LookPath("sudo")
-		if err != nil {
-			return nil, err
-		}
+	if _, err := os.Stat(ndsudoPath); err != nil {
+		return nil, fmt.Errorf("ndsudo executable not found: %v", err)
 	}
 
-	if sudoPath != "" {
-		ctx1, cancel1 := context.WithTimeout(context.Background(), n.Timeout.Duration())
-		defer cancel1()
-
-		if _, err := exec.CommandContext(ctx1, sudoPath, "-n", "-v").Output(); err != nil {
-			return nil, fmt.Errorf("can not run sudo on this host: %v", err)
-		}
-
-		ctx2, cancel2 := context.WithTimeout(context.Background(), n.Timeout.Duration())
-		defer cancel2()
-
-		if _, err := exec.CommandContext(ctx2, sudoPath, "-n", "-l", nvmePath).Output(); err != nil {
-			return nil, fmt.Errorf("can not run '%s' with sudo: %v", n.BinaryPath, err)
-		}
+	nvmeExec := &nvmeCLIExec{
+		ndsudoPath: ndsudoPath,
+		timeout:    n.Timeout.Duration(),
 	}
 
-	return &nvmeCLIExec{
-		sudoPath: sudoPath,
-		nvmePath: nvmePath,
-		timeout:  n.Timeout.Duration(),
-	}, nil
+	return nvmeExec, nil
 }
diff --git a/src/go/collectors/go.d.plugin/modules/nvme/metadata.yaml b/src/go/collectors/go.d.plugin/modules/nvme/metadata.yaml
index 71a5be2e7c..546f9c6c85 100644
--- a/src/go/collectors/go.d.plugin/modules/nvme/metadata.yaml
+++ b/src/go/collectors/go.d.plugin/modules/nvme/metadata.yaml
@@ -21,9 +21,10 @@ modules:
     overview:
       data_collection:
         metrics_description: >
-          This collector monitors the health of NVMe devices using the command line
-          tool [nvme](https://github.com/linux-nvme/nvme-cli#nvme-cli), which can only be run by the root user. It uses `sudo` and
-          assumes it is set up so that the netdata user can execute `nvme` as root without a password.
+          This collector monitors the health of NVMe devices.
+          It relies on the [`nvme`](https://github.com/linux-nvme/nvme-cli#nvme-cli) CLI tool but avoids directly executing the binary.
+          Instead, it utilizes `ndsudo`, a Netdata helper specifically designed to run privileged commands securely within the Netdata environment.
+          This approach eliminates the need to use `sudo`, improving security and potentially simplifying permission management.
         method_description: ""
       supported_platforms:
         include: []
@@ -69,10 +70,6 @@ modules:
               description: Recheck interval in seconds. Zero means no recheck will be scheduled.
               default_value: 0
               required: false
-            - name: binary_path
-              description: Path to nvme binary. The default is "nvme" and the executable is looked for in the directories specified in the PATH environment variable.
-              default_value: nvme
-              required: false
             - name: timeout
               description: nvme binary execution timeout.
               default_value: 2
@@ -82,12 +79,12 @@ modules:
             title: Config
             enabled: true
           list:
-            - name: Custom binary path
-              description: The executable is not in the directories specified in the PATH environment variable.
+            - name: Custom update_every
+              description: Allows you to override the default data collection interval.
               config: |
                 jobs:
                   - name: nvme
-                    binary_path: /usr/local/sbin/nvme
+                    update_every: 5  # Collect NVMe metrics every 5 seconds
     troubleshooting:
       problems:
         list: []
diff --git a/src/go/collectors/go.d.plugin/modules/nvme/nvme.go b/src/go/collectors/go.d.plugin/modules/nvme/nvme.go
index 9ed9fb7951..760317919b 100644
--- a/src/go/collectors/go.d.plugin/modules/nvme/nvme.go
+++ b/src/go/collectors/go.d.plugin/modules/nvme/nvme.go
@@ -27,8 +27,7 @@ func init() {
 func New() *NVMe {
 	return &NVMe{
 		Config: Config{
-			BinaryPath: "nvme",
-			Timeout:    web.Duration(time.Second * 2),
+			Timeout: web.Duration(time.Second * 2),
 		},
 
 		charts:           &module.Charts{},
@@ -41,7 +40,6 @@ func New() *NVMe {
 type Config struct {
 	UpdateEvery int          `yaml:"update_every" json:"update_every"`
 	Timeout     web.Duration `yaml:"timeout" json:"timeout"`
-	BinaryPath  string       `yaml:"binary_path" json:"binary_path"`
 }
 
 type (
@@ -69,17 +67,12 @@ func (n *NVMe) Configuration() any {
 }
 
 func (n *NVMe) Init() error {
-	if err := n.validateConfig(); err != nil {
-		n.Errorf("config validation: %v", err)
-		return err
-	}
-
-	v, err := n.initNVMeCLIExec()
+	nvmeExec, err := n.initNVMeCLIExec()
 	if err != nil {
 		n.Errorf("init nvme-cli exec: %v", err)
 		return err
 	}
-	n.exec = v
+	n.exec = nvmeExec
 
 	return nil
 }
diff --git a/src/go/collectors/go.d.plugin/modules/nvme/nvme_test.go b/src/go/collectors/go.d.plugin/modules/nvme/nvme_test.go
index 6ca15974aa..ab814442d6 100644
--- a/src/go/collectors/go.d.plugin/modules/nvme/nvme_test.go
+++ b/src/go/collectors/go.d.plugin/modules/nvme/nvme_test.go
@@ -45,20 +45,12 @@ func TestNVMe_ConfigurationSerialize(t *testing.T) {
 
 func TestNVMe_Init(t *testing.T) {
 	tests := map[string]struct {
-		prepare  func(n *NVMe)
+		config   Config
 		wantFail bool
 	}{
-		"fails if 'binary_path' not set": {
-			wantFail: true,
-			prepare: func(n *NVMe) {
-				n.BinaryPath = ""
-			},
-		},
-		"fails if can't locate nvme-cli": {
+		"fails if 'ndsudo' not found": {
 			wantFail: true,
-			prepare: func(n *NVMe) {
-				n.BinaryPath += "!!!"
-			},
+			config:   New().Config,
 		},
 	}
 
@@ -66,8 +58,6 @@ func TestNVMe_Init(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			nv := New()
 
-			test.prepare(nv)
-
 			if test.wantFail {
 				assert.Error(t, nv.Init())
 			} else {