-
Notifications
You must be signed in to change notification settings - Fork 25
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
Remove broken work-around for specifying build platforms for the operator image #49
Remove broken work-around for specifying build platforms for the operator image #49
Conversation
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.
Thanks for creating this PR @rooftopcellist !
One thing blocking this PR is that this only updates the testdata which is generated files and will be overwritten. This also means that only this testdata would be updated and future project scaffolding won't be fixed. Instead, the change should be made in
ansible-operator-plugins/pkg/plugins/ansible/v1/scaffolds/internal/templates/makefile.go
Lines 106 to 121 in bbe095f
# PLATFORMS defines the target platforms for the manager image be build to provide support to multiple | |
# architectures. (i.e. make docker-buildx IMG=myregistry/mypoperator:0.0.1). To use this option you need to: | |
# - able to use docker buildx . More info: https://docs.docker.com/build/buildx/ | |
# - have enable BuildKit, More info: https://docs.docker.com/develop/develop-images/build_enhancements/ | |
# - be able to push the image for your registry (i.e. if you do not inform a valid value via IMG=<myregistry/image:<tag>> than the export will fail) | |
# To properly provided solutions that supports more than one platform you should use this option. | |
PLATFORMS ?= linux/arm64,linux/amd64,linux/s390x,linux/ppc64le | |
.PHONY: docker-buildx | |
docker-buildx: test ## Build and push docker image for the manager for cross-platform support | |
# copy existing Dockerfile and insert --platform=${BUILDPLATFORM} into Dockerfile.cross, and preserve the original Dockerfile | |
sed -e '1 s/\(^FROM\)/FROM --platform=\$$\{BUILDPLATFORM\}/; t' -e ' 1,// s//FROM --platform=\$$\{BUILDPLATFORM\}/' Dockerfile > Dockerfile.cross | |
- docker buildx create --name project-v3-builder | |
docker buildx use project-v3-builder | |
- docker buildx build --push --platform=$(PLATFORMS) --tag ${IMG} -f Dockerfile.cross . | |
- docker buildx rm project-v3-builder | |
rm Dockerfile.cross |
make generate
0313bef
to
331ee70
Compare
Thanks for the review @everettraven ! I just pushed the change to the Let me know if any other changes are needed. |
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.
Thanks @rooftopcellist ! LGTM
@everettraven I think these CI failures are un-related and just need a re-kick, I could be wrong though. |
@rooftopcellist A re-kick will still fail as this is a build issue with the base image due to a vulnerability in one of the python packages. I'll create a separate PR to fix it (I don't want to ignore this because it will bite us later if we do) and once that is in I should be able to close and reopen this PR so it picks up those changes in CI |
…ator image Signed-off-by: Christian M. Adams <[email protected]>
331ee70
to
afeb162
Compare
Apologies for the delay on circling back around to this PR. the CI failures should have been addressed by #50 Pending a successful CI run this should be good to merge |
Description of the change:
Resolves: #48
Currently, the
make docker-buildx
target does not work for ansible operators as we found out in the github.com/ansible/eda-server-operator. See details here:Motivation for the change:
Users want to be able to build ansible operators for platforms other than the one on the build system or local machine.
cc @everettraven