#8146 #8147 Support systemd named inherited file descriptors (#11691)

Also, document the shortcomings of index-based identification.

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 <systemd>`.
 

docs/core/howto/listings/systemd/www.example.com.socket

@@ -1,5 +1,6 @@
 [Socket]
 ListenStream=0.0.0.0:80
+FileDescriptorName=my-web-port
 
 [Install]
 WantedBy=sockets.target

docs/core/howto/listings/systemd/www.example.com.socketactivated.service

@@ -1,13 +1,12 @@
 [Unit]
 Description=Example Web Server
+Requires=www.example.com.socket
 
 [Service]
 ExecStart=/usr/bin/twistd \
     --nodaemon \
     --pidfile= \
-    web --listen systemd:domain=INET:index=0 --path .
-
-NonBlocking=true
+    web --listen systemd:domain=INET:name=my-web-port --path .
 
 WorkingDirectory=/srv/www/www.example.com/static
 

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 <http://www.freedesktop.org/software/systemd/man/systemd.service.html>`_ file.
+The essential configuration file for a ``systemd`` service is the `service file <http://www.freedesktop.org/software/systemd/man/systemd.service.html>`_.
 
 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 <http://www.freedesktop.org/software/systemd/man/systemd.service.html>`_ file at ``/etc/systemd/system/www.example.com.service`` with the following content:
+Create the `service file <http://www.freedesktop.org/software/systemd/man/systemd.service.html>`_ at ``/etc/systemd/system/www.example.com.service`` with the following content:
 
 :download:`/etc/systemd/system/www.example.com.service <listings/systemd/www.example.com.static.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 <http://www.freedesktop.org/software/systemd/man/systemd.service.html>`_.
 
@@ -269,7 +269,12 @@ WantedBy=sockets.target
   This is
   a `special target <http://www.freedesktop.org/software/systemd/man/systemd.special.html#sockets.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:
+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 <listings/systemd/www.example.com.socketactivated.service>`
 
@@ -282,13 +287,14 @@ 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=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.
 
-NonBlocking
+Requires
 
-  This must be set to ``true`` to ensure that ``systemd`` passes non-blocking sockets to Twisted.
+  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]
 
@@ -353,7 +359,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=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.

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,34 @@ 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: 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")
+
+        if index is not None:
+            fileno = self._sddaemon.inheritedDescriptors()[int(index)]
+        else:
+            assert name is not None
+            fileno = self._sddaemon.inheritedNamedDescriptors()[name]
 
-        @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]
         addressFamily = getattr(socket, "AF_" + domain)
         return AdoptedStreamServerEndpoint(reactor, fileno, addressFamily)
 

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,16 @@ 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")
+
+    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):

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.

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.