From 72a1f36dcc1878843c592de927520829b6173ad3 Mon Sep 17 00:00:00 2001 From: Ilya Mashchenko Date: Thu, 11 Apr 2024 14:14:06 +0300 Subject: go.d intelgpu switch to using ndsudo (#17380) --- .../go.d.plugin/config/go.d/intelgpu.conf | 1 - .../modules/intelgpu/config_schema.json | 12 ---------- .../go.d.plugin/modules/intelgpu/exec.go | 15 +++++++++---- .../go.d.plugin/modules/intelgpu/init.go | 17 +++++++------- .../go.d.plugin/modules/intelgpu/intelgpu.go | 16 +++++-------- .../go.d.plugin/modules/intelgpu/intelgpu_test.go | 4 ++-- .../go.d.plugin/modules/intelgpu/metadata.yaml | 26 +++++----------------- .../modules/intelgpu/testdata/config.json | 3 +-- .../modules/intelgpu/testdata/config.yaml | 1 - .../collectors/go.d.plugin/modules/zfspool/init.go | 1 + 10 files changed, 34 insertions(+), 62 deletions(-) diff --git a/src/go/collectors/go.d.plugin/config/go.d/intelgpu.conf b/src/go/collectors/go.d.plugin/config/go.d/intelgpu.conf index 806f4158ae..3639076f1a 100644 --- a/src/go/collectors/go.d.plugin/config/go.d/intelgpu.conf +++ b/src/go/collectors/go.d.plugin/config/go.d/intelgpu.conf @@ -3,4 +3,3 @@ jobs: - name: intelgpu - binary_path: /usr/bin/intel_gpu_top diff --git a/src/go/collectors/go.d.plugin/modules/intelgpu/config_schema.json b/src/go/collectors/go.d.plugin/modules/intelgpu/config_schema.json index 4224129f04..ddee75e7af 100644 --- a/src/go/collectors/go.d.plugin/modules/intelgpu/config_schema.json +++ b/src/go/collectors/go.d.plugin/modules/intelgpu/config_schema.json @@ -10,17 +10,8 @@ "type": "integer", "minimum": 1, "default": 1 - }, - "binary_path": { - "title": "Binary path", - "description": "Path to the `intel_gpu_top` binary.", - "type": "string", - "default": "/usr/bin/intel_gpu_top" } }, - "required": [ - "binary_path" - ], "additionalProperties": false, "patternProperties": { "^name$": {} @@ -29,9 +20,6 @@ "uiSchema": { "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." } } } diff --git a/src/go/collectors/go.d.plugin/modules/intelgpu/exec.go b/src/go/collectors/go.d.plugin/modules/intelgpu/exec.go index 836c6f58a7..cb81bccb01 100644 --- a/src/go/collectors/go.d.plugin/modules/intelgpu/exec.go +++ b/src/go/collectors/go.d.plugin/modules/intelgpu/exec.go @@ -10,11 +10,14 @@ import ( "strconv" "sync" "time" + + "github.com/netdata/netdata/go/go.d.plugin/logger" ) -func newIntelGpuTopExec(binPath string, updateEvery int) (*intelGpuTopExec, error) { +func newIntelGpuTopExec(ndsudoPath string, updateEvery int, log *logger.Logger) (*intelGpuTopExec, error) { topExec := &intelGpuTopExec{ - binPath: binPath, + Logger: log, + ndsudoPath: ndsudoPath, updateEvery: updateEvery, } @@ -26,7 +29,9 @@ func newIntelGpuTopExec(binPath string, updateEvery int) (*intelGpuTopExec, erro } type intelGpuTopExec struct { - binPath string + *logger.Logger + + ndsudoPath string updateEvery int cmd *exec.Cmd @@ -42,7 +47,9 @@ func (e *intelGpuTopExec) run() error { refresh = e.updateEvery*1000 - 500 // milliseconds } - cmd := exec.Command(e.binPath, "-J", "-s", strconv.Itoa(refresh)) + cmd := exec.Command(e.ndsudoPath, "igt-json", "--interval", strconv.Itoa(refresh)) + + e.Debugf("executing '%s'", cmd) r, err := cmd.StdoutPipe() if err != nil { diff --git a/src/go/collectors/go.d.plugin/modules/intelgpu/init.go b/src/go/collectors/go.d.plugin/modules/intelgpu/init.go index a398b0fa1f..e36db7c163 100644 --- a/src/go/collectors/go.d.plugin/modules/intelgpu/init.go +++ b/src/go/collectors/go.d.plugin/modules/intelgpu/init.go @@ -5,18 +5,17 @@ package intelgpu import ( "fmt" "os" - "os/exec" + "path/filepath" + + "github.com/netdata/netdata/go/go.d.plugin/agent/executable" ) func (ig *IntelGPU) initIntelGPUTopExec() (intelGpuTop, error) { - binPath := ig.BinaryPath - if _, err := os.Stat(binPath); os.IsNotExist(err) { - path, err := exec.LookPath(ig.binName) - if err != nil { - return nil, fmt.Errorf("error on lookup '%s': %v", ig.binName, err) - } - binPath = path + ndsudoPath := filepath.Join(executable.Directory, ig.ndsudoName) + if _, err := os.Stat(ndsudoPath); err != nil { + return nil, fmt.Errorf("ndsudo executable not found: %v", err) + } - return newIntelGpuTopExec(binPath, ig.UpdateEvery) + return newIntelGpuTopExec(ndsudoPath, ig.UpdateEvery, ig.Logger) } diff --git a/src/go/collectors/go.d.plugin/modules/intelgpu/intelgpu.go b/src/go/collectors/go.d.plugin/modules/intelgpu/intelgpu.go index 5b355811ba..09dcffc768 100644 --- a/src/go/collectors/go.d.plugin/modules/intelgpu/intelgpu.go +++ b/src/go/collectors/go.d.plugin/modules/intelgpu/intelgpu.go @@ -21,18 +21,14 @@ func init() { func New() *IntelGPU { return &IntelGPU{ - Config: Config{ - BinaryPath: "/usr/bin/intel_gpu_top", - }, - binName: "intel_gpu_top", - charts: charts.Copy(), - engines: make(map[string]bool), + ndsudoName: "ndsudo", + charts: charts.Copy(), + engines: make(map[string]bool), } } type Config struct { - UpdateEvery int `yaml:"update_every" json:"update_every"` - BinaryPath string `yaml:"binary_path" json:"binary_path"` + UpdateEvery int `yaml:"update_every" json:"update_every"` } type ( @@ -42,8 +38,8 @@ type ( charts *module.Charts - exec intelGpuTop - binName string + exec intelGpuTop + ndsudoName string engines map[string]bool } diff --git a/src/go/collectors/go.d.plugin/modules/intelgpu/intelgpu_test.go b/src/go/collectors/go.d.plugin/modules/intelgpu/intelgpu_test.go index 8d897e2055..ac38727c60 100644 --- a/src/go/collectors/go.d.plugin/modules/intelgpu/intelgpu_test.go +++ b/src/go/collectors/go.d.plugin/modules/intelgpu/intelgpu_test.go @@ -39,10 +39,10 @@ func TestIntelGPU_Init(t *testing.T) { prepare func(igt *IntelGPU) wantFail bool }{ - "fails if can't find intel_gpu_top": { + "fails if can't locate ndsudo": { wantFail: true, prepare: func(igt *IntelGPU) { - igt.binName += "!!!" + igt.ndsudoName += "!!!" }, }, } 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 be77e73fd7..0aabcb6ceb 100644 --- a/src/go/collectors/go.d.plugin/modules/intelgpu/metadata.yaml +++ b/src/go/collectors/go.d.plugin/modules/intelgpu/metadata.yaml @@ -23,8 +23,10 @@ modules: overview: data_collection: metrics_description: | - This collector monitors Intel integrated GPUs performance metrics using - the [intel_gpu_top](https://manpages.debian.org/testing/intel-gpu-tools/intel_gpu_top.1.en.html) CLI tool. + This collector gathers performance metrics for Intel integrated GPUs. + It relies on the [`intel_gpu_top`](https://manpages.debian.org/testing/intel-gpu-tools/intel_gpu_top.1.en.html) 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 grant the CAP_PERFMON capability to `intel_gpu_top`, improving security and potentially simplifying permission management. method_description: "" supported_platforms: include: [] @@ -44,14 +46,6 @@ modules: list: - title: Install intel-gpu-tools description: Install `intel-gpu-tools` using your distribution's package manager. - - title: Add CAP_PERFMON to `intel_gpu_top` - description: | - When running as a normal user CAP_PERFMON is required to access performance monitoring. - See [capabilities(7)](https://man7.org/linux/man-pages/man7/capabilities.7.html) and [setcap(8)](https://man7.org/linux/man-pages/man8/setcap.8.html). - - ```bash - sudo setcap cap_perfmon=eip /usr/bin/intel_gpu_top - ``` configuration: file: name: go.d/intelgpu.conf @@ -66,21 +60,11 @@ modules: description: Data collection frequency. default_value: 1 required: false - - name: binary_path - description: Path to the `intel_gpu_top` binary. 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. - default_value: /usr/bin/intel_gpu_top - required: true examples: folding: title: Config enabled: true - list: - - name: Custom binary path - description: The executable is not in the directories specified in the PATH environment variable. - config: | - jobs: - - name: nvidia_smi - binary_path: /usr/local/sbin/intel_gpu_top + list: [] troubleshooting: problems: list: [] diff --git a/src/go/collectors/go.d.plugin/modules/intelgpu/testdata/config.json b/src/go/collectors/go.d.plugin/modules/intelgpu/testdata/config.json index 723a66ff24..0e3f7c4039 100644 --- a/src/go/collectors/go.d.plugin/modules/intelgpu/testdata/config.json +++ b/src/go/collectors/go.d.plugin/modules/intelgpu/testdata/config.json @@ -1,4 +1,3 @@ { - "update_every": 123, - "binary_path": "ok" + "update_every": 123 } diff --git a/src/go/collectors/go.d.plugin/modules/intelgpu/testdata/config.yaml b/src/go/collectors/go.d.plugin/modules/intelgpu/testdata/config.yaml index 7d0ab9437f..f21a3a7a06 100644 --- a/src/go/collectors/go.d.plugin/modules/intelgpu/testdata/config.yaml +++ b/src/go/collectors/go.d.plugin/modules/intelgpu/testdata/config.yaml @@ -1,2 +1 @@ update_every: 123 -binary_path: "ok" diff --git a/src/go/collectors/go.d.plugin/modules/zfspool/init.go b/src/go/collectors/go.d.plugin/modules/zfspool/init.go index d4debe3fa0..f640801dd1 100644 --- a/src/go/collectors/go.d.plugin/modules/zfspool/init.go +++ b/src/go/collectors/go.d.plugin/modules/zfspool/init.go @@ -32,6 +32,7 @@ func (z *ZFSPool) initZPoolCLIExec() (zpoolCLI, error) { } zpoolExec := newZpoolCLIExec(binPath, z.Timeout.Duration()) + zpoolExec.Logger = z.Logger return zpoolExec, nil } -- cgit v1.2.3