-
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 all 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,11 @@ def main(): # pylint: disable=too-many-branches,too-many-return-statements | |
| else: | ||
| args.sanitizer = constants.DEFAULT_SANITIZER | ||
|
|
||
| if (hasattr(args, 'architecture') and | ||
| args.architecture != constants.DEFAULT_ARCHITECTURE and | ||
| CONTAINER_TOOL != 'docker'): | ||
| raise RuntimeError('Non-default architectures require Docker.') | ||
|
Contributor
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
Collaborator
Author
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.
Contributor
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. FWIW I tested i386 builds in x86_64 VMs with podman last week and it did work in the sense that I was able to reproduce a build failure where |
||
|
|
||
| if args.command == 'generate': | ||
| result = generate(args) | ||
| elif args.command == 'build_image': | ||
|
|
@@ -585,7 +592,13 @@ def check_project_exists(project): | |
| 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, | ||
| ] | ||
| command.extend(['-v', '%s:/out' % project.out]) | ||
| command.append(BASE_RUNNER_IMAGE) | ||
|
|
||
|
|
@@ -750,10 +763,11 @@ def prepare_aarch64_emulation(): | |
|
|
||
|
|
||
| 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', '--privileged', '--shm-size=2g', '--platform', | ||
| platform | ||
| ] | ||
| if os.getenv('OSS_FUZZ_SAVE_CONTAINERS_NAME'): | ||
| command.append('--name') | ||
|
|
@@ -781,8 +795,8 @@ def docker_run(run_args, print_output=True, architecture='x86_64'): | |
|
|
||
|
|
||
| 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_build(build_args): | |
|
|
||
|
|
||
| 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 @@ def fuzzbench_build_fuzzers(args): | |
| ] | ||
| 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_TOOLisdockereven thoughpodmanis used under the hood. It would probably be safer to callCONTAINER_TOOLto 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
makedirsis 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
--privilegedI guess at some point it should be necessary to tell docker and podman apart.