Fixes #30731 - Make smart-proxy use SdNotify library

[adamruzicka]

No description provided.

[ekohl]

You need to remove test/sd_notify_test.rb as well.

[adamruzicka]

I knew I forgot something, fixed

[ekohl]

👍 pending packaging merge.

[ekohl]

Packaging was merged. Merging this so I can send the foreman-proxy.spec update PR.

[ekohl]

I opened theforeman/foreman-packaging#5750

[lzap]

For the record, this patch made my life way harder because the patch extended our notification helper with ready_all method and ability to send notification after all listeners are ready (HTTP + HTTPS). The library does not support that.

#623

I wish this was never merged, adding a new dependency to write one line through a socket does not provide much value IMHO. Systemd notification is a stable API, they will unlikely change it because many programs use it today. I will need to create some kind of SdNotify helper anyway if I want to keep the correct behavior. :-(

[lzap]

BTW if I revert this patch, my PR rebases cleanly and it's very much improved over this version which ignores that we actually listen on multiple endpoints. I propose to put our own SdNotify back reverting this change and the dependency.

[adamruzicka]

The idea here was to use a single library in sidekiq workers, smart proxy and smart proxy dynflow core, without having to copy our own implementation across three projects. So the value is not in writing a line through a socket, but in standardization.

Instead of reverting the whole thing, putting back our class and extending that can't you just make a subclass of the class provided by the dependency?

[ekohl]

I agree that reverting is a bad idea. The changes in https://github.com/theforeman/smart-proxy/pull/623/files#diff-250c2220401729101b8b210180fb87c2 have little to do with the actual systemd protocol and don't belong in that class. Also note that Puma itself likely to start using it (see puma/puma#2260).

However, I'm still not convinced using Puma in the smart proxy is actually a good idea. It complicates the code base a lot and I couldn't see a real difference in how it performed.

[lzap]

Ok I will make a wrapper called SdNotifyAll around that library to provide support for multiple jobs.