prometheus-community / ipmi_exporter

Remote IPMI exporter for Prometheus
MIT License
473 stars 133 forks source link

Add rpm build #132

Closed gabrieleiannetti closed 1 year ago

gabrieleiannetti commented 2 years ago

Hi,

I would like to propose a small and straightforward approach to build rpm packages.

A description has been added to the README.md file.

Best Gabriele

SuperQ commented 1 year ago

This needs to be done at the Prometheus central tools level, not on a repo-by-repo basis. The overhead to maintain this per-repo is too high as there are hundreds of exporters that need to all do the same thing and keep up to date with standards.

See: https://github.com/prometheus/promu/issues/93

bitfehler commented 1 year ago

I think there is no harm in some exporters providing something for their users until the next four years that the linked issue remains unresolved?

SuperQ commented 1 year ago

There actually is harm in adding this here. It creates inconsistency and maintenance burden to the ecosystem.

gabrieleiannetti commented 1 year ago

There actually is harm in adding this here. It creates inconsistency and maintenance burden to the ecosystem.

Well, if it is optional and located as stated by bitfehler in a separate directory, I disagree to your argument...

I am always open for improvements.

Anyway, having an automated rpm package build is very helpful for the community though...

bitfehler commented 1 year ago

There actually is harm in adding this here. It creates inconsistency and maintenance burden to the ecosystem.

What are the inconsistencies? Where is the burden, and to whom?

I am open for having a proper discussion about this, but neither referring to a non-existent feature that nobody is working on nor unspecific, generic phrases like that are really helpful.

The IPMI exporter is different from the vast majority of other exporters in that it actually requires more than just a Go binary. The setup of the surroundings (FreeIPMI, potentially with sudo as seen here) are one the most frequently brought-up issues here. It is my burden to take care of that. And for me, there is value in providing guidance around this, be it with documentation or an example of how to build a package.

So please, @SuperQ - do take a minute to make a proper argument. I promise I will listen and evaluate. But I need a little more than provided so far...

gabrieleiannetti commented 1 year ago

Thank you for your contribution! I am not familiar RPM at all, so I can't comment on any of the specifics there, but: could you call the folder contrib/rpm (instead of just rpm) and then put all the files you added in that folder (with sub-folders as you see fit)? All the files contain distribution-specific handling (e.g. the assumption that there is a "prometheus" user), and I'd like to avoid the confusion of people thinking that this is e.g. a generic (or officially endorsed) systemd unit.

It would also be great if you could put your additions to the README into a new README in that folder and then just add

### Building an RPM package

See <INSERT LINK>

to the main README. I've been trying to keep it short, though I will admit you might not notice from looking at it wink

Bonus points if you could call the output folder something less generic than build, but if that's some sort of default or a hassle to change then that's cool.

Thanks again!

Hi @bitfehler,

as suggested by you, I have cleanup the rpm build process to make it more clear (commit).

You welcome...

Best
Gabriele

bitfehler commented 1 year ago

Sorry for dropping the ball on this! Thanks for preparing this