slapproxy: Fix software URL migration

The `local-software-release-url` option allows migrating the software
URLs which are local paths by rebasing them on the path provided by
the option.

Do not migrate software release URLs if the old root path and the
new root path are subpaths or superpaths one of the other.

In addition, do not migrate an URL if the old one refers to an
existing file and the new one doesn't.

Also, create a backup of the database before migrating.

See merge request nexedi/slapos.core!338

slapos/proxy/views.py

@@ -203,23 +203,45 @@ def _updateLocalSoftwareReleaseRootPathIfNeeded():
   current_root_path = execute_db('local_software_release_root', 'SELECT * from %s', one=True)['path'] or os.sep
   new_root_path = app.config['local_software_release_root'] or os.sep
   execute_db('local_software_release_root', 'UPDATE %s SET path=?', [new_root_path])
+  # Check whether one is the same as or a subpath of the other
+  if current_root_path == new_root_path:
+    return
+  relpath = os.path.relpath(new_root_path, current_root_path)
+  if not relpath.startswith(os.pardir + os.sep):
+    app.logger.info('Do not rebase any URLs because %s is a subpath of %s', new_root_path, current_root_path)
+    return
+  elif os.path.basename(relpath) == os.pardir:
+    app.logger.info('Do not rebase any URLs because %s is a superpath of %s', new_root_path, current_root_path)
+    return
+  # Backup the database before migrating
+  database_path = app.config['DATABASE_URI']
+  backup_path = database_path + "-backup-%s.sql" % datetime.now().isoformat()
+  app.logger.info("Backuping database to %s", backup_path)
+  with open(backup_path, 'w') as f:
+    for line in g.db.iterdump():
+      f.write('%s\n' % line)
   # Rebase all URLs relative to the new root path
-  if current_root_path != new_root_path:
-    app.logger.info('Updating local software release root path: %s --> %s', current_root_path, new_root_path)
-    def migrate_url(url):
-      if not url or urlparse(url).scheme:
-        app.logger.debug('Migrate URL ? N: %s is not a path', url)
-        return url
-      rel = os.path.relpath(url, current_root_path)
-      if rel.startswith(os.pardir + os.sep):
-        app.logger.debug('Migrate URL ? N: %s is not a subpath', url)
-        return url
-      new = os.path.join(new_root_path, rel)
-      app.logger.debug('Migrate URL ? Y: %s -> %s', url, new)
-      return new
-    g.db.create_function('migrate_url', 1, migrate_url)
-    execute_db('software', 'UPDATE %s SET url=migrate_url(url)')
-    execute_db('partition', 'UPDATE %s SET software_release=migrate_url(software_release)')
+  app.logger.info('Rebase URLs on local software release root path')
+  app.logger.info('Old root path: %s', current_root_path)
+  app.logger.info('New root path: %s', new_root_path)
+  def migrate_url(url):
+    app.logger.debug('Examining URL %s', url)
+    if not url or urlparse(url).scheme:
+      app.logger.debug('  Do not rebase because it is not a path')
+      return url
+    rel = os.path.relpath(url, current_root_path)
+    if rel.startswith(os.pardir + os.sep):
+      app.logger.debug('  Do not rebase because it is not a subpath of %s', current_root_path)
+      return url
+    new = os.path.join(new_root_path, rel)
+    if not os.path.isfile(new) and os.path.isfile(url):
+      app.logger.debug('  Do not rebase because it refers to an existing file but %s does not', new)
+      return url
+    app.logger.debug('  Migrate to rebased URL %s', new)
+    return new
+  g.db.create_function('migrate_url', 1, migrate_url)
+  execute_db('software', 'UPDATE %s SET url=migrate_url(url)')
+  execute_db('partition', 'UPDATE %s SET software_release=migrate_url(software_release)')
 
 is_schema_already_executed = False
 @app.before_request

slapos/tests/test_slapproxy.py

@@ -2060,19 +2060,21 @@ class TestLocalSoftwareReleaseRootPathMigration(MasterMixin):
   def newRootDir(self):
     return os.path.join(self._tempdir, str(1 + int(os.path.basename(self._rootdir))))
 
-  def createSlapOSConfigurationFile(self):
+  def createSlapOSConfigurationFile(self, localdir=None):
+    localdir = localdir or os.path.join(self._rootdir, 'opt')
     super(TestLocalSoftwareReleaseRootPathMigration, self).createSlapOSConfigurationFile()
     with open(self.slapos_cfg, 'a') as f:
-      f.write("\nlocal_software_release_root = %s/opt" % self._rootdir)
-
-  def moveProxy(self, rootdir=None):
-    if not rootdir:
-      rootdir = self.newRootDir()
-    os.rename(self._rootdir, rootdir)
-    self._rootdir = rootdir
+      f.write("\nlocal_software_release_root = %s" % localdir)
+
+  def moveProxy(self, rootdir=None, localdir=None):
+    if not localdir:
+      if not rootdir:
+        rootdir = self.newRootDir()
+      os.rename(self._rootdir, rootdir)
+      self._rootdir = rootdir
     self.slapos_cfg = os.path.join(self._rootdir, 'slapos.cfg')
     self.proxy_db = os.path.join(self._rootdir, 'lib', 'proxy.db')
-    self.createSlapOSConfigurationFile()
+    self.createSlapOSConfigurationFile(localdir)
     views.is_schema_already_executed = False
     self.startProxy()
     os.environ.pop('SLAPGRID_INSTANCE_ROOT', None)
@@ -2084,23 +2086,23 @@ class TestLocalSoftwareReleaseRootPathMigration(MasterMixin):
   def assertPartitionUrl(self, partition_id, expected_url):
     self.assertEqual(self.getPartitionInformation(partition_id).getSoftwareRelease().getURI(), expected_url)
 
-  def checkSupplyUrl(self, initial_url, expected_url, rootdir=None):
+  def checkSupplyUrl(self, initial_url, expected_url, rootdir=None, localdir=None):
     self.supply(initial_url)
     self.assertSoftwareUrls(initial_url)
-    self.moveProxy(rootdir)
+    self.moveProxy(rootdir, localdir)
     self.assertSoftwareUrls(expected_url)
 
-  def checkRequestUrl(self, initial_url, expected_url, rootdir=None):
+  def checkRequestUrl(self, initial_url, expected_url, rootdir=None, localdir=None):
     self.format_for_number_of_partitions(1)
     partition = self.request(initial_url, None, 'MyInstance', 'slappart0')
     self.assertPartitionUrl(partition._partition_id, initial_url)
-    self.moveProxy(rootdir)
+    self.moveProxy(rootdir, localdir)
     self.assertPartitionUrl(partition._partition_id, expected_url)
 
   def test_supply_local_url(self):
-    initial_url = os.path.join(self._rootdir, 'opt', 'software.cfg')
+    initial_url = os.path.join(self._rootdir, 'opt', 'soft', 'software.cfg')
     new_rootdir = self.newRootDir()
-    expected_url = os.path.join(new_rootdir, 'opt', 'software.cfg')
+    expected_url = os.path.join(new_rootdir, 'opt', 'soft', 'software.cfg')
     self.checkSupplyUrl(initial_url, expected_url, new_rootdir)
 
   def test_supply_not_in_root_url(self):
@@ -2116,13 +2118,13 @@ class TestLocalSoftwareReleaseRootPathMigration(MasterMixin):
     self.checkSupplyUrl(url, url)
 
   def test_request_local_url(self):
-    initial_url = os.path.join(self._rootdir, 'opt', 'software.cfg')
+    initial_url = os.path.join(self._rootdir, 'opt', 'soft', 'software.cfg')
     new_rootdir = self.newRootDir()
-    expected_url = os.path.join(new_rootdir, 'opt', 'software.cfg')
+    expected_url = os.path.join(new_rootdir, 'opt', 'soft', 'software.cfg')
     self.checkRequestUrl(initial_url, expected_url, new_rootdir)
 
   def test_request_not_in_root_url(self):
-    url = os.path.join(self._rootdir, 'srv', 'software.cfg')
+    url = os.path.join(self._rootdir, 'srv', 'soft', 'software.cfg')
     self.checkRequestUrl(url, url)
 
   def test_request_http_url(self):
@@ -2134,10 +2136,10 @@ class TestLocalSoftwareReleaseRootPathMigration(MasterMixin):
     self.checkRequestUrl(url, url)
 
   def checkMultipleMoves(self, checkUrl):
-    initial_url = os.path.join(self._rootdir, 'opt', 'software.cfg')
+    initial_url = os.path.join(self._rootdir, 'opt', 'soft', 'software.cfg')
     for _ in range(5):
       new_rootdir = self.newRootDir()
-      expected_url = os.path.join(new_rootdir, 'opt', 'software.cfg')
+      expected_url = os.path.join(new_rootdir, 'opt', 'soft', 'software.cfg')
       checkUrl(initial_url, expected_url, new_rootdir)
       initial_url = expected_url
 
@@ -2149,8 +2151,8 @@ class TestLocalSoftwareReleaseRootPathMigration(MasterMixin):
 
   def test_move_logs(self):
     local_sr_root = os.path.join(self._rootdir, 'opt')
-    subpath_url = os.path.join(local_sr_root, 'software.cfg')
-    path_not_subpath_url = os.path.join(self._rootdir, 'srv', 'software.cfg')
+    subpath_url = os.path.join(local_sr_root, 'soft', 'software.cfg')
+    path_not_subpath_url = os.path.join(self._rootdir, 'srv', 'soft', 'software.cfg')
     http_url = "https://sr//"
 
     self.format_for_number_of_partitions(3)
@@ -2160,23 +2162,73 @@ class TestLocalSoftwareReleaseRootPathMigration(MasterMixin):
     self.moveProxy()
 
     new_local_sr_root = os.path.join(self._rootdir, 'opt')
-    new_subpath_url = os.path.join(new_local_sr_root, 'software.cfg')
+    new_subpath_url = os.path.join(new_local_sr_root, 'soft', 'software.cfg')
 
     with mock.patch.object(views.app, 'logger') as logger:
       # Request something to trigger update
       self.getFullComputerInformation()
 
-    logger.info.assert_called_once_with(
-      'Updating local software release root path: %s --> %s',
-      local_sr_root,
-      new_local_sr_root,
-    )
+    logger.info.assert_has_calls([
+      mock.call('Backuping database to %s' , mock.ANY),
+      mock.call('Rebase URLs on local software release root path'),
+      mock.call('Old root path: %s', local_sr_root),
+      mock.call('New root path: %s', new_local_sr_root)
+    ])
+
+    backup_path = logger.info.call_args_list[0][0][1]
+    with open(backup_path) as f:
+      dump = f.read()
+      self.assertIn("CREATE TABLE", dump)
+      self.assertIn('INSERT INTO', dump)
+
     logger.debug.assert_has_calls([
-      mock.call('Migrate URL ? Y: %s -> %s', subpath_url, new_subpath_url),
-      mock.call('Migrate URL ? N: %s is not a subpath', path_not_subpath_url),
-      mock.call('Migrate URL ? N: %s is not a path', http_url)
+      mock.call('Examining URL %s', subpath_url),
+      mock.call('  Migrate to rebased URL %s', new_subpath_url),
+      mock.call('  Do not rebase because it is not a subpath of %s', local_sr_root),
+      mock.call('  Do not rebase because it is not a path')
     ]*2, any_order=True)
 
+  def checkSupplyAndRequestUrl(self, initial_url, expected_url, rootdir=None, localdir=None):
+    self.format_for_number_of_partitions(1)
+    self.supply(initial_url)
+    partition = self.request(initial_url, None, 'MyInstance', 'slappart0')
+    self.assertSoftwareUrls(initial_url)
+    self.assertPartitionUrl(partition._partition_id, initial_url)
+    self.moveProxy(rootdir, localdir)
+    self.assertSoftwareUrls(expected_url)
+    self.assertPartitionUrl(partition._partition_id, expected_url)
+
+  def test_move_to_subpath(self):
+    initial_url = os.path.join(self._rootdir, 'opt', 'soft', 'software.cfg')
+    new_localdir = os.path.join(self._rootdir, 'opt', 'soft')
+    self.checkSupplyAndRequestUrl(initial_url, initial_url, None, localdir=new_localdir)
+
+  def test_move_to_superpath(self):
+    initial_url = os.path.join(self._rootdir, 'opt', 'soft', 'software.cfg')
+    self.checkSupplyAndRequestUrl(initial_url, initial_url, None, localdir=self._rootdir)
+
+  def createSoftware(self, rootdir):
+    softdir = os.path.join(rootdir, 'opt', 'soft')
+    url = os.path.join(softdir, 'software.cfg')
+    os.makedirs(softdir)
+    with open(url, 'w'):
+      pass
+    return url
+
+  def test_move_initial_exists(self):
+    initial_url = self.createSoftware(self._rootdir)
+    new_rootdir = self.newRootDir()
+    new_localdir = os.path.join(new_rootdir, 'opt')
+    os.mkdir(new_rootdir)
+    self.checkSupplyAndRequestUrl(initial_url, initial_url, new_rootdir, new_localdir)
+
+  def test_move_both_exist(self):
+    initial_url = self.createSoftware(self._rootdir)
+    new_rootdir = self.newRootDir()
+    expected_url = self.createSoftware(new_rootdir)
+    new_localdir = os.path.join(new_rootdir, 'opt')
+    self.checkSupplyAndRequestUrl(initial_url, expected_url, new_rootdir, new_localdir)
+
 
 class _MigrationTestCase(TestInformation, TestRequest, TestSlaveRequest, TestMultiNodeSupport):
   """