go.d nvme: drop using `nvme` directly (#17386)
Ilya Mashchenko committed
Apr 11, 2024 at 17:16 UTC
111bd27097166159375063bb829543dddaed900d
8 files changed
+37
-138
src/go/collectors/go.d.plugin/config/go.d/zfspool.conf
+1
-1
@@ -1,5 +1,5 @@
1
## All available configuration options, their descriptions and default values:
2
-## https://github.com/netdata/netdata/tree/master/src/go/collectors/go.d.plugin/modules/zpool#readme
2
+## https://github.com/netdata/netdata/tree/master/src/go/collectors/go.d.plugin/modules/zfspool#readme
3
4
jobs:
5
- name: zfspool
src/go/collectors/go.d.plugin/modules/intelgpu/metadata.yaml
+7
-1
@@ -64,7 +64,13 @@ modules:
64
folding:
65
title: Config
66
enabled: true
67
- list: []
67
+ list:
68
+ - name: Custom update_every
69
+ description: Allows you to override the default data collection interval.
70
+ config: |
71
+ jobs:
72
+ - name: intelgpu
73
+ update_every: 5 # Collect Intel iGPU metrics every 5 seconds
74
troubleshooting:
75
problems:
76
list: []
src/go/collectors/go.d.plugin/modules/nvme/config_schema.json
+3
-14
@@ -11,23 +11,15 @@
11
"minimum": 1,
12
"default": 10
13
},
14
- "binary_path": {
15
- "title": "Binary path",
16
- "description": "Path to the `nvme` binary.",
17
- "type": "string",
18
- "default": "nvme"
19
- },
14
"timeout": {
15
"title": "Timeout",
22
- "description": "Timeout for executing the binary, specified in seconds.",
16
+ "description": "Timeout for executing the `nvme`, specified in seconds.",
17
"type": "number",
18
"minimum": 0.5,
25
- "default": 10
19
+ "default": 2
20
}
21
},
28
- "required": [
29
- "binary_path"
30
- ],
22
+ "required": [],
23
"additionalProperties": false,
24
"patternProperties": {
25
"^name$": {}
@@ -37,9 +29,6 @@
29
"uiOptions": {
30
"fullPage": true
31
},
40
- "binary_path": {
41
- "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."
42
- },
32
"timeout": {
33
"ui:help": "Accepts decimals for precise control (e.g., type 1.5 for 1.5 seconds)."
34
}
src/go/collectors/go.d.plugin/modules/nvme/exec.go
+4
-32
@@ -54,27 +54,18 @@ func (n *nvmeNumber) UnmarshalJSON(b []byte) error {
54
}
55
56
type nvmeCLIExec struct {
57
- sudoPath string
58
- nvmePath string
57
ndsudoPath string
58
timeout time.Duration
59
}
60
61
func (n *nvmeCLIExec) list() (*nvmeDeviceList, error) {
64
- var data []byte
65
- var err error
66
-
67
- if n.ndsudoPath != "" {
68
- data, err = n.executeNdSudo("nvme-list")
69
- } else {
70
- data, err = n.execute("list", "--output-format=json")
71
- }
62
+ bs, err := n.execute("nvme-list")
63
if err != nil {
64
return nil, err
65
}
66
67
var v nvmeDeviceList
77
- if err := json.Unmarshal(data, &v); err != nil {
68
+ if err := json.Unmarshal(bs, &v); err != nil {
69
return nil, err
70
}
71
@@ -82,20 +73,13 @@ func (n *nvmeCLIExec) list() (*nvmeDeviceList, error) {
73
}
74
75
func (n *nvmeCLIExec) smartLog(devicePath string) (*nvmeDeviceSmartLog, error) {
85
- var data []byte
86
- var err error
87
-
88
- if n.ndsudoPath != "" {
89
- data, err = n.executeNdSudo("nvme-smart-log", "--device", devicePath)
90
- } else {
91
- data, err = n.execute("smart-log", devicePath, "--output-format=json")
92
- }
76
+ bs, err := n.execute("nvme-smart-log", "--device", devicePath)
77
if err != nil {
78
return nil, err
79
}
80
81
var v nvmeDeviceSmartLog
98
- if err := json.Unmarshal(data, &v); err != nil {
82
+ if err := json.Unmarshal(bs, &v); err != nil {
83
return nil, err
84
}
85
@@ -106,17 +90,5 @@ func (n *nvmeCLIExec) execute(arg ...string) ([]byte, error) {
90
ctx, cancel := context.WithTimeout(context.Background(), n.timeout)
91
defer cancel()
92
109
- if n.sudoPath != "" {
110
- args := append([]string{"-n", n.nvmePath}, arg...)
111
- return exec.CommandContext(ctx, n.sudoPath, args...).Output()
112
- }
113
-
114
- return exec.CommandContext(ctx, n.nvmePath, arg...).Output()
115
-}
116
-
117
-func (n *nvmeCLIExec) executeNdSudo(arg ...string) ([]byte, error) {
118
- ctx, cancel := context.WithTimeout(context.Background(), n.timeout)
119
- defer cancel()
120
-
93
return exec.CommandContext(ctx, n.ndsudoPath, arg...).Output()
94
}
src/go/collectors/go.d.plugin/modules/nvme/init.go
+9
-57
@@ -3,72 +3,24 @@
3
package nvme
4
5
import (
6
- "context"
7
- "errors"
6
"fmt"
7
"os"
10
- "os/exec"
8
"path/filepath"
12
-)
13
-
14
-func (n *NVMe) validateConfig() error {
15
- if n.BinaryPath == "" {
16
- return errors.New("'binary_path' can not be empty")
17
- }
9
19
- return nil
20
-}
10
+ "github.com/netdata/netdata/go/go.d.plugin/agent/executable"
11
+)
12
13
func (n *NVMe) initNVMeCLIExec() (nvmeCLI, error) {
23
- if exePath, err := os.Executable(); err == nil {
24
- ndsudoPath := filepath.Join(filepath.Dir(exePath), "ndsudo")
25
-
26
- if fi, err := os.Stat(ndsudoPath); err == nil {
27
- // executable by owner or group
28
- if fi.Mode().Perm()&0110 != 0 {
29
- n.Debug("using ndsudo")
30
- return &nvmeCLIExec{
31
- ndsudoPath: ndsudoPath,
32
- timeout: n.Timeout.Duration(),
33
- }, nil
34
- }
35
- }
36
- }
37
-
38
- // TODO: remove after next minor release of Netdata (latest is v1.44.0)
39
- // can't remove now because it will break "from source + stable channel" installations
40
- nvmePath, err := exec.LookPath(n.BinaryPath)
41
- if err != nil {
42
- return nil, err
43
- }
14
+ ndsudoPath := filepath.Join(executable.Directory, "ndsudo")
15
45
- var sudoPath string
46
- if os.Getuid() != 0 {
47
- sudoPath, err = exec.LookPath("sudo")
48
- if err != nil {
49
- return nil, err
50
- }
16
+ if _, err := os.Stat(ndsudoPath); err != nil {
17
+ return nil, fmt.Errorf("ndsudo executable not found: %v", err)
18
}
19
53
- if sudoPath != "" {
54
- ctx1, cancel1 := context.WithTimeout(context.Background(), n.Timeout.Duration())
55
- defer cancel1()
56
-
57
- if _, err := exec.CommandContext(ctx1, sudoPath, "-n", "-v").Output(); err != nil {
58
- return nil, fmt.Errorf("can not run sudo on this host: %v", err)
59
- }
60
-
61
- ctx2, cancel2 := context.WithTimeout(context.Background(), n.Timeout.Duration())
62
- defer cancel2()
63
-
64
- if _, err := exec.CommandContext(ctx2, sudoPath, "-n", "-l", nvmePath).Output(); err != nil {
65
- return nil, fmt.Errorf("can not run '%s' with sudo: %v", n.BinaryPath, err)
66
- }
20
+ nvmeExec := &nvmeCLIExec{
21
+ ndsudoPath: ndsudoPath,
22
+ timeout: n.Timeout.Duration(),
23
}
24
69
- return &nvmeCLIExec{
70
- sudoPath: sudoPath,
71
- nvmePath: nvmePath,
72
- timeout: n.Timeout.Duration(),
73
- }, nil
25
+ return nvmeExec, nil
26
}
src/go/collectors/go.d.plugin/modules/nvme/metadata.yaml
+7
-10
@@ -21,9 +21,10 @@ modules:
21
overview:
22
data_collection:
23
metrics_description: >
24
- This collector monitors the health of NVMe devices using the command line
25
- tool [nvme](https://github.com/linux-nvme/nvme-cli#nvme-cli), which can only be run by the root user. It uses `sudo` and
26
- assumes it is set up so that the netdata user can execute `nvme` as root without a password.
24
+ This collector monitors the health of NVMe devices.
25
+ It relies on the [`nvme`](https://github.com/linux-nvme/nvme-cli#nvme-cli) CLI tool but avoids directly executing the binary.
26
+ Instead, it utilizes `ndsudo`, a Netdata helper specifically designed to run privileged commands securely within the Netdata environment.
27
+ This approach eliminates the need to use `sudo`, improving security and potentially simplifying permission management.
28
method_description: ""
29
supported_platforms:
30
include: []
@@ -69,10 +70,6 @@ modules:
70
description: Recheck interval in seconds. Zero means no recheck will be scheduled.
71
default_value: 0
72
required: false
72
- - name: binary_path
73
- description: Path to nvme binary. The default is "nvme" and the executable is looked for in the directories specified in the PATH environment variable.
74
- default_value: nvme
75
- required: false
73
- name: timeout
74
description: nvme binary execution timeout.
75
default_value: 2
@@ -82,12 +79,12 @@ modules:
79
title: Config
80
enabled: true
81
list:
85
- - name: Custom binary path
86
- description: The executable is not in the directories specified in the PATH environment variable.
82
+ - name: Custom update_every
83
+ description: Allows you to override the default data collection interval.
84
config: |
85
jobs:
86
- name: nvme
90
- binary_path: /usr/local/sbin/nvme
87
+ update_every: 5 # Collect NVMe metrics every 5 seconds
88
troubleshooting:
89
problems:
90
list: []
src/go/collectors/go.d.plugin/modules/nvme/nvme.go
+3
-10
@@ -27,8 +27,7 @@ func init() {
27
func New() *NVMe {
28
return &NVMe{
29
Config: Config{
30
- BinaryPath: "nvme",
31
- Timeout: web.Duration(time.Second * 2),
30
+ Timeout: web.Duration(time.Second * 2),
31
},
32
33
charts: &module.Charts{},
@@ -41,7 +40,6 @@ func New() *NVMe {
40
type Config struct {
41
UpdateEvery int `yaml:"update_every" json:"update_every"`
42
Timeout web.Duration `yaml:"timeout" json:"timeout"`
44
- BinaryPath string `yaml:"binary_path" json:"binary_path"`
43
}
44
45
type (
@@ -69,17 +67,12 @@ func (n *NVMe) Configuration() any {
67
}
68
69
func (n *NVMe) Init() error {
72
- if err := n.validateConfig(); err != nil {
73
- n.Errorf("config validation: %v", err)
74
- return err
75
- }
76
-
77
- v, err := n.initNVMeCLIExec()
70
+ nvmeExec, err := n.initNVMeCLIExec()
71
if err != nil {
72
n.Errorf("init nvme-cli exec: %v", err)
73
return err
74
}
82
- n.exec = v
75
+ n.exec = nvmeExec
76
77
return nil
78
}
src/go/collectors/go.d.plugin/modules/nvme/nvme_test.go
+3
-13
@@ -45,20 +45,12 @@ func TestNVMe_ConfigurationSerialize(t *testing.T) {
45
46
func TestNVMe_Init(t *testing.T) {
47
tests := map[string]struct {
48
- prepare func(n *NVMe)
48
+ config Config
49
wantFail bool
50
}{
51
- "fails if 'binary_path' not set": {
52
- wantFail: true,
53
- prepare: func(n *NVMe) {
54
- n.BinaryPath = ""
55
- },
56
- },
57
- "fails if can't locate nvme-cli": {
51
+ "fails if 'ndsudo' not found": {
52
wantFail: true,
59
- prepare: func(n *NVMe) {
60
- n.BinaryPath += "!!!"
61
- },
53
+ config: New().Config,
54
},
55
}
56
@@ -66,8 +58,6 @@ func TestNVMe_Init(t *testing.T) {
58
t.Run(name, func(t *testing.T) {
59
nv := New()
60
69
- test.prepare(nv)
70
-
61
if test.wantFail {
62
assert.Error(t, nv.Init())
63
} else {