makefile: use a variable to choose "docker" command #260
Conversation
There was a problem hiding this comment.
@phlogistonjohn i think we need to update this one also
docker login -u "${QUAY_IO_USERNAME}" -p "${QUAY_IO_PASSWORD}" quay.io
Makefile
Outdated
| @@ -14,6 +14,8 @@ | |||
|
|
|||
| .PHONY: all rbdplugin cephfsplugin | |||
|
|
|||
| DOCKERCMD:=docker | |||
There was a problem hiding this comment.
| DOCKERCMD:=docker | |
| CONTAINER_RUNTIME_BINARY:=docker |
There was a problem hiding this comment.
I can make that change but I chose 'DOCKERCMD' because it matches the variable name in a similar change I made in rook. Let me know if you feel strongly about it though.
There was a problem hiding this comment.
Not really but if DOCKERCMD is podman then that does not mean much at the end ^^. That's why I proposed CONTAINER_RUNTIME_BINARY to make it as generic as possible.
There was a problem hiding this comment.
Sure. :-) But docker is the defacto standard and podman is compatible with docker cli.
I wasn't trying to replace all instances of docker, just make it so that I (and others) can build without it. Since deploy.sh is not referenced from the Makefile I think we can skip that for now. |
There was a problem hiding this comment.
I like adding some flexibility, what about just adding an eval for env variables though?
So similar to what we have for like RBD_IMAGE_NAME maybe add an variable/eval for something more generic like:
CONTAINER_RUNTIME
It might still be limited in the options; but maybe in the future it could be worth expanding to call a shell script to do different types of container builds?
Just a thought... meanwhile I would like to see L#17 changed to eval for an env variable either way though (and default to docker).
the reason why i suggested was, you have replaced for the command to push image also, log in with docker and pushing an image with different command does not feel right to me. |
This change allows the use of alternatives to or wrappers around the normal docker command for container builds. Example 1: make image-rbdplugin CONTAINER_CMD=podman Example 2: CONTAINER_CMD=podman make image-rbdplugin Signed-off-by: John Mulligan <jmulligan@redhat.com>
phlogistonjohn
force-pushed
the
jjm-dockercmd
branch
from
28f05bf to
23027af
Compare
@leseb @Madhu-1 @j-griffith PTAL |
This change allows the use of alternatives to or wrappers around the normal docker command when running the deploy.sh script. Example: CONTAINER_CMD=podman ./deploy.sh Signed-off-by: John Mulligan <jmulligan@redhat.com>
phlogistonjohn
force-pushed
the
jjm-dockercmd
branch
from
23027af to
33a2fb1
Compare
|
@leseb ping. I think I addressed your comments, but please let me know if I overlooked something. |
|
@phlogistonjohn looks good thanks. |
|
That was super fast! thanks! :-) |
makefile: use a variable to choose "docker" command
BUG 2264002: rbd: log sitestatuses and description
This change allows the use of alternatives to or wrappers around
the normal docker command for container builds.
Example: make image-rbdplugin DOCKERCMD=podman