Skip to content
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

External commands (eg reload) with v5.4 are broken in Docker #676

Closed
fredbcode opened this issue Apr 16, 2021 · 21 comments
Closed

External commands (eg reload) with v5.4 are broken in Docker #676

fredbcode opened this issue Apr 16, 2021 · 21 comments

Comments

@fredbcode
Copy link
Contributor

With v5.4 and e2guardian -N

get_pid() is not working

e2guardian -r
No e2guardian process found.

e2guardian -s
No e2guardian process found.

e2guardian 5.4.4

Built with:  '--prefix=/usr' '--enable-clamd=yes' '--with-proxyuser=e2guardian' '--with-proxygroup=e2guardian' '--sysconfdir=/etc' '--localstatedir=/var' '--with-piddir=/run/e2guardian' '--enable-icap=yes' '--enable-commandline=yes' '--enable-email=yes' '--enable-ntlm=yes' '--mandir=/usr/share/man' '--infodir=/usr/share/info' '--enable-pcre=yes' '--enable-sslmitm=yes' 'CPPFLAGS=-mno-sse2 -g -O2'

@fredbcode fredbcode changed the title e2guardian external commands (eg reload) and v5.4 are broken External commands (eg reload) with v5.4 are broken Apr 16, 2021
@fredbcode
Copy link
Contributor Author

Tested only with: e2guardian -N

@philipianpearce
Copy link
Contributor

I was not able to fully replicate this issue. I only found #678. Other signal functions worked ok for me.

Is this a file/directory permission problem??

@fredbcode
Copy link
Contributor Author

Maybe but there is nothing obvious for me

$ e2guardian --help
Usage: e2guardian [{-c ConfigFileName|-v|-P|-h|-N|-q|-s|-r|-g|-i}]
  -v gives the version number and build options.
  -h gives this message.
  -c allows you to specify a different configuration file location.
  -N Do not go into the background.
  -q causes e2guardian to kill any running copy.
  -Q kill any running copy AND start a new one with current options.
  -s shows the parent process PID and exits.
  -r reloads lists and group config files by issuing a HUP,
     but this does not reset the httpworkers option (amongst others).
  -g  same as -r  (Issues a USR1)
  -i read lists from stdin
$ e2guardian -r
No e2guardian process found.
$ ls -la /etc/e2guardian
total 280
drwxrwxrwx 10 e2guardian e2guardian  4096 Apr 16 15:52 .
drwxr-xr-x  1 root       root        4096 Apr 16 12:11 ..
drwxr-xr-x  2 e2guardian e2guardian  4096 Apr 12 10:19 authplugins
-rw-r--r--  1 e2guardian e2guardian 11133 Apr 13 09:09 common.story
drwxr-xr-x  2 e2guardian e2guardian  4096 Apr 12 10:19 contentscanners
drwxr-xr-x  2 e2guardian e2guardian  4096 Apr 12 10:19 downloadmanagers
-rw-r--r--  1 e2guardian e2guardian 43094 Apr 15 16:11 e2guardian.conf
-rw-r--r--  1 e2guardian e2guardian 31759 Apr 13 15:26 e2guardianf1.conf
-rw-r--r--  1 e2guardian e2guardian  2265 Apr 12 10:19 examplef1.story
drwxr-x--- 29 e2guardian e2guardian  4096 Apr 12 15:34 languages
drwxr-xr-x  9 e2guardian e2guardian  4096 Apr 13 12:58 lists
-rw-r--r--  1 e2guardian e2guardian  2149 Apr 13 15:20 preauth.story

$ ls -la /run/e2guardian
total 8
drwxr-xr-x 1 e2guardian root       4096 Apr 19 16:38 .
drwxr-xr-x 1 root       root       4096 Apr 16 11:51 ..
-rw-r--r-- 1 e2guardian e2guardian    0 Apr 19 16:38 e2.pid

 ps -edf | grep e2
e2guardian     2764  2747  0 16:36 ?        00:00:01 e2guardian -N

@philipianpearce
Copy link
Contributor

philipianpearce commented Apr 19, 2021 via email

@fredbcode
Copy link
Contributor Author

@fredbcode
Copy link
Contributor Author

docker pull fredbcode/e2guardian:v5.4-test if needed

@fredbcode
Copy link
Contributor Author

strace

``
read(3, "", 8191) = 0
close(3) = 0
openat(AT_FDCWD, "/run/e2guardian/e2.pid", O_RDONLY) = 3
read(3, "1\n", 31) = 2
close(3) = 0
write(2, "No e2guardian process found.", 28No e2guardian process found.) = 28
write(2, "\n", 1
) = 1
exit_group(1)

root@5972e029f412:/tmp# cat /run/e2guardian/e2.pid
1
root@5972e029f412:/tmp# ls -la /run/e2guardian/
total 12
drwxr-xr-x 1 e2guardian root 4096 Apr 19 16:58 .
drwxr-xr-x 1 root root 4096 Apr 16 11:51 ..
-rw-r--r-- 1 e2guardian e2guardian 2 Apr 19 16:58 e2.pid
root@5972e029f412:/tmp# pidof e2guardian
1

@fredbcode
Copy link
Contributor Author

Ok I found something

pid < 6 = error

Reproduce easily with

echo 4 > /run/e2guardian/e2.pid 
root@5972e029f412:/tmp# e2guardian -g
No e2guardian process found.
root@5972e029f412:/tmp# echo 6 > /run/e2guardian/e2.pid 
root@5972e029f412:/tmp# e2guardian -g
root@5972e029f412:/tmp#

fredbcode added a commit that referenced this issue Apr 20, 2021
fredbcode added a commit that referenced this issue Apr 20, 2021
fredbcode added a commit that referenced this issue Apr 20, 2021
@philipianpearce philipianpearce changed the title External commands (eg reload) with v5.4 are broken External commands (eg reload) with v5.4 are broken in Docker Apr 20, 2021
@philipianpearce
Copy link
Contributor

Can you try latest?

Problem seems to be that Docker is using pid 1 for e2g. In Linux/BSD pid 1 is reserved for init process so is illegal.

I've now added a conditional check so that pid 1 is allowed when "dockermode" is enabled.

Can you see if that fixes it?

@fredbcode
Copy link
Contributor Author

Same issue, be careful the limit is 5 not just 1, with 6 it works

1,2,3,4,5 -> No process e2guardian found
6,7,8, etc -> ok

FI, process with docker can be 1 but also another, in my case yes it's 1

philipianpearce added a commit that referenced this issue Apr 20, 2021
@fredbcode
Copy link
Contributor Author

With strace I'm seeing there is no Kill test 0 in the issue case (1..5)

You can "test" with echo 6 > /path/e2.pid

@philipianpearce
Copy link
Contributor

There is something odd happening here - there is no exclusion of id less than 6 in e2g code - it must be something to do with how pids are handled in Docker.

Also, adding id above 5 to e2.pid manually should not work as this is not the pid of e2g (1). So it may say OK but it reporting/killing another process and not e2g!

@fredbcode
Copy link
Contributor Author

Also, adding id above 5 to e2.pid manually should not work as this is not the pid of e2g (1). So it may say OK but it reporting/killing another process and not e2g!

Of course it didn't works, just the returns from e2 is an error message in this this case and not beyond 5

Do you have e2 on physical machine for testing this ?

@philipianpearce
Copy link
Contributor

Yes, I have tested on a physical machine and all works OK.

It looks as if Docker reserves pid 1-5 and does not allow the system kill() call to them. i.e. It will not allow a kill (even a NOOP signal) to the container id.

This issue with Docker would be in previous versions too.

Looking at Docker site - would not a better approach with Docker be to send the signal to the Docker container with the docker kill command rather than using the e2g flags?

@fredbcode
Copy link
Contributor Author

I should see a kill call with strace, no ?

Maybe we could generated a number beyond 5, when we are in docker mode ? in sysv_writetpidefile ?

@fredbcode
Copy link
Contributor Author

Ah, of course not

@fredbcode
Copy link
Contributor Author

Philip I found a solution, I'm using "init" flag and now my process is ...
6 !

The container’s main process is responsible for managing all processes that it starts. In some cases, the main process isn’t well-designed, and doesn’t handle “reaping” (stopping) child processes gracefully when the container exits. If your process falls into this category, you can use the --init option when you run the container. The --init flag inserts a tiny init-process into the container as the main process, and handles reaping of all processes when the container exits. Handling such processes this way is superior to using a full-fledged init process such as sysvinit, upstart, or systemd to handle process lifecycle within your container.

Sorry for the noise ...

@fredbcode
Copy link
Contributor Author

There is now a public docker image, I updated the release note

https://hub.docker.com/r/fredbcode/e2guardian

@philipianpearce
Copy link
Contributor

That is great!

OK to close?

@fredbcode
Copy link
Contributor Author

Yep

@fredbcode
Copy link
Contributor Author

Hum, maybe we should remove your commit ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants