Skip to content

Remove broken work-around for specifying build platforms for the operator image#49

Merged
everettraven merged 1 commit into
operator-framework:mainfrom
rooftopcellist:cross-compilation-target
Jan 22, 2024
Merged

Remove broken work-around for specifying build platforms for the operator image#49
everettraven merged 1 commit into
operator-framework:mainfrom
rooftopcellist:cross-compilation-target

Conversation

@rooftopcellist

@rooftopcellist rooftopcellist commented Jan 8, 2024

Copy link
Copy Markdown
Contributor

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

@openshift-ci openshift-ci Bot requested review from anik120 and jberkhahn January 8, 2024 19:07
@rooftopcellist rooftopcellist mentioned this pull request Jan 8, 2024
Closed
everettraven
everettraven requested changes Jan 9, 2024
View reviewed changes

@everettraven everettraven left a comment

Copy link
Copy Markdown
Collaborator

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
and the testdata regenerated with make generate

@rooftopcellist rooftopcellist force-pushed the cross-compilation-target branch from 0313bef to 331ee70 Compare January 11, 2024 04:56
@rooftopcellist

rooftopcellist commented Jan 11, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for the review @everettraven ! I just pushed the change to the makefile.go, I left in the diff for the testdata as I figured it wouldn't hurt.

Let me know if any other changes are needed.

everettraven
everettraven approved these changes Jan 11, 2024
View reviewed changes

@everettraven everettraven left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @rooftopcellist ! LGTM

@rooftopcellist

rooftopcellist commented Jan 11, 2024

Copy link
Copy Markdown
Contributor Author

@everettraven I think these CI failures are un-related and just need a re-kick, I could be wrong though.

@everettraven

everettraven commented Jan 12, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@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

@kurokobo kurokobo mentioned this pull request Jan 12, 2024
Closed
@rooftopcellist
Remove broken work-around for specifying build platforms for the oper…
afeb162 
…ator image

Signed-off-by: Christian M. Adams <chadams@redhat.com>
@rooftopcellist rooftopcellist force-pushed the cross-compilation-target branch from 331ee70 to afeb162 Compare January 15, 2024 22:49
@everettraven

everettraven commented Jan 22, 2024

Copy link
Copy Markdown
Collaborator

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

@everettraven everettraven merged commit d76a8db into operator-framework:main Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@everettraven everettraven everettraven approved these changes

@anik120 anik120 Awaiting requested review from anik120

@jberkhahn jberkhahn Awaiting requested review from jberkhahn

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cross Platform docker-buildx target does not work

2 participants

@rooftopcellist @everettraven