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

support arm architecture #2803

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

support arm architecture #2803

wants to merge 3 commits into from

Conversation

zeetabit
Copy link

@zeetabit zeetabit commented Jul 21, 2023

To be able build not only amd64 images we need to support golang downloading for proper architecture.

Example:
docker buildx build --platform=linux/arm64 -t sameersbn/gitlab .

You can even adjust (in future) your automation builds to push multiarch builds to your docker registries. This is buildkit docker feature OOTB.

Example:
docker buildx build --platform=linux/arm64,linux/arm64 -t <repo tag> --push .
To speedup build you can use previous build as cache
docker buildx build --platform=linux/arm64,linux/arm64 --cache-from <prev build tag> -t <new tag> --push .
and so on.

To be able build not only amd64 images we need to support golang downloading for proper architecture.

Example:
docker buildx build --platform=linux/arm64 -t sameersbn/gitlab .
Copy link
Contributor

@kkimurak kkimurak left a comment

Choose a reason for hiding this comment

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

Short and smart code. I like it. Thank you for your contribution.

For your information:

  1. There is a concurrent work Add support to build arm64 image #2732 .

  2. From man page of dpkg-architecture(1) :

    In any case, you should never use dpkg --print-architecture to get architecture
    information during a package build.

    What do you think about it? If you plan to use dpkg, the document seems to recommend using dpkg-architecture -qDEB_HOST_ARCH. Also, We must ensure that the output of dpkg-architecture matches the architecture name used in golang archive filenames.

@zeetabit
Copy link
Author

zeetabit commented Aug 24, 2023

Thank you for detail answer.
I fine with alternate solution - why not ;)
I don't think that it's critical to use dpkg --print-architecture as we are not building packages, we just downloading ready ones. We do not need to initialise them as we are not modifying them at package build steps (like in makefile at compilation steps, so on).

 Usage in debian/rules
       The environment variables set by dpkg-architecture are passed to
       debian/rules as make variables (see make documentation). However, you
       should not rely on them, as this breaks manual invocation of the
       script. Instead, you should always initialize them using dpkg-
       architecture with the -q option. Here are some examples, which also
       show how you can improve the cross compilation support in your package:

       Retrieving the GNU system type and forwarding it to ./configure:

BTW I checked only arm64/amd64 builds. If you are going to support other architectures for all binaries in Dockerfile , for sure we should check golang binaries (and other (!) binaries), for golang list is there https://storage.googleapis.com/golang/, for example I see that i386 architecture stored without i, so script with if/else or switch/case blocks is needed (like at alternate solution). Golang documentation https://go.dev/doc/install/source
For now I would say it will be overcomplicated without real requirement to have it, but can be implemented for sure in future.

@zeetabit
Copy link
Author

zeetabit commented Aug 24, 2023

So I will adjust PR from

dpkgArch="$(dpkg --print-architecture)"

to

dpkgArch="$(dpkg-architecture -qDEB_HOST_ARCH)"

and that's all. It still will support arm64/amd64, but other architectures should be adjusted additionally (and other binaries, starting from basic ubuntu image architecture list support).
But to be honest I do not see any improve on it.

@zeetabit
Copy link
Author

updated as mentioned, checked by
docker buildx build --platform=linux/amd64 --cache-from sameersbn/gitlab -t sameersbn/gitlab .
docker buildx build --platform=linux/arm64 --cache-from sameersbn/gitlab -t sameersbn/gitlab .

@nohnaimer
Copy link

Hello,
You can build it like this:

~$ docker buildx create --use
~$ docker buildx build --platform=linux/amd64,linux/arm64 --cache-from sameersbn/gitlab -t sameersbn/gitlab:16.4.1 .

But improvements are needed for the arm, because now it gives an error:

#0 176.2 go: downloading google.golang.org/appengine v1.6.7
#0 189.3 # runtime/cgo
#0 189.3 gcc: error: unrecognized command line option '-m64'
#0 239.0 make: *** [Makefile:99: bin/gitlab-shell] Error 1
------
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
Dockerfile:60
--------------------
  58 |
  59 |     COPY assets/build/ ${GITLAB_BUILD_DIR}/
  60 | >>> RUN bash ${GITLAB_BUILD_DIR}/install.sh
  61 |
  62 |     COPY assets/runtime/ ${GITLAB_RUNTIME_DIR}/
--------------------
ERROR: failed to solve: process "/bin/sh -c bash ${GITLAB_BUILD_DIR}/install.sh" did not complete successfully: exit code: 2

@gjrtimmer
Copy link
Contributor

So it looks like a need to write patches that update the CGO flags in GoLang to set the reight flags for compilation for arm64

@scooby22400
Copy link

To be able build not only amd64 images we need to support golang downloading for proper architecture.

Example: docker buildx build --platform=linux/arm64 -t sameersbn/gitlab .

You can even adjust (in future) your automation builds to push multiarch builds to your docker registries. This is buildkit docker feature OOTB.

Example: docker buildx build --platform=linux/arm64,linux/arm64 -t --push . To speedup build you can use previous build as cache docker buildx build --platform=linux/arm64,linux/arm64 --cache-from -t --push . and so on.

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.

None yet

5 participants