-
Notifications
You must be signed in to change notification settings - Fork 71
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
False positive for containerd #317
Comments
UPDATE 2: reported it at https://bugs.launchpad.net/ubuntu/+source/needrestart/+bug/2089193 So I guess it should be closed here? |
Will need to validate - A patch was recently merged from upstream for https://ubuntu.com/security/notices/USN-7117-1 Can you also run this upstream in the machine and see if it behaves the same? |
Likewise for Debian 12 (Bookworm) with |
I cloned needrestart locally and with bisect found that the problem was introduced in the following commit: 6ce6136
|
We have seen downstream reports in Debian which might be related: But they are not yet further analyzed. |
Thanks! Could you please provide the output of |
I picked just a random process, as I wasn't sure what |
Thanks @zerkms ! Tested on Debian bookworm with git HEAD after spawning a docker container (with containerd) but I was not able to reproduce it, yet 🤔 Looking at all the downstream bugs and your bisect:
|
UPDATE: okay, with just docker I cannot reproduce it either (and what is also unfortunate - I never in my life coded in perl so cannot give a hand in debugging it locally). But it looks like running it through docker triggers some other internal machinery:
So it knows about docker. But possibly it does not recognise it as a containerised process when it's naked containerd/runc? |
okay, here is a repro with containerd (from a host that runs docker):
second terminal:
Relevant output:
(just ensure that the host filesystem does not have |
🤦 This is the trigger! Proc::ProcessTable does not provide a value in the I need to take a look at how to deal with this as the patch prevents a race condition. Sorry it's too late for me to look at it just now. |
I have the same problem, but the non-existing file in the host filesystem is not the distinguishing feature. Both examples are from inside a docker container. This is the obsolete binary that triggers a restart:
but this does not (and does not look for the parent, but detects the container):
Both binaries are not part of the host filessystem:
The host system is debian 11 in a VM on proxmox and I'll try to reproduce the problem, but until then, I don't change anything on this machine and could help with any data that you need for debugging. |
Some more information since I'm able to reproduce the problem. I have pinned the docker packages from the docker repo, since the vm is running some legacy images (see at the end for the versions). Now I can reproduce the problem with this docker command:
The important switch is The docker packages used are:
|
Test with a clean debian 12 VM shows the same with current updates and needrestart 3.6-4+deb12u2:
|
The suspect line line 533 was added to avoid unnecessary further tests. This is wrong because of of the way the Proc::ProcessTable module works. Dropping the line fixes the example provided by @zerkms on my host. I will do some more testing and review before merging this into he master branch. Feel free to give 42af5d3 a try and report back here, thanks! |
The change from 42af5d3 fixes the problem both for my debian 11 and debian 12 occurences. Thank you! |
FWIW, I have uploaded for Debian unstable https://tracker.debian.org/news/1588733/accepted-needrestart-37-32-source-into-unstable/ and wanted to give it a bit more exposure before doing a regression update. I see you just ammended the commit with 63c0f1b so I guess it's worth waiting. |
IMPORTANT: it's a report for needrestart v3.6 since it's the latest version available on ubuntu's. If it was fixed in one of the later releases - my apologies, I don't see anything relevant in the changelog though.
Affected software:
ubuntu 24.04
needrestart: 3.6 (3.6-7ubuntu4.3)
containerd: 1.7.12-0ubuntu4.1
It started happening just this morning, so I guess it's canonical's needrestart packaging of needrestart that is to blame?
UPDATE: confirmed, after downgrading to
3.6-7ubuntu4.1
it works as expected. So it's3.6-7ubuntu4.3
release that is at fault. Not sure where to report it though.The text was updated successfully, but these errors were encountered: