prometheus-community/ipmi_exporter

Wrong sudo binary location "/usr/sbin/sudo"

happycouak opened this issue · 10 comments

Using prometheus-ipmi-exporter 1.4.0+ds-3 on Ubuntu 22.04:

prometheus-ipmi-exporter --version
ipmi_exporter, version 1.4.0+ds (branch: debian/sid, revision: 1.4.0+ds-3)
  build user:       team+pkg-go@tracker.debian.org
  build date:       20220103-19:07:33
  go version:       go1.17.3
  platform:         linux/amd64

The exporter try "/usr/sbin/sudo" instead of "/usr/bin/sudo":

Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.913Z caller=collector.go:87 level=debug msg="Running collector" collector=bmc
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.915Z caller=freeipmi.go:133 level=debug msg=Executing command=/usr/sbin/sudo args="unsupported value type"
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.916Z caller=collector_bmc.go:40 level=error msg="Failed to collect BMC data" target=[local] error="error running /usr/sbin/sudo: fork/exec /usr/sbin/sudo: no such file or directory: "
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.916Z caller=collector.go:87 level=debug msg="Running collector" collector=ipmi
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.916Z caller=freeipmi.go:133 level=debug msg=Executing command=/usr/sbin/sudo args="unsupported value type"
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.916Z caller=collector_ipmi.go:128 level=error msg="Failed to collect sensor data" target=[local] error="error running /usr/sbin/sudo: fork/exec /usr/sbin/sudo: no such file or directory: "
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.916Z caller=collector.go:87 level=debug msg="Running collector" collector=dcmi
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.916Z caller=freeipmi.go:133 level=debug msg=Executing command=/usr/sbin/sudo args="unsupported value type"
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.917Z caller=collector_dcmi.go:40 level=error msg="Failed to collect DCMI data" target=[local] error="error running /usr/sbin/sudo: fork/exec /usr/sbin/sudo: no such file or directory: "
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.917Z caller=collector.go:87 level=debug msg="Running collector" collector=chassis
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.917Z caller=freeipmi.go:133 level=debug msg=Executing command=/usr/sbin/sudo args="unsupported value type"
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.918Z caller=collector_chassis.go:40 level=error msg="Failed to collect chassis data" target=[local] error="error running /usr/sbin/sudo: fork/exec /usr/sbin/sudo: no such file or directory: "
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.918Z caller=collector.go:87 level=debug msg="Running collector" collector=sel
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.918Z caller=freeipmi.go:133 level=debug msg=Executing command=/usr/sbin/sudo args="unsupported value type"
Apr 01 16:24:32 pool1 prometheus-ipmi-exporter[1996141]: ts=2023-04-01T14:24:32.918Z caller=collector_sel.go:47 level=error msg="Failed to collect SEL data" target=[local] error="error running /usr/sbin/sudo: fork/exec /usr/sbin/sudo: no such file or directory: "

Here is the used configuration:

modules:
    default:
        collectors:
        - bmc
        - ipmi
        - dcmi
        - chassis
        - sel
        collector_cmd:
            bmc: sudo
            ipmi: sudo
            dcmi: sudo
            chassis: sudo
            sel: sudo
        custom_args:
            bmc:
            - "bmc-info"
            ipmi:
            - "ipmimonitoring"
            dcmi:
            - "ipmi-dcmi"
            chassis:
            - "ipmi-chassis"
            sel:
            - "ipmi-sel"

Also note that sudo configuration is correct:

root@pool1:~# su -m prometheus -c "sudo bmc-info"
Device ID             : 32
Device Revision       : 1
Device SDRs           : supported
Firmware Revision     : 2.65
Device Available      : yes (normal operation)
IPMI Version          : 2.0
Sensor Device         : supported
SDR Repository Device : supported
SEL Device            : supported
FRU Inventory Device  : supported
IPMB Event Receiver   : supported
IPMB Event Generator  : unsupported
Bridge                : supported
Chassis Device        : supported
Manufacturer ID       : Dell Inc. (674)

Any pointers welcome.

Are you running the exporter with -freeipmi.path=/usr/sbin? If so, try removing that flag. Note that this might be set by the package-provided service definition.

I just ran into this as well. I'm on Ubuntu and Debian systems, using the distro supplied packages. I'm overriding the ARGS variable in /etc/default/prometheus-ipmi-exporter, which the service definition uses, and can verify from ps that the freeipmi.path variable isn't being passed to the service, but it still seems to be defaulting to this behaviour.

is there a way to override this option's builtin default, and just use the system PATH instead? that feels like a better approach than this weird config-option-to-define-paths situation you've got here, especially since there doesn't seem to be a way to deal with sudo being in one location (/usr/bin) and other tools being in another (/usr/sbin).

Lol, so it took me a while to figure that out, but this is Debian being Debian: https://salsa.debian.org/go-team/packages/prometheus-ipmi-exporter/-/blob/debian/sid/debian/patches/01-debian-defaults.patch

Please complain to them to get this fixed...

This is not a Debian-specific problem, and will affect any distro (even when using an unpatched ipmi_exporter) that stores sudo and the freeipmi tools in different paths. This includes (but is probably not limited to) Debian (and its derivatives, such as Ubuntu etc.), Alpine Linux, RPM-based distros such as Centos, OpenSUSE, Alma etc.

In fact, from my research, the only distro I found so far which does store the freeipmi tools in /usr/bin (i.e., in the same location as sudo) is Arch Linux. The vast majority of distros store sudo in /usr/bin (which makes sense, as non-root users need it on their default PATH), and the freeipmi utils in /usr/sbin. So setting the --freeipmi.path is going to break operation with sudo on a large number of distros.

Looking at the code, it seems that the flag name is a bit of a misnomer, since the variable name executablesPath is a more accurate reflection of what this option is actually used for, as it is applied to all commands executed by the exporter.

    executablesPath = kingpin.Flag(
        "freeipmi.path",
        "Path to FreeIPMI executables (default: rely on $PATH).",
    ).String()

I would therefore argue that this issue really ought to be fixed in the exporter itself.

@dswarbrick I know your handle and respect your work, and also appreciate you getting involved here. But frankly, that comment feels a bit egregious to me. Let me explain:

This is not a Debian-specific problem

Debian goes out of its way to apply a patch (updated link) to change a hardcoded default value, for no obvious reason, thereby breaking the assumptions of all upstream documentation, causing wasted time for their users and upstream maintainers trying to help them. This is a textbook example of a Debian problem.

and will affect any distro (even when using an unpatched ipmi_exporter)

No.

So setting the --freeipmi.path is going to break operation with sudo

Yeah. So maybe - don't do it? This argument was provided as advanced switch for folks who have to use custom or multiple FreeIPMI versions in unusual places, where $PATH doesn't cut it. So why force it onto people? And honestly, why force it onto people so sneakily? If you think that setting is important for some reason, why not apply your default value in the unit file, where people at least have a chance to see what is happening?

Looking at the code, it seems that the flag name is a bit of a misnomer

Maybe, because it pre-dates the option to replace the executable being run. But renaming it would change... nothing? The patch would still need to be dropped as much as it should be right now.

I would therefore argue that this issue really ought to be fixed in the exporter itself.

There is nothing to fix here. But if you are interested in improving the interface, PRs welcome.

@bitfehler

@dswarbrick

This is not a Debian-specific problem and will affect any distro (even when using an unpatched ipmi_exporter) that stores sudo and the freeipmi tools in different paths.

No.

Actually, yes.

Using an unpatched, official release:

$ ./ipmi_exporter --version
ipmi_exporter, version 1.8.0 (branch: HEAD, revision: 0401ab9f809ca02709589cf90069afce42d10a10)
  build user:       root@5a0f00808880
  build date:       20240123-15:35:19
  go version:       go1.21.6
  platform:         linux/amd64
  tags:             netgo static_build

and a minimal config that uses sudo:

modules:
  default:
    collectors:
    - ipmi
    - sel
    collector_cmd:
      ipmi: sudo
      sel: sudo
    custom_args:
      ipmi:
      - "ipmimonitoring"
      sel:
      - "ipmi-sel"

the exporter will fail when the freeipmi.path flag is specified, because it assumes that all executables are in said location.

./ipmi_exporter --freeipmi.path /usr/sbin
...
ts=2024-04-16T11:10:59.765Z caller=collector_ipmi.go:151 level=error msg="Failed to collect sensor data" target=[local] error="error running /usr/sbin/sudo: fork/exec /usr/sbin/sudo: no such file or directory: "

As I already stated, the only distro I found so far that actually puts the freeipmi utils in /usr/bin (i.e., alongside sudo) is Arch. I would think that that suggests quite a large number of users on other distros will be confused when attempting to use this exporter with sudo as documented does not work.

So I think that dismissively labeling this as a "Debian problem" is a bit of stretch (no pun intended).

Essentially, the sudo method and the freeipmi.path flag are incompatible with each other (on most distros), and I think this could at least be made clearer in the docs, if not fixed altogether.

the exporter will fail when the freeipmi.path flag is specified

Did you even read the rest of my comment? Why do you insist on setting this? If you were to simply not do that, it would just work. And with that Debian patch, Debian becomes the only distro to force breakage onto their users.

Fun fact: that whole sudo thing is also in fact not officially supported. It just happens to work as a side effect of how the config is structured. But we expect people to know what they are doing. However, even after attentively reading our documentation, they cannot know what they are doing if you force some unnecessary config defaults on them.

Why do you insist on setting this? If you were to simply not do that, it would just work.

The original need to specify the freeipmi.path explicitly is due to the fact that on Debian, Linux Mint (LMDE), and possibly others, /usr/sbin is not in the PATH for non-root users. See /etc/profile:

if [ "$(id -u)" -eq 0 ]; then
  PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
else
  PATH="/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games"
fi
export PATH

Assuming that most installations will run the exporter non-root, in its default unpatched state, ipmi_exporter would not find the freeipmi tools on such distros.

Beyond requiring users to figure this out on their own and always specify the path, there are a few options open:

  • Override the PATH in the systemd service / init script - but this won't help if the exporter is just casually run from the command line
  • Use the well-known /etc/default/foo method of supplying args to services (whether launched via systemd or sysvinit) - but this won't help if the exporter is just casually run from the command line
  • Patch the exporter default with the explicit path of the freeipmi utils for the environment in which it is being packaged - which will also be honoured when the exporter is casually run from the command line

Debian becomes the only distro to force breakage onto their users.

That's a bold assumption.

I work with Alpine and Arch, and this issue does not exist on either.

There is obviously a glaring inconsistency between "we're hiding the tools in /usr/sbin so regular users don't see them" and "we want regular users to use these tools out of the box, even 'casually from the command line'". I mean, if you can't see the silly in that, I don't know what gives.

But if you manage to come up with an interface that is generic and does not break the way this works on other distros, cool. I'll merge it. But saying this is a deficiency of this tool, that is the real stretch.

I work with Alpine and Arch, and this issue does not exist on either.

It's not a problem on Arch because they store both sudo and the freeipmi utils in /usr/bin, so even if one were to explicitly specify --freeipmi.path /usr/bin, there would be no issue.

Despite Alpine storing the freeipmi utils in /usr/sbin (like Debian and a bunch of other distros), I suspect that they provide /usr/sbin in the default PATH, even to non-root users, so ipmi_exporter is able to find it on the PATH.

There is obviously a glaring inconsistency between "we're hiding the tools in /usr/sbin so regular users don't see them" and "we want regular users to use these tools out of the box, even 'casually from the command line'". I mean, if you can't see the silly in that, I don't know what gives.

The Filesystem Hierarchy Standard (FHS), to which Debian pretty closely adheres, has this to say about /usr/sbin: "Non-essential system binaries (e.g., daemons for various network services)."

Debian is not trying to "hide" tools by putting them in /usr/sbin. The freeipmi package maintainer apparently just does not deem them essential enough to put them in /usr/bin, where the really essential stuff lives, like util-linux and shells. The practice of excluding /usr/sbin from non-root users' PATH probably has its origins way back in the early days of Unix. Having said that, a lot of non-essential stuff ends up in /usr/bin these days anyway, so hooray for consistency.

But saying this is a deficiency of this tool, that is the real stretch.

I never said that. I think this is a bit of a rough edge that could be smoothed out a bit to enhance the end-user experience, and perhaps clarified better in the docs. If anything is being "hidden" here, it's that ipmi_exporter will use the same freeipmi.path prefix for all executables, which is only obvious if one inspects the source code.

I think that ipmi_exporter is a useful bit of software, which I've used a lot in the past. I no longer have need for it, but I still actively co-maintain the Debian package. So I occasionally find myself caught between differing points of view when Debian users raise issues.