Accelerated container image for AMD #316
Conversation
kwozyman
requested review from
MichaelClifford,
rhatdan,
sallyom,
lmilbaum and
cgwalters
as code owners
|
|
||
| .PHONY: amd | ||
| amd: | ||
| make -C amd/ image |
There was a problem hiding this comment.
I think you would need to change this to
make -C amd/ image FROM=$(FROM) REGISTRY=$(REGISTRY) REGISTRY_ORG=$(REGISTRY_ORG)...
There was a problem hiding this comment.
I think these get inherited as environment variables. At least on my Fedora 39 a make amd REGISTRY=foo gets me the correct name just with the above
There was a problem hiding this comment.
If they do that is great, ignore my comment.
training/amd/Makefile
Outdated
| "${CONTAINER_TOOL}" build \ | ||
| --file Containerfile \ | ||
| --tag "${REGISTRY}/${REGISTRY_ORG}/${IMAGE_NAME}:${IMAGE_TAG}" \ | ||
| --build-arg BASEIMAGE=${FROM} \ |
There was a problem hiding this comment.
In the other repos we have been using
--from $(FROM) rather then --build-arg.
training/amd/Containerfile
Outdated
| @@ -0,0 +1,8 @@ | |||
| ARG BASEIMAGE="quay.io/centos-bootc/centos-bootc:stream9" | |||
|
|
|||
| FROM ${BASEIMAGE} | |||
There was a problem hiding this comment.
I would prefer to use FROM quay.io/centos-bootc/centos-bootc:stream9 and use `--from $(FROM) in the Makefile like we do in the rest of the repo.
|
Looks good, just a couple of nits. |
|
@kwozyman you need to sign your commits, and then we can merge. |
Signed-off-by: Costin Gamenț <cgament@redhat.com>
|
LGTM |
I've added the first bootc bare metal accelerated bootc container image and it's README.