-
Notifications
You must be signed in to change notification settings - Fork 533
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
Multi-arch support #324
Multi-arch support #324
Conversation
f785a6f
to
ea02a45
Compare
Update:I've gotten it to be able to build different architecture images from one config.
Running
WDYT @andyshinn @tianon |
Almost There!I've got the library spec generator working with multi-arch.
Then you just copy that into the The last step is to figure out what to do with multi-arch images for gliderlabs. I'll reach out to them. They may want to just keep their images as x86_64. Once that's figured out and I fix a few more things, this PR will be all set. Apologies to all for clogging up this PR and the related issues with updates, etc. I am using this as a sort of documentation in place. Eventually, if requested, I could put it all in a markdown file in After this PR is good, the maintainers will decide which versions to roll out multi-arch on. Currently the only config I've changed is One thing I wonder is if explicitly specifying only amd64 instead of the default inferred architectures will have implications on the i386 alpine images. I hope it won't break anything. |
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.
In the commit
function there is a glob on versions/**/rootfs.tar.xz
which no longer works correctly in bash. This might be simply solved with shopt -s globstar
somewhere in the script (maybe the top is fine). It requires bash 4.x but I think we can just switch the CIrcleCI builder to Ubuntu 14.04 to get bash 4.
The push
function also isn't aware of the arches. But since we don't append the -$arch
to the tag for x86_64
and only use push
for the gliderlabs org this is probably fine.
I think we want to enable arm for all the library versions that support it. I'm not sure if @tianon had other plans for the Alpine arm variants. But if the builder and build script now support them here, we can just host them here.
So, other than the globbing issue for commiting the tarballs from CI, this looks ready for merge.
Edit: some minor linting issues as well (master branch is currently green in shellcheck
so maybe give the changes a run through it to see).
build
Outdated
for arch in "${ARCHS[@]}"; do | ||
# Generate rootfs | ||
mkdir -p "$version_dir/$arch" | ||
docker run -e "TRACE=$TRACE" --rm "$BUILDER_IMAGE" -a $arch "${BUILD_OPTIONS[@]}" \ |
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.
$arch
should probably be double quoted here.
build
Outdated
convert() { | ||
# takes a space-separated list of alpine-linux architectures, and returns a space-separated list of docker architectures | ||
local i=0 | ||
for arch in $@; do |
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.
I'm guessing $@
here should be double quoted as well.
build
Outdated
echo "${tag#*:}:" \ | ||
"git://github.com/gliderlabs/docker-alpine@${refs:0:40}" \ | ||
"versions/library-${RELEASE#v}" | ||
tags=${TAGS[@]} |
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.
Instead of assigning a string to an array, you can just expand the array with tags="${TAGS[*]}"
.
build
Outdated
[[ "$ARCHS" ]] || { # compatability with older configs that were only written for x86_64 architectures | ||
ARCHS=("x86_64") | ||
} | ||
a=$(convert ${ARCHS[@]}) |
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.
Double quotes around ${ARCHS[@]}
.
build
Outdated
} | ||
a=$(convert ${ARCHS[@]}) | ||
|
||
printf "\nTags:" && echo ${tags// /, } |
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.
Double quotes around the following 3 echoed variables as well.
Fixed the linting issues, added |
I think this supersedes #252, because this is more fleshed out and complete. It should allow us to add aarch64 (#190) or any other currently supported architecture in both docker and alpine. |
Hmm, it looks like the |
I think the best option is to loop over the |
1bf6b51
to
44c7ab5
Compare
That should fix the |
44c7ab5
to
c2585bf
Compare
This seems reasonable. I'll check it out and test tomorrow. |
build
Outdated
case "$arch" in # converts Alpine Linux arch strings to Docker arch strings | ||
x86_64) echo -n "amd64";; | ||
x86) echo -n "i386";; | ||
armhf) echo -n "arm32v7";; # TODO: What about arm32v6? |
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.
In Alpine, armhf
is arm32v6
, not v7 -- Alpine doesn't officially support v7 directly (only indirectly via v6, which is fine).
build
Outdated
} | ||
|
||
echo | ||
echo "# maintainer: Glider Labs <[email protected]> (@gliderlabs)" |
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.
To convert to the new file format, this line needs to be part of the initial "global" block, ala:
Maintainers: Glider Labs <[email protected]> (@gliderlabs)
GitRepo: https://github.com/gliderlabs/docker-alpine.git
build
Outdated
echo | ||
echo "Tags: ${tags// /, }" | ||
echo "Architectures: ${a// /, }" | ||
echo "GitCommit: ${refs:0:40}" |
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.
If these commits are not going to be on the master
branch, then each block will also need a GitFetch
line:
Tags: 3.6, latest
Architectures: foo, bar, baz
GitFetch: refs/heads/rootfs/library-3.6
GitCommit: c6cb2bbffbc92d4c35cdaf584dd1c43a860104ea
foo-Directory: versions/library-3.6/foo
...
Thanks for the review. All fixed now. |
Looks good. But CI is failing to build the armhf variant. Maybe this is just an older Docker issue (I think CircleCI is running 1.9)? I'll research this more. The same builds fine locally. |
Yeah, it also fails to build on play-with-docker.com, which is basically an alpine dev/demo environment. I've done some digging, and it seems that the What's really confusing is that they don't fail locally. I'll keep digging and update you guys on this. |
OK, after some more research it seems this was only really working because of https://docs.docker.com/docker-for-mac/multi-arch/. The trigger scripts for arm fail in normal Linux Docker environments. This... seems really strange to me... I'm not sure why Docker for Mac is built in such a way to encourage non-deterministic behavior like this. Frustrating... |
I see. The unlucky issue of us both using Macs. |
I'll take a look at the |
I think worst case is maybe I will just remove |
Yup, @andyshinn Probably a good idea while we figure out this issue. |
It seems that the I'll dig some more and try to get a fix tomorrow. |
The install scripts from the packages just fail. I've opened an upstream issue: https://bugs.alpinelinux.org/issues/7767 There are alternative methods to still implement multi-arch functionality, such as just automatically downloading the This method would limit customization, as we would have to extract, change stuff, then re-compress. Maybe that's a good thing for the official images, but would be harder for the gliderlabs variants. WDYT @andyshinn about alternative methods |
There is a The scripts that are failing (in my build at least) are:
Might be worth trying. If we were going to go the route of just storing the upstream tarballs I'd think about just rewriting the script to pull those down and commit them. I'm not sure if extracting, testing, and modifying them makes sense in that case. I'd assume that whatever testing of how they were built is the responsibility of upstream. |
I suspect we'll get an unusable system, because |
Well, just tried it out, and you can use |
Just pulling the tarballs and committing them without any changes would limit some of the customization options that we currently have. This would probably eliminate:
However, this might be a good thing if the goal is to closely mirror upstream without many changes. Of course, if these options are very important, we could just extract to a temporary working directory, make the changes as before, and then re-compress to the rootfs. I will start working on a version that just pulls down the tarballs. |
* Add -a flag for multiarch support in alpine-builder. * Add ARCHS variable for building different architectures. * Add an example of multi-arch * Update docs for multi-arch * Fix typo in library-edge options * Update library spec generator for multi-arch * Fix tagging of images for multi-arch * Fix style shellcheck issues, add comments * Fix rootfs commit glob issue by adding globstar option. * Update .gitignore for new multi-arch structure. * Rewrite commit loop based on options files, fix commit issues. * Fix library spec issues
See #237 and #304
Todo:
Last tasks