-
Notifications
You must be signed in to change notification settings - Fork 73
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
Logic for detection of processes in docker containers seems wrong #80
Comments
I don't think there is any need to restart processes within docker containers by design (read: within a container running a single app there should never be outdated binaries... updates are handled by new docker images). For the binary path needrestart only checks if the symlink target of needrestart looks for mapped files at Both checks should not match on docker containers... and I'm not able to reproduce your behavior. Which version of needrestart did you use? |
I agree that there is no need to restart processes within docker containers. This is Debian jessie, needrestart in version 2.11-2~bpo8+1. I have a docker container running mysqld. The process list shows as: root@delgado[~]# ps -ef | grep mysqld root@delgado[~]# cd /proc/6694 root@delgado[6694]# ls -l exe There is no (deleted) suffix, so no need to check. But: root@delgado[~]# needrestart -vv This looks to me as if NR were not able to get the docker image for a running process, maybe because the kernel is too old? I tried patching NR and adding some additional debug output, which results in this output: root@delgado[needrestart]# perl -I perl/lib needrestart -vv This time the correct containers are detected, but still NR supposes a restart. |
Thanks for the details. I think the docker stuff in needrestart is broken by design. It was derived from the LXC module since LXC containers can run outdated binaries. We had just agreed that there is no need to restart docker containers after software updates (one would need to update the docker image and restart the container or services after that - but that is beyond the scope of needrestart). I'm going to change needrestart to ignore any process running within a docker container namespace. |
Hi, just checked your commit. If I run the new master with option "-v" everything is fine: root@libai[needrestart]# perl -Iperl/lib needrestart -v But if I run it without the '-v' switch, I get root@libai[needrestart]# perl -Iperl/lib needrestart Something must still be br0ken. Cheers, Christopher |
I was not able to reproduce this behavior. With needrestart 2.11-3 (Debian stretch) it tries to restart Might it be just a problem of the perl include path? |
This might be triggered by the debconf perl package which does fork and executes a new needrestart process which seems to use the wrong version of the needrestart packages. |
Hi,
we have a number of debian machines running docker containers. On all of these machines needrestart keeps complaining about the usage of obsolete files even if I restart the containers or even the whole machine.
After debugging the code I found the following logic being used:
This logic misses an important fact: If a process belongs to a container, it uses a different root directory which can be found by looking at the process' cgroup (and the docker config of course).
So currently NR reports
[main] #6722 uses obsolete /usr/local/bin/node
The file /usr/local/bin/node does not exist indeed. But NR should look for the file
/var/lib/docker/aufs/mnt/8b581d3068682ebf7ed7f45298b78a563192446aa1e4cbd2bd0bfc4c4d42f917/usr/local/bin/node
which DOES exist - and is older than the process start time, so there is nothing to do.
So my guess would be that the logic should be changed in the following way:
for each process:
Am I right?
Thanks,
Christopher
The text was updated successfully, but these errors were encountered: