-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
setpgid(0,0)
fails if dcron is process group leader
#13
Comments
On Sun, Aug 28, 2016, at 02:42 PM, Gilles wrote:
Hi, thanks for this report. Offhand, your diagnosis sounds right to me. moment. But I hope to get to it sometime this fall. |
this seams important for running dcron in a docker container, |
Same problem here ;) |
Same problem with alpine linux 3.5.2 and crond 4.5 I have observe the following : I strace it and have the same conclusion about the fatal setpgid
I absolutely don't understand the purpose of setpgid/getpgid neither why dcron needs it, so no patch.
|
A docker #!/bin/sh
set -e
# see: https://github.com/dubiousjim/dcron/issues/13
# ignore using `exec` for `dcron` to get another pid instead of `1`
# exec "$@"
"$@" |
Imho it should just specialcase EPERM.
|
In short: dcron works if it's started as container PID1, works if it's started as non-PID1, but fails if some other process started as PID1 and then Easy workaround: start container using docker's init as PID1 ( |
Having run in to this myself, I figured I'd share the patch I use in case anyone else in the future runs in to this problem and wants a fix. It just special cases diff -Naur dcron-4.5.orig/main.c dcron-4.5.setpgid/main.c
--- dcron-4.5.orig/main.c 2011-05-01 19:00:17.000000000 -0400
+++ dcron-4.5.setpgid/main.c 2023-07-14 13:31:39.470797778 -0400
@@ -270,8 +270,10 @@
/* stay in existing session, but start a new process group */
if (setpgid(0,0)) {
- perror("setpgid");
- exit(1);
+ if (EPERM != errno) {
+ perror("setpgid");
+ exit(1);
+ }
}
/* stderr stays open, start SIGHUP ignoring, SIGCHLD handling */ This happens a lot because many init systems or service managers already give services their own process group; it isn't specifically a PID 1 issue. |
I was going to file an issue in the supposedly maintained fork. But the issues there are disabled. I wonder if @ptchinster will notice my message here. Meanwhile I'll leave the information here:
FROM alpine:3.20
RUN apk add git build-base \
&& git clone https://github.com/ptchinster/dcron \
&& cd dcron \
&& make
#!/bin/sh -eu
dcron/crond -f
#!/bin/sh -eu
exec dcron/crond -f
With the following patch (credit):
--- main.c
+++ main.c
@@ -270,8 +270,10 @@
/* stay in existing session, but start a new process group */
if (setpgid(0,0)) {
- perror("setpgid");
- exit(1);
+ if (errno != EPERM) {
+ perror("setpgid");
+ exit(1);
+ }
}
/* stderr stays open, start SIGHUP ignoring, SIGCHLD handling */
FROM alpine:3.20
COPY patch .
RUN apk add git build-base \
&& git clone https://github.com/ptchinster/dcron \
&& cd dcron \
&& patch < /patch \
&& make all the cases work except for the last one for some reason. For now running it with Also at some point I tried different alternatives. The program I liked the most is |
It looks interesting, but… The problem is supercronic kills email reporting cron feature. Sure, it make sense to use other ways for reporting in a docker (e.g. prometheus metrics), but cron tasks often used email not just to report failures but also to report success/stats/etc. This means nowadays they should use something like a pushgateway to report metrics instead. It would be nice if supercronic provide such a feature for cron tasks as a replacement for dropped emailing feature. Also metrics reported by supercronic about task failures should be configurable by task names (I suppose this feature is not supported yet just because it cron format has no special field for task name) - otherwise it would be hard to know which task fail and thus will result in overhead like "run one supercronic container per each cron task". P.S. Actually one supercronic container per one cron task (or several tightly related tasks) is probably a good idea anyway in a docker world. It'll both separate each task metrics/alerts and also their logs. |
Issues are now open on the fork |
Using
s6-rc
, dcron dies becausesetpgid(0,0)
fails.My
s6-rc/dcron/
directory has the following structureRunning
s6-rc -u change dcron
results indcron-log
capturing thefollowing:
However, running
sudo /usr/sbin/crond -M /bin/true -f
in a TTY worksjust fine. After discussion with the s6 folks, it appears this is because
s6 makes the service it supervises the session and group leader, and
indeed
setpgid(2)
will fail withEPERM
if the process is currently asession leader.
I'm not very familiar with why the details behind dcron creating a new
group, but from the comments, it looks like
EPERM
isn't necessarilyan error condition, since dcron is already in the state it wants to
change to. Replacing the relevant code (around
main.c:272
) withmight do what's desired (or perhaps
getpgrp()
in place ofgetsid(0)
,depending on exactly what dcron needs?). I'm unsure enough of the inner
workings of dcron that I'm not submitting that as a pull request, however.
The text was updated successfully, but these errors were encountered: