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

Publish docker images via GHA #1134

Merged
merged 20 commits into from
Sep 15, 2021
Merged

Publish docker images via GHA #1134

merged 20 commits into from
Sep 15, 2021

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Sep 9, 2021

I tested this by running on my repo with an access token for my dockerhub account.

This run shows what a successful workflow should look like.

To publish these images to matrixdotorg we'll need an access token secret in this repository. I've requsted that in vector-im/sre-internal#162.

David Robertson added 16 commits September 9, 2021 11:57
> The workflow is not valid. .github/workflows/docker.yml: Anchors are
not currently supported. Remove the anchor 'buildx
already exists at the org level in the primary sytest repo
interesting to know it's possible, but I don't understand what it's
doing and it hasn't saved any time. One for a rainy day.
@DMRobertson DMRobertson marked this pull request as ready for review September 9, 2021 12:50
@DMRobertson DMRobertson requested a review from a team September 9, 2021 12:53
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

This looks sensible to me (don't forget to remove dmr/publish-docker-images from the branch list before you merge ;p), but a couple of questions which probably want feedback from someone more opinionated.

docker/build.sh Outdated Show resolved Hide resolved
strategy:
matrix:
include:
- base_image: ubuntu:bionic
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this PR is the time for this or not, but @richvdh told me not to use bionic when I was asking about which image to use — because it's old.

Should we be bumping this to something like focal (20.04 LTS) (or hirsute [21.04] though probably not this one because non-LTS releases run out of life fairly quickly)?

Copy link
Member

Choose a reason for hiding this comment

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

I generally prefer debian to ubuntu for this sort of thing - it has a longer support lifetime than the non-LTS ubuntus, and is bettter maintained than the LTS ones

suggest debian:bullseye-slim.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do. (I was just trying to replicate the existing behaviour as faithfully as I could.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thoughts, I'll make that a second PR.

docker/README.md Outdated Show resolved Hide resolved
@DMRobertson DMRobertson requested review from a team, reivilibre and richvdh and removed request for a team September 10, 2021 11:20
docker/README.md Outdated Show resolved Hide resolved
@reivilibre
Copy link
Contributor

reivilibre commented Sep 14, 2021

LGTM otherwise (though we should get ops to put the secret in before merge) it's already there

Co-authored-by: reivilibre <38398653+reivilibre@users.noreply.github.com>
@DMRobertson DMRobertson merged commit 888762f into matrix-org:develop Sep 15, 2021
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