Skip to content
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

feat: Update makefile to support building with podman #266

Merged
merged 1 commit into from
Oct 26, 2022
Merged

feat: Update makefile to support building with podman #266

merged 1 commit into from
Oct 26, 2022

Conversation

anishasthana
Copy link
Contributor

@anishasthana anishasthana commented Oct 22, 2022

This PR will update the makefile and build scripts to allow developers to use podman as the build engine.
The default behaviour for the makefile will remain the same.

I ran the following to build images:

make build.develop ENGINE=podman
make build ENGINE=podman

I don't have a working docker env to test in.
cc @njhill

Copy link
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @anishasthana, this looks good to me. I thought docker aliased to podman anyhow though?

Should we also add --format=docker to the build command in the podman case?

scripts/build_devimage.sh Outdated Show resolved Hide resolved
@njhill njhill changed the title Update makefile to support building with podman feat: Update makefile to support building with podman Oct 25, 2022
@anishasthana
Copy link
Contributor Author

It only happens if you've actually set up the alias ahead of time. I hadn't :-)

Should we also add --format=docker to the build command in the podman case?

I actually haven't used the --format=docker flag before -- never found the need to. There doesn't seem to be a major difference in the image specs.

@anishasthana
Copy link
Contributor Author

I spoke to one of the podman maintainers. She said that the --format=docker is unnecessary unless there's something very specific to the docker format that our image requires. The OCI format is standard now and should be the default -- any runtimes that are OCI-compliant will work with OCI images.

@njhill
Copy link
Member

njhill commented Oct 26, 2022

It only happens if you've actually set up the alias ahead of time.

Ah interesting, the RHEL install I use had it already so I assumed it was configured by default.

I spoke to one of the podman maintainers. She said that the --format=docker is unnecessary unless there's something very specific to the docker format that our image requires. The OCI format is standard now and should be the default -- any runtimes that are OCI-compliant will work with OCI images.

OK sure, I just recall that I'd had to add it at some point in the past, maybe was for something obscure.

@kserve-oss-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anishasthana, njhill

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@njhill
Copy link
Member

njhill commented Oct 26, 2022

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants