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

Publish latest images from CI on successful builds. #1337

Merged
merged 1 commit into from
Aug 24, 2015

Conversation

tomwilkie
Copy link
Contributor

Fixes #1295

@rade
Copy link
Member

rade commented Aug 20, 2015

This does a lot less than make publish. Notably it doesn't tag the images with the github revision. As a result we will have no idea what code people are running.

Why does this not just invoke make publish?

@tomwilkie
Copy link
Contributor Author

This does a lot less than make publish. Notably it doesn't tag the images with the github revision.

The only thing it doesn't do is tag the image with the github revision. So not "a lot less".

As a result we will have no idea what code people are running.

We'll still know what people are running because the binaries have the git version encoded in them.

Why does this not just invoke make publish?

I didn't know about make publish.

How does tagging the image with the git ref help? If I pull weaveworks/weave:latest, I end up with an image tagged with latest (and not with the git ref). Docker doesn't seem to pull the other tags, and the docker hub is less than helpful: https://hub.docker.com/r/weaveworks/weave/tags/

@rade
Copy link
Member

rade commented Aug 20, 2015

Docker doesn't seem to pull the other tags

Good point. That means our weavexec version reporting won't work for snapshot releases. Which is annoying. File a bug.

The tag is still useful because it allows us to point people at specific revisions, e.g. VERSION=git-... weave setup. Plus we wouldn't want the CI publish to behave differently from a manual publish.

@tomwilkie
Copy link
Contributor Author

I also move the git-version-tagging to the docker build stage - if you'd done a commit between doing a make and a make publish you'd be tagging with the wrong version.

@rade
Copy link
Member

rade commented Aug 20, 2015

I also move the git-version-tagging to the docker build stage

Good idea, but needs a separate issue.

@@ -83,7 +83,7 @@ $(WEAVEWAIT_EXE) $(SIGPROXY_EXE) $(COVER_EXE) $(RUNNER_EXE):
go build -o $@ ./$(@D)

$(WEAVER_UPTODATE): prog/weaver/Dockerfile $(WEAVER_EXE)
$(SUDO) docker build -t $(WEAVER_IMAGE) prog/weaver
$(SUDO) docker build -t $(WEAVER_IMAGE) -t $(WEAVER_IMAGE):$(WEAVE_VERSION) prog/weaver

This comment was marked as abuse.

@rade
Copy link
Member

rade commented Aug 20, 2015

I also wonder whether the version tagging ends up creating a trail of garbage. i.e. when performing repeated builds, will all those images be left lying around? Though perhaps that is happening anyway.

@tomwilkie
Copy link
Contributor Author

I also wonder whether the version tagging ends up creating a trail of garbage. i.e. when performing repeated builds, will all those images be left lying around? Though perhaps that is happening anyway.

Kinda, yeah. When doing repeated builds on the same cset, it doesn't. But when you git pull or checkout a new one, it does - and they are not cleaned up by the rmi dangling.

vagrant@ubuntu-14:~/src/github.com/weaveworks/weave$ docker images
REPOSITORY             TAG                 IMAGE ID            CREATED             VIRTUAL SIZE
weaveworks/weaveexec   git-c17178e94447    adfb792e4158        2 seconds ago       55.29 MB
weaveworks/weaveexec   latest              adfb792e4158        2 seconds ago       55.29 MB
weaveworks/weaveexec   git-ca0aad04e7a6    2665f2eb1a1f        39 seconds ago      55.29 MB
<none>                 <none>              0d8316053677        3 minutes ago       55.29 MB
weaveworks/weave       latest              603be2f11ab8        4 minutes ago       17.27 MB
weaveworks/weave       git-c17178e94447    603be2f11ab8        4 minutes ago       17.27 MB
weaveworks/weave       git-ca0aad04e7a6    603be2f11ab8        4 minutes ago       17.27 MB
gliderlabs/alpine      latest              5bd56d818842        9 weeks ago         5.255 MB
vagrant@ubuntu-14:~/src/github.com/weaveworks/weave$ docker rmi $(docker images -q -f dangling=true)
Deleted: 0d8316053677e81394fd991ab64e1d58744e16de72ae24886f06af8dcfd70e89
Deleted: 0789a737e3b3c58514250a714afa83a71ff550dc659485a0479d1d701aacff66
Deleted: 2a8ba24d84452fa91e8f72886e1203499126d9763992fc991f5543984ef13d5a
Deleted: 28fe0ece1f9cbe4772f941c2478ead354f793da4468c2ca32367f20dc8b486bc
vagrant@ubuntu-14:~/src/github.com/weaveworks/weave$ docker images
REPOSITORY             TAG                 IMAGE ID            CREATED              VIRTUAL SIZE
weaveworks/weaveexec   git-c17178e94447    adfb792e4158        29 seconds ago       55.29 MB
weaveworks/weaveexec   latest              adfb792e4158        29 seconds ago       55.29 MB
weaveworks/weaveexec   git-ca0aad04e7a6    2665f2eb1a1f        About a minute ago   55.29 MB
weaveworks/weave       git-c17178e94447    603be2f11ab8        4 minutes ago        17.27 MB
weaveworks/weave       git-ca0aad04e7a6    603be2f11ab8        4 minutes ago        17.27 MB
weaveworks/weave       latest              603be2f11ab8        4 minutes ago        17.27 MB
gliderlabs/alpine      latest              5bd56d818842        9 weeks ago          5.255 MB

@rade
Copy link
Member

rade commented Aug 20, 2015

when you git pull or checkout a new one, it does - and they are not cleaned up by the rmi dangling.

That's not great. I think there's a better fix:

--- a/Makefile
+++ b/Makefile
@@ -105,7 +105,7 @@ $(DOCKER_DISTRIB):
 tests: $(COVER_EXE)
        @test/units.sh

-$(PUBLISH): publish_%:
+$(PUBLISH): publish_%: $(IMAGES_UPTODATE)
        $(SUDO) docker tag -f $(DOCKERHUB_USER)/$* $(DOCKERHUB_USER)/$*:$(WEAVE_VERSION)
        $(SUDO) docker push   $(DOCKERHUB_USER)/$*:$(WEAVE_VERSION)
        $(SUDO) docker push   $(DOCKERHUB_USER)/$*:latest

I think this does the right thing, namely ensure that images are up to date before publishing them. And some quick testing appears to confirm that. But the publish target is slightly magic, so I'd like @dpw to check this really does what we want.

@rade
Copy link
Member

rade commented Aug 21, 2015

I think this does the right thing

Having re-read the GNU make documentation, it does. So I have applied this change to master.

PR lgtm, but I don't know enough about circle, e.g. where do the credentials for the docker login come from? @paulbellamy pls review.

@tomwilkie
Copy link
Contributor Author

I've put the credentials in the circle environment manually.

paulbellamy added a commit that referenced this pull request Aug 24, 2015
Publish latest images from CI on successful builds.
@paulbellamy paulbellamy merged commit 5054805 into master Aug 24, 2015
@paulbellamy paulbellamy deleted the 1295-push-on-green branch August 24, 2015 09:58
errordeveloper added a commit that referenced this pull request Aug 24, 2015
@rade rade added this to the n/a milestone Aug 29, 2015
@rade rade modified the milestones: 1.2.0, n/a Oct 1, 2015
@rade rade modified the milestones: n/a, 1.2.0 Oct 20, 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.

3 participants