diff options
| -rw-r--r-- | changelog.d/15860.removal | 1 | ||||
| -rw-r--r-- | docs/upgrade.md | 15 | ||||
| -rw-r--r-- | docs/usage/configuration/config_documentation.md | 45 | ||||
| -rw-r--r-- | docs/workers.md | 3 | ||||
| -rw-r--r-- | synapse/config/workers.py | 50 | ||||
| -rw-r--r-- | tests/app/test_homeserver_start.py | 6 | ||||
| -rw-r--r-- | tests/config/test_workers.py | 27 |
7 files changed, 49 insertions, 98 deletions
diff --git a/changelog.d/15860.removal b/changelog.d/15860.removal new file mode 100644 index 0000000000..1993bf0299 --- /dev/null +++ b/changelog.d/15860.removal @@ -0,0 +1 @@ +Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options. diff --git a/docs/upgrade.md b/docs/upgrade.md index 384f4010b4..b94d13c4da 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -98,6 +98,21 @@ You will need Python 3.8 to run Synapse v1.88.0 (due out July 18th, 2023). If you use current versions of the Matrix.org-distributed Debian packages or Docker images, no action is required. +## Removal of `worker_replication_*` settings + +As mentioned previously in [Upgrading to v1.84.0](#upgrading-to-v1840), the following deprecated settings +are being removed in this release of Synapse: + +* [`worker_replication_host`](https://matrix-org.github.io/synapse/v1.86/usage/configuration/config_documentation.html#worker_replication_host) +* [`worker_replication_http_port`](https://matrix-org.github.io/synapse/v1.86/usage/configuration/config_documentation.html#worker_replication_http_port) +* [`worker_replication_http_tls`](https://matrix-org.github.io/synapse/v1.86/usage/configuration/config_documentation.html#worker_replication_http_tls) + +Please ensure that you have migrated to using `main` on your shared configuration's `instance_map` +(or create one if necessary). This is required if you have ***any*** workers at all; +administrators of single-process (monolith) installations don't need to do anything. + +For an illustrative example, please see [Upgrading to v1.84.0](#upgrading-to-v1840) below. + # Upgrading to v1.86.0 diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 89a92c4682..04e8390ffe 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -4107,51 +4107,6 @@ Example configuration: worker_name: generic_worker1 ``` --- -### `worker_replication_host` -*Deprecated as of version 1.84.0. Place `host` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* - -The HTTP replication endpoint that it should talk to on the main Synapse process. -The main Synapse process defines this with a `replication` resource in -[`listeners` option](#listeners). - -Example configuration: -```yaml -worker_replication_host: 127.0.0.1 -``` ---- -### `worker_replication_http_port` -*Deprecated as of version 1.84.0. Place `port` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* - -The HTTP replication port that it should talk to on the main Synapse process. -The main Synapse process defines this with a `replication` resource in -[`listeners` option](#listeners). - -Example configuration: -```yaml -worker_replication_http_port: 9093 -``` ---- -### `worker_replication_http_tls` -*Deprecated as of version 1.84.0. Place `tls` under `main` entry on the [`instance_map`](#instance_map) in your shared yaml configuration instead.* - -Whether TLS should be used for talking to the HTTP replication port on the main -Synapse process. -The main Synapse process defines this with the `tls` option on its [listener](#listeners) that -has the `replication` resource enabled. - -**Please note:** by default, it is not safe to expose replication ports to the -public Internet, even with TLS enabled. -See [`worker_replication_secret`](#worker_replication_secret). - -Defaults to `false`. - -*Added in Synapse 1.72.0.* - -Example configuration: -```yaml -worker_replication_http_tls: true -``` ---- ### `worker_listeners` A worker can handle HTTP requests. To do so, a `worker_listeners` option diff --git a/docs/workers.md b/docs/workers.md index 303e0f0e7a..03415c6eb3 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -145,9 +145,6 @@ In the config file for each worker, you must specify: with an `http` listener. * **Synapse 1.72 and older:** if handling the `^/_matrix/client/v3/keys/upload` endpoint, the HTTP URI for the main process (`worker_main_http_uri`). This config option is no longer required and is ignored when running Synapse 1.73 and newer. - * **Synapse 1.83 and older:** The HTTP replication endpoint that the worker should talk to on the main synapse process - ([`worker_replication_host`](usage/configuration/config_documentation.md#worker_replication_host) and - [`worker_replication_http_port`](usage/configuration/config_documentation.md#worker_replication_http_port)). If using Synapse 1.84 and newer, these are not needed if `main` is defined on the [shared configuration](#shared-configuration) `instance_map` For example: diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 0b9789160c..5c81eb5c67 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -41,11 +41,17 @@ Synapse version. Please use ``%s: name_of_worker`` instead. _MISSING_MAIN_PROCESS_INSTANCE_MAP_DATA = """ Missing data for a worker to connect to main process. Please include '%s' in the -`instance_map` declared in your shared yaml configuration, or optionally(as a deprecated -solution) in every worker's yaml as various `worker_replication_*` settings as defined -in workers documentation here: +`instance_map` declared in your shared yaml configuration as defined in configuration +documentation here: +`https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#instance_map` +""" + +WORKER_REPLICATION_SETTING_DEPRECATED_MESSAGE = """ +'%s' is no longer a supported worker setting, please place '%s' onto your shared +configuration under `main` inside the `instance_map`. See workers documentation here: `https://matrix-org.github.io/synapse/latest/workers.html#worker-configuration` """ + # This allows for a handy knob when it's time to change from 'master' to # something with less 'history' MAIN_PROCESS_INSTANCE_NAME = "master" @@ -237,22 +243,37 @@ class WorkerConfig(Config): ) # A map from instance name to host/port of their HTTP replication endpoint. - # Check if the main process is declared. Inject it into the map if it's not, - # based first on if a 'main' block is declared then on 'worker_replication_*' - # data. If both are available, default to instance_map. The main process - # itself doesn't need this data as it would never have to talk to itself. + # Check if the main process is declared. The main process itself doesn't need + # this data as it would never have to talk to itself. instance_map: Dict[str, Any] = config.get("instance_map", {}) if self.instance_name is not MAIN_PROCESS_INSTANCE_NAME: + # TODO: The next 3 condition blocks can be deleted after some time has + # passed and we're ready to stop checking for these settings. # The host used to connect to the main synapse main_host = config.get("worker_replication_host", None) + if main_host: + raise ConfigError( + WORKER_REPLICATION_SETTING_DEPRECATED_MESSAGE + % ("worker_replication_host", main_host) + ) # The port on the main synapse for HTTP replication endpoint main_port = config.get("worker_replication_http_port") + if main_port: + raise ConfigError( + WORKER_REPLICATION_SETTING_DEPRECATED_MESSAGE + % ("worker_replication_http_port", main_port) + ) # The tls mode on the main synapse for HTTP replication endpoint. # For backward compatibility this defaults to False. main_tls = config.get("worker_replication_http_tls", False) + if main_tls: + raise ConfigError( + WORKER_REPLICATION_SETTING_DEPRECATED_MESSAGE + % ("worker_replication_http_tls", main_tls) + ) # For now, accept 'main' in the instance_map, but the replication system # expects 'master', force that into being until it's changed later. @@ -262,22 +283,9 @@ class WorkerConfig(Config): ] del instance_map[MAIN_PROCESS_INSTANCE_MAP_NAME] - # This is the backwards compatibility bit that handles the - # worker_replication_* bits using setdefault() to not overwrite anything. - elif main_host is not None and main_port is not None: - instance_map.setdefault( - MAIN_PROCESS_INSTANCE_NAME, - { - "host": main_host, - "port": main_port, - "tls": main_tls, - }, - ) - else: # If we've gotten here, it means that the main process is not on the - # instance_map and that not enough worker_replication_* variables - # were declared in the worker's yaml. + # instance_map. raise ConfigError( _MISSING_MAIN_PROCESS_INSTANCE_MAP_DATA % MAIN_PROCESS_INSTANCE_MAP_NAME diff --git a/tests/app/test_homeserver_start.py b/tests/app/test_homeserver_start.py index cd117b7394..0201933b04 100644 --- a/tests/app/test_homeserver_start.py +++ b/tests/app/test_homeserver_start.py @@ -25,9 +25,9 @@ class HomeserverAppStartTestCase(ConfigFileTestCase): # Add a blank line as otherwise the next addition ends up on a line with a comment self.add_lines_to_config([" "]) self.add_lines_to_config(["worker_app: test_worker_app"]) - self.add_lines_to_config(["worker_replication_host: 127.0.0.1"]) - self.add_lines_to_config(["worker_replication_http_port: 0"]) - + self.add_lines_to_config(["worker_log_config: /data/logconfig.config"]) + self.add_lines_to_config(["instance_map:"]) + self.add_lines_to_config([" main:", " host: 127.0.0.1", " port: 1234"]) # Ensure that starting master process with worker config raises an exception with self.assertRaises(ConfigError): synapse.app.homeserver.setup(["-c", self.config_file]) diff --git a/tests/config/test_workers.py b/tests/config/test_workers.py index 086359fd71..2a643ae4f3 100644 --- a/tests/config/test_workers.py +++ b/tests/config/test_workers.py @@ -17,7 +17,7 @@ from unittest.mock import Mock from immutabledict import immutabledict from synapse.config import ConfigError -from synapse.config.workers import InstanceLocationConfig, WorkerConfig +from synapse.config.workers import WorkerConfig from tests.unittest import TestCase @@ -323,28 +323,3 @@ class WorkerDutyConfigTestCase(TestCase): ) self.assertTrue(worker2_config.should_notify_appservices) self.assertFalse(worker2_config.should_update_user_directory) - - def test_worker_instance_map_compat(self) -> None: - """ - Test that `worker_replication_*` settings are compatibly handled by - adding them to the instance map as a `main` entry. - """ - - worker1_config = self._make_worker_config( - worker_app="synapse.app.generic_worker", - worker_name="worker1", - extras={ - "notify_appservices_from_worker": "worker2", - "update_user_directory_from_worker": "worker1", - "worker_replication_host": "127.0.0.42", - "worker_replication_http_port": 1979, - }, - ) - self.assertEqual( - worker1_config.instance_map, - { - "master": InstanceLocationConfig( - host="127.0.0.42", port=1979, tls=False - ), - }, - ) |