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 a cloudbuild configuration for building the docker image and propagating the new tag to a kubernetes manifest #92

Merged
merged 13 commits into from
Jun 5, 2024

Conversation

sjahl
Copy link
Collaborator

@sjahl sjahl commented Jun 3, 2024

No description provided.

@sjahl sjahl requested a review from rileyhgrant June 3, 2024 19:35
@sjahl sjahl self-assigned this Jun 3, 2024
@sjahl sjahl force-pushed the docker-cloudbuild branch from fccf0b7 to d005dcd Compare June 4, 2024 17:07
@sjahl
Copy link
Collaborator Author

sjahl commented Jun 5, 2024

We can ignore the cloudbuild failure here -- that was due to https://github.com/broadinstitute/gnomad-terraform/pull/141 not being merged yet (and the trigger was pointed at this branch instead of main)

@@ -136,6 +136,12 @@ If you would like to extend your deployment with things like an Ingress, additio

### Production Deployment

## Building the Docker image

For production, docker images are automatically built after a push to the main branch. Once the build succeeds, retrieve the image tag from the build in Cloud Build, and update update the deployment spec in the [gnomad-deployments](https://github.com/broadinstitute/gnoma-deployments) repository.
Copy link
Contributor

Choose a reason for hiding this comment

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

Very minor -- I think there's a typo here, should this link to "https://github.com/broadinstitute/gnomad-deployments"?

The current link is missing the "d" - "...gnoma-deployments"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yup, thanks, will fix

cloudbuild.yaml Outdated

- name: "gcr.io/cloud-builders/docker"
- name: 'gcr.io/kaniko-project/executor:latest'
Copy link
Contributor

Choose a reason for hiding this comment

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

Personal question out of curiosity -- what are the benefits and downsides of using latest here, as opposed to pinning to a specific version.

I ran into issues a while ago with GeniE once with Kaniko caching incorrectly that were seemingly quite old and would have been fixed if we were using latest, but on the other hand that same issue seemed to have been introduced as a regression, so a process that worked fine previously using latest could start spontaneously failing.

I have no issue with using latest, this comment is really just me being curious as to your thoughts on this front.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, the pros and cons are pretty much what you mentioned there. For build-time environment dependencies, I'm sometimes OK with the latest tag, whereas I probably wouldn't be happy with that for a production deployment where you need to know exactly what versions of things you're running.

In this case, i think it's a good idea to have used the latest tag during development, but now that we have a version that we know works, we can pin to that (this will also help keep a record of which kaniko built which exome results image).

@rileyhgrant
Copy link
Contributor

rileyhgrant commented Jun 5, 2024

Approved with one minor typo and one question -- also with caveat that the exome-results-browsers-build step is failing, but I defer to you and assume you know that and its doing that for a good reason.

Edit: oops saw your comment about being able to ignore the failures -- my bad on not reading that. Ignore the above mention of that.

@sjahl sjahl merged commit bdf5f2f into main Jun 5, 2024
5 checks passed
@sjahl sjahl deleted the docker-cloudbuild branch June 5, 2024 15:51
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