From 08f778543ef53b965d02df5caa3987e59297abb3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 10:35:23 -0400 Subject: [PATCH 01/21] Add support for named descriptors to ListenFDs --- src/twisted/python/systemd.py | 98 ++++++++--- src/twisted/python/test/strategies.py | 26 +++ src/twisted/python/test/test_systemd.py | 219 ++++++++++++------------ 3 files changed, 210 insertions(+), 133 deletions(-) create mode 100644 src/twisted/python/test/strategies.py diff --git a/src/twisted/python/systemd.py b/src/twisted/python/systemd.py index fe4f4a69467..5f98cb97ecb 100644 --- a/src/twisted/python/systemd.py +++ b/src/twisted/python/systemd.py @@ -13,9 +13,12 @@ __all__ = ["ListenFDs"] from os import getpid -from typing import Iterable, List, Mapping, Optional +from typing import Dict, List, Mapping, Optional, Sequence +from attrs import Factory, define + +@define class ListenFDs: """ L{ListenFDs} provides access to file descriptors inherited from systemd. @@ -30,16 +33,15 @@ class ListenFDs: @ivar _descriptors: A C{list} of C{int} giving the descriptors which were inherited. + + @ivar _names: A L{Sequence} of C{str} giving the names of the descriptors + which were inherited. """ - _START = 3 + _descriptors: Sequence[int] + _names: Sequence[str] = Factory(tuple) - def __init__(self, descriptors: List[int]) -> None: - """ - @param descriptors: The descriptors which will be returned from calls to - C{inheritedDescriptors}. - """ - self._descriptors = descriptors + _START = 3 @classmethod def fromEnvironment( @@ -68,26 +70,70 @@ def fromEnvironment( if start is None: start = cls._START - descriptors: List[int] = [] - - try: - pid = int(environ["LISTEN_PID"]) - except (KeyError, ValueError): - pass + if str(getpid()) == environ.get("LISTEN_PID"): + descriptors: List[int] = _parseDescriptors(start, environ) + names: Sequence[str] = _parseNames(environ) else: - if pid == getpid(): - try: - count = int(environ["LISTEN_FDS"]) - except (KeyError, ValueError): - pass - else: - descriptors = list(range(start, start + count)) - del environ["LISTEN_PID"], environ["LISTEN_FDS"] - - return cls(descriptors) - - def inheritedDescriptors(self) -> Iterable[int]: + descriptors = [] + names = () + + # They may both be missing (consistent with not running under systemd + # at all) or they may both be present (consistent with running under + # systemd 227 or newer). It is not allowed for only one to be present + # or for the values to disagree with each other. + if len(names) != len(descriptors): + return cls([], ()) + + return cls(descriptors, names) + + def inheritedDescriptors(self) -> List[int]: """ @return: The configured descriptors. """ return list(self._descriptors) + + def inheritedNamedDescriptors(self) -> Dict[str, int]: + """ + @return: A mapping from the names of configured descriptors to + their integer values. + """ + return dict(zip(self._names, self._descriptors)) + + +def _parseDescriptors(start: int, environ: Mapping[str, str]) -> List[int]: + """ + Parse the I{LISTEN_FDS} environment variable supplied by systemd. + + @param start: systemd provides only a count of the number of descriptors + that have been inherited. This is the integer value of the first + inherited descriptor. Subsequent inherited descriptors are numbered + counting up from here. See L{ListenFDs._START}. + + @param environ: The environment variable mapping in which to look for the + value to parse. + + @return: The integer values of the inherited file descriptors, in order. + """ + try: + count = int(environ["LISTEN_FDS"]) + except (KeyError, ValueError): + return [] + else: + descriptors = list(range(start, start + count)) + del environ["LISTEN_PID"], environ["LISTEN_FDS"] + return descriptors + + +def _parseNames(environ: Mapping[str, str]) -> Sequence[str]: + """ + Parse the I{LISTEN_FDNAMES} environment variable supplied by systemd. + + @param environ: The environment variable mapping in which to look for the + value to parse. + + @return: The names of the inherited descriptors, in order. + """ + names = environ.get("LISTEN_FDNAMES", "") + if len(names) > 0: + return tuple(names.split(":")) + return () diff --git a/src/twisted/python/test/strategies.py b/src/twisted/python/test/strategies.py new file mode 100644 index 00000000000..93c7d12fb38 --- /dev/null +++ b/src/twisted/python/test/strategies.py @@ -0,0 +1,26 @@ +from hypothesis.strategies import SearchStrategy, characters, text + + +def systemd_descriptor_names() -> SearchStrategy[str]: + """ + Build strings that are legal values for the systemd + I{FileDescriptorName} field. + """ + # systemd.socket(5) says: + # + # > Names may contain any ASCII character, but must exclude control + # > characters and ":", and must be at most 255 characters in length. + return text( + # The docs don't say there is a min size so I'm guessing... + min_size=1, + max_size=255, + alphabet=characters( + # These constraints restrict us to ASCII. + min_codepoint=0, + max_codepoint=127, + # This one excludes control characters. + blacklist_categories=("Cc",), + # And this excludes the separator. + blacklist_characters=(":",), + ), + ) diff --git a/src/twisted/python/test/test_systemd.py b/src/twisted/python/test/test_systemd.py index 597239abbca..0e60178f2f9 100644 --- a/src/twisted/python/test/test_systemd.py +++ b/src/twisted/python/test/test_systemd.py @@ -7,158 +7,163 @@ import os +from typing import Dict, Mapping, Sequence -from twisted.python.systemd import ListenFDs -from twisted.trial.unittest import TestCase - - -class InheritedDescriptorsMixin: - """ - Mixin for a L{TestCase} subclass which defines test methods for some kind of - systemd sd-daemon class. In particular, it defines tests for a - C{inheritedDescriptors} method. - """ - - def test_inheritedDescriptors(self): - """ - C{inheritedDescriptors} returns a list of integers giving the file - descriptors which were inherited from systemd. - """ - sddaemon = self.getDaemon(7, 3) - self.assertEqual([7, 8, 9], sddaemon.inheritedDescriptors()) +from hamcrest import assert_that, equal_to, not_ +from hypothesis import given +from hypothesis.strategies import dictionaries, integers, lists - def test_repeated(self): - """ - Any subsequent calls to C{inheritedDescriptors} return the same list. - """ - sddaemon = self.getDaemon(7, 3) - self.assertEqual( - sddaemon.inheritedDescriptors(), sddaemon.inheritedDescriptors() - ) +from twisted.python.systemd import ListenFDs +from twisted.trial.unittest import SynchronousTestCase +from .strategies import systemd_descriptor_names -class MemoryOnlyMixin: +def initializeEnvironment(count: int, pid: object) -> Dict[str, str]: """ - Mixin for a L{TestCase} subclass which creates creating a fake, in-memory - implementation of C{inheritedDescriptors}. This provides verification that - the fake behaves in a compatible way with the real implementation. + Create a copy of the process environment and add I{LISTEN_FDS} and + I{LISTEN_PID} (the environment variables set by systemd) to it. """ - - def getDaemon(self, start, count): - """ - Invent C{count} new I{file descriptors} (actually integers, attached to - no real file description), starting at C{start}. Construct and return a - new L{ListenFDs} which will claim those integers represent inherited - file descriptors. - """ - return ListenFDs(range(start, start + count)) + result = os.environ.copy() + result["LISTEN_FDS"] = str(count) + result["LISTEN_FDNAMES"] = ":".join([f"{n}.socket" for n in range(count)]) + result["LISTEN_PID"] = str(pid) + return result -class EnvironmentMixin: +class ListenFDsTests(SynchronousTestCase): """ - Mixin for a L{TestCase} subclass which creates a real implementation of - C{inheritedDescriptors} which is based on the environment variables set by - systemd. To facilitate testing, this mixin will also create a fake - environment dictionary and add keys to it to make it look as if some - descriptors have been inherited. + Apply tests to L{ListenFDs}, constructed based on an environment dictionary. """ - def initializeEnvironment(self, count, pid): - """ - Create a copy of the process environment and add I{LISTEN_FDS} and - I{LISTEN_PID} (the environment variables set by systemd) to it. - """ - result = os.environ.copy() - result["LISTEN_FDS"] = str(count) - result["LISTEN_PID"] = str(pid) - return result + @given(lists(systemd_descriptor_names(), min_size=0, max_size=10)) + def test_fromEnvironmentEquivalence(self, names: Sequence[str]) -> None: + """ + The L{ListenFDs} and L{ListenFDs.fromEnvironment} constructors are + equivalent for their respective representations of the same + information. + + @param names: The names of the file descriptors to represent as + inherited in the test environment given to the parser. The number + of descriptors represented will equal the length of this list. + """ + numFDs = len(names) + descriptors = list(range(ListenFDs._START, ListenFDs._START + numFDs)) + fds = ListenFDs.fromEnvironment( + { + "LISTEN_PID": str(os.getpid()), + "LISTEN_FDS": str(numFDs), + "LISTEN_FDNAMES": ":".join(names), + } + ) + assert_that(fds, equal_to(ListenFDs(descriptors, tuple(names)))) - def getDaemon(self, start, count): + def test_defaultEnviron(self) -> None: """ - Create a new L{ListenFDs} instance, initialized with a fake environment - dictionary which will be set up as systemd would have set it up if - C{count} descriptors were being inherited. The descriptors will also - start at C{start}. + If the process environment is not explicitly passed to + L{ListenFDs.fromEnvironment}, the real process environment dictionary + is used. """ - fakeEnvironment = self.initializeEnvironment(count, os.getpid()) - return ListenFDs.fromEnvironment(environ=fakeEnvironment, start=start) - - -class MemoryOnlyTests(MemoryOnlyMixin, InheritedDescriptorsMixin, TestCase): - """ - Apply tests to L{ListenFDs}, explicitly constructed with some fake file - descriptors. - """ - - -class EnvironmentTests(EnvironmentMixin, InheritedDescriptorsMixin, TestCase): - """ - Apply tests to L{ListenFDs}, constructed based on an environment dictionary. - """ + self.patch(os, "environ", initializeEnvironment(5, os.getpid())) + sddaemon = ListenFDs.fromEnvironment() + self.assertEqual(list(range(3, 3 + 5)), sddaemon.inheritedDescriptors()) - def test_secondEnvironment(self): + def test_secondEnvironment(self) -> None: """ - Only a single L{Environment} can extract inherited file descriptors. + L{ListenFDs.fromEnvironment} removes information about the inherited + file descriptors from the environment mapping. """ - fakeEnvironment = self.initializeEnvironment(3, os.getpid()) - first = ListenFDs.fromEnvironment(environ=fakeEnvironment) - second = ListenFDs.fromEnvironment(environ=fakeEnvironment) + env = initializeEnvironment(3, os.getpid()) + first = ListenFDs.fromEnvironment(environ=env) + second = ListenFDs.fromEnvironment(environ=env) self.assertEqual(list(range(3, 6)), first.inheritedDescriptors()) self.assertEqual([], second.inheritedDescriptors()) - def test_mismatchedPID(self): + def test_mismatchedPID(self) -> None: """ - If the current process PID does not match the PID in the environment, no - inherited descriptors are reported. + If the current process PID does not match the PID in the environment, + no inherited descriptors are reported. """ - fakeEnvironment = self.initializeEnvironment(3, os.getpid() + 1) - sddaemon = ListenFDs.fromEnvironment(environ=fakeEnvironment) + env = initializeEnvironment(3, os.getpid() + 1) + sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) - def test_missingPIDVariable(self): + def test_missingPIDVariable(self) -> None: """ If the I{LISTEN_PID} environment variable is not present, no inherited descriptors are reported. """ - fakeEnvironment = self.initializeEnvironment(3, os.getpid()) - del fakeEnvironment["LISTEN_PID"] - sddaemon = ListenFDs.fromEnvironment(environ=fakeEnvironment) + env = initializeEnvironment(3, os.getpid()) + del env["LISTEN_PID"] + sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) - def test_nonIntegerPIDVariable(self): + def test_nonIntegerPIDVariable(self) -> None: """ If the I{LISTEN_PID} environment variable is set to a string that cannot be parsed as an integer, no inherited descriptors are reported. """ - fakeEnvironment = self.initializeEnvironment(3, "hello, world") - sddaemon = ListenFDs.fromEnvironment(environ=fakeEnvironment) + env = initializeEnvironment(3, "hello, world") + sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) - def test_missingFDSVariable(self): + def test_missingFDSVariable(self) -> None: """ - If the I{LISTEN_FDS} environment variable is not present, no inherited - descriptors are reported. + If the I{LISTEN_FDS} and I{LISTEN_FDNAMES} environment variables + are not present, no inherited descriptors are reported. """ - fakeEnvironment = self.initializeEnvironment(3, os.getpid()) - del fakeEnvironment["LISTEN_FDS"] - sddaemon = ListenFDs.fromEnvironment(environ=fakeEnvironment) + env = initializeEnvironment(3, os.getpid()) + del env["LISTEN_FDS"] + del env["LISTEN_FDNAMES"] + sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) - def test_nonIntegerFDSVariable(self): + def test_nonIntegerFDSVariable(self) -> None: """ If the I{LISTEN_FDS} environment variable is set to a string that cannot be parsed as an integer, no inherited descriptors are reported. """ - fakeEnvironment = self.initializeEnvironment("hello, world", os.getpid()) - sddaemon = ListenFDs.fromEnvironment(environ=fakeEnvironment) + env = initializeEnvironment(3, os.getpid()) + env["LISTEN_FDS"] = "hello, world" + sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) - def test_defaultEnviron(self): + @given(lists(integers(min_value=0, max_value=10), unique=True)) + def test_inheritedDescriptors(self, descriptors: Sequence[int]) -> None: """ - If the process environment is not explicitly passed to - L{Environment.__init__}, the real process environment dictionary is - used. + L{ListenFDs.inheritedDescriptors} returns a copy of the inherited + descriptors list. """ - self.patch(os, "environ", {"LISTEN_PID": str(os.getpid()), "LISTEN_FDS": "5"}) - sddaemon = ListenFDs.fromEnvironment() - self.assertEqual(list(range(3, 3 + 5)), sddaemon.inheritedDescriptors()) + names = tuple(map(str, descriptors)) + fds = ListenFDs(descriptors, names) + fdsCopy = fds.inheritedDescriptors() + assert_that(descriptors, equal_to(fdsCopy)) + fdsCopy.append(1) + assert_that(descriptors, not_(equal_to(fdsCopy))) + + @given(dictionaries(systemd_descriptor_names(), integers(min_value=0), max_size=10)) + def test_inheritedNamedDescriptors(self, expected: Mapping[str, int]) -> None: + """ + L{ListenFDs.inheritedNamedDescriptors} returns a mapping from the + descriptor names to their integer values, with items formed by + pairwise combination of the input descriptors and names. + """ + items = list(expected.items()) + names = [name for name, _ in items] + descriptors = [fd for _, fd in items] + fds = ListenFDs(descriptors, names) + assert_that(fds.inheritedNamedDescriptors(), equal_to(expected)) + + @given(lists(integers(min_value=0, max_value=10), unique=True)) + def test_repeated(self, descriptors: Sequence[int]) -> None: + """ + Any subsequent calls to C{inheritedDescriptors} and + C{inheritedNamedDescriptors} return the same list. + """ + names = tuple(map(str, descriptors)) + sddaemon = ListenFDs(descriptors, names) + self.assertEqual( + sddaemon.inheritedDescriptors(), sddaemon.inheritedDescriptors() + ) + self.assertEqual( + sddaemon.inheritedNamedDescriptors(), sddaemon.inheritedNamedDescriptors() + ) From 67f52942810d25a4423e32cf53fb6457a978dadf Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 10:35:31 -0400 Subject: [PATCH 02/21] Add support for using named descriptors to the systemd endpoint parser --- src/twisted/internet/endpoints.py | 44 +++++++++---- src/twisted/internet/test/test_endpoints.py | 72 +++++++++++++++++---- 2 files changed, 94 insertions(+), 22 deletions(-) diff --git a/src/twisted/internet/endpoints.py b/src/twisted/internet/endpoints.py index 80579933800..1f61cc03b3a 100644 --- a/src/twisted/internet/endpoints.py +++ b/src/twisted/internet/endpoints.py @@ -17,6 +17,7 @@ import re import socket import warnings +from typing import Optional from unicodedata import normalize from zope.interface import directlyProvides, implementer, provider @@ -35,6 +36,7 @@ from twisted.internet.interfaces import ( IHostnameResolver, IReactorPluggableNameResolver, + IReactorSocket, IResolutionReceiver, IStreamClientEndpointStringParserWithReactor, IStreamServerEndpointStringParser, @@ -1504,7 +1506,13 @@ class _SystemdParser: prefix = "systemd" - def _parseServer(self, reactor, domain, index): + def _parseServer( + self, + reactor: IReactorSocket, + domain: str, + index: Optional[str] = None, + name: Optional[str] = None, + ) -> AdoptedStreamServerEndpoint: """ Internal parser function for L{_parseServer} to convert the string arguments for a systemd server endpoint into structured arguments for @@ -1513,21 +1521,35 @@ def _parseServer(self, reactor, domain, index): @param reactor: An L{IReactorSocket} provider. @param domain: The domain (or address family) of the socket inherited - from systemd. This is a string like C{"INET"} or C{"UNIX"}, ie the - name of an address family from the L{socket} module, without the - C{"AF_"} prefix. - @type domain: C{str} - - @param index: An offset into the list of file descriptors inherited from - systemd. - @type index: C{str} + from systemd. This is a string like C{"INET"} or C{"UNIX"}, ie + the name of an address family from the L{socket} module, without + the C{"AF_"} prefix. + + @param index: If given, the decimal representation of an integer + giving the offset into the list of file descriptors inherited from + systemd. Since the order of descriptors received from systemd is + hard to predict, this option should only be used if only one + descriptor is being inherited. Even in that case, C{name} is + probably a better idea. Either C{index} or C{name} must be given. + + @param name: If given, the name (as defined by C{FileDescriptorName} + in the C{[Socket]} section of a systemd service definition) of an + inherited file descriptor. Either C{index} or C{name} must be + given. @return: A two-tuple of parsed positional arguments and parsed keyword arguments (a tuple and a dictionary). These can be used to construct an L{AdoptedStreamServerEndpoint}. """ - index = int(index) - fileno = self._sddaemon.inheritedDescriptors()[index] + if (index is None) == (name is None): + raise ValueError("Specify exactly one of descriptor index or name") + + if index is not None: + fileno = self._sddaemon.inheritedDescriptors()[int(index)] + else: + assert name is not None + fileno = self._sddaemon.inheritedNamedDescriptors()[name] + addressFamily = getattr(socket, "AF_" + domain) return AdoptedStreamServerEndpoint(reactor, fileno, addressFamily) diff --git a/src/twisted/internet/test/test_endpoints.py b/src/twisted/internet/test/test_endpoints.py index f4fce891832..f2b94ff53a7 100644 --- a/src/twisted/internet/test/test_endpoints.py +++ b/src/twisted/internet/test/test_endpoints.py @@ -8,7 +8,7 @@ """ from errno import EPERM -from socket import AF_INET, AF_INET6, IPPROTO_TCP, SOCK_STREAM, gaierror +from socket import AF_INET, AF_INET6, IPPROTO_TCP, SOCK_STREAM, AddressFamily, gaierror from types import FunctionType from unicodedata import normalize from unittest import skipIf @@ -3786,10 +3786,12 @@ def test_interface(self): verifyObject(interfaces.IStreamServerEndpointStringParser, parser) ) - def _parseStreamServerTest(self, addressFamily, addressFamilyString): + def _parseIndexStreamServerTest( + self, addressFamily: AddressFamily, addressFamilyString: str + ) -> None: """ - Helper for unit tests for L{endpoints._SystemdParser.parseStreamServer} - for different address families. + Helper for tests for L{endpoints._SystemdParser.parseStreamServer} + for different address families with a descriptor identified by index. Handling of the address family given will be verify. If there is a problem a test-failing exception will be raised. @@ -3802,10 +3804,11 @@ def _parseStreamServerTest(self, addressFamily, addressFamilyString): """ reactor = object() descriptors = [5, 6, 7, 8, 9] + names = ["5.socket", "6.socket", "foo", "8.socket", "9.socket"] index = 3 parser = self._parserClass() - parser._sddaemon = ListenFDs(descriptors) + parser._sddaemon = ListenFDs(descriptors, names) server = parser.parseStreamServer( reactor, domain=addressFamilyString, index=str(index) @@ -3814,19 +3817,66 @@ def _parseStreamServerTest(self, addressFamily, addressFamilyString): self.assertEqual(server.addressFamily, addressFamily) self.assertEqual(server.fileno, descriptors[index]) - def test_parseStreamServerINET(self): + def _parseNameStreamServerTest( + self, addressFamily: AddressFamily, addressFamilyString: str + ) -> None: + """ + Like L{_parseIndexStreamServerTest} but for descriptors identified by + name. + """ + reactor = object() + descriptors = [5, 6, 7, 8, 9] + names = ["5.socket", "6.socket", "foo", "8.socket", "9.socket"] + name = "foo" + + parser = self._parserClass() + parser._sddaemon = ListenFDs(descriptors, names) + + server = parser.parseStreamServer( + reactor, + domain=addressFamilyString, + name=name, + ) + self.assertIs(server.reactor, reactor) + self.assertEqual(server.addressFamily, addressFamily) + self.assertEqual(server.fileno, descriptors[names.index(name)]) + + def test_parseIndexStreamServerINET(self) -> None: + """ + IPv4 can be specified using the string C{"INET"}. + """ + self._parseIndexStreamServerTest(AF_INET, "INET") + + def test_parseIndexStreamServerINET6(self) -> None: + """ + IPv6 can be specified using the string C{"INET6"}. + """ + self._parseIndexStreamServerTest(AF_INET6, "INET6") + + def test_parseIndexStreamServerUNIX(self) -> None: + """ + A UNIX domain socket can be specified using the string C{"UNIX"}. + """ + try: + from socket import AF_UNIX + except ImportError: + raise unittest.SkipTest("Platform lacks AF_UNIX support") + else: + self._parseIndexStreamServerTest(AF_UNIX, "UNIX") + + def test_parseNameStreamServerINET(self) -> None: """ IPv4 can be specified using the string C{"INET"}. """ - self._parseStreamServerTest(AF_INET, "INET") + self._parseNameStreamServerTest(AF_INET, "INET") - def test_parseStreamServerINET6(self): + def test_parseNameStreamServerINET6(self) -> None: """ IPv6 can be specified using the string C{"INET6"}. """ - self._parseStreamServerTest(AF_INET6, "INET6") + self._parseNameStreamServerTest(AF_INET6, "INET6") - def test_parseStreamServerUNIX(self): + def test_parseNameStreamServerUNIX(self) -> None: """ A UNIX domain socket can be specified using the string C{"UNIX"}. """ @@ -3835,7 +3885,7 @@ def test_parseStreamServerUNIX(self): except ImportError: raise unittest.SkipTest("Platform lacks AF_UNIX support") else: - self._parseStreamServerTest(AF_UNIX, "UNIX") + self._parseNameStreamServerTest(AF_UNIX, "UNIX") class TCP6ServerEndpointPluginTests(unittest.TestCase): From b9c1dde2b5c0f946444967802e0aa5171ca37323 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 10:41:25 -0400 Subject: [PATCH 03/21] Document the new feature of the endpoint parser --- docs/core/howto/endpoints.rst | 7 +++++-- docs/core/howto/listings/systemd/www.example.com.socket | 1 + .../systemd/www.example.com.socketactivated.service | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/core/howto/endpoints.rst b/docs/core/howto/endpoints.rst index d3c6887b1eb..5cf0069ea2c 100644 --- a/docs/core/howto/endpoints.rst +++ b/docs/core/howto/endpoints.rst @@ -376,11 +376,14 @@ UNIX For example, ``unix:address=/var/run/web.sock:lockfile=1``. systemd - Supported arguments: ``domain``, ``index``. + Supported arguments: ``domain``, ``name``, and ``index``. ``domain`` indicates which socket domain the inherited file descriptor belongs to (eg INET, INET6). + ``name`` indicates the name of a file descriptor inherited from systemd. + The is set by the systemd configuration for the socket. ``index`` indicates an offset into the array of file descriptors which have been inherited from systemd. + ``name`` should be preferred over ``index`` because the order of the descriptors can be difficult to predict. - For example, ``systemd:domain=INET6:index=3``. + For example, ``systemd:domain=INET6:name=my-web-server``. See also :doc:`Deploying Twisted with systemd `. diff --git a/docs/core/howto/listings/systemd/www.example.com.socket b/docs/core/howto/listings/systemd/www.example.com.socket index 551d102224f..da4748da30e 100644 --- a/docs/core/howto/listings/systemd/www.example.com.socket +++ b/docs/core/howto/listings/systemd/www.example.com.socket @@ -1,5 +1,6 @@ [Socket] ListenStream=0.0.0.0:80 +FileDescriptorName=www [Install] WantedBy=sockets.target diff --git a/docs/core/howto/listings/systemd/www.example.com.socketactivated.service b/docs/core/howto/listings/systemd/www.example.com.socketactivated.service index 5e273619c9f..7dec4ba7711 100644 --- a/docs/core/howto/listings/systemd/www.example.com.socketactivated.service +++ b/docs/core/howto/listings/systemd/www.example.com.socketactivated.service @@ -5,7 +5,7 @@ Description=Example Web Server ExecStart=/usr/bin/twistd \ --nodaemon \ --pidfile= \ - web --listen systemd:domain=INET:index=0 --path . + web --listen systemd:domain=INET:name=www --path . NonBlocking=true From dc583f08bb241b7397909dce98719b1c5ff5391d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 10:41:32 -0400 Subject: [PATCH 04/21] news fragment --- src/twisted/newsfragments/8147.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 src/twisted/newsfragments/8147.feature diff --git a/src/twisted/newsfragments/8147.feature b/src/twisted/newsfragments/8147.feature new file mode 100644 index 00000000000..d9d337948ae --- /dev/null +++ b/src/twisted/newsfragments/8147.feature @@ -0,0 +1 @@ +The ``systemd:`` endpoint parser now supports "named" file descriptors. This is a more reliable mechanism for choosing among several inherited descriptors. \ No newline at end of file From ba3ae171f3ddd806915c4cb13d59de084f101374 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 10:44:37 -0400 Subject: [PATCH 05/21] another news fragment --- src/twisted/newsfragments/8146.doc | 1 + 1 file changed, 1 insertion(+) create mode 100644 src/twisted/newsfragments/8146.doc diff --git a/src/twisted/newsfragments/8146.doc b/src/twisted/newsfragments/8146.doc new file mode 100644 index 00000000000..50fe3ae8164 --- /dev/null +++ b/src/twisted/newsfragments/8146.doc @@ -0,0 +1 @@ +The ``systemd`` endpoint parser's ``index`` parameter is now documented as leading to non-deterministic results in which descriptor is selected. The new ``name`` parameter is now documented as preferred. From a333fcace3cea0fc8a3d551283834d66d89d9b48 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 11:37:18 -0400 Subject: [PATCH 06/21] test a new error case from the parser --- src/twisted/internet/test/test_endpoints.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/twisted/internet/test/test_endpoints.py b/src/twisted/internet/test/test_endpoints.py index f2b94ff53a7..a0ab3dba752 100644 --- a/src/twisted/internet/test/test_endpoints.py +++ b/src/twisted/internet/test/test_endpoints.py @@ -3887,6 +3887,15 @@ def test_parseNameStreamServerUNIX(self) -> None: else: self._parseNameStreamServerTest(AF_UNIX, "UNIX") + def test_indexAndNameMutuallyExclusive(self) -> None: + """ + The endpoint cannot be defined using both C{index} and C{name}. + """ + parser = self._parserClass() + parser._sddaemon = ListenFDs([], ()) + with self.assertRaises(ValueError): + parser.parseStreamServer(reactor, domain="INET", index=0, name="foo") + class TCP6ServerEndpointPluginTests(unittest.TestCase): """ From 352ab9ed70ccb0a08a87f123b3b6d03bd44be4ef Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 26 Sep 2022 12:40:07 -0400 Subject: [PATCH 07/21] add some module boilerplate --- src/twisted/python/test/strategies.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/twisted/python/test/strategies.py b/src/twisted/python/test/strategies.py index 93c7d12fb38..bb393daf070 100644 --- a/src/twisted/python/test/strategies.py +++ b/src/twisted/python/test/strategies.py @@ -1,3 +1,10 @@ +# Copyright (c) Twisted Matrix Laboratories. +# See LICENSE for details. + +""" +Hypothesis strategies for values related to L{twisted.python}. +""" + from hypothesis.strategies import SearchStrategy, characters, text From 5cd0003efd21726143627e3b5997e977d85df4bc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Sep 2022 08:57:33 -0400 Subject: [PATCH 08/21] Reflect the use of `name=...` in the howto --- docs/core/howto/systemd.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/core/howto/systemd.rst b/docs/core/howto/systemd.rst index 98933efc2b8..779c3d47e11 100644 --- a/docs/core/howto/systemd.rst +++ b/docs/core/howto/systemd.rst @@ -282,7 +282,7 @@ ExecStart The ``domain=INET`` endpoint argument makes ``twistd`` treat the inherited file descriptor as an IPv4 socket. - The ``index=0`` endpoint argument makes ``twistd`` adopt the first file descriptor inherited from ``systemd``\ . + The ``name=www`` endpoint argument makes ``twistd`` adopt the file descriptor inherited from ``systemd`` named ``www``. Socket activation is also technically possible with other socket families and types, but Twisted currently only accepts IPv4 and IPv6 TCP sockets. See :ref:`limitations` below. @@ -353,7 +353,7 @@ You can verify this by using systemctl to report the status of the service. eg Active: active (running) since Tue 2013-01-29 15:02:20 GMT; 3s ago Main PID: 25605 (twistd) CGroup: name=systemd:/system/www.example.com.service - └─25605 /usr/bin/python /usr/bin/twistd --nodaemon --pidfile= web --port systemd:domain=INET:index=0 --path . + └─25605 /usr/bin/python /usr/bin/twistd --nodaemon --pidfile= web --port systemd:domain=INET:name=www --path . Jan 29 15:02:20 zorin.lan systemd[1]: Started Example Web Server. Jan 29 15:02:20 zorin.lan twistd[25605]: 2013-01-29 15:02:20+0000 [-] Log opened. From d62447002f90fa668ef034e6c8e08c2021fe2205 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 1 Oct 2022 17:04:05 -0400 Subject: [PATCH 09/21] Give the file descriptor a more descriptive name --- docs/core/howto/listings/systemd/www.example.com.socket | 2 +- docs/core/howto/systemd.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/core/howto/listings/systemd/www.example.com.socket b/docs/core/howto/listings/systemd/www.example.com.socket index da4748da30e..b4c091f4f63 100644 --- a/docs/core/howto/listings/systemd/www.example.com.socket +++ b/docs/core/howto/listings/systemd/www.example.com.socket @@ -1,6 +1,6 @@ [Socket] ListenStream=0.0.0.0:80 -FileDescriptorName=www +FileDescriptorName=my-web-port [Install] WantedBy=sockets.target diff --git a/docs/core/howto/systemd.rst b/docs/core/howto/systemd.rst index 779c3d47e11..03058e58c36 100644 --- a/docs/core/howto/systemd.rst +++ b/docs/core/howto/systemd.rst @@ -282,7 +282,7 @@ ExecStart The ``domain=INET`` endpoint argument makes ``twistd`` treat the inherited file descriptor as an IPv4 socket. - The ``name=www`` endpoint argument makes ``twistd`` adopt the file descriptor inherited from ``systemd`` named ``www``. + The ``name=my-web-port`` endpoint argument makes ``twistd`` adopt the file descriptor inherited from ``systemd`` named ``my-web-port``. Socket activation is also technically possible with other socket families and types, but Twisted currently only accepts IPv4 and IPv6 TCP sockets. See :ref:`limitations` below. From 5f8e8cf3900561d24157c95804f5e40f7e8a0955 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 1 Oct 2022 17:04:58 -0400 Subject: [PATCH 10/21] Try to be more clear when talking about the systemd service file --- docs/core/howto/systemd.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/core/howto/systemd.rst b/docs/core/howto/systemd.rst index 03058e58c36..bcf870c9a28 100644 --- a/docs/core/howto/systemd.rst +++ b/docs/core/howto/systemd.rst @@ -65,15 +65,15 @@ Twisted Basic Systemd Service Configuration ----------------------------------- -The essential configuration file for a ``systemd`` service is the `service `_ file. +The essential configuration file for a ``systemd`` service is the `service file `_. Later in this tutorial, you will learn about some other types of configuration file, which are used to control when and how your service is started. But we will begin by configuring ``systemd`` to start a Twisted web server immediately on system boot. -Create a systemd.service file +Create a systemd service file ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Create the `service `_ file at ``/etc/systemd/system/www.example.com.service`` with the following content: +Create the `service file `_ at ``/etc/systemd/system/www.example.com.service`` with the following content: :download:`/etc/systemd/system/www.example.com.service ` @@ -190,7 +190,7 @@ Later in this tutorial you will learn how to use another special unit - the ``so Test that the service is automatically restarted ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The ``Restart=always`` option in the ``systemd.service`` file ensures that ``systemd`` will restart the ``twistd`` process if and when it exits unexpectedly. +The ``Restart=always`` option in the ``www.example.com.service`` file ensures that ``systemd`` will restart the ``twistd`` process if and when it exits unexpectedly. You can read about other ``Restart`` options in the `systemd.service man page `_. @@ -269,7 +269,7 @@ WantedBy=sockets.target This is a `special target `_ used by all socket activated services. ``systemd`` will automatically bind to all such socket activation ports during boot up. -You also need to modify the ``systemd.service`` file as follows: +You also need to modify the ``www.example.com.service`` file as follows: :download:`/etc/systemd/system/www.example.com.service ` From 6c441e3647e8e19f7127c1fd817a0ad73fdde459 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 1 Oct 2022 17:05:19 -0400 Subject: [PATCH 11/21] Discuss FileDescriptorName --- docs/core/howto/systemd.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/core/howto/systemd.rst b/docs/core/howto/systemd.rst index bcf870c9a28..861afc70c3b 100644 --- a/docs/core/howto/systemd.rst +++ b/docs/core/howto/systemd.rst @@ -269,6 +269,11 @@ WantedBy=sockets.target This is a `special target `_ used by all socket activated services. ``systemd`` will automatically bind to all such socket activation ports during boot up. +FileDescriptorName=my-web-port + + This option names the file descriptor for the socket. + The name allows a specific inherited descriptor to be chosen reliably out of set of several inherited descriptors. + You also need to modify the ``www.example.com.service`` file as follows: :download:`/etc/systemd/system/www.example.com.service ` From ccb97184dca46770877e2601f927bc60f5a1d32b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 3 Oct 2022 16:13:03 -0400 Subject: [PATCH 12/21] Fix the file descriptor name in the systemd unit definition Co-authored-by: Tom Most --- .../listings/systemd/www.example.com.socketactivated.service | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/core/howto/listings/systemd/www.example.com.socketactivated.service b/docs/core/howto/listings/systemd/www.example.com.socketactivated.service index 7dec4ba7711..237d9a73b50 100644 --- a/docs/core/howto/listings/systemd/www.example.com.socketactivated.service +++ b/docs/core/howto/listings/systemd/www.example.com.socketactivated.service @@ -5,7 +5,7 @@ Description=Example Web Server ExecStart=/usr/bin/twistd \ --nodaemon \ --pidfile= \ - web --listen systemd:domain=INET:name=www --path . + web --listen systemd:domain=INET:name=my-web-port --path . NonBlocking=true From 6f2affdd53cae2d86c5bcabb9b2a450d047b06e2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 3 Oct 2022 16:13:29 -0400 Subject: [PATCH 13/21] Fix the endpoint string in the example systemd output Co-authored-by: Tom Most --- docs/core/howto/systemd.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/core/howto/systemd.rst b/docs/core/howto/systemd.rst index 861afc70c3b..e8a323bbbc6 100644 --- a/docs/core/howto/systemd.rst +++ b/docs/core/howto/systemd.rst @@ -358,7 +358,7 @@ You can verify this by using systemctl to report the status of the service. eg Active: active (running) since Tue 2013-01-29 15:02:20 GMT; 3s ago Main PID: 25605 (twistd) CGroup: name=systemd:/system/www.example.com.service - └─25605 /usr/bin/python /usr/bin/twistd --nodaemon --pidfile= web --port systemd:domain=INET:name=www --path . + └─25605 /usr/bin/python /usr/bin/twistd --nodaemon --pidfile= web --port systemd:domain=INET:name=my-web-port --path . Jan 29 15:02:20 zorin.lan systemd[1]: Started Example Web Server. Jan 29 15:02:20 zorin.lan twistd[25605]: 2013-01-29 15:02:20+0000 [-] Log opened. From 5b011897cd58caede3d678114368f24224d64c17 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 1 Oct 2022 18:36:18 -0400 Subject: [PATCH 14/21] note that systemd _always_ gives us names, even if we don't ask for them --- src/twisted/python/systemd.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/twisted/python/systemd.py b/src/twisted/python/systemd.py index 5f98cb97ecb..c5906bbfe07 100644 --- a/src/twisted/python/systemd.py +++ b/src/twisted/python/systemd.py @@ -80,7 +80,10 @@ def fromEnvironment( # They may both be missing (consistent with not running under systemd # at all) or they may both be present (consistent with running under # systemd 227 or newer). It is not allowed for only one to be present - # or for the values to disagree with each other. + # or for the values to disagree with each other (per + # systemd.socket(5), systemd will use a default value based on the + # socket unit name if the socket unit doesn't explicitly define a name + # with FileDescriptorName). if len(names) != len(descriptors): return cls([], ()) From 6e69b110ac0927b63805be0be79cdaee9661f436 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 1 Oct 2022 18:46:56 -0400 Subject: [PATCH 15/21] add some explanation for the weird environment mutation behavior --- src/twisted/python/systemd.py | 12 ++++++++++++ src/twisted/python/test/test_systemd.py | 6 ++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/twisted/python/systemd.py b/src/twisted/python/systemd.py index c5906bbfe07..51911160eb9 100644 --- a/src/twisted/python/systemd.py +++ b/src/twisted/python/systemd.py @@ -123,6 +123,18 @@ def _parseDescriptors(start: int, environ: Mapping[str, str]) -> List[int]: return [] else: descriptors = list(range(start, start + count)) + + # Remove the information from the environment so that a second + # `ListenFDs` cannot find the same information. This is a precaution + # against some application code accidentally trying to handle the same + # inherited descriptor more than once - which probably wouldn't work. + # + # This precaution is perhaps somewhat questionable since it is up to + # the application itself to know whether its handling of the file + # descriptor will actually be safe. Also, nothing stops an + # application from getting the same descriptor more than once using + # multiple calls to `ListenFDs.inheritedDescriptors()` on the same + # `ListenFDs` instance. del environ["LISTEN_PID"], environ["LISTEN_FDS"] return descriptors diff --git a/src/twisted/python/test/test_systemd.py b/src/twisted/python/test/test_systemd.py index 0e60178f2f9..aafeb7cd8d3 100644 --- a/src/twisted/python/test/test_systemd.py +++ b/src/twisted/python/test/test_systemd.py @@ -69,8 +69,10 @@ def test_defaultEnviron(self) -> None: def test_secondEnvironment(self) -> None: """ - L{ListenFDs.fromEnvironment} removes information about the inherited - file descriptors from the environment mapping. + L{ListenFDs.fromEnvironment} removes information about the + inherited file descriptors from the environment mapping so that the + same inherited file descriptors cannot be handled repeatedly from + multiple L{ListenFDs} instances. """ env = initializeEnvironment(3, os.getpid()) first = ListenFDs.fromEnvironment(environ=env) From 3cf25c1e7095e87ecbaf9c64c9fa20686d99b987 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 1 Oct 2022 18:52:13 -0400 Subject: [PATCH 16/21] Fix the new strategy name to adhere to the convention --- src/twisted/python/test/strategies.py | 2 +- src/twisted/python/test/test_systemd.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/twisted/python/test/strategies.py b/src/twisted/python/test/strategies.py index bb393daf070..d11fbf74dbf 100644 --- a/src/twisted/python/test/strategies.py +++ b/src/twisted/python/test/strategies.py @@ -8,7 +8,7 @@ from hypothesis.strategies import SearchStrategy, characters, text -def systemd_descriptor_names() -> SearchStrategy[str]: +def systemdDescriptorNames() -> SearchStrategy[str]: """ Build strings that are legal values for the systemd I{FileDescriptorName} field. diff --git a/src/twisted/python/test/test_systemd.py b/src/twisted/python/test/test_systemd.py index aafeb7cd8d3..04da160a181 100644 --- a/src/twisted/python/test/test_systemd.py +++ b/src/twisted/python/test/test_systemd.py @@ -15,7 +15,7 @@ from twisted.python.systemd import ListenFDs from twisted.trial.unittest import SynchronousTestCase -from .strategies import systemd_descriptor_names +from .strategies import systemdDescriptorNames def initializeEnvironment(count: int, pid: object) -> Dict[str, str]: @@ -35,7 +35,7 @@ class ListenFDsTests(SynchronousTestCase): Apply tests to L{ListenFDs}, constructed based on an environment dictionary. """ - @given(lists(systemd_descriptor_names(), min_size=0, max_size=10)) + @given(lists(systemdDescriptorNames(), min_size=0, max_size=10)) def test_fromEnvironmentEquivalence(self, names: Sequence[str]) -> None: """ The L{ListenFDs} and L{ListenFDs.fromEnvironment} constructors are @@ -142,7 +142,7 @@ def test_inheritedDescriptors(self, descriptors: Sequence[int]) -> None: fdsCopy.append(1) assert_that(descriptors, not_(equal_to(fdsCopy))) - @given(dictionaries(systemd_descriptor_names(), integers(min_value=0), max_size=10)) + @given(dictionaries(systemdDescriptorNames(), integers(min_value=0), max_size=10)) def test_inheritedNamedDescriptors(self, expected: Mapping[str, int]) -> None: """ L{ListenFDs.inheritedNamedDescriptors} returns a mapping from the From 5a57382ca8e0f4b0abaa980fc60352881ca39d9a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 1 Oct 2022 18:58:46 -0400 Subject: [PATCH 17/21] rename and otherwise improve `initializeEnvironment` --- src/twisted/python/test/test_systemd.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/twisted/python/test/test_systemd.py b/src/twisted/python/test/test_systemd.py index 04da160a181..fda16217dbd 100644 --- a/src/twisted/python/test/test_systemd.py +++ b/src/twisted/python/test/test_systemd.py @@ -18,10 +18,14 @@ from .strategies import systemdDescriptorNames -def initializeEnvironment(count: int, pid: object) -> Dict[str, str]: +def buildEnvironment(count: int, pid: object) -> Dict[str, str]: """ - Create a copy of the process environment and add I{LISTEN_FDS} and - I{LISTEN_PID} (the environment variables set by systemd) to it. + @param count: The number of file descriptors to indicate as inherited. + + @param pid: The pid of the inheriting process to indicate. + + @return: A copy of the current process environment with the I{systemd} + file descriptor inheritance-related environment variables added to it. """ result = os.environ.copy() result["LISTEN_FDS"] = str(count) @@ -63,7 +67,7 @@ def test_defaultEnviron(self) -> None: L{ListenFDs.fromEnvironment}, the real process environment dictionary is used. """ - self.patch(os, "environ", initializeEnvironment(5, os.getpid())) + self.patch(os, "environ", buildEnvironment(5, os.getpid())) sddaemon = ListenFDs.fromEnvironment() self.assertEqual(list(range(3, 3 + 5)), sddaemon.inheritedDescriptors()) @@ -74,7 +78,7 @@ def test_secondEnvironment(self) -> None: same inherited file descriptors cannot be handled repeatedly from multiple L{ListenFDs} instances. """ - env = initializeEnvironment(3, os.getpid()) + env = buildEnvironment(3, os.getpid()) first = ListenFDs.fromEnvironment(environ=env) second = ListenFDs.fromEnvironment(environ=env) self.assertEqual(list(range(3, 6)), first.inheritedDescriptors()) @@ -85,7 +89,7 @@ def test_mismatchedPID(self) -> None: If the current process PID does not match the PID in the environment, no inherited descriptors are reported. """ - env = initializeEnvironment(3, os.getpid() + 1) + env = buildEnvironment(3, os.getpid() + 1) sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) @@ -94,7 +98,7 @@ def test_missingPIDVariable(self) -> None: If the I{LISTEN_PID} environment variable is not present, no inherited descriptors are reported. """ - env = initializeEnvironment(3, os.getpid()) + env = buildEnvironment(3, os.getpid()) del env["LISTEN_PID"] sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) @@ -104,7 +108,7 @@ def test_nonIntegerPIDVariable(self) -> None: If the I{LISTEN_PID} environment variable is set to a string that cannot be parsed as an integer, no inherited descriptors are reported. """ - env = initializeEnvironment(3, "hello, world") + env = buildEnvironment(3, "hello, world") sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) @@ -113,7 +117,7 @@ def test_missingFDSVariable(self) -> None: If the I{LISTEN_FDS} and I{LISTEN_FDNAMES} environment variables are not present, no inherited descriptors are reported. """ - env = initializeEnvironment(3, os.getpid()) + env = buildEnvironment(3, os.getpid()) del env["LISTEN_FDS"] del env["LISTEN_FDNAMES"] sddaemon = ListenFDs.fromEnvironment(environ=env) @@ -124,7 +128,7 @@ def test_nonIntegerFDSVariable(self) -> None: If the I{LISTEN_FDS} environment variable is set to a string that cannot be parsed as an integer, no inherited descriptors are reported. """ - env = initializeEnvironment(3, os.getpid()) + env = buildEnvironment(3, os.getpid()) env["LISTEN_FDS"] = "hello, world" sddaemon = ListenFDs.fromEnvironment(environ=env) self.assertEqual([], sddaemon.inheritedDescriptors()) From d4d9323c29fe8464597216c67aed0bf2ada9096c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 1 Oct 2022 19:03:45 -0400 Subject: [PATCH 18/21] Add detail to some of the pid behavior-checking tests --- src/twisted/python/test/test_systemd.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/twisted/python/test/test_systemd.py b/src/twisted/python/test/test_systemd.py index fda16217dbd..8208f483b2a 100644 --- a/src/twisted/python/test/test_systemd.py +++ b/src/twisted/python/test/test_systemd.py @@ -86,8 +86,11 @@ def test_secondEnvironment(self) -> None: def test_mismatchedPID(self) -> None: """ - If the current process PID does not match the PID in the environment, - no inherited descriptors are reported. + If the current process PID does not match the PID in the + environment then the systemd variables in the environment were set for + a different process (perhaps our parent) and the inherited descriptors + are not intended for this process so L{ListenFDs.inheritedDescriptors} + returns an empty list. """ env = buildEnvironment(3, os.getpid() + 1) sddaemon = ListenFDs.fromEnvironment(environ=env) @@ -95,8 +98,10 @@ def test_mismatchedPID(self) -> None: def test_missingPIDVariable(self) -> None: """ - If the I{LISTEN_PID} environment variable is not present, no inherited - descriptors are reported. + If the I{LISTEN_PID} environment variable is not present then + there is no clear indication that any file descriptors were inherited + by this process so L{ListenFDs.inheritedDescriptors} returns an empty + list. """ env = buildEnvironment(3, os.getpid()) del env["LISTEN_PID"] From 45bec8e1962e4e6701b41f5fa1efe0f63b2243a0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 3 Oct 2022 19:25:56 -0400 Subject: [PATCH 19/21] Add the missing `Requires` declaration --- .../listings/systemd/www.example.com.socketactivated.service | 1 + docs/core/howto/systemd.rst | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/docs/core/howto/listings/systemd/www.example.com.socketactivated.service b/docs/core/howto/listings/systemd/www.example.com.socketactivated.service index 237d9a73b50..033df2fe63f 100644 --- a/docs/core/howto/listings/systemd/www.example.com.socketactivated.service +++ b/docs/core/howto/listings/systemd/www.example.com.socketactivated.service @@ -1,5 +1,6 @@ [Unit] Description=Example Web Server +Requires=www.example.com.socket [Service] ExecStart=/usr/bin/twistd \ diff --git a/docs/core/howto/systemd.rst b/docs/core/howto/systemd.rst index e8a323bbbc6..88c8b1c59d0 100644 --- a/docs/core/howto/systemd.rst +++ b/docs/core/howto/systemd.rst @@ -295,6 +295,11 @@ NonBlocking This must be set to ``true`` to ensure that ``systemd`` passes non-blocking sockets to Twisted. +Requires + + The service no longer knows how to bind the listening port for itself. + The corresponding socket unit must be started so it can pass the listening port on to the ``twistd`` process. + [Install] In this example, the ``[Install]`` section has been moved to the socket configuration file. From 2115c401bdaa676153cfd6ae35ac2b3a77f13e31 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 3 Oct 2022 19:28:38 -0400 Subject: [PATCH 20/21] Fix incorrect return documentation for the systemd endpoint parser --- src/twisted/internet/endpoints.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/twisted/internet/endpoints.py b/src/twisted/internet/endpoints.py index 1f61cc03b3a..6b9adb9ff1b 100644 --- a/src/twisted/internet/endpoints.py +++ b/src/twisted/internet/endpoints.py @@ -1537,9 +1537,8 @@ def _parseServer( inherited file descriptor. Either C{index} or C{name} must be given. - @return: A two-tuple of parsed positional arguments and parsed keyword - arguments (a tuple and a dictionary). These can be used to - construct an L{AdoptedStreamServerEndpoint}. + @return: An L{AdoptedStreamServerEndpoint} which will adopt the + inherited listening port when it is used to listen. """ if (index is None) == (name is None): raise ValueError("Specify exactly one of descriptor index or name") From 55789f23b1a4e4cfd233ad06cfcc139dae666e9d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 3 Oct 2022 19:30:46 -0400 Subject: [PATCH 21/21] They will be placed into non-blocking mode automatically. --- .../listings/systemd/www.example.com.socketactivated.service | 2 -- docs/core/howto/systemd.rst | 4 ---- 2 files changed, 6 deletions(-) diff --git a/docs/core/howto/listings/systemd/www.example.com.socketactivated.service b/docs/core/howto/listings/systemd/www.example.com.socketactivated.service index 033df2fe63f..356b9b12bc1 100644 --- a/docs/core/howto/listings/systemd/www.example.com.socketactivated.service +++ b/docs/core/howto/listings/systemd/www.example.com.socketactivated.service @@ -8,8 +8,6 @@ ExecStart=/usr/bin/twistd \ --pidfile= \ web --listen systemd:domain=INET:name=my-web-port --path . -NonBlocking=true - WorkingDirectory=/srv/www/www.example.com/static User=nobody diff --git a/docs/core/howto/systemd.rst b/docs/core/howto/systemd.rst index 88c8b1c59d0..7f9859cbf0a 100644 --- a/docs/core/howto/systemd.rst +++ b/docs/core/howto/systemd.rst @@ -291,10 +291,6 @@ ExecStart Socket activation is also technically possible with other socket families and types, but Twisted currently only accepts IPv4 and IPv6 TCP sockets. See :ref:`limitations` below. -NonBlocking - - This must be set to ``true`` to ensure that ``systemd`` passes non-blocking sockets to Twisted. - Requires The service no longer knows how to bind the listening port for itself.