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

Add docker build/push/test scripts #2

Merged
merged 5 commits into from
Jul 26, 2024
Merged

Add docker build/push/test scripts #2

merged 5 commits into from
Jul 26, 2024

Conversation

richfitz
Copy link
Contributor

@richfitz richfitz commented Jul 26, 2024

This PR merges into #1, not main.

This PR sets up our normal docker approach. At some point I'd like to explore using github's container registry (gcr) because dockerhub is quite annoying to work with. A consequence of this annoyingness is that the namespace here is mrcide because that's the one we have a (fairly tenuous) pro account on.

The instructions as posted won't work as written - you'll need to use :jidea-46 in place of :latest in the pull and run, so:

docker pull mrcide/daedalus.api:jidea-46
docker run -d --rm -p 8001:8001 --name daedalus-api mrcide/daedalus.api:jidea-46
curl -s http://localhost:8001 | jq
docker stop daedalus-api

Buildkite should report back on build, and this is configured: https://buildkite.com/mrc-ide/daedalus-dot-api/settings/repository (see "Update commit statuses"). This is not happening, but I hope that once we get things onto main it will start, otherwise let's investigate. The webhooks are definitely making it. You can see build history here https://buildkite.com/mrc-ide/daedalus-dot-api (once you have logged in, invite has gone to your college address)

@richfitz richfitz marked this pull request as ready for review July 26, 2024 09:51
Copy link
Contributor

@pratikunterwegs pratikunterwegs left a comment

Choose a reason for hiding this comment

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

Thanks for adding - I'll update the Readme instructions in #1 once this is merged.

Just looking at Dockerhub, I see multiple tags for the image - one per commit and then one for the PR branch. Is it worth cutting down on the number of pushes to Dockerhub if our (over) use of it is becoming an issue?

@richfitz
Copy link
Contributor Author

Thanks for adding - I'll update the Readme instructions in #1 once this is merged.

This PR merges into your PR so will update the readme instructions, we can merge this into yours if you're happy, and you should not need to change them there. Then we merge your PR.

Just looking at Dockerhub, I see multiple tags for the image - one per commit and then one for the PR branch. Is it worth cutting down on the number of pushes to Dockerhub if our (over) use of it is becoming an issue?

This is what we do for everything; it's actually required for a multi-stage build to work in buildkite. Docker images are clever though and only layers that are changed are duplicated, even if the total size is large. So if you push 10x in one day and the build cache is not invalidated you'll have a few tiny different layers containing different copies of the code in this package, with all other layers being shared

@pratikunterwegs
Copy link
Contributor

Thanks for adding - I'll update the Readme instructions in #1 once this is merged.

This PR merges into your PR so will update the readme instructions, we can merge this into yours if you're happy, and you should not need to change them there. Then we merge your PR.

I was thinking docker pull mrcide/daedalus.api:latest should perhaps be daedalus.api:main? latest is just my own tag for the manual build - unless that's a valid and reserved tag on Dockerhub?

Just looking at Dockerhub, I see multiple tags for the image - one per commit and then one for the PR branch. Is it worth cutting down on the number of pushes to Dockerhub if our (over) use of it is becoming an issue?

This is what we do for everything; it's actually required for a multi-stage build to work in buildkite. Docker images are clever though and only layers that are changed are duplicated, even if the total size is large. So if you push 10x in one day and the build cache is not invalidated you'll have a few tiny different layers containing different copies of the code in this package, with all other layers being shared

Ah I see thanks!

@richfitz
Copy link
Contributor Author

This section of the push file will make sure you have a latest https://github.com/jameel-institute/daedalus.api/pull/2/files#diff-7b7ef1299c5b4369a024f7ac93bf4aabb66df4102e21e4600ca1cd4241ded1e8R12-R17 - latest is the default tag so we always want to produce one. I had this wrong up until 95eb863 because the place I'd copied it from was still using master!

@pratikunterwegs
Copy link
Contributor

Great, thanks for the pointer.

@pratikunterwegs pratikunterwegs merged commit bf52d34 into jidea-39 Jul 26, 2024
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.

2 participants