Skip to content
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

Make edits for s390x support #1399

Merged
merged 5 commits into from
Aug 10, 2023
Merged

Conversation

Joshua-Beha
Copy link
Contributor

This closes #1396

Note to reviewers: remember to look at the commits in this PR and consider if they can be squashed

Summary Of Changes

Use docker buildx to create a multi-arch manifest that includes support for amd64 and s390x architectures.

Additional Context

NA

Local Testing

Works as expected on both s390x Kubernetes cluster and x86 Kubernetes cluster.

Please ensure you run the unit, integration and system tests before approving the PR.

To run the unit and integration tests:

$ make unit-tests integration-tests

You will need to target a k8s cluster and have the operator deployed for running the system tests.

For example, for a Kubernetes context named dev-bunny:

$ kubectx dev-bunny
$ make destroy deploy-dev
# wait for operator to be deployed
$ make system-tests

@Zerpet Zerpet self-assigned this Jul 17, 2023
@Zerpet Zerpet self-requested a review July 17, 2023 12:12
Copy link
Collaborator

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

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

The change is valid. Attempting to build for s390x arch before this PR resulted in an error:

error output
 => ERROR [etc-builder 2/3] RUN echo "rabbitmq-cluster-operator:x:1000:" > /etc/group &&     echo "rabbitmq-cluster-operator:x:1000:1000::/home/rabbitm  0.3s
------
 > [etc-builder 2/3] RUN echo "rabbitmq-cluster-operator:x:1000:" > /etc/group &&     echo "rabbitmq-cluster-operator:x:1000:1000::/home/rabbitmq-cluster-operator:/usr/sbin/nologin" > /etc/passwd:
0.324 exec /bin/sh: exec format error
------
Dockerfile:28
--------------------
  27 |
  28 | >>> RUN echo "rabbitmq-cluster-operator:x:1000:" > /etc/group && \
  29 | >>>     echo "rabbitmq-cluster-operator:x:1000:1000::/home/rabbitmq-cluster-operator:/usr/sbin/nologin" > /etc/passwd
  30 |
--------------------
ERROR: failed to solve: process "/bin/sh -c echo \"rabbitmq-cluster-operator:x:1000:\" > /etc/group &&     echo \"rabbitmq-cluster-operator:x:1000:1000::/home/rabbitmq-cluster-operator:/usr/sbin/nologin\" > /etc/passwd" did not complete successfully: exit code: 1

Notice exec /bin/sh: exec format error

After this PR, this builds successfully. The changes to the Dockerfile are the ground work to make it possible to build in linux s390x, however, this won't suffice to start producing images in that architecture. That will require changes to our CI, which may not be trivial.

There's also a change request to the Makefile to not build by default on s390x architecture.

Makefile Outdated Show resolved Hide resolved
@Sapana-Khemkar
Copy link
Contributor

@Joshua-Beha look at PR #1421, you have to do similar changes in CI workflow for s390.

@Joshua-Beha
Copy link
Contributor Author

Updated to add s390x architecture image to the automation and remove s390x from being build by default from the Makefile

@Zerpet
Copy link
Collaborator

Zerpet commented Aug 9, 2023

The changes look good. There are conflicts with the main branch, kinda expected since #1421 changed the same file of this PR. Can you rebase your branch on top of main?

@Joshua-Beha
Copy link
Contributor Author

Should be rebased now

@Sapana-Khemkar
Copy link
Contributor

@Joshua-Beha just do changes in build-test-publish.yml. Revert all other changes as they are not required

@Zerpet Zerpet merged commit 3d252f2 into rabbitmq:main Aug 10, 2023
@Zerpet
Copy link
Collaborator

Zerpet commented Aug 10, 2023

Thank you!

@Joshua-Beha Joshua-Beha deleted the multi-arch-support branch August 10, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support s390x Architecture
3 participants