Bug 258488 - net/rabbitmq: fix pidfile in rabbitmq rc.d script
Summary: net/rabbitmq: fix pidfile in rabbitmq rc.d script
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Dave Cottlehuber
URL: https://reviews.freebsd.org/D32312
Keywords: regression
Depends on:
Blocks:
 
Reported: 2021-09-14 00:04 UTC by Oleg Ginzburg
Modified: 2024-03-13 08:09 UTC (History)
4 users (show)

See Also:
bugzilla: maintainer-feedback? (erlang)
koobs: maintainer-feedback? (dch)
koobs: merge-quarterly?


Attachments
net/rabbitmq: fix pidfile in rabbitmq rc.d script (1.49 KB, patch)
2021-09-14 00:04 UTC, Oleg Ginzburg
no flags Details | Diff
net/rabbitmq: fix pidfile in rabbitmq rc.d script (1.50 KB, patch)
2021-09-14 06:29 UTC, Oleg Ginzburg
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oleg Ginzburg 2021-09-14 00:04:05 UTC
Created attachment 227887 [details]
net/rabbitmq: fix pidfile in rabbitmq rc.d script

with https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256686 update we got regression related to empty pid file, due to:

https://www.rabbitmq.com/rabbitmq-server.8.html:

-detached
    Start the server process in the background. Note that this will cause the pid not to be written to the pid file


I suggest going back to running through the daemon(8), patch attached.

PS: Also, we return a zero code on stop/shutdown: it affects some management systems that expect a positive code.
Comment 1 Oleg Ginzburg 2021-09-14 06:29:21 UTC
Created attachment 227890 [details]
net/rabbitmq: fix pidfile in rabbitmq rc.d script

use return insead of exit
Comment 2 Dave Cottlehuber freebsd_committer freebsd_triage 2021-10-05 11:13:44 UTC
This gets messy fast! None of the options so far produce a good result.

- RabbitMQ in general expects its pidfile to be the BEAM process
- using daemon(8) sets the FreeBSD pidfile to the rabbitmq-server shell script
- rabbitmq-server docs are in fact incorrect, the pidfile is written
    even when `-detached` is set
- we don't know what the pidfile is going to be inside RabbitMQ really
- RabbitMQ *needs* to have a writable pidfile location as its Elixir helper scripts will wait for the file to appear (so truncation is not an option)

According to https://www.rabbitmq.com/relocate.html RabbitMQ has its
pidfile defined here:

RABBITMQ_PID_FILE

Which expands to (by default):

/var/db/rabbitmq/mnesia/rabbit@${HOSTNAME}.pid

Which is not at all FreeBSD hier(7) nor POLA, but is exactly where a
RabbitMQ person would look for it, and where all the tools expect it to
be.

The rc.d script sets this pidfile variable, but RabbitMQ ignores it as env vars are not passed through to su nor daemon.

```
# ps -dU rabbitmq
PID    STAT TIME   COMMAND
47303  I    0:00.01 /bin/sh /usr/local/sbin/rabbitmq-server
71950  I    0:07.19 - /usr/local/lib/erlang24/erts-12.1.1/bin/beam.smp ...
41316  Ss   0:00.10 `-- erl_child_setup 65000
   69  Is   0:00.00   `-- inet_gethost 4
15065  I    0:00.00     `-- inet_gethost 4

# cat /var/db/rabbitmq/mnesia/rabbit@wintermute.pid
71950

# cat /var/run/rabbitmq.pid
47303

# cat /var/run/rabbitmq-daemon.pid
43693

# pgrep -ilf daemon|grep rabb
43693 daemon: /usr/local/sbin/rabbitmq-server[47303]
```

wrt using daemon(8), I'm generally not in favour of this for
network-facing systems; if the application crashes, we want to fix that,
and not just restart and YOLO our way through whatever issue crops up.

For example, if the app crashes on malicious input, then it may be
possible to use this crash as a side channel to extract credentials.

That said, if you have need of it, I'm ok with switching back, I just
need to understand what's broken so I am sure we fix it this time.

I can roll this into the 3.9.7 update I'm preparing just now, and
backport that to quarterly.

Does the rc.d script behave correctly for your needs?
Do you need knowledge of /var/run/rabbitmq.pid outside of the rc.d script at all?

I don't think daemon(8) helps here, it just makes more incorrect
pidfiles. And the only sensible choice is the rabbitmq default,
which would mean we have no $pidfile at all in the rc.d script.

What should we do here?
Comment 3 Dave Cottlehuber freebsd_committer freebsd_triage 2021-10-05 11:22:20 UTC
we don't need pidfile at all here, it's only used to wait for rabbitmq to start up, which we can leave to run in background.

we can switch to `rabbitmqctl shutdown` for halting, which doesn't require pidfile either then.

See https://reviews.freebsd.org/D32312 for possible solution.

But if you need external pidfile for other reason, then this won't work.
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2024-01-19 14:34:43 UTC
^Triage: to dch@: now that D32312 has landed, is this PR still relevant?
Comment 5 Dave Cottlehuber freebsd_committer freebsd_triage 2024-03-13 08:09:37 UTC
Oleg I think krion@ alrady resolved this. Let me know if we need to reopen it. thanks!