Update docker.yml of GitHub Action, use docker/metadata-action #12573

henryclw · 2022-04-28T07:07:18Z

Update docker.yml of GitHub Action, use docker/metadata-action to generate docker image tags.

https://github.com/docker/metadata-action

Signed-off-by: Henry 97804910+henryclw@users.noreply.github.com

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file. The entry should:
- Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
- Use markdown where necessary, mostly for code blocks.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
Pull request includes a sign off
Code style is correct
(run the linters)

…erate docker image tags. https://github.com/docker/metadata-action

henryclw · 2022-04-28T07:13:56Z

@richvdh Maybe this is want you might want?

DMRobertson

Seems reasonable at first glance. I'm not sure how we'd test it (maybe for an RC?). Would like other input from the team.

One suggestion and one question:

DMRobertson · 2022-04-28T17:39:51Z

.github/workflows/docker.yml

-          echo "::set-output name=tag::$tag"
+        uses: docker/metadata-action@master
+        with:
+          images: ${{ secrets.DOCKER_HUB_USERNAME }}/synapse


We use matrixdotorg below (line +53) and that's presumably not necessarily secret. Would be nice to be consistent though; perhaps change that line too?

Thank you for your advice.
I've tried to resolve this in cab89bc, would you mind taking a look?

DMRobertson · 2022-04-28T17:40:59Z

.github/workflows/docker.yml

+            type=ref,event=branch
+            type=semver,pattern={{version}}
+            # set latest tag for master branch
+            type=raw,value=latest,enable=${{ github.ref == format('refs/heads/{0}', 'master') }}


Is there a reason we use format('refs/heads/{0}', 'master') instead of the string lilteral 'refs/heads/master'?

Maybe this will look better?

type=raw,value=latest,enable=${{ github.ref == "refs/heads/master" }}

Thank you for your advice.

I've tried to resolve this in 8d9b75e, would you mind taking a look?

Sorry, I found out that changing into string lilteral 'refs/heads/master' won't work, I've updated this back to the format as same as docker/metadata-action README
in 084bccd

The testing job that didn't worked with 'refs/heads/master' is https://github.com/henryclw/synapse-dummy-action/actions/runs/2266949247 and the testing job that worked with github.ref == format('refs/heads/{0}', 'master') is https://github.com/henryclw/synapse-dummy-action/actions/runs/2266951441

Maybe it's the double quotes that's making it fail? Could you try the string literal with single quotes?

Maybe it's the double quotes that's making it fail? Could you try the string literal with single quotes?

Yes it works. Nice, please refer to the following links for more information:

henryclw/synapse-dummy-action@170e7b3

https://github.com/henryclw/synapse-dummy-action/runs/6301952066

I've changed to using single quotes in bf204b9

richvdh · 2022-04-28T17:53:07Z

please could someone explain the purpose of this change?

DMRobertson · 2022-04-28T17:55:23Z

please could someone explain the purpose of this change?

I think this comes from your suggestion in #11810 :

synapse/.github/workflows/docker.yml

Lines 37 to 38 in 2d327d2

    
           # TODO: consider using https://github.com/docker/metadata-action instead of this 
        
           # custom magic

Resolve matrix-org#12573 (comment)

matrix-org#12573 (comment)

henryclw · 2022-04-28T23:37:19Z

please could someone explain the purpose of this change?

I think this comes from your suggestion in #11810 :

synapse/.github/workflows/docker.yml

Lines 37 to 38 in 2d327d2

# TODO: consider using https://github.com/docker/metadata-action instead of this

# custom magic

Yeah, this is want I mean, I saw this todo

henryclw · 2022-04-28T23:40:02Z

But I think this is a little bit dangerous, is there a nice way to check the output instead of directly deploying this github action?

squahtx

Thanks for taking this change on!

To test this change, would you be willing to set up a dummy public repo, with a workflow that prints the output of both versions of "Calculate docker image tag" (the shell script and docker/metadata-action), and push to { develop, master, main } / tag { v1.57.0, v1.57.0rc1 }?
We'd like to make sure that both versions of the step produce the same output.

These review comments are mostly speculative, since I haven't been able to test the workflow:

.github/workflows/docker.yml

changelog.d/12573.docker

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>

matrix-org#12573 (comment)

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>

https://github.com/matrix-org/synapse/pull/12573/files#r862032542

Refer to https://github.com/docker/metadata-action/blob/b2391d37b4157fa4aa2e118d643f417910ff3242/README.md?plain=1#L638-L642

henryclw · 2022-05-04T00:25:37Z

Thanks for taking this change on!

To test this change, would you be willing to set up a dummy public repo, with a workflow that prints the output of both versions of "Calculate docker image tag" (the shell script and docker/metadata-action), and push to { develop, master, main } / tag { v1.57.0, v1.57.0rc1 }? We'd like to make sure that both versions of the step produce the same output.

These review comments are mostly speculative, since I haven't been able to test the workflow:

I've done the testing, the CI code is this one:

https://github.com/henryclw/synapse-dummy-action/blob/5b8d3ed7b997cab46b28d8cb20bd208481377f6a/.github/workflows/docker.yml#L1-L70

the results are as the followings:

For me, they seems to be as expected, but do please review.

Feel free to contact me if you have any feedback, thank you for your time.

squahtx · 2022-05-04T10:30:41Z

Great work getting it tested! The results look good.

I've left one last question about the format(...) vs string literal. Otherwise it looks good, thank you!

matrix-org/synapse#12573 (comment)

matrix-org#12573 (comment)

…n_docker

henryclw · 2022-05-05T07:24:10Z

Great work getting it tested! The results look good.

I've left one last question about the format(...) vs string literal. Otherwise it looks good, thank you!

I've changed to using single quotes in bf204b9, and the testing result is as the #12573 (comment) shows

squahtx

Looks good! Thanks for taking this on and for the exhaustive testing.

henryclw · 2022-05-05T18:15:19Z

Thank you for all the works. Please keep an eye on the docker build push behavior just to make sure things work as expected. If there's any problem, feel free to contact me.

@SimonBrandner

Synapse 1.59.0rc1 (2022-05-10) ============================== This release makes several changes that server administrators should be aware of: - Device name lookup over federation is now disabled by default. ([\#12616](#12616)) - The `synapse.app.appservice` and `synapse.app.user_dir` worker application types are now deprecated. ([\#12452](#12452), [\#12654](#12654)) See [the upgrade notes](https://github.com/matrix-org/synapse/blob/develop/docs/upgrade.md#upgrading-to-v1590) for more details. Additionally, this release removes the non-standard `m.login.jwt` login type from Synapse. It can be replaced with `org.matrix.login.jwt` for identical behaviour. This is only used if `jwt_config.enabled` is set to `true` in the configuration. ([\#12597](#12597)) Features -------- - Support [MSC3266](matrix-org/matrix-spec-proposals#3266) room summaries over federation. ([\#11507](#11507)) - Implement [changes](matrix-org/matrix-spec-proposals@4a77139) to [MSC2285 (hidden read receipts)](matrix-org/matrix-spec-proposals#2285). Contributed by @SimonBrandner. ([\#12168](#12168), [\#12635](#12635), [\#12636](#12636), [\#12670](#12670)) - Extend the [module API](https://github.com/matrix-org/synapse/blob/release-v1.59/synapse/module_api/__init__.py) to allow modules to change actions for existing push rules of local users. ([\#12406](#12406)) - Add the `notify_appservices_from_worker` configuration option (superseding `notify_appservices`) to allow a generic worker to be designated as the worker to send traffic to Application Services. ([\#12452](#12452)) - Add the `update_user_directory_from_worker` configuration option (superseding `update_user_directory`) to allow a generic worker to be designated as the worker to update the user directory. ([\#12654](#12654)) - Add new `enable_registration_token_3pid_bypass` configuration option to allow registrations via token as an alternative to verifying a 3pid. ([\#12526](#12526)) - Implement [MSC3786](matrix-org/matrix-spec-proposals#3786): Add a default push rule to ignore `m.room.server_acl` events. ([\#12601](#12601)) - Add new `mau_appservice_trial_days` configuration option to specify a different trial period for users registered via an appservice. ([\#12619](#12619)) Bugfixes -------- - Fix a bug introduced in Synapse 1.48.0 where the latest thread reply provided failed to include the proper bundled aggregations. ([\#12273](#12273)) - Fix a bug introduced in Synapse 1.22.0 where attempting to send a large amount of read receipts to an application service all at once would result in duplicate content and abnormally high memory usage. Contributed by Brad & Nick @ Beeper. ([\#12544](#12544)) - Fix a bug introduced in Synapse 1.57.0 which could cause `Failed to calculate hosts in room` errors to be logged for outbound federation. ([\#12570](#12570)) - Fix a long-standing bug where status codes would almost always get logged as `200!`, irrespective of the actual status code, when clients disconnect before a request has finished processing. ([\#12580](#12580)) - Fix race when persisting an event and deleting a room that could lead to outbound federation breaking. ([\#12594](#12594)) - Fix a bug introduced in Synapse 1.53.0 where bundled aggregations for annotations/edits were incorrectly calculated. ([\#12633](#12633)) - Fix a long-standing bug where rooms containing power levels with string values could not be upgraded. ([\#12657](#12657)) - Prevent memory leak from reoccurring when presence is disabled. ([\#12656](#12656)) Updates to the Docker image --------------------------- - Explicitly opt-in to using [BuildKit-specific features](https://github.com/moby/buildkit/blob/master/frontend/dockerfile/docs/syntax.md) in the Dockerfile. This fixes issues with building images in some GitLab CI environments. ([\#12541](#12541)) - Update the "Build docker images" GitHub Actions workflow to use `docker/metadata-action` to generate docker image tags, instead of a custom shell script. Contributed by @henryclw. ([\#12573](#12573)) Improved Documentation ---------------------- - Update SQL statements and replace use of old table `user_stats_historical` in docs for Synapse Admins. ([\#12536](#12536)) - Add missing linebreak to `pipx` install instructions. ([\#12579](#12579)) - Add information about the TCP replication module to docs. ([\#12621](#12621)) - Fixes to the formatting of `README.rst`. ([\#12627](#12627)) - Fix docs on how to run specific Complement tests using the `complement.sh` test runner. ([\#12664](#12664)) Deprecations and Removals ------------------------- - Remove unstable identifiers from [MSC3069](matrix-org/matrix-spec-proposals#3069). ([\#12596](#12596)) - Remove the unspecified `m.login.jwt` login type and the unstable `uk.half-shot.msc2778.login.application_service` from [MSC2778](matrix-org/matrix-spec-proposals#2778). ([\#12597](#12597)) - Synapse now requires at least Python 3.7.1 (up from 3.7.0), for compatibility with the latest Twisted trunk. ([\#12613](#12613)) Internal Changes ---------------- - Use supervisord to supervise Postgres and Caddy in the Complement image to reduce restart time. ([\#12480](#12480)) - Immediately retry any requests that have backed off when a server comes back online. ([\#12500](#12500)) - Use `make_awaitable` instead of `defer.succeed` for return values of mocks in tests. ([\#12505](#12505)) - Consistently check if an object is a `frozendict`. ([\#12564](#12564)) - Protect module callbacks with read semantics against cancellation. ([\#12568](#12568)) - Improve comments and error messages around access tokens. ([\#12577](#12577)) - Improve docstrings for the receipts store. ([\#12581](#12581)) - Use constants for read-receipts in tests. ([\#12582](#12582)) - Log status code of cancelled requests as 499 and avoid logging stack traces for them. ([\#12587](#12587), [\#12663](#12663)) - Remove special-case for `twisted` logger from default log config. ([\#12589](#12589)) - Use `getClientAddress` instead of the deprecated `getClientIP`. ([\#12599](#12599)) - Add link to documentation in Grafana Dashboard. ([\#12602](#12602)) - Reduce log spam when running multiple event persisters. ([\#12610](#12610)) - Add extra debug logging to federation sender. ([\#12614](#12614)) - Prevent remote homeservers from requesting local user device names by default. ([\#12616](#12616)) - Add a consistency check on events which we read from the database. ([\#12620](#12620)) - Remove use of the `constantly` library and switch to enums for `EventRedactBehaviour`. Contributed by @andrewdoh. ([\#12624](#12624)) - Remove unused code related to receipts. ([\#12632](#12632)) - Minor improvements to the scripts for running Synapse in worker mode under Complement. ([\#12637](#12637)) - Move `pympler` back in to the `all` extras. ([\#12652](#12652)) - Fix spelling of `M_UNRECOGNIZED` in comments. ([\#12665](#12665)) - Release script: confirm the commit to be tagged before tagging. ([\#12556](#12556)) - Fix a typo in the announcement text generated by the Synapse release development script. ([\#12612](#12612)) - Fix scripts-dev to pass typechecking. ([\#12356](#12356)) - Add some type hints to datastore. ([\#12485](#12485)) - Remove unused `# type: ignore`s. ([\#12531](#12531)) - Allow unused `# type: ignore` comments in bleeding edge CI jobs. ([\#12576](#12576)) - Remove redundant lines of config from `mypy.ini`. ([\#12608](#12608)) - Update to mypy 0.950. ([\#12650](#12650)) - Use `Concatenate` to better annotate `_do_execute`. ([\#12666](#12666)) - Use `ParamSpec` to refine type hints. ([\#12667](#12667)) - Fix mypy against latest pillow stubs. ([\#12671](#12671))

henryclw added 2 commits April 27, 2022 23:51

Update docker.yml of GitHub Action, use docker/metadata-action to gen…

75cfe1b

…erate docker image tags. https://github.com/docker/metadata-action

Create changelog.d/12573.docker

9febe54

henryclw requested a review from a team as a code owner April 28, 2022 07:07

DMRobertson self-assigned this Apr 28, 2022

DMRobertson reviewed Apr 28, 2022

View reviewed changes

DMRobertson requested a review from a team April 28, 2022 17:44

DMRobertson removed their assignment Apr 28, 2022

henryclw added 2 commits April 28, 2022 16:34

Use matrixdotorg/synapse directly.

cab89bc

Resolve matrix-org#12573 (comment)

Use refs/heads/master directly in docker.yml

8d9b75e

matrix-org#12573 (comment)

Merge branch 'develop' into henry/github_action_docker

17e3c2b

squahtx reviewed Apr 29, 2022

View reviewed changes

squahtx self-assigned this Apr 29, 2022

henryclw and others added 4 commits April 29, 2022 10:27

Update .github/workflows/docker.yml, use tags rather than tag

25bbd58

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>

Update .github/workflows/docker.yml, update tag version format

0a3bfc6

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>

Update docker.yml, add flavor latest=false

f427bad

matrix-org#12573 (comment)

Update changelog.d/12573.docker

4853548

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>

squahtx added the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label May 3, 2022

henryclw added 3 commits May 3, 2022 16:10

Update docker.yml, tag develop and latest for different branch

ce28168

https://github.com/matrix-org/synapse/pull/12573/files#r862032542

Merge branch 'matrix-org:develop' into henry/github_action_docker

57f4421

Use github.ref format in docker.yml

084bccd

Refer to https://github.com/docker/metadata-action/blob/b2391d37b4157fa4aa2e118d643f417910ff3242/README.md?plain=1#L638-L642

henryclw added a commit to henryclw/synapse-dummy-action that referenced this pull request May 5, 2022

Test using string literal with single quotes

170e7b3

matrix-org/synapse#12573 (comment)

henryclw added 2 commits May 5, 2022 00:20

Using string literal with single quotes

bf204b9

matrix-org#12573 (comment)

Merge remote-tracking branch 'origin/develop' into henry/github_actio…

769a100

…n_docker

squahtx approved these changes May 5, 2022

View reviewed changes

squahtx enabled auto-merge (squash) May 5, 2022 12:16

squahtx merged commit b8fa24b into matrix-org:develop May 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update docker.yml of GitHub Action, use docker/metadata-action #12573

Update docker.yml of GitHub Action, use docker/metadata-action #12573

henryclw commented Apr 28, 2022 •

edited

Loading

henryclw commented Apr 28, 2022

DMRobertson left a comment

DMRobertson Apr 28, 2022

henryclw Apr 28, 2022

DMRobertson Apr 28, 2022

henryclw Apr 28, 2022

henryclw Apr 28, 2022

henryclw May 3, 2022

squahtx May 4, 2022

henryclw May 5, 2022

henryclw May 5, 2022

richvdh commented Apr 28, 2022

DMRobertson commented Apr 28, 2022

henryclw commented Apr 28, 2022

henryclw commented Apr 28, 2022

squahtx left a comment •

edited

Loading

henryclw commented May 4, 2022

squahtx commented May 4, 2022 •

edited

Loading

henryclw commented May 5, 2022

squahtx left a comment

henryclw commented May 5, 2022

Update docker.yml of GitHub Action, use docker/metadata-action #12573

Update docker.yml of GitHub Action, use docker/metadata-action #12573

Conversation

henryclw commented Apr 28, 2022 • edited Loading

Pull Request Checklist

henryclw commented Apr 28, 2022

DMRobertson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh commented Apr 28, 2022

DMRobertson commented Apr 28, 2022

henryclw commented Apr 28, 2022

henryclw commented Apr 28, 2022

squahtx left a comment • edited Loading

Choose a reason for hiding this comment

henryclw commented May 4, 2022

squahtx commented May 4, 2022 • edited Loading

henryclw commented May 5, 2022

squahtx left a comment

Choose a reason for hiding this comment

henryclw commented May 5, 2022

henryclw commented Apr 28, 2022 •

edited

Loading

squahtx left a comment •

edited

Loading

squahtx commented May 4, 2022 •

edited

Loading