manager: Switch to lsblk devperm

lsblk OS inspection is the way to have realiable results for querying the
devices on the system, so fully switch to it, while still supporting symlinks.

Disks and partitions are supported.

slapos/manager/devperm.py

@@ -4,6 +4,7 @@ import logging
 import os
 import pwd
 import grp
+import subprocess
 from zope.interface import implementer
 from slapos.manager import interface
 
@@ -60,6 +61,40 @@ class Manager(object):
     """
     self.instanceTearDown(partition)
 
+  def _getLsblkJsonDict(self):
+    try:
+      lsblk_json_dict = json.loads(subprocess.check_output([
+        'lsblk', '--json', '--output-all']))
+    except Exception:
+      logger.info('lsblk call failed', exc_info=True)
+      return {}
+    if not isinstance(lsblk_json_dict, dict):
+      logger.info('lsblk output not supported')
+      return {}
+    return lsblk_json_dict
+
+  def _getLsblkDiskList(self):
+    lsblk_dict = self._getLsblkJsonDict()
+
+    if 'blockdevices' not in lsblk_dict:
+      logger.info('lsblk output not supported')
+      return []
+
+    if not isinstance(lsblk_dict['blockdevices'], list):
+      logger.info('lsblk output not supported')
+
+    disk_list = []
+    for block_device in lsblk_dict['blockdevices']:
+      if 'path' in block_device and 'type' in block_device:
+        if block_device['type'] == 'disk':
+          disk_list.append(block_device['path'])
+        if 'children' in block_device and isinstance(block_device['children'], list):
+          for partition in block_device['children']:
+            if 'path' in partition and 'type' in partition:
+              if partition['type'] == 'part':
+                disk_list.append(partition['path'])
+    return disk_list
+
   def instanceTearDown(self, partition):
     """Method  called after `slapos node instance` phase.
 
@@ -77,6 +112,7 @@ class Manager(object):
         logger.warning('Bad disk configuration file', exc_info=True)
         return
 
+    lsblk_disk_list = self._getLsblkDiskList()
     for entry in disk_list:
       disk = entry.get("disk", None)
       if disk is None:
@@ -97,16 +133,8 @@ class Manager(object):
           logger.warning('Disk %s not in allowed disk list %s', disk, ', '.join(self.allowed_disk_for_vm))
           continue
 
-      if not disk.startswith("/dev/"):
-        logger.warning("Bad disk definition: %s " % disk_list, exc_info=True)
-        continue
-
-      if len(disk[len("/dev/"):]) > 6:
-        logger.warning("Bad disk definition: %s " % disk_list, exc_info=True)
-        continue
-
-      if not os.path.exists(disk):
-        logger.warning("Disk don't exist: %s " % disk_list, exc_info=True)
+      if disk not in lsblk_disk_list:
+        logger.warning("Disk %r is not detected by lsblk list %r", disk, lsblk_disk_list)
         continue
 
       uid = os.stat(partition.instance_path).st_uid

slapos/tests/test_slapgrid.py

@@ -3148,9 +3148,21 @@ class TestSlapgridWithPortRedirection(MasterMixin, unittest.TestCase):
       self.assertNotIn('socat HTCPCP4-LISTEN:1234,fork HTCPCP4:127.0.0.1:4321', partition_supervisord_config)
 
 
-class TestSlapgridWithDevPerm(MasterMixin, unittest.TestCase):
+class TestSlapgridWithDevPermLsblk(MasterMixin, unittest.TestCase):
   config = {'manager_list': 'devperm'}
 
+  def _getLsblkJsonDict(self):
+    return {
+      "blockdevices": [{
+        "path": "/dev/tst",
+        "type": "disk",
+        "children": [{
+          "path": "/dev/toolong",
+          "type": "part"
+        }]
+      }]
+    }
+
   def setUpExpected(self):
     gid = grp.getgrnam("disk").gr_gid
     uid = os.stat(os.environ['HOME']).st_uid
@@ -3162,6 +3174,10 @@ class TestSlapgridWithDevPerm(MasterMixin, unittest.TestCase):
       ['/dev/tst', uid, gid],
       ['/dev/tst', uid, gid],
     ]
+    self.test_link_os_chown_call_list_long = [
+      ['/dev/toolong', uid, gid],
+      ['/dev/toolong', uid, gid],
+    ]
 
   def setUp(self):
     self.setUpExpected()
@@ -3183,7 +3199,8 @@ class TestSlapgridWithDevPerm(MasterMixin, unittest.TestCase):
       patch.object(os, 'stat', new=self.os_stat),
       patch.object(os, 'chown', new=self.os_chown),
       patch.object(os, 'readlink', new=self.os_readlink),
-      patch.object(os.path, 'islink', new=self.os_path_islink)
+      patch.object(os.path, 'islink', new=self.os_path_islink),
+      patch.object(slapmanager.devperm.Manager, '_getLsblkJsonDict', new=self._getLsblkJsonDict)
     ]
     [q.start() for q in self.patcher_list]
 
@@ -3202,7 +3219,7 @@ class TestSlapgridWithDevPerm(MasterMixin, unittest.TestCase):
       return original(path)
 
   def os_stat(self, path, original=os.stat):
-    if path == '/dev/tst':
+    if path in ['/dev/tst', '/dev/toolong']:
       class dummy():
         pass
       mocked = dummy()
@@ -3212,7 +3229,7 @@ class TestSlapgridWithDevPerm(MasterMixin, unittest.TestCase):
       return original(path)
 
   def os_chown(self, path, uid, gid, original=os.chown):
-    if path.startswith('/dev/tst'):
+    if path.startswith('/dev/tst') or path.startswith('/dev/toolong'):
       self.os_chown_call_list.append([path, uid, gid])
       return
     else:
@@ -3289,7 +3306,7 @@ class TestSlapgridWithDevPerm(MasterMixin, unittest.TestCase):
 
       self.assertEqual(
         self.os_chown_call_list,
-        []
+        self.test_link_os_chown_call_list_long
       )
 
   def test_not_existing(self):
@@ -3308,14 +3325,14 @@ class TestSlapgridWithDevPerm(MasterMixin, unittest.TestCase):
       )
 
 
-class TestSlapgridWithDevPermManagerDevPermEmpty(TestSlapgridWithDevPerm):
+class TestSlapgridWithDevPermManagerDevPermEmptyLsblk(TestSlapgridWithDevPermLsblk):
   config = {
     'manager_list': 'devperm',
     'manager': {'devperm': {}}
   }
 
 
-class TestSlapgridWithDevPermManagerDevPermListEmpty(TestSlapgridWithDevPerm):
+class TestSlapgridWithDevPermManagerDevPermListEmptyLsblk(TestSlapgridWithDevPermLsblk):
   config = {
     'manager_list': 'devperm',
     'manager': {'devperm': {'allowed-disk-for-vm': ''}}
@@ -3325,9 +3342,11 @@ class TestSlapgridWithDevPermManagerDevPermListEmpty(TestSlapgridWithDevPerm):
     ]
     self.test_link_os_chown_call_list = [
     ]
+    self.test_link_os_chown_call_list_long = [
+    ]
 
 
-class TestSlapgridWithDevPermManagerDevPermDisallow(TestSlapgridWithDevPerm):
+class TestSlapgridWithDevPermManagerDevPermDisallowLsblk(TestSlapgridWithDevPermLsblk):
   config = {
     'manager_list': 'devperm',
     'manager': {'devperm': {'allowed-disk-for-vm': '/dev/quo'}}
@@ -3337,13 +3356,28 @@ class TestSlapgridWithDevPermManagerDevPermDisallow(TestSlapgridWithDevPerm):
     ]
     self.test_link_os_chown_call_list = [
     ]
+    self.test_link_os_chown_call_list_long = [
+    ]
 
 
-class TestSlapgridWithDevPermManagerDevPermAllow(TestSlapgridWithDevPerm):
+class TestSlapgridWithDevPermManagerDevPermAllowLsblk(TestSlapgridWithDevPermLsblk):
   config = {
     'manager_list': 'devperm',
     'manager': {'devperm': {'allowed-disk-for-vm': '/dev/tst'}}
   }
+  def setUpExpected(self):
+    gid = grp.getgrnam("disk").gr_gid
+    uid = os.stat(os.environ['HOME']).st_uid
+    self.test_os_chown_call_list = [
+      ['/dev/tst', uid, gid],
+      ['/dev/tst', uid, gid],
+    ]
+    self.test_link_os_chown_call_list = [
+      ['/dev/tst', uid, gid],
+      ['/dev/tst', uid, gid],
+    ]
+    self.test_link_os_chown_call_list_long = [
+    ]
 
 
 class TestSlapgridManagerLifecycle(MasterMixin, unittest.TestCase):