Merge the Complement testing Docker images into a single, multi-purpose image. #12881

reivilibre · 2022-05-26T11:31:57Z

Fixes #12847.
We now only have 3 images, but 1 of them is user-facing and untouchable, the next (Synapse-workers) is in use by maybe a few people for testing purposes and the last is this multi-purpose Complement image.

I don't mind this situation too much because they layer fairly well.

This multi-purpose image supports:

Monolith/SQLite (as before; our usual configuration in CI so far)
Monolith/Postgres (NEW! This would have caught some issues already; I'd like to enable it in CI right after this PR)
Workers/Postgres (Too slow to run in CI as-is; I have a PR for splitting tests among 4 jobs or I now have a new idea that should seriously cut down the startup time...)

I've tried to make this easier to review commit-by-commit, so you may find that nicer rather than the overall diff.

(It causes test errors!)

Signed-off-by: Olivier Wilkinson (reivilibre) <[email protected]>

richvdh

Please make sure that all these options are clearly documented in https://github.com/matrix-org/synapse/blob/develop/docker/README-testing.md, and that that file is generally updated to match the changes!

richvdh · 2022-05-30T11:49:21Z

docker/complement/Dockerfile

+#
+# Instructions for building this image from those it depends on is detailed in this guide:
+# https://github.com/matrix-org/synapse/blob/develop/docker/README-testing.md#testing-with-postgresql-and-single-or-multi-process-synapse
+FROM matrixdotorg/synapse-workers


the ability to run Complement against various versions of Synapse by changing the SYNAPSE_VERSION build param is something I've used from time to time to bisect where a problem landed, and it would be quite nice to retain that ability, presumably by parameterising docker/Dockerfile-workers.

Good point, thanks — I should be able to support that easily.

Out of interest: Does using git bisect/checking out the relevant version of the source tree and using complement.sh not do what you want?

sure it does, at the expense of building the entire Synapse image from scratch for each version ;)

Ah, you're running it with the pre-builts? Yeah, that sounds like it'd be good to support

I will note just in case you think it would be useful: we could revive #11852 to publish the synapse-worker images to GHCR if that would make this kind of investigation easier.

richvdh · 2022-05-30T11:53:55Z

docker/complement/Dockerfile


-# generate a signing key
-RUN generate_signing_key -o /conf/server.signing.key


do we not still need a signing key? or is that generated somewhere else?

This isn't something I've touched (although the overall diff will make that less than obvious) — presumably it generated a signing key before. That said, this one eluded me for a bit!

It's in start.py (called by configure_workers_and_start.py):

synapse/docker/start.py

Lines 115 to 126 in 7f92ac4

# Hopefully we already have a signing key, but generate one if not.

args = [

sys.executable,

"-m",

"synapse.app.homeserver",

"--config-path",

config_path,

# tell synapse to put generated keys in /data rather than /compiled

"--keys-directory",

config_dir,

"--generate-keys",

]

I think it is something you've changed... or at least, I don't understand why you're saying it's not.

Anyway, if start.py sorts it out, I guess that's fine.

I didn't write that file — it was already here: e4137c1 (#12881); the diff is just confusing because I deleted the monolith-only image which is what the red part of this diff (that your comment is on) corresponds to

docker/complement/Dockerfile

docker/Dockerfile-workers

docker/configure_workers_and_start.py

docker/complement/Dockerfile

But still retain the word 'Complement' so we know it's Complement-specific

richvdh

generally lgtm

docker/Dockerfile-workers

docker/README-testing.md

docker/complement/Dockerfile

richvdh · 2022-05-31T10:47:06Z

docker/complement/Dockerfile


-# generate a signing key
-RUN generate_signing_key -o /conf/server.signing.key


I think it is something you've changed... or at least, I don't understand why you're saying it's not.

Anyway, if start.py sorts it out, I guess that's fine.

docker/complement/Dockerfile

docker/complement/conf/start_for_complement.sh

Co-authored-by: Richard van der Hoff <[email protected]>

richvdh

🚢

…ther than appending the files

reivilibre · 2022-06-01T15:45:39Z

@richvdh Sorry — another commit for you. Seems like I need to disable the faster joins work when workers are used, or else I get errors.

docker/complement/conf/workers-shared-extra.yaml.j2

richvdh

So yeah, the fact that docker/configure_workers_and_start.py appends to any existing file is super confusing, but fixing that is maybe a job for another day.

LGTM other than the suggestions below.

docker/configure_workers_and_start.py

docker/complement/conf/workers-shared-extra.yaml.j2

Co-authored-by: Richard van der Hoff <[email protected]>

reivilibre · 2022-06-08T09:44:02Z

So yeah, the fact that docker/configure_workers_and_start.py appends to any existing file is super confusing

Yes, yes it is. It took me ages to spot that! :/

@Thumbscrew

Synapse 1.62.0 (2022-07-05) =========================== No significant changes since 1.62.0rc3. Authors of spam-checker plugins should consult the [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.62/docs/upgrade.md#upgrading-to-v1620) to learn about the enriched signatures for spam checker callbacks, which are supported with this release of Synapse. Synapse 1.62.0rc3 (2022-07-04) ============================== Bugfixes -------- - Update the version of the [ldap3 plugin](https://github.com/matrix-org/matrix-synapse-ldap3/) included in the `matrixdotorg/synapse` DockerHub images and the Debian packages hosted on `packages.matrix.org` to 0.2.1. This fixes [a bug](matrix-org/matrix-synapse-ldap3#163) with usernames containing uppercase characters. ([\matrix-org#13156](matrix-org#13156)) - Fix a bug introduced in Synapse 1.62.0rc1 affecting unread counts for users on small servers. ([\matrix-org#13168](matrix-org#13168)) Synapse 1.62.0rc2 (2022-07-01) ============================== Bugfixes -------- - Fix unread counts for users on large servers. Introduced in v1.62.0rc1. ([\matrix-org#13140](matrix-org#13140)) - Fix DB performance when deleting old push notifications. Introduced in v1.62.0rc1. ([\matrix-org#13141](matrix-org#13141)) Synapse 1.62.0rc1 (2022-06-28) ============================== Features -------- - Port the spam-checker API callbacks to a new, richer API. This is part of an ongoing change to let spam-checker modules inform users of the reason their event or operation is rejected. ([\matrix-org#12857](matrix-org#12857), [\matrix-org#13047](matrix-org#13047)) - Allow server admins to customise the response of the `/.well-known/matrix/client` endpoint. ([\matrix-org#13035](matrix-org#13035)) - Add metrics measuring the CPU and DB time spent in state resolution. ([\matrix-org#13036](matrix-org#13036)) - Speed up fetching of device list changes in `/sync` and `/keys/changes`. ([\matrix-org#13045](matrix-org#13045), [\matrix-org#13098](matrix-org#13098)) - Improve URL previews for sites which only provide Twitter Card metadata, e.g. LWN.net. ([\matrix-org#13056](matrix-org#13056)) Bugfixes -------- - Update [MSC3786](matrix-org/matrix-spec-proposals#3786) implementation to check `state_key`. ([\matrix-org#12939](matrix-org#12939)) - Fix a bug introduced in Synapse 1.58 where Synapse would not report full version information when installed from a git checkout. This is a best-effort affair and not guaranteed to be stable. ([\matrix-org#12973](matrix-org#12973)) - Fix a bug introduced in Synapse 1.60 where Synapse would fail to start if the `sqlite3` module was not available. ([\matrix-org#12979](matrix-org#12979)) - Fix a bug where non-standard information was required when requesting the `/hierarchy` API over federation. Introduced in Synapse v1.41.0. ([\matrix-org#12991](matrix-org#12991)) - Fix a long-standing bug which meant that rate limiting was not restrictive enough in some cases. ([\matrix-org#13018](matrix-org#13018)) - Fix a bug introduced in Synapse 1.58 where profile requests for a malformed user ID would ccause an internal error. Synapse now returns 400 Bad Request in this situation. ([\matrix-org#13041](matrix-org#13041)) - Fix some inconsistencies in the event authentication code. ([\matrix-org#13087](matrix-org#13087), [\matrix-org#13088](matrix-org#13088)) - Fix a long-standing bug where room directory requests would cause an internal server error if given a malformed room alias. ([\matrix-org#13106](matrix-org#13106)) Improved Documentation ---------------------- - Add documentation for how to configure Synapse with Workers using Docker Compose. Includes example worker config and docker-compose.yaml. Contributed by @Thumbscrew. ([\matrix-org#12737](matrix-org#12737)) - Ensure the [Poetry cheat sheet](https://matrix-org.github.io/synapse/develop/development/dependencies.html) is available in the online documentation. ([\matrix-org#13022](matrix-org#13022)) - Mention removed community/group worker endpoints in upgrade.md. Contributed by @olmari. ([\matrix-org#13023](matrix-org#13023)) - Add instructions for running Complement with `gotestfmt`-formatted output locally. ([\matrix-org#13073](matrix-org#13073)) - Update OpenTracing docs to reference the configuration manual rather than the configuration file. ([\matrix-org#13076](matrix-org#13076)) - Update information on downstream Debian packages. ([\matrix-org#13095](matrix-org#13095)) - Remove documentation for the Delete Group Admin API which no longer exists. ([\matrix-org#13112](matrix-org#13112)) Deprecations and Removals ------------------------- - Remove the unspecced `DELETE /directory/list/room/{roomId}` endpoint, which hid rooms from the [public room directory](https://spec.matrix.org/v1.3/client-server-api/#listing-rooms). Instead, `PUT` to the same URL with a visibility of `"private"`. ([\matrix-org#13123](matrix-org#13123)) Internal Changes ---------------- - Add tests for cancellation of `GET /rooms/$room_id/members` and `GET /rooms/$room_id/state` requests. ([\matrix-org#12674](matrix-org#12674)) - Report login failures due to unknown third party identifiers in the same way as failures due to invalid passwords. This prevents an attacker from using the error response to determine if the identifier exists. Contributed by Daniel Aloni. ([\matrix-org#12738](matrix-org#12738)) - Merge the Complement testing Docker images into a single, multi-purpose image. ([\matrix-org#12881](matrix-org#12881), [\matrix-org#13075](matrix-org#13075)) - Simplify the database schema for `event_edges`. ([\matrix-org#12893](matrix-org#12893)) - Clean up the test code for client disconnection. ([\matrix-org#12929](matrix-org#12929)) - Remove code generating comments in configuration. ([\matrix-org#12941](matrix-org#12941)) - Add `Cross-Origin-Resource-Policy: cross-origin` header to content repository's thumbnail and download endpoints. ([\matrix-org#12944](matrix-org#12944)) - Replace noop background updates with `DELETE` delta. ([\matrix-org#12954](matrix-org#12954), [\matrix-org#13050](matrix-org#13050)) - Use lower isolation level when inserting read receipts to avoid serialization errors. Contributed by Nick @ Beeper. ([\matrix-org#12957](matrix-org#12957)) - Reduce the amount of state we pull from the DB. ([\matrix-org#12963](matrix-org#12963)) - Enable testing against PostgreSQL databases in Complement CI. ([\matrix-org#12965](matrix-org#12965), [\matrix-org#13034](matrix-org#13034)) - Fix an inaccurate comment. ([\matrix-org#12969](matrix-org#12969)) - Remove the `delete_device` method and always call `delete_devices`. ([\matrix-org#12970](matrix-org#12970)) - Use a GitHub form for issues rather than a hard-to-read, easy-to-ignore template. ([\matrix-org#12982](matrix-org#12982)) - Move [MSC3715](matrix-org/matrix-spec-proposals#3715) behind an experimental config flag. ([\matrix-org#12984](matrix-org#12984)) - Add type hints to tests. ([\matrix-org#12985](matrix-org#12985), [\matrix-org#13099](matrix-org#13099)) - Refactor macaroon tokens generation and move the unsubscribe link in notification emails to `/_synapse/client/unsubscribe`. ([\matrix-org#12986](matrix-org#12986)) - Fix documentation for running complement tests. ([\matrix-org#12990](matrix-org#12990)) - Faster joins: add issue links to the TODO comments in the code. ([\matrix-org#13004](matrix-org#13004)) - Reduce DB usage of `/sync` when a large number of unread messages have recently been sent in a room. ([\matrix-org#13005](matrix-org#13005), [\matrix-org#13096](matrix-org#13096), [\matrix-org#13118](matrix-org#13118)) - Replaced usage of PyJWT with methods from Authlib in `org.matrix.login.jwt`. Contributed by Hannes Lerchl. ([\matrix-org#13011](matrix-org#13011)) - Modernize the `contrib/graph/` scripts. ([\matrix-org#13013](matrix-org#13013)) - Remove redundant `room_version` parameters from event auth functions. ([\matrix-org#13017](matrix-org#13017)) - Decouple `synapse.api.auth_blocking.AuthBlocking` from `synapse.api.auth.Auth`. ([\matrix-org#13021](matrix-org#13021)) - Add type annotations to `synapse.storage.databases.main.devices`. ([\matrix-org#13025](matrix-org#13025)) - Set default `sync_response_cache_duration` to two minutes. ([\matrix-org#13042](matrix-org#13042)) - Rename CI test runs. ([\matrix-org#13046](matrix-org#13046)) - Increase timeout of complement CI test runs. ([\matrix-org#13048](matrix-org#13048)) - Refactor entry points so that they all have a `main` function. ([\matrix-org#13052](matrix-org#13052)) - Refactor the Dockerfile-workers configuration script to use Jinja2 templates in Synapse workers' Supervisord blocks. ([\matrix-org#13054](matrix-org#13054)) - Add headers to individual options in config documentation to allow for linking. ([\matrix-org#13055](matrix-org#13055)) - Make Complement CI logs easier to read. ([\matrix-org#13057](matrix-org#13057), [\matrix-org#13058](matrix-org#13058), [\matrix-org#13069](matrix-org#13069)) - Don't instantiate modules with keyword arguments. ([\matrix-org#13060](matrix-org#13060)) - Fix type checking errors against Twisted trunk. ([\matrix-org#13061](matrix-org#13061)) - Allow MSC3030 `timestamp_to_event` calls from anyone on world-readable rooms. ([\matrix-org#13062](matrix-org#13062)) - Add a CI job to check that schema deltas are in the correct folder. ([\matrix-org#13063](matrix-org#13063)) - Avoid rechecking event auth rules which are independent of room state. ([\matrix-org#13065](matrix-org#13065)) - Reduce the duplication of code that invokes the rate limiter. ([\matrix-org#13070](matrix-org#13070)) - Add a Subject Alternative Name to the certificate generated for Complement tests. ([\matrix-org#13071](matrix-org#13071)) - Add more tests for room upgrades. ([\matrix-org#13074](matrix-org#13074)) - Pin dependencies maintained by matrix.org to [semantic version](https://semver.org/) bounds. ([\matrix-org#13082](matrix-org#13082)) - Correctly report prometheus DB stats for `get_earliest_token_for_stats`. ([\matrix-org#13085](matrix-org#13085)) - Fix a long-standing bug where a finished logging context would be re-started when Synapse failed to persist an event from federation. ([\matrix-org#13089](matrix-org#13089)) - Simplify the alias deletion logic as an application service. ([\matrix-org#13093](matrix-org#13093)) - Add type annotations to `tests.test_server`. ([\matrix-org#13124](matrix-org#13124))

reivilibre added 10 commits May 24, 2022 14:35

Mutiny: oust the monolith image and replace with the workers image

e4137c1

Update references in Dockerfile

865fa26

Update complement.sh to use the unified image

edc7fcd

Make it possible to disable Redis with an env var

15215bd

Require START_POSTGRES=1 to start Postgres

97699e3

Make the Complement Synapse launcher able to handle all 3 scenarios

9cd96d2

Don't enable Redis for the monolith case

3a588a0

(It causes test errors!)

Burn env vars into the image (sad and hopefully temporary)

7bc03a6

Newsfile

bf37bf3

Signed-off-by: Olivier Wilkinson (reivilibre) <[email protected]>

Bring back the experimental features

facccd0

reivilibre marked this pull request as ready for review May 26, 2022 13:43

reivilibre requested a review from a team as a code owner May 26, 2022 13:43

richvdh self-assigned this May 30, 2022

richvdh suggested changes May 30, 2022

View reviewed changes

richvdh removed their assignment May 30, 2022

reivilibre added 8 commits May 30, 2022 16:37

Apply suggestions from Rich

7e9884a

Remove mention of 'worker' from entrypoint

423e212

But still retain the word 'Complement' so we know it's Complement-specific

Pass the environment variables straight through, with no burning!

470fabe

Add back SYNAPSE_VERSION build arguments

b0241e7

Automatically disable Redis only for no-worker instances

3537762

Improve Complement documentation about 'Stuff' :)

8ae3da7

Update the contributing guide

33aeb67

Update README-testing.md

9f8d317

reivilibre requested a review from richvdh May 31, 2022 10:30

Fix sentence that dropped off

9531da6

richvdh reviewed May 31, 2022

View reviewed changes

reivilibre and others added 4 commits May 31, 2022 12:18

Update docker/README-testing.md

2beae8f

Co-authored-by: Richard van der Hoff <[email protected]>

Apply suggestions from code review

bc5d529

Co-authored-by: Richard van der Hoff <[email protected]>

Remove obsolete START_REDIS

166b046

Fix comment explaining the overwritten config file

4a739ca

reivilibre requested a review from richvdh May 31, 2022 11:25

richvdh approved these changes May 31, 2022

View reviewed changes

Convert workers-shared.yaml to Jinja2 and import the original file ra…

1af38ef

…ther than appending the files

reivilibre requested a review from richvdh June 1, 2022 15:45

richvdh reviewed Jun 1, 2022

View reviewed changes

docker/complement/conf/workers-shared-extra.yaml.j2 Show resolved Hide resolved

Add comment about how it fits in

6daac8a

reivilibre requested a review from richvdh June 6, 2022 11:29

Merge branch 'develop' into rei/one_complement_image

6c01955

richvdh approved these changes Jun 7, 2022

View reviewed changes

docker/configure_workers_and_start.py Outdated Show resolved Hide resolved

docker/complement/conf/workers-shared-extra.yaml.j2 Outdated Show resolved Hide resolved

reivilibre and others added 2 commits June 8, 2022 10:08

Update docker/configure_workers_and_start.py

2fd415b

Co-authored-by: Richard van der Hoff <[email protected]>

Move comment to top of file and rephrase slightly

b548132

reivilibre enabled auto-merge (squash) June 8, 2022 09:28

reivilibre merged commit 67f51c8 into develop Jun 8, 2022

reivilibre deleted the rei/one_complement_image branch June 8, 2022 09:57

reivilibre mentioned this pull request Jun 9, 2022

Fix CI by building the Synapse-workers image (needed to build the Synapse-Complement image) matrix-org/complement#391

Merged

richvdh mentioned this pull request Jun 15, 2022

Complement: use SQLite by default #13075

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge the Complement testing Docker images into a single, multi-purpose image. #12881

Merge the Complement testing Docker images into a single, multi-purpose image. #12881

reivilibre commented May 26, 2022 •

edited

Loading

richvdh left a comment

richvdh May 30, 2022

reivilibre May 30, 2022

richvdh May 30, 2022

reivilibre May 30, 2022

reivilibre May 30, 2022

richvdh May 30, 2022

reivilibre May 30, 2022

richvdh May 31, 2022

reivilibre May 31, 2022

richvdh left a comment

richvdh May 31, 2022

richvdh left a comment

reivilibre commented Jun 1, 2022

richvdh left a comment

reivilibre commented Jun 8, 2022


		# generate a signing key
		RUN generate_signing_key -o /conf/server.signing.key

	# Hopefully we already have a signing key, but generate one if not.
	args = [
	sys.executable,
	"-m",
	"synapse.app.homeserver",
	"--config-path",
	config_path,
	# tell synapse to put generated keys in /data rather than /compiled
	"--keys-directory",
	config_dir,
	"--generate-keys",
	]

Merge the Complement testing Docker images into a single, multi-purpose image. #12881

Merge the Complement testing Docker images into a single, multi-purpose image. #12881

Conversation

reivilibre commented May 26, 2022 • edited Loading

richvdh 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 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

reivilibre commented Jun 1, 2022

richvdh left a comment

Choose a reason for hiding this comment

reivilibre commented Jun 8, 2022

reivilibre commented May 26, 2022 •

edited

Loading