-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Experimental support for podman. #13973
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,6 +100,8 @@ | |
INDEXER_PREBUILT_URL = ('https://clusterfuzz-builds.storage.googleapis.com/' | ||
'oss-fuzz-artifacts/indexer') | ||
|
||
CONTAINER_TOOL = os.getenv('OSS_FUZZ_CONTAINER_TOOL', 'docker') | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
if sys.version_info[0] >= 3: | ||
|
@@ -222,6 +224,10 @@ | |
else: | ||
args.sanitizer = constants.DEFAULT_SANITIZER | ||
|
||
if (hasattr(args, 'architecture') and | ||
args.architecture != constants.DEFAULT_ARCHITECTURE): | ||
raise RuntimeError('Non-default architectures require Docker.') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be OK to pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks! This is entirely un-tested territory, so I'd prefer not to support this for now. |
||
|
||
if args.command == 'generate': | ||
result = generate(args) | ||
elif args.command == 'build_image': | ||
|
@@ -585,7 +591,14 @@ | |
def _check_fuzzer_exists(project, fuzzer_name, architecture='x86_64'): | ||
"""Checks if a fuzzer exists.""" | ||
platform = 'linux/arm64' if architecture == 'aarch64' else 'linux/amd64' | ||
command = ['docker', 'run', '--rm', '--platform', platform] | ||
command = [ | ||
CONTAINER_TOOL, | ||
'run', | ||
'--rm', | ||
'--platform', | ||
platform, | ||
"--pull=never", | ||
|
||
] | ||
command.extend(['-v', '%s:/out' % project.out]) | ||
command.append(BASE_RUNNER_IMAGE) | ||
|
||
|
@@ -750,10 +763,11 @@ | |
|
||
|
||
def docker_run(run_args, print_output=True, architecture='x86_64'): | ||
"""Calls `docker run`.""" | ||
"""Calls `CONTAINER_TOOL run`.""" | ||
platform = 'linux/arm64' if architecture == 'aarch64' else 'linux/amd64' | ||
command = [ | ||
'docker', 'run', '--privileged', '--shm-size=2g', '--platform', platform | ||
CONTAINER_TOOL, 'run', '--pull=never', '--privileged', '--shm-size=2g', | ||
'--platform', platform | ||
] | ||
if os.getenv('OSS_FUZZ_SAVE_CONTAINERS_NAME'): | ||
command.append('--name') | ||
|
@@ -781,8 +795,8 @@ | |
|
||
|
||
def docker_build(build_args): | ||
"""Calls `docker build`.""" | ||
command = ['docker', 'build'] | ||
"""Calls `CONTAINER_TOOL build`.""" | ||
command = [CONTAINER_TOOL, 'build'] | ||
command.extend(build_args) | ||
logger.info('Running: %s.', _get_command_string(command)) | ||
|
||
|
@@ -796,8 +810,8 @@ | |
|
||
|
||
def docker_pull(image): | ||
"""Call `docker pull`.""" | ||
command = ['docker', 'pull', image] | ||
"""Call `CONTAINER_TOOL pull`.""" | ||
command = [CONTAINER_TOOL, 'pull', image] | ||
logger.info('Running: %s', _get_command_string(command)) | ||
|
||
try: | ||
|
@@ -1032,7 +1046,7 @@ | |
] | ||
tag = f'gcr.io/oss-fuzz/{args.project.name}' | ||
subprocess.run([ | ||
'docker', 'tag', 'gcr.io/oss-fuzz-base/base-builder-fuzzbench', | ||
CONTAINER_TOOL, 'tag', 'gcr.io/oss-fuzz-base/base-builder-fuzzbench', | ||
'gcr.io/oss-fuzz-base/base-builder' | ||
], | ||
check=True) | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can be confusing in the sense that podman is often installed with podman-docker pointing docker to podman (https://packages.fedoraproject.org/pkgs/podman/podman-docker/) so
CONTAINER_TOOL
isdocker
even thoughpodman
is used under the hood. It would probably be safer to callCONTAINER_TOOL
to figure out what it is.The result can then be used to get around differences like #9439.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should avoid handling these differences as much as we can.
For #9439, is this still a problem? Can we just add the makedirs as a default behaviour for docker also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. I have been applying that patch since then. I'll double-check to see if it's still needed a bit later today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It failed without that patch with
so it looks like
makedirs
is still required.I think it should be fine (I'm not 100% sure though)
Agreed. I think it should probably be possible to avoid that to cover most use cases but given that for example #4774 was supposed to get it to work with rootless podman containers with SELinux enabled without
--privileged
I guess at some point it should be necessary to tell docker and podman apart.