CIFuzz: switch to systemd-detect-virt to detect docker #4101
Conversation
infra/utils.py
Outdated
|
|
||
| Returns: | ||
| Container name or None if not in a container. | ||
| """ | ||
| with open('/proc/self/cgroup') as file_handle: | ||
| if 'docker' not in file_handle.read(): | ||
| if b'docker' not in subprocess.run('systemd-detect-virt -c', shell=True, stdout=subprocess.PIPE).stdout: |
There was a problem hiding this comment.
Thank you so much! you rock!
can you please do
result = subprocess.run('systemd-detect-virt -c', shell=True, stdout=subprocess.PIPE)
if b'docker' in result.stdout:
There was a problem hiding this comment.
stdout=subprocess.PIPE can be skipped as well i think
There was a problem hiding this comment.
b'docker' in subprocess.run(['systemd-detect-virt', '-c'], stdout=subprocess.PIPE).stdout should work.
There was a problem hiding this comment.
According to https://docs.python.org/3/library/subprocess.html#subprocess.run, run doesn't capture stdout by default. Another option would be to pass capture_output=True to it but it was added relatively recently so I don't expect it to be available on Ubuntu (where python-3.5 is still used).
There was a problem hiding this comment.
Why not just subprocess.check_output
This is equivalent to:
run(..., check=True, stdout=PIPE).stdout
There was a problem hiding this comment.
systemd-detect-virt -c returns 1 when it isn't run in containers, which makes check_output throw exceptions left and right. I don't think it's desirable given that that code can be run elsewhere.
There was a problem hiding this comment.
ok then, please move this to a local var, that line is too long. rest lgtm.
There was a problem hiding this comment.
I replaced shell=True with ['systemd-detect-virt', '-c'] as @jonathanmetzman suggested.
I'll add result=... shortly
evverx
force-pushed
the
systemd-detect-virt
branch
2 times, most recently
from
906221d to
c563c92
Compare
|
Minor lint failure in presubmit in https://github.com/google/oss-fuzz/pull/4101/checks?check_run_id=855435834 |
evverx
force-pushed
the
systemd-detect-virt
branch
from
c563c92 to
69cb2fe
Compare
|
The presubmit check is still failing unfortunately with |
it is ok, we will fix it sometime later. |
"check" is omitted intentionally there. A follow-up to google#4101
Until then it would probably make sense to ignore subprocess-run-check there. I've just opened #4102. |
"check" is omitted intentionally there. A follow-up to #4101
Drop the older container detection attempts because they are not reliable to detect being run as root in a container in github actions. <cloph> google/oss-fuzz#4093 (comment) "It appears some GitHub actions are run with docker.service (where docker is in /proc/self/cgroup) while the zstd actions are run with containerd.service where /proc/self/cgroup looks like […]" <cloph> google/oss-fuzz#4101 → probably also just use systemd-detect-virt instead of the grepping ourselves... if we're root and systemd-detect-virt doesn't exist or it claims we're not in a container then continue to abort the build using LIB_FUZZING_ENGINE for the oss-fuzz specific case worked fine, but lets try something a little more generic. Change-Id: I59711b01dfcd052b5af899ad41ae5890f849eacb Reviewed-on: https://gerrit.libreoffice.org/c/core/+/113738 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Closes #4093