BSD: find_devs_with_ refactoring (#298)

Refactoring of the `find_devs_with_*bsd()` methods:

- centralize everything in `util.py`
- add test coverage

4 files changed, 136 insertions, 32 deletions

diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py
index 71d5fc2e..72d86c41 100644
--- a/cloudinit/sources/DataSourceNoCloud.py
+++ b/cloudinit/sources/DataSourceNoCloud.py
@@ -36,27 +36,14 @@ class DataSourceNoCloud(sources.DataSource):
         return "%s [seed=%s][dsmode=%s]" % (root, self.seed, self.dsmode)
 
     def _get_devices(self, label):
-        if util.is_FreeBSD():
-            devlist = [
-                p for p in ['/dev/msdosfs/' + label, '/dev/iso9660/' + label]
-                if os.path.exists(p)]
-        elif util.is_NetBSD():
-            out, _err = util.subp(['sysctl', '-n', 'hw.disknames'], rcs=[0])
-            devlist = []
-            for dev in out.split():
-                mscdlabel_out, _ = util.subp(['mscdlabel', dev], rcs=[0, 1])
-                if ('label "%s"' % label) in mscdlabel_out:
-                    devlist.append('/dev/' + dev)
-                    devlist.append('/dev/' + dev + 'a')  # NetBSD 7
-        else:
-            fslist = util.find_devs_with("TYPE=vfat")
-            fslist.extend(util.find_devs_with("TYPE=iso9660"))
+        fslist = util.find_devs_with("TYPE=vfat")
+        fslist.extend(util.find_devs_with("TYPE=iso9660"))
 
-            label_list = util.find_devs_with("LABEL=%s" % label.upper())
-            label_list.extend(util.find_devs_with("LABEL=%s" % label.lower()))
+        label_list = util.find_devs_with("LABEL=%s" % label.upper())
+        label_list.extend(util.find_devs_with("LABEL=%s" % label.lower()))
 
-            devlist = list(set(fslist) & set(label_list))
-            devlist.sort(reverse=True)
+        devlist = list(set(fslist) & set(label_list))
+        devlist.sort(reverse=True)
         return devlist
 
     def _get_data(self):
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 818904b1..4cae7ec8 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1245,19 +1245,44 @@ def close_stdin():
         os.dup2(fp.fileno(), sys.stdin.fileno())
 
 
+def find_devs_with_freebsd(criteria=None, oformat='device',
+                           tag=None, no_cache=False, path=None):
+    if not criteria:
+        return glob.glob("/dev/msdosfs/*") + glob.glob("/dev/iso9660/*")
+    if criteria.startswith("LABEL="):
+        label = criteria.lstrip("LABEL=")
+        devlist = [
+            p for p in ['/dev/msdosfs/' + label, '/dev/iso9660/' + label]
+            if os.path.exists(p)]
+    elif criteria == "TYPE=vfat":
+        devlist = glob.glob("/dev/msdosfs/*")
+    elif criteria == "TYPE=iso9660":
+        devlist = glob.glob("/dev/iso9660/*")
+    return devlist
+
+
 def find_devs_with_netbsd(criteria=None, oformat='device',
                           tag=None, no_cache=False, path=None):
-    if not path:
-        path = "/dev/cd0"
-    cmd = ["mscdlabel", path]
-    out, _ = subp(cmd, capture=True, decode="replace", rcs=[0, 1])
-    result = out.split()
-    if result and len(result) > 2:
-        if criteria == "TYPE=iso9660" and "ISO" in result:
-            return [path]
-        if criteria == "LABEL=CONFIG-2" and '"config-2"' in result:
-            return [path]
-    return []
+    devlist = []
+    label = None
+    _type = None
+    if criteria:
+        if criteria.startswith("LABEL="):
+            label = criteria.lstrip("LABEL=")
+        if criteria.startswith("TYPE="):
+            _type = criteria.lstrip("TYPE=")
+    out, _err = subp(['sysctl', '-n', 'hw.disknames'], rcs=[0])
+    for dev in out.split():
+        if label or _type:
+            mscdlabel_out, _ = subp(['mscdlabel', dev], rcs=[0, 1])
+        if label and not ('label "%s"' % label) in mscdlabel_out:
+            continue
+        if _type == "iso9660" and "ISO filesystem" not in mscdlabel_out:
+            continue
+        if _type == "vfat" and "ISO filesystem" in mscdlabel_out:
+            continue
+        devlist.append('/dev/' + dev)
+    return devlist
 
 
 def find_devs_with_openbsd(criteria=None, oformat='device',
@@ -1290,7 +1315,10 @@ def find_devs_with(criteria=None, oformat='device',
       LABEL=<label>
       UUID=<uuid>
     """
-    if is_NetBSD():
+    if is_FreeBSD():
+        return find_devs_with_freebsd(criteria, oformat,
+                                      tag, no_cache, path)
+    elif is_NetBSD():
         return find_devs_with_netbsd(criteria, oformat,
                                      tag, no_cache, path)
     elif is_OpenBSD():
diff --git a/tests/unittests/test_datasource/test_nocloud.py b/tests/unittests/test_datasource/test_nocloud.py
index 18bea0b9..2e6b53ff 100644
--- a/tests/unittests/test_datasource/test_nocloud.py
+++ b/tests/unittests/test_datasource/test_nocloud.py
@@ -288,8 +288,23 @@ class TestNoCloudDataSource(CiTestCase):
         self.mocks.enter_context(
             mock.patch.object(util, 'is_FreeBSD', return_value=True))
 
+        def _mfind_devs_with_freebsd(
+                criteria=None, oformat='device',
+                tag=None, no_cache=False, path=None):
+            if not criteria:
+                return ["/dev/msdosfs/foo", "/dev/iso9660/foo"]
+            if criteria.startswith("LABEL="):
+                return ["/dev/msdosfs/foo", "/dev/iso9660/foo"]
+            elif criteria == "TYPE=vfat":
+                return ["/dev/msdosfs/foo"]
+            elif criteria == "TYPE=iso9660":
+                return ["/dev/iso9660/foo"]
+            return []
+
         self.mocks.enter_context(
-            mock.patch.object(os.path, 'exists', return_value=True))
+            mock.patch.object(
+                util, 'find_devs_with_freebsd',
+                side_effect=_mfind_devs_with_freebsd))
 
         dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths)
         ret = dsrc._get_devices('foo')
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index 2900997b..e45bedc1 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -1190,4 +1190,78 @@ def test_find_devs_with_openbsd_with_criteria(m_subp):
     assert devlist == ['/dev/cd0a']
 
 
+@mock.patch('glob.glob')
+def test_find_devs_with_freebsd(m_glob):
+    def fake_glob(pattern):
+        msdos = ["/dev/msdosfs/EFISYS"]
+        iso9660 = ["/dev/iso9660/config-2"]
+        if pattern == "/dev/msdosfs/*":
+            return msdos
+        elif pattern == "/dev/iso9660/*":
+            return iso9660
+        raise Exception
+    m_glob.side_effect = fake_glob
+
+    devlist = util.find_devs_with_freebsd()
+    assert set(devlist) == set([
+        '/dev/iso9660/config-2', '/dev/msdosfs/EFISYS'])
+    devlist = util.find_devs_with_freebsd(criteria="TYPE=iso9660")
+    assert devlist == ['/dev/iso9660/config-2']
+    devlist = util.find_devs_with_freebsd(criteria="TYPE=vfat")
+    assert devlist == ['/dev/msdosfs/EFISYS']
+
+
+@mock.patch("cloudinit.util.subp")
+def test_find_devs_with_netbsd(m_subp):
+    side_effect_values = [
+        ("ld0 dk0 dk1 cd0", ""),
+        (
+            (
+                "mscdlabel: CDIOREADTOCHEADER: "
+                "Inappropriate ioctl for device\n"
+                "track (ctl=4) at sector 0\n"
+                "disklabel not written\n"
+            ),
+            "",
+        ),
+        (
+            (
+                "mscdlabel: CDIOREADTOCHEADER: "
+                "Inappropriate ioctl for device\n"
+                "track (ctl=4) at sector 0\n"
+                "disklabel not written\n"
+            ),
+            "",
+        ),
+        (
+            (
+                "mscdlabel: CDIOREADTOCHEADER: "
+                "Inappropriate ioctl for device\n"
+                "track (ctl=4) at sector 0\n"
+                "disklabel not written\n"
+            ),
+            "",
+        ),
+        (
+            (
+                "track (ctl=4) at sector 0\n"
+                'ISO filesystem, label "config-2", '
+                "creation time: 2020/03/31 17:29\n"
+                "adding as 'a'\n"
+            ),
+            "",
+        ),
+    ]
+    m_subp.side_effect = side_effect_values
+    devlist = util.find_devs_with_netbsd()
+    assert set(devlist) == set(
+        ["/dev/ld0", "/dev/dk0", "/dev/dk1", "/dev/cd0"]
+    )
+    m_subp.side_effect = side_effect_values
+    devlist = util.find_devs_with_netbsd(criteria="TYPE=iso9660")
+    assert devlist == ["/dev/cd0"]
+    m_subp.side_effect = side_effect_values
+    devlist = util.find_devs_with_netbsd(criteria="TYPE=vfat")
+    assert devlist == ["/dev/ld0", "/dev/dk0", "/dev/dk1"]
+
 # vi: ts=4 expandtab