feat: added support for Podman as container engine#96
Conversation
User Test ResultsTest specification and instructions User tests are not required |
mcdurdin
left a comment
There was a problem hiding this comment.
This looks clean and straightforward, just rename BUILDER_ENGINE to CONTAINER_ENGINE please for clarity
| fi | ||
| if [[ $# -ge 4 ]]; then | ||
| TARGET="-f $4 ." | ||
| FILE="-f $4 " |
There was a problem hiding this comment.
This may need additional tweaks later for api.keyman.com which has two Dockerfiles (one for db, one for site).
There was a problem hiding this comment.
Should I do anything about it, or should I leave it as it is for now?
| elif [[ "${CONTAINER_ENGINE}" == "docker" ]]; then | ||
| FILE="-f Dockerfile " | ||
| elif [[ "${CONTAINER_ENGINE}" == "podman" ]]; then | ||
| FILE="-f Podmanfile " |
There was a problem hiding this comment.
Are these really needed? Won't podman use Podmanfile by default just as docker uses Dockerfile by default?
There was a problem hiding this comment.
Podman uses "Dockerfile" for default to be compatible. Usually you can run Podman with the same file as Docker uses but the Dockerfile must address specific requirements of Podman e.g. full path to images.
If you like you can test the build on Docker with the Podmanfile, it should work.
There was a problem hiding this comment.
So if we can get to the point where Podman and Docker can use the same file, that will definitely be better for maintenance. The differences seem pretty minimal at present, so can we get to a point where we can merge them?
|
Test-bot: skip |
No description provided.