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

Automate docker image build and push to dockerhub #55

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

quarkytale
Copy link
Contributor

Adding Github action workflow to automatically upload mbari_wec docker image to mbari_wec docker repository. This would be triggered when a new tag on this repository is created.

  • Need help with adding secrets on this repo for this to work. Mainly dockerhun username and password, steps here.
  • The default base is nvidia_opengl_ubuntu22, is that what we want?
  • I haven't tested it yet, should we try the candidate release with this script? Or open to suggestions how I can test it

Signed-off-by: Dharini Dutia <[email protected]>
@quarkytale quarkytale requested review from mabelzhang and andermi June 13, 2023 22:31
@github-actions
Copy link

github-actions bot commented Jun 13, 2023

PR Preview Action v1.4.4
🚀 Deployed preview to https://osrf.github.io/mbari_wec/pr-preview/pr-55/
on branch gh-pages at 2023-09-25 18:52 UTC

Copy link
Collaborator

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

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

I think we should decouple this from the first release, because this is a nice-to-have. The first release has a timeline we need to do ASAP, whereas the GitHub Actions to automatically push the image doesn't have a time constraint.

We also said we are not going to do a candidate release, because the timeline is short now. So the first release will be the official 1.0. That image on DockerHub needs to work, so we should just push that manually.

Since we already expect updates to be made to the documentation as minor releases after the 1.0, this PR could be tested with similar minor releases, e.g. 1.0.x.

In fact, it is probably better to test this PR with dedicated prereleases, so that it isn't coupled with any actual releases. Some naming convention examples are here on Drupal and a blog post here. "Tags ending in -alpha, -beta, and -rc, followed by a number, are pre-releases. Read more about the distinctions between Alpha, Beta and RC."

In short, we could test the GitHub Action with 1.0.0-rc1, 1.0.0-rc2, etc., increment the last rc number until it works. @andermi?

I don't know about the credentials... whether it is safe for us to store it here. @andermi how is the other MBARI repo storing credentials to push? Is it in a private repo? That would be a difference...

.github/workflows/docker_build.yaml Outdated Show resolved Hide resolved
@mabelzhang
Copy link
Collaborator

The default base is nvidia_opengl_ubuntu22, is that what we want?

Yeah, I used that for the ICRA tutorial, and that has worked for people. If GitHub Actions is able to find that image in the directory and build it successfully, then it's a tested option.

@quarkytale
Copy link
Contributor Author

The idea is to have a latest tag and all previous git versions tags. Currently, a -rc suffix would be added by default, which should be removed after testing this workflow. Once the dockerhub login is sorted out, this can be tested.

We can also add a test job to test the image before pushing, see https://docs.docker.com/build/ci/github-actions/test-before-push/

@mabelzhang
Copy link
Collaborator

all previous git versions tags

That could get too many. With the foreseeable documentation patches, the changes could be very minor. I think it could be enough to only keep the latest tag of each major version. Open to other options.

@quarkytale
Copy link
Contributor Author

So I couldn't find how to delete or retag already uploaded images. But I can put up a check to only trigger the upload workflow on only major version changes.

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.

3 participants