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

Switch to golang:1.8.4-alpine3.6 #633

Merged
merged 1 commit into from
Oct 27, 2017

Conversation

seemethere
Copy link
Contributor

@seemethere seemethere commented Oct 20, 2017

Signed-off-by: Eli Uriegas [email protected]

- What I did
golang:1.8.4-alpine does not have multi-arch images available in the
manifest. (s390x, ppc64le, etc.)

This makes it so that if you are trying to compile on different
arches you aren't forced to have to write your own Dockerfile and can
instead use the one bundled with the CLI repo.

- How I did it
Switch the images to golang:1.8.4-alpine3.6 and switch the package management commands to
apt-get

- How to verify it
make -f docker.Makefile binary

- Description for the changelog

Switched build image to allow for multi-arch compilation

- A picture of a cute animal (not mandatory but encouraged)
kayak

@seemethere
Copy link
Contributor Author

I understand this takes up more space on our machines but I think the benefit of being able to do multi-arch things more than makes up for some MBs

@thaJeztah
Copy link
Member

These are the "dev" and "lint" images; given that we don't do any multi-arch in this repository, wondering if this change should be in the packaging repo

@dnephin
Copy link
Contributor

dnephin commented Oct 20, 2017

The problem with non-alpine images isn't so much the space, more that downloading the base image and installing packages is much slower, which makes CI much slower. (not sure why CI hasn't run yet on this PR)

Can't we just wait for gliderlabs/docker-alpine#337 which will give us alpine multi-arch? It sounds like it's almost ready.

@dnephin
Copy link
Contributor

dnephin commented Oct 20, 2017

Alternatively, I think we could build our own mutli-arch golang alpine image and use that until the official one is ready.

@seemethere
Copy link
Contributor Author

@thaJeztah We already use golang:1.8.4 to build the images anyway when creating static binaries. This would make it easier so that we don't have to write mechanisms like:

	docker run --rm -i -e VERSION=$(VERSION) -e GITCOMMIT=$(GITCOMMIT) \
		-v $(CURDIR)/docker-ce/components/cli:/go/src/github.com/docker/cli \
		-w /go/src/github.com/docker/cli \
		golang:1.8.4 make binary

Instead we could just do a:

make -C docker-ce/components/cli -f docker.Makefile binary

@thaJeztah
Copy link
Member

@seemethere I agree we should eventually move to that; my concern was that that change would currently be a step back for the way the image is used in this repo; is there a real urgency to change it right now?

@seemethere
Copy link
Contributor Author

@thaJeztah Our workarounds seem to work for now but it'd be nice to eventually get multi-arch support in the CLI repo outside of the cross target which doesn't actually get used downstream.

@dnephin
Copy link
Contributor

dnephin commented Oct 23, 2017

Yes, once the multi-arch golang-alpine images are ready it should "just work" without us having to make any changes.

@thaJeztah
Copy link
Member

Alpine is multi arch now; gliderlabs/docker-alpine#352

@thaJeztah
Copy link
Member

Golang is almost there as well; docker-library/official-images#3624

@tophj-ibm
Copy link
Contributor

golang is there now too 🎉 Just tested with golang:1.8.4-alpine3.6 and that works.

docker manifest inspect golang:1.8.4-alpine3.6 | grep architecture
            "architecture": "amd64",
            "architecture": "arm",
            "architecture": "arm64",
            "architecture": "386",
            "architecture": "ppc64le",
            "architecture": "s390x",

@dnephin
Copy link
Contributor

dnephin commented Oct 26, 2017

Great, so I guess we just need to change the base image to golang:1.8.4-alpine3.6. @seemethere would you like to do that in this PR?

@seemethere
Copy link
Contributor Author

I will update the PR later today!!

@thaJeztah
Copy link
Member

@seemethere is there any change needed to the current version in this repo? Those should now automatically pick the right architecture, correct?

@thaJeztah
Copy link
Member

oh, never mind; we wanted to bump to alpine 3.6

golang:1.8.4-alpine does not have multi-arch images available in the
manifest. (s390x, ppc64le, etc.)

This makes it so that if you are trying to compile on different
arches you aren't forced to have to write your own Dockerfile and can
instead use the one bundled with the CLI repo.

Signed-off-by: Eli Uriegas <[email protected]>
@seemethere seemethere force-pushed the switch_to_multi_arch_images branch from 371f696 to 9d1d9f2 Compare October 26, 2017 18:42
@seemethere
Copy link
Contributor Author

Updated with a bump to golang:1.8.4-alpine3.6

@codecov-io
Copy link

codecov-io commented Oct 26, 2017

Codecov Report

Merging #633 into master will decrease coverage by 0.29%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master    #633     +/-   ##
========================================
- Coverage   49.69%   49.4%   -0.3%     
========================================
  Files         209     208      -1     
  Lines       17258   17190     -68     
========================================
- Hits         8577    8493     -84     
- Misses       8255    8264      +9     
- Partials      426     433      +7

@seemethere seemethere changed the title Switch to golang:1.8.4 Switch to golang:1.8.4-alpine3.6 Oct 26, 2017
@vieux
Copy link
Contributor

vieux commented Oct 26, 2017

@seemethere go1.8.5 was just released

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@vieux vieux left a comment

Choose a reason for hiding this comment

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

LGTM

@vieux vieux merged commit 50e1161 into docker:master Oct 27, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.11.0 milestone Oct 27, 2017
@seemethere seemethere deleted the switch_to_multi_arch_images branch October 27, 2017 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants