grid: Keep using the previous socket path name if it still exists

In 58fbaabf (svcbackend/standalone: use shorter names for supervisor sockets
, 2020-07-17) we changed the socket path from supervisord.sock to a shorter
sv.sock, but this caused issues when updating running slapos node instances,
because this was trying to start a new supervisord process using the new
socket path, while the old one was still running.

To keep services running after upgrade, introduce a compatibility layer in
this socket path - if we still have a supervisord running with a socket at
this previous path, keep using this old socket name.

slapos/cli/supervisord.py

@@ -51,7 +51,7 @@ class SupervisordCommand(ConfigCommand):
           supervisord_additional_argument_list = ['--nodaemon']
         else:
           supervisord_additional_argument_list = []
-        createSupervisordConfiguration(instance_root)
+        createSupervisordConfiguration(instance_root, logger=self.app.log)
         launchSupervisord(
             instance_root=instance_root, logger=self.app.log,
             supervisord_additional_argument_list=supervisord_additional_argument_list

slapos/grid/slapgrid.py

@@ -361,7 +361,7 @@ class Slapgrid(object):
     self.instance_root = os.path.abspath(instance_root)
     self.master_url = master_url
     self.computer_id = computer_id
-    self.supervisord_socket = _getSupervisordSocketPath(instance_root)
+    self.supervisord_socket = _getSupervisordSocketPath(instance_root, logger)
     self.key_file = key_file
     self.cert_file = cert_file
     self.master_ca_file = master_ca_file
@@ -528,7 +528,10 @@ stderr_logfile_backups=1
     if not os.path.isdir(self.software_root):
       raise OSError('%s does not exist.' % self.software_root)
 
-    createSupervisordConfiguration(self.instance_root, self._getWatchdogLine())
+    createSupervisordConfiguration(
+        self.instance_root,
+        logger=self.logger,
+        watchdog_command=self._getWatchdogLine())
     self._generateFirewallSupervisorConf()
     self._generateDbusSupervisorConf()
 

slapos/grid/svcbackend.py

@@ -64,7 +64,11 @@ def getSupervisorRPC(socket):
     with server_proxy as s:
       yield s.supervisor
 
-def _getSupervisordSocketPath(instance_root):
+def _getSupervisordSocketPath(instance_root, logger):
+  legacy_socket_path = os.path.join(instance_root, 'supervisord.socket')
+  if os.path.exists(legacy_socket_path):
+    logger.info("Using legacy supervisor socket path %s", legacy_socket_path)
+    return legacy_socket_path
   return os.path.join(instance_root, 'sv.sock')
 
 def _getSupervisordConfigurationFilePath(instance_root):
@@ -73,7 +77,7 @@ def _getSupervisordConfigurationFilePath(instance_root):
 def _getSupervisordConfigurationDirectory(instance_root):
   return os.path.join(instance_root, 'etc', 'supervisord.conf.d')
 
-def createSupervisordConfiguration(instance_root, watchdog_command=''):
+def createSupervisordConfiguration(instance_root, logger, watchdog_command=''):
   """
   Create supervisord related files and directories.
   """
@@ -82,7 +86,7 @@ def createSupervisordConfiguration(instance_root, watchdog_command=''):
 
   supervisord_configuration_file_path = _getSupervisordConfigurationFilePath(instance_root)
   supervisord_configuration_directory = _getSupervisordConfigurationDirectory(instance_root)
-  supervisord_socket = _getSupervisordSocketPath(instance_root)
+  supervisord_socket = _getSupervisordSocketPath(instance_root, logger)
 
   # Create directory accessible for the instances.
   var_directory = os.path.join(instance_root, 'var')
@@ -140,7 +144,7 @@ def _updateWatchdog(socket):
 def launchSupervisord(instance_root, logger,
                       supervisord_additional_argument_list=None):
   configuration_file = _getSupervisordConfigurationFilePath(instance_root)
-  socket = _getSupervisordSocketPath(instance_root)
+  socket = _getSupervisordSocketPath(instance_root, logger)
   if os.path.exists(socket):
     trynum = 1
     while trynum < 6:

slapos/slap/standalone.py

@@ -455,7 +455,7 @@ class StandaloneSlapOS(object):
 
     This should be used as a context manager.
     """
-    return getSupervisorRPC(_getSupervisordSocketPath(self._instance_root))
+    return getSupervisorRPC(_getSupervisordSocketPath(self._instance_root, self._logger))
 
   def format(
       self,

slapos/tests/test_slapgrid.py

@@ -32,6 +32,7 @@ import random
 import shutil
 import signal
 import socket
+import subprocess
 import sys
 import stat
 import tempfile
@@ -50,6 +51,7 @@ from zope.interface import implementer
 
 import slapos.slap.slap
 import slapos.grid.utils
+import slapos.grid.svcbackend
 from slapos.grid import slapgrid
 from slapos.grid.utils import md5digest
 from slapos.grid.watchdog import Watchdog
@@ -3758,12 +3760,61 @@ class TestSVCBackend(unittest.TestCase):
     """Proper message is displayed when supervisor can not be started.
     """
     logger = mock.create_autospec(logging.Logger)
-    from slapos.grid.svcbackend import launchSupervisord
+
     with self.assertRaisesRegexp(
         RuntimeError,
         """Failed to launch supervisord:
 Error: could not find config file /not/exist/etc/supervisord.conf
 For help, use -c -h"""):
-      launchSupervisord('/not/exist', logger)
+      slapos.grid.svcbackend.launchSupervisord('/not/exist', logger)
 
     logger.warning.assert_called()
+
+  def test_supervisor_socket_legacy_path(self):
+    """Check that supervisor uses legacy socket path if available.
+
+    We changed the path of supervisor socket from supervisord.socket to sv.sock
+    but we don't want createSupervisordConfiguration or launchSupervisord to use
+    the new path if a supervisor is still running at the previous path.
+    """
+    instance_root = tempfile.mkdtemp()
+    self.addCleanup(shutil.rmtree, instance_root)
+    logger = mock.create_autospec(logging.Logger)
+
+    slapos.grid.svcbackend.createSupervisordConfiguration(instance_root, logger)
+    supervisord_config_file_path = os.path.join(instance_root, 'etc', 'supervisord.conf')
+    with open(supervisord_config_file_path) as f:
+      supervisord_config_file_content = f.read()
+    self.assertIn('/sv.sock', supervisord_config_file_content)
+    # change this config to use old socket
+    with open(supervisord_config_file_path, 'w') as f:
+      f.write(supervisord_config_file_content.replace('/sv.sock', '/supervisord.socket'))
+    # start a supervisor on the old socket - this create the state of a running slapos
+    # in the state before, when supervisord.socket was used as a socket name.
+    supervisord_process = subprocess.Popen(
+        ['supervisord', '--nodaemon', '-c', supervisord_config_file_path],
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+    )
+    self.addCleanup(supervisord_process.wait)
+    self.addCleanup(supervisord_process.terminate)
+
+    # wait for process to be started
+    supervisord_legacy_socket_path = os.path.join(instance_root, 'supervisord.socket')
+    for i in range(20):
+      if os.path.exists(supervisord_legacy_socket_path):
+        break
+      time.sleep(.1 * i)
+    if supervisord_process.poll() is not None:
+      self.fail(supervisord_process.communicate())
+
+    # if we create config again, the legacy path will be used and config will not be overwritten
+    slapos.grid.svcbackend.createSupervisordConfiguration(instance_root, logger)
+    with open(supervisord_config_file_path) as f:
+      supervisord_config_file_content = f.read()
+    self.assertIn('/supervisord.socket', supervisord_config_file_content)
+    self.assertNotIn('/sv.sock', supervisord_config_file_content)
+    logger.info.assert_called_with('Using legacy supervisor socket path %s', supervisord_legacy_socket_path)
+
+    slapos.grid.svcbackend.launchSupervisord(instance_root, logger)
+    logger.debug.assert_called_with('Supervisord already running.')