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

feat: docker build and tag from ci #6949

Merged
merged 15 commits into from
Mar 13, 2020
Merged

Conversation

olizilla
Copy link
Member

@olizilla olizilla commented Mar 4, 2020

  • build docker image in ci so we can see failures.
  • tag and push to dockerhub from ci so we have more control and visibility on the process.
  • use a script to decide what docket tag to apply given the current branch name or tag name.
  • automatically create docker tags with a build number and short git commit hash for each build from the feat/stabilize-dht branch on go-ipfs. e.g.
    • bifrost-1-a44c610c
    • bifrost-2-78347be9
    • bifrost-<build number>-<short git sha>
    • the build number lets us operators talk about "rolling back" and "rolling forward" through the tags
    • the commit sha means we all know exactly which version of go-ipfs we are talking about, and lets us find out how far behind the latest HEAD of the branch we are.

I've recreated the rest of the dockerhub autobuild rules we have currently, but it is worth taking a moment to review them as we may not want all of them, see:
Screenshot 2020-03-04 at 14 44 54
https://hub.docker.com/repository/docker/ipfs/go-ipfs/builds

Of note the alternative here is to keep using dockerhub for autobuilds, and just have a job to build the docker image in CI to verify it builds, and a job to do the custom bifrost tag... but the dockerhub config is not version controlled, and is a faff to change, so I went the whole hog and ported the lot. Very much up for discussion though.

Also of note, the current set up means the workflow we have called test is executed for all branches but not tags (which is how its always been). I've made it so that the docker-build step runs in parallel, but the docker-push where we tag and publish to dockerhub requires all the tests to pass, which is much stricter than the current dockerhub autobuild, which will run regardless of our test failures.

I've created a seperate workflow to trigger the docker steps when the repo is tagged, but it doesn't currently require any test jobs. It probably should, but I wanted to get a sanity check on what's here so far first.

This PR currently has the safety swtiches engaged in that it's currently configured to publish to an image on dockerhub called olizilla/test-go-ipfs rather than ipfs/go-ipfs for now, and the script to do the tag and push to docker is in dry-run mode, so it would just log out the tag and push step rather than do them.

TODO:

  • create dockerhub token
  • create circleci context called dockerhub scoped to the go-ipfs release github team
  • remove dryrun flag from call to push-docker-tags.sh
  • set IMAGE_NAME to be ipfs/go-ipfs
  • decide on test requirements for branch and tag builds before pushing to dockerhub
  • remove autobuilds from dockerhub

License: MIT
Signed-off-by: Oli Evans oli@tableflip.io

- build in docker in ci so we can see failures.
- tag and push to dockerhub from ci so we have more control and visibility on the process.

Note, docker workflows run on all branches and no tags by default. You need to opt in to having builds trigger when a git tag is pushed. The filter definition to opt in to tags needs to be present on your job and all dependendent jobs, which is dull. See https://circleci.com/docs/2.0/workflows/#executing-workflows-for-a-git-tag

I've recreated the dockerhub autobuild rules we have currently, but it is worthing taking a moment to review them.

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla olizilla requested a review from lidel March 4, 2020 14:58
@olizilla
Copy link
Member Author

olizilla commented Mar 4, 2020

This PR was based on ipfs-inactive/ci-websites#10 which was inspired by the handy article on exactly this topic https://circleci.com/blog/using-circleci-workflows-to-replicate-docker-hub-automated-builds/

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
- run:
name: Publish Docker Image to Docker Hub
command: |
echo "$DOCKERHUB_PASS" | docker login -u "$DOCKERHUB_USERNAME" --password-stdin
Copy link
Member

Choose a reason for hiding this comment

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

When are these credentials provided?

Copy link
Member

Choose a reason for hiding this comment

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

Every branch, it looks like? That's not a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

I keep running into this same wall. 🤦‍♂

Copy link
Member

Choose a reason for hiding this comment

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

Resolution: only users in a specific group will get this token. That will also be the group with release permissions.

@olizilla
Copy link
Member Author

olizilla commented Mar 5, 2020

@hsanjuan is the cluster-{\1} autobuild still needed?

@olizilla
Copy link
Member Author

olizilla commented Mar 5, 2020

I've discovered docker hub supports in repo hooks that allow us to add extra tags for a given auto build, and drafting a PR to try it out. https://docs.docker.com/docker-hub/builds/advanced/#override-build-test-or-push-commands

@hsanjuan
Copy link
Contributor

hsanjuan commented Mar 5, 2020

@hsanjuan is the cluster-{\1} autobuild still needed?

No

olizilla added a commit to olizilla/go-ipfs that referenced this pull request Mar 5, 2020
Dockerhub allows us to add scripts to a hooks directory in our repo, that allow us to alter the steps of an autobuild!

This PR adds a post_push dockerhub hook that adds additional tags for the stabilize-dht and master branch in the form `<branch>-<short date>-<short commit sha>`, e.g `bifrost-2020-02-26-a44c610` or `master-2020-03-01-18ca5ab`

This is an alternative to ipfs#6949 that requires no credentials in CI, but does not ensure our published docker tags are tested.

See ipfs#6949 for details

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
olizilla and others added 3 commits March 6, 2020 09:37
Thanks to @Mr0grog

Co-Authored-By: Rob Brackett <rob@robbrackett.com>
- use a circleci context to provide dockerhub credentials
- use a yaml alias to reduce repetition for branch filters (thanks @Mr0grog)
- limit docker-push step from test workflow to only run on relevant branches

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Member Author

Ok. I'm going to drag this version over the line. I'm going with date stamps rather than using circleci build numbers as a datestamp does the same job of telling you that a given image was created before or after another, and isn't tied to circleci...if we move ci in the future it would be dull to have the build number reset.

I'm going to test it out with my personal docker account, and the credentails will be added to a circle context with access limted to a specific admin github team https://github.com/orgs/ipfs/teams/admin/members

@olizilla
Copy link
Member Author

@Stebalien ...sanity check on our current docker builds in dockerhub, whatever gets pushed to the release branch gets tagged as release and latest on docker, and git tags that look like semver get tagged on docker with a copy of that git tag name.

How do you handle releases, in terms of git branches and git tags? do you merge to the release branch and tag from there? How so you manage patch releases? do you keep a version branch going so you can backport things from master?

...the release docker tag feels redundant, and it seems a little racey that it's set from a release branch rather than a pointer to a git tag.

I could fix it in this PR so that we have docker tags just for:

  1. semver docker tags for semver git tags
  2. latest as a pointer to the latest semver tag
  3. master-<date>-<commit> - for testground testing
  4. master-latest pointing to latest master - for testground canary testing
  5. bifrost-<date>-<commit> - for infra testing, until we can merge the streams.
  6. bifrost-latest pointing to latest stabilize-dht. - for infra canary testing

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Member Author

pushing from version tag is work! (tested with a tag on my fork of go-ipfs, and on my personal dockerhub profile)

Screenshot 2020-03-12 at 19 18 37

@Stebalien
Copy link
Member

I'm going to test it out with my personal docker account, and the credentails will be added to a circle context with access limted to a specific admin github team

Can we restrict this to some docker-release team? Or even a go-ipfs-release team?

@Stebalien
Copy link
Member

Stebalien commented Mar 12, 2020

How do you handle releases, in terms of git branches and git tags? do you merge to the release branch and tag from there? How so you manage patch releases? do you keep a version branch going so you can backport things from master?

To release, we currently:

  1. Tag on master.
  2. Merge the tag into release.

Same for patch releases.

For the next major release, we're probably going to have to perform some git magic to clobber the patch release changes.

...the release docker tag feels redundant, and it seems a little racey that it's set from a release branch rather than a pointer to a git tag.

I'm not actually sure why we do this. Possibly because "latest" isn't reliable? Latest also includes RCs, IIRC.

@whyrusleeping says "no reason really"

@Stebalien
Copy link
Member

I could fix it in this PR so that we have docker tags just for:

I'm fine with that as long as latest actually points to the latest release. But is there any way to make release an alias for latest?

@olizilla
Copy link
Member Author

Possibly because "latest" isn't reliable? Latest also includes RCs, IIRC.

With the current dockerhub autobuilds, the docker tags latest and release both point to whatever is the head of the release branch.

The proposal is to have the docker tag latest point to the same docker image as the most recent semver tag, and drop the release tag... aka, have none of our docker tags pointing to the head of the release branch.

@olizilla
Copy link
Member Author

olizilla commented Mar 13, 2020

@Stebalien in it's current form, the circleci config requires that all tests pass on master or the stabilize-dht branch for use to push a new master-<date>-<commit> docker tag, while semver tags only require that the docker-build step succeeds.

This may seem a little backwards... we may well want bleeding edge docker tags fo branches that dont always pass all tests, but we dont want to publish a semver release until all the tests are green... in its current form, it's left up to the releaser to decide when a commit is good enough to tag, and as long as it builds, there will be a corresponding docker tag, regardless of tests, which may be fine.

Do you have preferences here? I'm thinking we could reduce the requirement for branch builds. Where should we set the bar for those? I'm inclined to leave the tag builds as they are, at the discression of the releaser, on the assumption the are tagging from a branch that has passing tests already. If you decide to tag a release in spite of a failing test, it would be annoying to not automatically get a docker tag for that because of ci.

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Member Author

Branch tagging also works!
https://app.circleci.com/pipelines/github/olizilla/go-ipfs/20/workflows/cdd10c75-998f-48df-8009-80b0c4732371/jobs/152

Tagging ********/test-go-ipfs:bifrost-2020-03-13-a527a7a and pushing to dockerhub
The push refers to repository [docker.io/********/test-go-ipfs]

b3a71461: Preparing 
120503b3: Preparing 
5cbec343: Preparing 
c9d8d2e3: Preparing 
34e54786: Preparing 
83601870: Preparing 
b220ea20: Preparing 
04fb0a52: Preparing 
63bdbaf4: Preparing 
9e45ad9e: Preparing 
eaf85be0: Preparing 
335f47a8: Preparing 
c115622f: Preparing 
bifrost-2020-03-13-a527a7a: digest: sha256:722b1f290829c45cd5a437ceece44fc38fb281984c34b09c7298fe62cf14eb34 size: 3241
Tagging ********/test-go-ipfs:bifrost-latest and pushing to dockerhub
The push refers to repository [docker.io/********/test-go-ipfs]

b3a71461: Preparing 
120503b3: Preparing 
5cbec343: Preparing 
c9d8d2e3: Preparing 
34e54786: Preparing 
83601870: Preparing 
b220ea20: Preparing 
04fb0a52: Preparing 
63bdbaf4: Preparing 
9e45ad9e: Preparing 
eaf85be0: Preparing 
335f47a8: Preparing 
c115622f: Preparing 
bifrost-latest: digest: sha256:722b1f290829c45cd5a437ceece44fc38fb281984c34b09c7298fe62cf14eb34 size: 3241

CircleCI received exit code 0

a wild bifrost-latest tag and a bifrost-2020-03-13-a527a7a tag appear! (on the test repo)

Screenshot 2020-03-13 at 15 42 18

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Member Author

The dockerhub context is set up on circleci and limited to the go-ipfs release github team. the image name is updated to use the real deal ipfs/go-ipfs.

To land this we just need to verify that we're happy with branch builds requiring all the tests to pass, while tags require none, but assume the tagger will make the call... then remove all the autobuilds from dockerhub and merge it.

@Stebalien
Copy link
Member

Do you have preferences here?

I'd say everything must always pass. If this becomes an issue, we can reduce the requirements (or just fix the issue).

@Stebalien Stebalien merged commit ba28580 into ipfs:master Mar 13, 2020
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

master-${BUILD_NUM}-${GIT_SHA1_SHORT} is extremely useful to have, thank you! 👍

olizilla added a commit to filecoin-project/lily that referenced this pull request Sep 22, 2020
Adds script and ci config to push to docker hub when
- the master or main branch changes
- semver like tags are pushed to the repo

Pushing a semver like tag pushes a docker image with the same tag. The docker latest tag points to the last semver version tag

Pushing changes to main or master gets you a docker tag of the form <branch name>-<date>-<truncated commit sha>
e.g main-2020-f435621. a <branch>-latest docker tag is provided as a convenince if you really want the bleeding edge. The branch name is included in the docker tag name to make that clearer.

This is a tweak on what I set up for go-ipfs in ipfs/kubo#6949

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
placer14 pushed a commit to filecoin-project/lily that referenced this pull request Sep 22, 2020
Adds script and ci config to push to docker hub when
- the master or main branch changes
- semver like tags are pushed to the repo

Pushing a semver like tag pushes a docker image with the same tag. The docker latest tag points to the last semver version tag

Pushing changes to main or master gets you a docker tag of the form <branch name>-<date>-<truncated commit sha>
e.g main-2020-f435621. a <branch>-latest docker tag is provided as a convenince if you really want the bleeding edge. The branch name is included in the docker tag name to make that clearer.

This is a tweak on what I set up for go-ipfs in ipfs/kubo#6949

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
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.

5 participants