Bug 256221 - dns/knot-resolver: the kresd init script won't stop the service
Summary: dns/knot-resolver: the kresd init script won't stop the service
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-28 17:43 UTC by Chris Hutchinson
Modified: 2021-06-22 22:20 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (freebsd)
portmaster: maintainer-feedback? (freebsd)


Attachments
dns/knot-resolver: replacement for rc.d/kresd (1.53 KB, text/plain)
2021-06-04 05:51 UTC, Chris Hutchinson
no flags Details
dns/knot-resolver: replacement for rc.d/kresd (final version) (1.47 KB, text/plain)
2021-06-04 06:06 UTC, Chris Hutchinson
no flags Details
dns/knot-resolver: replacement for rc.d/kresd (version 3) (1.53 KB, text/plain)
2021-06-04 16:17 UTC, Chris Hutchinson
no flags Details
git diff for dns/knot-resolver (1.80 KB, patch)
2021-06-04 20:01 UTC, Chris Hutchinson
no flags Details | Diff
git diff for dns:/knot-resolver (version 2) (1.81 KB, patch)
2021-06-07 05:07 UTC, Chris Hutchinson
freebsd: maintainer-approval-
Details | Diff
knot-resolver fixes and cleanup (3.51 KB, patch)
2021-06-22 13:34 UTC, Leo Vandewoestijne
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Hutchinson 2021-05-28 17:43:46 UTC
The initialization script, kresd
(/usr/local/rc.d/kresd) starts the resolver
process as intended. But there is no way; short of
killing off all the related PIDs, to actually
kill kresd.
IOW Initiating
service kresd stop
does absolutely nothing. Isn't there any way to
cat/kill/remove the associated PIDs for this?

Thanks.

--Chris
Comment 1 Chris Hutchinson 2021-05-28 17:49:27 UTC
This works. But seems kind of awkward:

for all in `ps waux | grep kresd | awk '{print $2;}'`
do
    kill -HUP $all
done
Comment 2 Leo Vandewoestijne 2021-05-31 10:15:40 UTC
(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

...?
Comment 3 Leo Vandewoestijne 2021-05-31 10:18:53 UTC
(In reply to Leo Vandewoestijne from comment #2)

kill -HUP  doesn't actually kill - I do think we'll need -9 here.
Comment 4 Chris Hutchinson 2021-06-02 16:31:30 UTC
(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
Comment 5 Chris Hutchinson 2021-06-02 16:32:51 UTC
(In reply to Chris Hutchinson from comment #4)
/serviced/systemd/
Comment 6 Chris Hutchinson 2021-06-02 20:33:52 UTC
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
Comment 7 Chris Hutchinson 2021-06-02 20:48:39 UTC
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. ;-)
Comment 8 Chris Hutchinson 2021-06-02 21:26:03 UTC
Maybe this[1] would be of some assistance (untested)?
1) https://github.com/akhilvij/systemd-to-sysvinit-converter

--Chris
Comment 9 Vladimír Čunát 2021-06-03 06:33:01 UTC
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
Comment 10 Chris Hutchinson 2021-06-03 17:57:26 UTC
(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
Comment 11 Chris Hutchinson 2021-06-04 05:51:26 UTC
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
Comment 12 Chris Hutchinson 2021-06-04 06:06:45 UTC
Created attachment 225536 [details]
dns/knot-resolver: replacement for rc.d/kresd (final version)

Sorry. Attached the wrong file last time.
Comment 13 Chris Hutchinson 2021-06-04 16:17:48 UTC
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
Comment 14 Chris Hutchinson 2021-06-04 20:01:00 UTC
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
Comment 15 Chris Hutchinson 2021-06-07 05:07:05 UTC
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
Comment 16 Chris Hutchinson 2021-06-07 18:11:39 UTC
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
Comment 17 Vladimír Čunát 2021-06-07 18:21:00 UTC
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.
Comment 18 Chris Hutchinson 2021-06-07 18:34:19 UTC
(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
Comment 19 Vladimír Čunát 2021-06-14 09:06:46 UTC
> 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...
Comment 20 Chris Hutchinson 2021-06-14 17:43:41 UTC
(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 21 Leo Vandewoestijne 2021-06-22 13:05:08 UTC
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!).
Comment 22 Leo Vandewoestijne 2021-06-22 13:34:03 UTC
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.
Comment 23 Chris Hutchinson 2021-06-22 21:54:02 UTC
(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
Comment 24 Chris Hutchinson 2021-06-22 22:20:52 UTC
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