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

Operation not permitted from prometheus.exporters.smartctl #176524

Closed
MalteT opened this issue Jun 6, 2022 · 6 comments · Fixed by #176553
Closed

Operation not permitted from prometheus.exporters.smartctl #176524

MalteT opened this issue Jun 6, 2022 · 6 comments · Fixed by #176553

Comments

@MalteT
Copy link
Contributor

MalteT commented Jun 6, 2022

Describe the bug

prometheus-smartctl-exporter cannot access my devices. Here's an example log:

Jun 05 21:02:22 faunus-ater smartctl_exporter[195740]: [Error] Smartctl open device: /dev/sde failed: Operation not permitted
Jun 05 21:02:22 faunus-ater smartctl_exporter[195740]: [Error] smartctl returned bad data for device /dev/sde
Jun 05 21:02:22 faunus-ater smartctl_exporter[195740]: [Warning] S.M.A.R.T. output reading error: exit status 2
Jun 05 21:02:22 faunus-ater smartctl_exporter[195740]: [Warning] The device error log contains records of errors.

I'm almost certain, that the issue is an empty DeviceAllow=, as the service runs fine without it, but I'd like a second opinion before sending a PR.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Configure services.prometheus and set services.prometheus.exporters.smartctl.enable = true
  2. Watch journalctl -u prometheus-smartctl-exporter
  3. [Optional] Override DeviceAllow list:
    systemd.services."prometheus-smartctl-exporter".serviceConfig.DeviceAllow = lib.mkOverride 10 [
        "block-blkext rw"
        "block-sd rw"
        "char-nvme rw"
    ]
    The service will work now.

Expected behavior

smartctl should run without errors.

Additional context

Adding services.prometheus.exporters.smartctl.devices = [ ... ] has the same issue, overriding DeviceAllow fixes it aswell.
Possible offending code:

serviceConfig.DeviceAllow = [ "" ];

Not sure why the override is not working, though:
DeviceAllow = lib.mkOverride 100 (
if cfg.devices != [] then
cfg.devices
else [
"block-blkext rw"
"block-sd rw"
"char-nvme rw"
]
);

Notify maintainers

@mweinelt

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 5.17.2, NixOS, 22.05 (Quokka), 22.05.20220413.ff9efb0`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.8.0pre20220411_f7276bc`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`
@mweinelt
Copy link
Member

mweinelt commented Jun 6, 2022

What does your resulting systemd unit look like?

# systemctl cat prometheus-smartctl-exporter.service | grep DeviceAllow=
DeviceAllow=block-sd rw
DeviceAllow=block-blkext rw
DeviceAllow=char-nvme rw

Also default priority is 1000, so overriding it with priority 100 should work.

@MalteT
Copy link
Contributor Author

MalteT commented Jun 6, 2022

I agree, it should work, here is the before and after (created with systemctl cat)

Stock nixpkgs:

[root@faunus-ater:~]# cat /tmp/old | rg DeviceAllow=
DeviceAllow=block-blkext rw
DeviceAllow=block-sd rw
DeviceAllow=char-nvme rw
DeviceAllow=

With my changes:

[root@faunus-ater:~]# cat /tmp/new | rg DeviceAllow=
DeviceAllow=block-blkext rw
DeviceAllow=block-sd rw
DeviceAllow=char-nvme rw

Evaluating systemd.services."prometheus-smartctl-exporter".serviceConfig.DeviceAllow results in [ "block-blkext rw" "block-sd rw" "char-nvme rw" "" ] for me.

@mweinelt
Copy link
Member

mweinelt commented Jun 6, 2022

Yup, that looks like the problem. That would mean they share the same priority, so we should likely try going down to 50 with the override.

@MalteT
Copy link
Contributor Author

MalteT commented Jun 6, 2022

I'm not sure if the wiki is up to date, but 100 seems to be the default, 1000 is the default for defaults.
It might be more future-proof to change

serviceConfig.DeviceAllow = [ "" ];

to mkDefault [ "" ], but it's probably more disruptive for the other modules. Overriding with 50 seems to be the smarter option.

I could assemble a PR, seems like a very small fix

@mweinelt
Copy link
Member

mweinelt commented Jun 6, 2022

Feel free!

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/smartctl-exporter/19110/7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants