Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

separate the image build and export phases #994

Merged
merged 2 commits into from
Jun 24, 2015

Conversation

rade
Copy link
Member

@rade rade commented Jun 23, 2015

That way the export can be just a single tar, which is more convenient.

@rade rade assigned dpw and rade and unassigned dpw Jun 23, 2015
@rade rade force-pushed the separate_image_build_and_export branch from 87ff8e0 to af0356d Compare June 23, 2015 18:59
@rade
Copy link
Member Author

rade commented Jun 23, 2015

@tomwilkie so this is breaking CircleCI because it looks like the docker version on the build VM is too old - multi-arg docker save got introduced in 1.3. Is this easily fixable?

@tomwilkie
Copy link
Contributor

We could potentially build in a container with a newer version of docker,
but it might not be fixable.

On Tuesday, 23 June 2015, Matthias Radestock [email protected]
wrote:

@tomwilkie https://github.com/tomwilkie so this is breaking CircleCI
https://circleci.com/gh/weaveworks/weave/1237 because it looks like the
docker version on the build VM is too old - multi-arg docker save got introduced
in 1.3
moby/moby@e64131d.
Is this easily fixable?


Reply to this email directly or view it on GitHub
#994 (comment).

@rade
Copy link
Member Author

rade commented Jun 23, 2015

where is that ancient version coming from? CircleCI? GCE?

@rade
Copy link
Member Author

rade commented Jun 23, 2015

isn't this using the weaveworks/weave-build image, and doesn't that contain a version of docker?

@rade
Copy link
Member Author

rade commented Jun 23, 2015

isn't this using the weaveworks/weave-build image, and doesn't that contain a version of docker?

I reckon that's the problem. weave-build's Dockerfile installs docker.io, not lxc-docker.

Am I on the right track?

@tomwilkie
Copy link
Contributor

Old version comes from circle.

We do build in the container, and we bind mount the docker socket in. But
we might be able to self host a newer docker.

On Tuesday, 23 June 2015, Matthias Radestock [email protected]
wrote:

isn't this using the weaveworks/weave-build image, and doesn't that
contain a version of docker?

I reckon that's the problem. weave-build's Dockerfile installs docker.io
https://github.com/weaveworks/weave/blob/master/build/Dockerfile#L3,
not lxc-docker.

Am I on the right track?


Reply to this email directly or view it on GitHub
#994 (comment).

@rade
Copy link
Member Author

rade commented Jun 23, 2015

Hmm. Allegedly CircleCI runs Docker 1.4.1. Which does support multi-arg save. Something isn't right here.

@rade
Copy link
Member Author

rade commented Jun 23, 2015

So I reckon the issue is what I said earlier. The multi-arg save gets rejected because the docker client inside the build container is too old a version, since it gets installed from the docker.io instead of lxc-docker package.

@rade
Copy link
Member Author

rade commented Jun 23, 2015

Note that we wouldn't be able to just switch to lxc-docker; we'd have to pin the version to 1.4.1 too, since otherwise the client won't be able to talk to the CircleCI-provided docker daemon.

That way the export can be just a single tar, which is more
convenient. And without introducing unnecessary image builds.
@rade rade force-pushed the separate_image_build_and_export branch from af0356d to 6dea9bd Compare June 24, 2015 02:00
to 1.3.1, the same version we use everywhere else, e.g. in weavexec.
@rade rade force-pushed the separate_image_build_and_export branch from 6dea9bd to bdda9d0 Compare June 24, 2015 02:25
@rade rade assigned dpw and unassigned rade Jun 24, 2015
@rade
Copy link
Member Author

rade commented Jun 24, 2015

Success!

I changed the weave-build image to include docker 1.3.1, which is what we use elsewhere, notably in the weaveexec image. It's a bit gross since it requires two apt-get -y update - the first to install apt-transport-https in order for apt to be able to access the docker.io repo. If anybody has a better idea, let me know.

@tomwilkie I had to Rebuild without the cache in CircleCI in order to force a rebuild of weave-build. Will this need to be done on a few other builds until we have circled through all the build VMs we have lying around? If so, is there a better way?

@rade
Copy link
Member Author

rade commented Jun 24, 2015

is there a better way

@tomwilkie says to "just press the button until it works".

@dpw dpw merged commit bdda9d0 into master Jun 24, 2015
dpw added a commit that referenced this pull request Jun 24, 2015
separate the image build and export phases
@rade rade deleted the separate_image_build_and_export branch June 24, 2015 14:43
@dpw
Copy link
Contributor

dpw commented Jun 26, 2015

I just noticed a surprising consequence of this change, although it seems benign:

I have a VM where I had previously done docker load for each of the three separate tar files. When I docker load the new unified tar file, I get:

The image weaveworks/weave:latest already exists, renaming the old one with ID 97543ea4be18 to empty string
The image weaveworks/weavedns:latest already exists, renaming the old one with ID e0da4ecffe48 to empty string
The image weaveworks/weaveexec:latest already exists, renaming the old one with ID a53e12301065 to empty string

These appear to be just warnings, and don't appear for subsequent loads of the single tar file. But I don't know what is special about this case to trigger them.

I can reproduce this by checking out 1.0 and building and loading separate tar files again, and then loading the single tar file again.

I expect these warnings are confined to docker load, so they are not serious. But it's still a bit of a mystery to me.

@bboreham
Copy link
Contributor

@rade
Copy link
Member Author

rade commented Jun 26, 2015

It's a new feature in Docker 1.7

@dpw
Copy link
Contributor

dpw commented Jun 26, 2015

Indeed it is: I get the same warnings when loading the single tar file after a build that changed things.

@rade rade modified the milestone: 1.1.0 Jul 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants