Bug 256221

This works. But seems kind of awkward:

for all in `ps waux | grep kresd | awk '{print $2;}'`
do
    kill -HUP $all
done

(In reply to Chris Hutchinson from comment #1)

I know the problem, but I use it in dedicated jails.
But indeed the behaviour is incorrect, and I fail to see what is the cause.
And manually I used to do the same command.


What about this in files/krescachegc.in

for all in `pgrep -x kresd`
do
    kill -HUP $all
done

and this is files/kresd.in

for all in `pgrep -x kres-cache-gc
do
    kill -HUP $all
done

...?

(In reply to Leo Vandewoestijne from comment #2)

kill -HUP  doesn't actually kill - I do think we'll need -9 here.

(In reply to Leo Vandewoestijne from comment #3)
-HUP (HangUP) _should_ be enough. But kresd(8)
isn't cleaning up after itself with either
-HUP || -9
IOW the pid files are still left within
kresd/ && kresd/control
I think this is going to be a bit more involved
than it initially appeared. Pretty sure it's
because its service handling is so intimately
tied to serviced. :-(

--Chris

(In reply to Chris Hutchinson from comment #4)
/serviced/systemd/

OK after many iterations of several
different possibilities. This is the
only way I can guarantee a successful
termination of kresd(8) that insures that
it also cleans up after itself:


for pid in `ps waux | grep kresd | grep daemon | awk '{print $2;}'`
do
	kill -TERM $pid
done

I created a script in my ~/bin/ folder called
kresd-stop:
#!/bin/sh -

for pid in `ps waux | grep kresd | grep daemon | awk '{print $2;}'`
do
	kill -TERM $pid
done

exit

HTH

--Chris

Somebody(tm) (not me) should look at the source
and try to untangle how they currently try to
initiate && terminate kresd(8). To add ifdef(s)
for freebsd. So that we can (initi|termin)ate
it in a *BSD friendly way. ;-)

Maybe this[1] would be of some assistance (untested)?
1) https://github.com/akhilvij/systemd-to-sysvinit-converter

--Chris

Hello, upstream here.

> kill -HUP  doesn't actually kill - I do think we'll need -9 here.

-HUP isn't supported, -TERM is the way (for both kresd and kres-cache-gc).  Is that unfriendly for FreeBSD?

pidfiles: we don't use them at all.  I'm not familiar with /usr/sbin/daemon (which you use as a wrapper).

--Vladimir

(In reply to Vladimír Čunát from comment #9)
> -HUP isn't supported, -TERM is the way (for both kresd and kres-cache-gc).
-HUP *does* remove kresd from the process list. So
I think we can safely assume it gets killed. BUT I
completely agree that -TERM is the correct answer here.
daemon(8) appears to me to be unnecessary overhead,
and just muddies the water. eg;
# ps waux | grep kres
root       26098   0.0  0.0    11004    2024  -  Is   13:30        0:00.00 daemon: /usr/local/sbin/kresd[26553] (daemon)
root       26553   0.0  0.1   140312   40388  -  S    13:30        0:46.35 /usr/local/sbin/kresd -c /usr/local/etc/knot-resolver/kr

NOTE above that process 26553 is the daemon(8) initiation
process. Whereas 26098 is the kresd(8) process itself. So in
using daemon(8), we end up with 2 procs/pids. Whereas simply
starting kresd(8) alone as a daemon reduces that to the
kresd(8) proc/pid (kresd.pid).

I'm cobbling up a new rc.d/kresd init script now that currently
provides START && STOP. But want to add RESTART to it as well.
I'd have already posted it here. But I'm juggling this along side
$WORK and all my testing is on a production server that actually
depends on kresd already. So testing is a little strained.
I should be able to provide something here for further examination
within a couple hours.

--Chris

Created attachment 225535 [details]
dns/knot-resolver: replacement for rc.d/kresd

OK sorry for the delay. Been a real busy day with many
interruptions.
Anyway, the attached proposed rc.d/kresd file, while not
elegant, serves its purpose quite well.
Ideally; to better deal with daemon(8). I'd create a
sup script && install it in ${prefix}/sbin/
That way one can get more finite control over the process
that daemon(8) is launching.
But this gets the job done pretty well, just the same. :-)

--Chris

Created attachment 225536 [details]
dns/knot-resolver: replacement for rc.d/kresd (final version)

Sorry. Attached the wrong file last time.

Created attachment 225548 [details]
dns/knot-resolver: replacement for rc.d/kresd (version 3)

OK I've found the time to add restart.
The attached rc.d/kresd file provides the following:
service (start | stop | restart )
While it's not perfect. I'm a bit strained for time
and this should pretty well cover the needs for the
initialization of kresd -- it WFM. :-)

I'm working in a dirty environment, or I would have
submitted this as a proper git diff. Sorry.

--Chris

Created attachment 225552 [details]
git diff for dns/knot-resolver

OK I used my lunchtime to do this right.
Please find attached a git diff proper for
dns/knot-resolver.

Changes
files/kresd.in

Adds the ability to:
service kresd (start | stop | restart)

That's it. Works as intended for everything
rc.subr is intended to provide.

--Chris

Created attachment 225620 [details]
git diff for dns:/knot-resolver (version 2)

OK after a great deal of additional testing. I
discovered I wasn't sending the process name we're
interested in to syslog(3) -- kresd. The git diff
attached, corrects that.
That's it. I'm done here. I might also send a pull
request upstream for the addition of this to their
source.

Please commit.

--Chris

OK I was going to submit a pull/merge request
with upstream. But after filling in all the
registration fields, and pressing "register".
I'm informed that:
There was an error with the reCAPTCHA.
Please solve the reCAPTCHA again.

However. No reCAPTCHA is ever presented.

So upstream is apparently broken. I have
a GitLab account and have no problems there.
But it's not possible at gitlab.nic.cz. :-(

Vladimír?

--Chris

I'll forward the information to the administrators.  If you have a GitHub account, I'd personally try that button for logging via that provider.

(In reply to Vladimír Čunát from comment #17)
> I'll forward the information to the administrators.
Thank you kindly, Vladimír! :-)

> If you have a GitHub account, I'd personally try that
> button for logging via that provider.
Ah, I see. I'm afraid I only have GitLab accounts.

Thanks again! :-)

--Chris

> I was going to submit a pull/merge request with upstream.

The registration problem was worked around within a few hours (reportedly), but I don't really get what you're about to submit.  I don't think it makes sense for us (upstream) to include the FreeBSD init scripts, as none of us knows about writing (FreeBSD) init scripts and we can't directly affect their packages/ports anyway.

Maybe it might've made sense in order to share the scripts among other systems, but I don't know if they're really portable and I'm not aware of Knot Resolver packages existing for some such other system anyway.


By the way, finding out PID by a `ps` call feels a bit fragile to me (though I expect it's better to get some improvement than nothing).  I assumed that the pidfiles are meant to keep this state, but I don't really know init script stuff...

(In reply to Vladimír Čunát from comment #19)
> The registration problem was worked around within a few hours (reportedly),
> but I don't really get what you're about to submit.  I don't think it makes
> sense for us (upstream) to include the FreeBSD init scripts, as none of us
> knows about writing (FreeBSD) init scripts and we can't directly affect
> their packages/ports anyway.
Thanks for your efforts, Vladimír!
Sure. Probably better to keep it local (here) to the FreeBSD ports.

> By the way, finding out PID by a `ps` call feels a bit fragile to me
> (though I expect it's better to get some improvement than nothing).
> I assumed that the pidfiles are meant to keep this state, but I don't
> really know init script stuff...
Wouldn't have been my first choice either. But, given the use of daemon(8).
We end up with 2 pids...
/var/run/kresd/kresd.pid &&
/var/run/kresd/control/<procnum>.pid
and worse; different versions of knot-resolver result in a differently
named kresd subdir. But to my credit, it always works as intended. :-)
The _correct_ solution would be to remove systemd, and use proper sysV
initialization. Or _add_ it;
#ifdef (FreeBSD...

But until, or unless... this gets the job done.

Thanks again, Vladimír.

--Chris

Comment on attachment 225620 [details]
git diff for dns:/knot-resolver (version 2)

Spotted an typo at
 +kgroup=$name}

I will catch it in a short while, however will decline this patch
(but please don't feel offended; your efforts are much appreciated!).

Created attachment 225993 [details]
knot-resolver fixes and cleanup

`daemon` seems needed as otherwise I can't get it to background,
at least not in a way that I think is decent.

About the most recent patch:

- The printf in the 'for pid loop' was inside the loop, being a bit too verbose.

- The install of the possible missing var dir was inside the if/elif/else loop;  and then start still would fail in that particular case.

- I simplified the restart routine but then discovered it all was identical to what was inside start/stop already, and so I don't think a restart routine is needed (as restart will work without having a custom one defined).

- I do share the thoughts about using "ps | grep | etc" for finding pid's.
And do share the view something is better than nothing.

poudriere complained about @sample / .sample
Fixed.

portfmt complained about all sorts of minor things.
Fixed a few.

IMHO this patch should be fine, but gladly welcome any major or nitpick comments.

(In reply to Leo Vandewoestijne from comment #22)
No offence taken. :-)

 -Too verbose? I may have gotten carried away. But trimming
  a little output/formatting to taste was always available. :-)

 -error(s)? In my final version, I was unable to produce any
  testing on 3 different versions of knot-resolver, on 3
  different versions of FreeBSD -- not saying your wrong.
  Just saying my experiences were different. :-)

 -nit; no restart command.

 -nit; echo "${name} seems already running.":
  _should_ read:
  echo "${name} is already running."
  ||
  echo "${name} already seems to be running."

Hey, you asked. ;-)

--Chris

Had a thought about the (missing) restart command.

At the end of your start command:

+	else
+		echo "${name} seems already running."
+	fi

Maybe change it as follows:

+	else
+		echo "${name} is already running!"
+		echo "Restarting ${name}..."
+		run_rc_command stop
+		run_rc_command start
+		echo "${name} restarted."
+	fi

Seems simple, and won't break/harm anything. Yet only adds
4 additional lines. :-)

--Chris

Created attachment 226234 [details]
knot-resolver final

(In reply to Chris Hutchinson from comment #24)

I did applied your grammar pointers.
But I did not applied the restart routine, because without a separate restart routine defined the restart parameter would execute stop/start, and so output the very same warnings (but with less code).

Further I tested with portlint, poudriere, portfmt and portclippy and made minor changes in order to please them as much as possible.

This seems kind of stuck, or at least I can't see the port changing.

Anyway, since today's release 5.4.0 it's possible to log directly through the syslog(3) interface.  https://knot-resolver.readthedocs.io/en/stable/config-logging-monitoring.html

You could e.g. pass an extra configuration file in the service command-line to add such distro-specific settings, similarly to what we do with systemd by default:
https://gitlab.nic.cz/knot/knot-resolver/-/blob/v5.4.0/systemd/kresd@.service.in#L15
https://gitlab.nic.cz/knot/knot-resolver/-/blob/v5.4.0/daemon/lua/distro-preconfig.lua.in

(In reply to Vladimír Čunát from comment #27)
OK, thanks, I've submitted PR 257553

Chris, can you please close this one?

Sure. For future ref. You can use the Blocks: field
in a new pr(1) to block older/other PRs as required. :-)

--Chris

*** This bug has been marked as a duplicate of bug 257553 ***