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

Add arm32 Dockerfile #5370

Closed
wants to merge 5 commits into from
Closed

Add arm32 Dockerfile #5370

wants to merge 5 commits into from

Conversation

johnelliott
Copy link

@johnelliott johnelliott commented Aug 10, 2018

fixes #4931

...or just related, looking for assistance form maintainers for what additional work to do here to maintain the image

@ghost
Copy link

ghost commented Aug 10, 2018

Looks good -- but are libld path and tini filename the only differences? If so, is there a way we could just parameterize the existing dockerfile?

@johnelliott
Copy link
Author

According to @yrzr here and my experience building, that was the only thing required. They mentioned that it is a convention, however given the small difference I was wondering the same thing.

@johnelliott
Copy link
Author

I've looked at some ways to parameterize the Dockerfile but haven't begun work on it.

@Stebalien Stebalien added the need/author-input Needs input from the original author label Aug 16, 2018
@johnelliott
Copy link
Author

johnelliott commented Aug 18, 2018

Side note here: The GitCop template links respond with 404.

@johnelliott
Copy link
Author

@lgierth I updated the existing Dockerfiles with parameters and successfully built them on Mac and Raspberry Pi. I need assistance knowing what to do next. I was looking for how to test, if there are automated tests for this.

I'm also curious if we can hook this up to CI so others can just pull down an arm image.

@Stebalien Stebalien requested a review from a user August 20, 2018 21:08
@Stebalien Stebalien added need/review Needs a review and removed need/author-input Needs input from the original author labels Aug 20, 2018
@ghost
Copy link

ghost commented Aug 23, 2018

Nice, this looks good to me!

The next steps are documenting the build args in the Dockerfiles themselves and in the README.md docker section.

And maybe (later) we can have Makefile targets a la make docker_amd64 and make docker_armv7.

ghost
ghost previously approved these changes Aug 23, 2018
@ghost ghost dismissed their stale review August 23, 2018 21:12

Sorry, too early -- need docs, see my comment

@johnelliott
Copy link
Author

OK, I will first see if a convention exists for such documentation and add some notes. I see the existing comment about maintaining the faster-building file. I'll update the readme also.

@Stebalien Stebalien added need/author-input Needs input from the original author need_docs and removed need/review Needs a review labels Aug 23, 2018
kvm2116 and others added 4 commits September 7, 2018 22:00
License: MIT
Signed-off-by: Kunal Mahajan <mkunal@cs.columbia.edu>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
Allow, for example:

  $ docker -v build . -f Dockerfile.fast --build-arg base=arm32v7/golang:1.10-stretch --build-arg tini_executable=tini-armhf --build-arg 'busybox_base=arm32v7/busybox:1-glibc' --build-arg glibc_shared_lib_arch=arm-linux-gnueabihf

to build for a Raspberry Pi (arm32v7)

License: MIT
Signed-off-by: John Elliott <johnelliott703@gmail.com>
@johnelliott
Copy link
Author

johnelliott commented Sep 8, 2018

I added documentation of the arguments and some notes to the README with example build commands

@Stebalien Stebalien removed need_docs need/author-input Needs input from the original author labels Sep 11, 2018
@Stebalien Stebalien requested a review from a user September 11, 2018 01:03
@Stebalien Stebalien added the need/review Needs a review label Sep 11, 2018
@johnelliott
Copy link
Author

Looks like I'll need to give this some more attention until a maintainer can review and merge this.

@doodlemania2
Copy link

any update on this? looking for an ipfs docker container for armhf

@yrzr
Copy link

yrzr commented Dec 1, 2018

@doodlemania2 Seems they haven't added the docker images yet. You can use my builds for arm32v7(armhf) and arm64v8(aarch) first. Or you can build your own images following #4931.

@johnelliott
Copy link
Author

johnelliott commented Dec 1, 2018

@doodlemania2 I got started with @yrzr's work and this PR is based on his work. Your fastest path forward is using their builds. I used them successfully before starting this branch

I just updated my fork via the website and I may need to pull it down to a license commit.

@doodlemania2
Copy link

Thanks all!

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

This looks good modulo some out of date documentation.

However, as I'm not familiar with docker, I'd also like to sanity check a few things (see the review comments)

# Go base image
ARG base=golang:1.11-stretch
# Busybox base image
ARG busybox_base=busybox:1-glibc
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a docker expert, but shouldn't this just go down at line 46? Is there a reason to duplicate it?

@@ -317,6 +317,48 @@ Stop the running container:

docker stop ipfs_host

### Building Docker images
Copy link
Member

Choose a reason for hiding this comment

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

Should be updated for go 1.11.

#### `busybox_base`
Possible values:
- `busybox:1-glibc` (default)
- `arm32v7/golang:1.10-stretch`
Copy link
Member

Choose a reason for hiding this comment

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

busybox.

- `base`: The base go image for the target platform
- `busybox_base`: The busybox base image for the target platform
- `tini_executable`: The tini executable path for the target platform
- `glibc_shared_lib_arch`: The target platform shared lib for glibc supplementing busybox
Copy link
Member

Choose a reason for hiding this comment

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

Is there really no better way to do this (i.e., with one variable). From what I can tell, there simply isn't but I'm not a Docker expert so I'd like to double check.

Copy link
Author

Choose a reason for hiding this comment

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

Same here. This was seemingly the only way.

@Stebalien Stebalien added need/author-input Needs input from the original author and removed need/review Needs a review labels Dec 3, 2018
@johnelliott
Copy link
Author

I may get to this this week :)

@johnelliott
Copy link
Author

There doesn't seem to be sufficient desire for this change yet so I'm closing this PR. Anyone else can feel free to take it up again :)

@johnelliott johnelliott closed this Jan 3, 2019
@Stebalien Stebalien removed the need/author-input Needs input from the original author label Jan 3, 2019
@Stebalien
Copy link
Member

Yeah, sorry about that. This looks useful but nobody (that I know of) on the current team has extensive knowledge of docker or the need to run go-ipfs on arm. That makes this a bit difficult to validate, test, and maintain.

However, if someone has a real need for this, please comment and we can reopen (or continue in a new PR).

@doodlemania2
Copy link

Thanks all - I ended up rolling my own: https://github.com/doodlemania2/ethereum-armhf based on other's work.

@master-hax
Copy link

btw the go-ipfs docker images provided by linuxserver.io work on armhf: https://hub.docker.com/r/linuxserver/ipfs

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.

docker image for ARM
6 participants