From f45f364896198d240d9cab64a624db09090cb5da Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 23 Nov 2021 11:26:34 -0800 Subject: [PATCH 1/7] Add check to catch syanpse master process starting when workers are configured --- synapse/app/homeserver.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 7e09530ad23c..1f60311ac14b 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -357,6 +357,13 @@ def setup(config_options: List[str]) -> SynapseHomeServer: # generating config files and shouldn't try to continue. sys.exit(0) + if config.worker: + raise ConfigError( + "You have specified worker app/s in the config but are attempting to start a non-worker " + "instance. Please use `python -m synapse.app.generic_worker` instead." + ) + sys.exit(1) + events.USE_FROZEN_DICTS = config.server.use_frozen_dicts synapse.util.caches.TRACK_MEMORY_USAGE = config.caches.track_memory_usage From 12b86172f5b98e49c2f5be5772d5e065596b2328 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 23 Nov 2021 11:28:07 -0800 Subject: [PATCH 2/7] add test to verify that starting master process with worker config raises error --- tests/app/test_homeserver_start.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/app/test_homeserver_start.py diff --git a/tests/app/test_homeserver_start.py b/tests/app/test_homeserver_start.py new file mode 100644 index 000000000000..eab8454ac3af --- /dev/null +++ b/tests/app/test_homeserver_start.py @@ -0,0 +1,29 @@ +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import synapse.app.homeserver +from synapse.config._base import ConfigError + +from tests.config.utils import ConfigFileTestCase + + +class HomeserverAppStartTestCase(ConfigFileTestCase): + def test_wrong_start_caught(self): + # Generate a config with a worker_app + self.generate_config() + self.add_lines_to_config(["worker_app: test_worker_app"]) + + # Ensure that starting master process with worker config raises an exception + with self.assertRaises(ConfigError): + synapse.app.homeserver.setup(["-c", self.config_file]) From 34af093e6168b61b966a6174fe7c3cdf235a8cb3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 23 Nov 2021 13:33:55 -0800 Subject: [PATCH 3/7] newsfragment --- changelog.d/11416.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11416.misc diff --git a/changelog.d/11416.misc b/changelog.d/11416.misc new file mode 100644 index 000000000000..a5c3aeda83a9 --- /dev/null +++ b/changelog.d/11416.misc @@ -0,0 +1 @@ +Add a check to ensure that users cannot start the Synapse master process when `worker_app` is set. \ No newline at end of file From 90ee6771cf091e3e45d9275d634e716fe7563def Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 29 Nov 2021 10:15:03 -0800 Subject: [PATCH 4/7] specify config.worker.worker_app in check --- synapse/app/homeserver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 1f60311ac14b..dc2a7db3f3da 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -357,7 +357,7 @@ def setup(config_options: List[str]) -> SynapseHomeServer: # generating config files and shouldn't try to continue. sys.exit(0) - if config.worker: + if config.worker.worker_app: raise ConfigError( "You have specified worker app/s in the config but are attempting to start a non-worker " "instance. Please use `python -m synapse.app.generic_worker` instead." From 6a24428b560f84bb2e77495d9c05337dacc298a9 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 29 Nov 2021 10:15:26 -0800 Subject: [PATCH 5/7] update test --- tests/app/test_homeserver_start.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/app/test_homeserver_start.py b/tests/app/test_homeserver_start.py index eab8454ac3af..cbcada04517e 100644 --- a/tests/app/test_homeserver_start.py +++ b/tests/app/test_homeserver_start.py @@ -22,6 +22,8 @@ class HomeserverAppStartTestCase(ConfigFileTestCase): def test_wrong_start_caught(self): # Generate a config with a worker_app self.generate_config() + # 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"]) # Ensure that starting master process with worker config raises an exception From bcef4a61c58c1cfb2187573bf6efa7eccadfda44 Mon Sep 17 00:00:00 2001 From: Shay Date: Tue, 30 Nov 2021 09:20:48 -0800 Subject: [PATCH 6/7] report specific config option that triggered the error Co-authored-by: reivilibre --- synapse/app/homeserver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index dc2a7db3f3da..9508366729a0 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -359,7 +359,7 @@ def setup(config_options: List[str]) -> SynapseHomeServer: if config.worker.worker_app: raise ConfigError( - "You have specified worker app/s in the config but are attempting to start a non-worker " + "You have specified `worker_app` in the config but are attempting to start a non-worker " "instance. Please use `python -m synapse.app.generic_worker` instead." ) sys.exit(1) From 78356ed92afafddcc8da7531294d38b44f14a925 Mon Sep 17 00:00:00 2001 From: Shay Date: Tue, 30 Nov 2021 09:24:44 -0800 Subject: [PATCH 7/7] clarify error message Co-authored-by: reivilibre --- synapse/app/homeserver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 9508366729a0..579cf31351fa 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -360,7 +360,7 @@ def setup(config_options: List[str]) -> SynapseHomeServer: if config.worker.worker_app: raise ConfigError( "You have specified `worker_app` in the config but are attempting to start a non-worker " - "instance. Please use `python -m synapse.app.generic_worker` instead." + "instance. Please use `python -m synapse.app.generic_worker` instead (or remove the option if this is the main process)." ) sys.exit(1)