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

Push a master-tagged image #142

Merged
merged 4 commits into from
Oct 15, 2018
Merged

Push a master-tagged image #142

merged 4 commits into from
Oct 15, 2018

Conversation

JaimeLennox
Copy link
Contributor

@JaimeLennox JaimeLennox commented Oct 13, 2018

Push a new docker image with tag "master" for every new build on the
master branch. This gives users wishing to run with the latest
development changes an easy way to stay up to date.

Note: when a tag is created, Travis sets the $TRAVIS_BRANCH envvar to
the tag, rather than the branch, hence why we only use this variable
now.

Closes: #139.

@aschmolck
Copy link
Contributor

aschmolck commented Oct 13, 2018

Good idea! But now just doing make w/o passing VERSION expliciltly will misbehave. One way to fix would be for the makefile to check if the variable is unset and blow up. But a sane default that only gets overriden when necessary might work better. I was going to suggest:

# top of Makefile
VERSION?=$$(cat version)

But maybe using the branch name is a better default:

VERSION?=$$(git rev-parse --abbrev-ref HEAD)

I can't remember why we introduced the version file, but it's also something that we probably could get rid off (in favor of just querying the tag).

Finally, maybe we should nuke the smarkets HQ images at last with the next "proper" release or do a final release.

I also think there is a bit of a support risk with making it too easy for people to be on bleeding edge, so it might be better to give it a less enticing name (like unstable or bleeding-edge).

tags: true
condition: "$TRAVIS_TAG = $(cat version)"
all_branches: true
condition: "$TRAVIS_BRANCH = $(cat version) || $TRAVIS_BRANCH = master"
Copy link
Contributor

@aschmolck aschmolck Oct 13, 2018

Choose a reason for hiding this comment

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

Don't you want TRAVIS_TAG here somwhere? I don't get how we'd still get correctly v3.2.1 style tagged docker images with this (since what you docker push is always tagged as master, as far as I can tell).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the Travis docs (https://docs.travis-ci.com/user/environment-variables/#default-environment-variables):
TRAVIS_BRANCH: for builds triggered by a tag, this is the same as the name of the tag (TRAVIS_TAG).
So I think this should work as expected.

@JaimeLennox
Copy link
Contributor Author

Yeah, I was thinking of getting rid of smarketshq images as well. We can nuke it after the next release, I guess.

I think this tag should be called master - its different enough from the standard docker conventions that I don't think anyone choosing this would find it unexpected, and it's much clearer what it represents like this. Maybe we should only update the latest tag on new versions and not on master builds instead? And/or we could mention this in the README.

I'll add the default VERSION as the branch, I guess that makes sense to do.

@JaimeLennox
Copy link
Contributor Author

I added a stable tag, which corresponds to the latest versioned tag, and updated the README with a bit of info about this. What do you think?

Push a new docker image with tag "master" for every new build on the
master branch. This gives users wishing to run with the latest
development changes an easy way to stay up to date.

Note: when a tag is created, Travis sets the $TRAVIS_BRANCH envvar to
the tag, rather than the branch, hence why we only use this variable
now.
This will correspond to to tagged versions for Marge, so users will be
able to use the latest versioned update automatically.
Explain how we tag our images and the recommended approach.
Makefile Outdated
@@ -25,10 +27,15 @@ docker-push:
docker login; \
fi
docker tag smarkets/marge-bot:$$(cat version) smarkets/marge-bot:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

I think :latest should be :stable (meaning we can probably drop :stable, although I'm not opposed to it either). If you docker run smarkets/marge-bot, you should get a release version, IMO.

@aschmolck
Copy link
Contributor

I think the connotations of master differ by community; e.g. go's package mangement will install master, so that better be stable not bleeding edge. I'm fine w/ using it though, if it turns out to cause confusion to some users, we can change to unstable or whatever. I don't think :latest should equal :master though; I think :latest always ought to be the latest release version, if you want bleeding edge you should have to say so explicitly, rather than the other way around where you need to say you wan :stable.

By default, we want users to get a stable version. If they want the
latest updates, they can use the master tag instead.
@JaimeLennox
Copy link
Contributor Author

Ok, I'm happy for it to behave that way too. I've left the stable tag in as it's more explicit.

@aschmolck aschmolck merged commit 9ae1d40 into master Oct 15, 2018
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.

None yet

2 participants