Merge ~goneri/cloud-init:freebsd_nocloud into cloud-init:master

Proposed by Gonéri Le Bouder on 2019-04-07
Status: Merged
Approved by: Chad Smith on 2019-05-24
Approved revision: df7f3963cc86811a7d166243e913fc1c4833ad8f
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~goneri/cloud-init:freebsd_nocloud
Merge into: cloud-init:master
Diff against target: 108 lines (+43/-19)
3 files modified
cloudinit/sources/DataSourceNoCloud.py (+23/-17)
config/cloud.cfg.tmpl (+2/-2)
tests/unittests/test_datasource/test_nocloud.py (+18/-0)
Reviewer Review Type Date Requested Status
Chad Smith 2019-04-07 Approve on 2019-05-24
Server Team CI bot continuous-integration Approve on 2019-05-24
Review via email: mp+365630@code.launchpad.net

Commit message

freebsd: NoCloud data source support

blkid is a Linux-only command. With this patch, cloud-init uses another
approach to find the data source on FreeBSD.

LP: #1645824

To post a comment you must log in.
do3meli (d-info-e) wrote :

it might be useful to have some test cases that cover the linux way using blkid and having a case that covers the FreeBSD way so future addition/changes to the data source don't break things up again without noticing.

apart from that - looking at the code diff without really testing it - the change looks reasonable to me.

Gonéri Le Bouder (goneri) wrote :

Done.

do3meli (d-info-e) wrote :

added 2 comments - don't understand that in detail yet...

Gonéri Le Bouder (goneri) :
Chad Smith (chad.smith) wrote :

Thanks for this branch Gonéri. Minor nits from me, but otherwise looks good.

Please reset 'Needs review' status when you get a chance to address or resolve comments.

Gonéri Le Bouder (goneri) wrote :

Done.

Chad Smith (chad.smith) :
Chad Smith (chad.smith) :

PASSED: Continuous integration, rev:df7f3963cc86811a7d166243e913fc1c4833ad8f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/729/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/729/rebuild

review: Approve (continuous-integration)
Chad Smith (chad.smith) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py
2index fcf5d58..8a9e5dd 100644
3--- a/cloudinit/sources/DataSourceNoCloud.py
4+++ b/cloudinit/sources/DataSourceNoCloud.py
5@@ -35,6 +35,26 @@ class DataSourceNoCloud(sources.DataSource):
6 root = sources.DataSource.__str__(self)
7 return "%s [seed=%s][dsmode=%s]" % (root, self.seed, self.dsmode)
8
9+ def _get_devices(self, label):
10+ if util.is_FreeBSD():
11+ devlist = [
12+ p for p in ['/dev/msdosfs/' + label, '/dev/iso9660/' + label]
13+ if os.path.exists(p)]
14+ else:
15+ # Query optical drive to get it in blkid cache for 2.6 kernels
16+ util.find_devs_with(path="/dev/sr0")
17+ util.find_devs_with(path="/dev/sr1")
18+
19+ fslist = util.find_devs_with("TYPE=vfat")
20+ fslist.extend(util.find_devs_with("TYPE=iso9660"))
21+
22+ label_list = util.find_devs_with("LABEL=%s" % label.upper())
23+ label_list.extend(util.find_devs_with("LABEL=%s" % label.lower()))
24+
25+ devlist = list(set(fslist) & set(label_list))
26+ devlist.sort(reverse=True)
27+ return devlist
28+
29 def _get_data(self):
30 defaults = {
31 "instance-id": "nocloud",
32@@ -99,20 +119,7 @@ class DataSourceNoCloud(sources.DataSource):
33
34 label = self.ds_cfg.get('fs_label', "cidata")
35 if label is not None:
36- # Query optical drive to get it in blkid cache for 2.6 kernels
37- util.find_devs_with(path="/dev/sr0")
38- util.find_devs_with(path="/dev/sr1")
39-
40- fslist = util.find_devs_with("TYPE=vfat")
41- fslist.extend(util.find_devs_with("TYPE=iso9660"))
42-
43- label_list = util.find_devs_with("LABEL=%s" % label.upper())
44- label_list.extend(util.find_devs_with("LABEL=%s" % label.lower()))
45-
46- devlist = list(set(fslist) & set(label_list))
47- devlist.sort(reverse=True)
48-
49- for dev in devlist:
50+ for dev in self._get_devices(label):
51 try:
52 LOG.debug("Attempting to use data from %s", dev)
53
54@@ -120,9 +127,8 @@ class DataSourceNoCloud(sources.DataSource):
55 seeded = util.mount_cb(dev, _pp2d_callback,
56 pp2d_kwargs)
57 except ValueError:
58- if dev in label_list:
59- LOG.warning("device %s with label=%s not a"
60- "valid seed.", dev, label)
61+ LOG.warning("device %s with label=%s not a"
62+ "valid seed.", dev, label)
63 continue
64
65 mydata = _merge_new_seed(mydata, seeded)
66diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl
67index 25db43e..684c747 100644
68--- a/config/cloud.cfg.tmpl
69+++ b/config/cloud.cfg.tmpl
70@@ -32,8 +32,8 @@ preserve_hostname: false
71
72 {% if variant in ["freebsd"] %}
73 # This should not be required, but leave it in place until the real cause of
74-# not beeing able to find -any- datasources is resolved.
75-datasource_list: ['ConfigDrive', 'Azure', 'OpenStack', 'Ec2']
76+# not finding -any- datasources is resolved.
77+datasource_list: ['NoCloud', 'ConfigDrive', 'Azure', 'OpenStack', 'Ec2']
78 {% endif %}
79 # Example datasource config
80 # datasource:
81diff --git a/tests/unittests/test_datasource/test_nocloud.py b/tests/unittests/test_datasource/test_nocloud.py
82index b785362..18bea0b 100644
83--- a/tests/unittests/test_datasource/test_nocloud.py
84+++ b/tests/unittests/test_datasource/test_nocloud.py
85@@ -278,6 +278,24 @@ class TestNoCloudDataSource(CiTestCase):
86 self.assertEqual(netconf, dsrc.network_config)
87 self.assertNotIn(gateway, str(dsrc.network_config))
88
89+ @mock.patch("cloudinit.util.blkid")
90+ def test_nocloud_get_devices_freebsd(self, m_is_lxd, fake_blkid):
91+ populate_dir(os.path.join(self.paths.seed_dir, "nocloud"),
92+ {'user-data': b"ud", 'meta-data': "instance-id: IID\n"})
93+
94+ sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}}
95+
96+ self.mocks.enter_context(
97+ mock.patch.object(util, 'is_FreeBSD', return_value=True))
98+
99+ self.mocks.enter_context(
100+ mock.patch.object(os.path, 'exists', return_value=True))
101+
102+ dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths)
103+ ret = dsrc._get_devices('foo')
104+ self.assertEqual(['/dev/msdosfs/foo', '/dev/iso9660/foo'], ret)
105+ fake_blkid.assert_not_called()
106+
107
108 class TestParseCommandLineData(CiTestCase):
109

Subscribers

People subscribed via source and target branches