Merge ~smoser/cloud-init:feature/freebsd-variant into cloud-init:master

Proposed by Scott Moser
Status: Merged
Approved by: Chad Smith
Approved revision: c6af5b9a1648c208c82b3a2704668391abadd8ab
Merged at revision: ecb408afa1104fe49ce6eb1dc5708be56abd5cb2
Proposed branch: ~smoser/cloud-init:feature/freebsd-variant
Merge into: cloud-init:master
Diff against target: 273 lines (+40/-49)
8 files modified
cloudinit/config/cc_growpart.py (+1/-1)
cloudinit/config/cc_power_state_change.py (+1/-1)
cloudinit/sources/DataSourceAzure.py (+1/-1)
cloudinit/util.py (+19/-27)
config/cloud.cfg.tmpl (+9/-11)
tests/unittests/test_handler/test_handler_ntp.py (+1/-1)
tests/unittests/test_util.py (+6/-3)
tools/build-on-freebsd (+2/-4)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
cloud-init Commiters Pending
Review via email: mp+325760@code.launchpad.net

Commit message

freebsd: Make freebsd a variant, fix unittests and tools/build-on-freebsd.

 - Simplify the logic of 'variant' in util.system_info
   much of the data from
   https://github.com/hpcugent/easybuild/wiki/OS_flavor_name_version
 - fix get_resource_disk_on_freebsd when running on a system without
   an Azure resource disk.
 - fix tools/build-on-freebsd to replace oauth with oauthlib and add
   bash which is a dependency for tests.
 - update a fiew places that were checking for freebsd but not using
   the util.is_FreeBSD()

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

Looks pretty good, I'd like to see other unit tests specifically looking at return vals from util.system_info() but that can come in a later branch. Minor nits but code looks good.
I have no bsd system available otherwise I'd check it out with a couple of runs.

Revision history for this message
Scott Moser (smoser) :
Revision history for this message
Scott Moser (smoser) wrote :

actually, on further thought, lets leve suse out right now.
there is a 'sles' distro (cloudinit/distros/sles.py) but per
 https://github.com/hpcugent/easybuild/wiki/OS_flavor_name_version
we would see 'suse' as the dist.

not sure what the right change is.

i didn't *change* any behavior here i dont think.
so i'd rather leave it is and just enable the freebsd.

and we can/should fix suse/sles later.

Revision history for this message
Chad Smith (chad.smith) wrote :

+1 on that suggestion

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/config/cc_growpart.py b/cloudinit/config/cc_growpart.py
2index d2bc6e6..bafca9d 100644
3--- a/cloudinit/config/cc_growpart.py
4+++ b/cloudinit/config/cc_growpart.py
5@@ -214,7 +214,7 @@ def device_part_info(devpath):
6
7 # FreeBSD doesn't know of sysfs so just get everything we need from
8 # the device, like /dev/vtbd0p2.
9- if util.system_info()["platform"].startswith('FreeBSD'):
10+ if util.is_FreeBSD():
11 m = re.search('^(/dev/.+)p([0-9])$', devpath)
12 return (m.group(1), m.group(2))
13
14diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py
15index c1c6fe7..eba58b0 100644
16--- a/cloudinit/config/cc_power_state_change.py
17+++ b/cloudinit/config/cc_power_state_change.py
18@@ -71,7 +71,7 @@ def givecmdline(pid):
19 # Example output from procstat -c 1
20 # PID COMM ARGS
21 # 1 init /bin/init --
22- if util.system_info()["platform"].startswith('FreeBSD'):
23+ if util.is_FreeBSD():
24 (output, _err) = util.subp(['procstat', '-c', str(pid)])
25 line = output.splitlines()[1]
26 m = re.search('\d+ (\w|\.|-)+\s+(/\w.+)', line)
27diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
28index 71e7c55..4fe0d63 100644
29--- a/cloudinit/sources/DataSourceAzure.py
30+++ b/cloudinit/sources/DataSourceAzure.py
31@@ -101,7 +101,7 @@ def get_dev_storvsc_sysctl():
32 sysctl_out, err = util.subp(['sysctl', 'dev.storvsc'])
33 except util.ProcessExecutionError:
34 LOG.debug("Fail to execute sysctl dev.storvsc")
35- return None
36+ sysctl_out = ""
37 return sysctl_out
38
39
40diff --git a/cloudinit/util.py b/cloudinit/util.py
41index ec68925..c93b6d7 100644
42--- a/cloudinit/util.py
43+++ b/cloudinit/util.py
44@@ -573,7 +573,7 @@ def is_ipv4(instr):
45
46
47 def is_FreeBSD():
48- return system_info()['platform'].startswith('FreeBSD')
49+ return system_info()['variant'] == "freebsd"
50
51
52 def get_cfg_option_bool(yobj, key, default=False):
53@@ -598,37 +598,29 @@ def get_cfg_option_int(yobj, key, default=0):
54 def system_info():
55 info = {
56 'platform': platform.platform(),
57+ 'system': platform.system(),
58 'release': platform.release(),
59 'python': platform.python_version(),
60 'uname': platform.uname(),
61- 'dist': platform.linux_distribution(), # pylint: disable=W1505
62+ 'dist': platform.dist(), # pylint: disable=W1505
63 }
64- plat = info['platform'].lower()
65- # Try to get more info about what it actually is, in a format
66- # that we can easily use across linux and variants...
67- if plat.startswith('darwin'):
68- info['variant'] = 'darwin'
69- elif plat.endswith("bsd"):
70- info['variant'] = 'bsd'
71- elif plat.startswith('win'):
72- info['variant'] = 'windows'
73- elif 'linux' in plat:
74- # Try to get a single string out of these...
75- linux_dist, _version, _id = info['dist']
76- linux_dist = linux_dist.lower()
77- if linux_dist in ('ubuntu', 'linuxmint', 'mint'):
78- info['variant'] = 'ubuntu'
79+ system = info['system'].lower()
80+ var = 'unknown'
81+ if system == "linux":
82+ linux_dist = info['dist'][0].lower()
83+ if linux_dist in ('centos', 'fedora', 'debian'):
84+ var = linux_dist
85+ elif linux_dist in ('ubuntu', 'linuxmint', 'mint'):
86+ var = 'ubuntu'
87+ elif linux_dist == 'redhat':
88+ var = 'rhel'
89 else:
90- for prefix, variant in [('redhat', 'rhel'),
91- ('centos', 'centos'),
92- ('fedora', 'fedora'),
93- ('debian', 'debian')]:
94- if linux_dist.startswith(prefix):
95- info['variant'] = variant
96- if 'variant' not in info:
97- info['variant'] = 'linux'
98- if 'variant' not in info:
99- info['variant'] = 'unknown'
100+ var = 'linux'
101+ elif system in ('windows', 'darwin', "freebsd"):
102+ var = system
103+
104+ info['variant'] = var
105+
106 return info
107
108
109diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl
110index 5af2a88..f4b9069 100644
111--- a/config/cloud.cfg.tmpl
112+++ b/config/cloud.cfg.tmpl
113@@ -2,7 +2,7 @@
114 # The top level settings are used as module
115 # and system configuration.
116
117-{% if variant in ["bsd"] %}
118+{% if variant in ["freebsd"] %}
119 syslog_fix_perms: root:wheel
120 {% endif %}
121 # A set of users which may be applied and/or used by various modules
122@@ -13,7 +13,7 @@ users:
123
124 # If this is set, 'root' will not be able to ssh in and they
125 # will get a message to login instead as the default $user
126-{% if variant in ["bsd"] %}
127+{% if variant in ["freebsd"] %}
128 disable_root: false
129 {% else %}
130 disable_root: true
131@@ -30,7 +30,7 @@ ssh_pwauth: 0
132 # This will cause the set+update hostname module to not operate (if true)
133 preserve_hostname: false
134
135-{% if variant in ["bsd"] %}
136+{% if variant in ["freebsd"] %}
137 # This should not be required, but leave it in place until the real cause of
138 # not beeing able to find -any- datasources is resolved.
139 datasource_list: ['ConfigDrive', 'Azure', 'OpenStack', 'Ec2']
140@@ -53,13 +53,13 @@ cloud_init_modules:
141 - write-files
142 - growpart
143 - resizefs
144-{% if variant not in ["bsd"] %}
145+{% if variant not in ["freebsd"] %}
146 - disk_setup
147 - mounts
148 {% endif %}
149 - set_hostname
150 - update_hostname
151-{% if variant not in ["bsd"] %}
152+{% if variant not in ["freebsd"] %}
153 - update_etc_hosts
154 - ca-certs
155 - rsyslog
156@@ -87,7 +87,7 @@ cloud_config_modules:
157 - apt-pipelining
158 - apt-configure
159 {% endif %}
160-{% if variant not in ["bsd"] %}
161+{% if variant not in ["freebsd"] %}
162 - ntp
163 {% endif %}
164 - timezone
165@@ -108,7 +108,7 @@ cloud_final_modules:
166 - landscape
167 - lxd
168 {% endif %}
169-{% if variant not in ["bsd"] %}
170+{% if variant not in ["freebsd"] %}
171 - puppet
172 - chef
173 - salt-minion
174@@ -130,10 +130,8 @@ cloud_final_modules:
175 # (not accessible to handlers/transforms)
176 system_info:
177 # This will affect which distro class gets used
178-{% if variant in ["centos", "debian", "fedora", "rhel", "ubuntu"] %}
179+{% if variant in ["centos", "debian", "fedora", "rhel", "ubuntu", "freebsd"] %}
180 distro: {{ variant }}
181-{% elif variant in ["bsd"] %}
182- distro: freebsd
183 {% else %}
184 # Unknown/fallback distro.
185 distro: ubuntu
186@@ -182,7 +180,7 @@ system_info:
187 cloud_dir: /var/lib/cloud/
188 templates_dir: /etc/cloud/templates/
189 ssh_svcname: sshd
190-{% elif variant in ["bsd"] %}
191+{% elif variant in ["freebsd"] %}
192 # Default user name + that default users groups (if added/used)
193 default_user:
194 name: freebsd
195diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
196index c4299d9..7f27864 100644
197--- a/tests/unittests/test_handler/test_handler_ntp.py
198+++ b/tests/unittests/test_handler/test_handler_ntp.py
199@@ -62,7 +62,7 @@ class TestNtp(FilesystemMockingTestCase):
200 def test_ntp_rename_ntp_conf(self):
201 """When NTP_CONF exists, rename_ntp moves it."""
202 ntpconf = self.tmp_path("ntp.conf", self.new_root)
203- os.mknod(ntpconf)
204+ util.write_file(ntpconf, "")
205 with mock.patch("cloudinit.config.cc_ntp.NTP_CONF", ntpconf):
206 cc_ntp.rename_ntp_conf()
207 self.assertFalse(os.path.exists(ntpconf))
208diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
209index 014aa6a..a73fd26 100644
210--- a/tests/unittests/test_util.py
211+++ b/tests/unittests/test_util.py
212@@ -20,6 +20,9 @@ except ImportError:
213 import mock
214
215
216+BASH = util.which('bash')
217+
218+
219 class FakeSelinux(object):
220
221 def __init__(self, match_what):
222@@ -544,17 +547,17 @@ class TestReadSeeded(helpers.TestCase):
223
224 class TestSubp(helpers.TestCase):
225
226- stdin2err = ['bash', '-c', 'cat >&2']
227+ stdin2err = [BASH, '-c', 'cat >&2']
228 stdin2out = ['cat']
229 utf8_invalid = b'ab\xaadef'
230 utf8_valid = b'start \xc3\xa9 end'
231 utf8_valid_2 = b'd\xc3\xa9j\xc8\xa7'
232- printenv = ['bash', '-c', 'for n in "$@"; do echo "$n=${!n}"; done', '--']
233+ printenv = [BASH, '-c', 'for n in "$@"; do echo "$n=${!n}"; done', '--']
234
235 def printf_cmd(self, *args):
236 # bash's printf supports \xaa. So does /usr/bin/printf
237 # but by using bash, we remove dependency on another program.
238- return(['bash', '-c', 'printf "$@"', 'printf'] + list(args))
239+ return([BASH, '-c', 'printf "$@"', 'printf'] + list(args))
240
241 def test_subp_handles_utf8(self):
242 # The given bytes contain utf-8 accented characters as seen in e.g.
243diff --git a/tools/build-on-freebsd b/tools/build-on-freebsd
244index ccc10b4..ff9153a 100755
245--- a/tools/build-on-freebsd
246+++ b/tools/build-on-freebsd
247@@ -8,6 +8,7 @@ fail() { echo "FAILED:" "$@" 1>&2; exit 1; }
248 # Check dependencies:
249 depschecked=/tmp/c-i.dependencieschecked
250 pkgs="
251+ bash
252 dmidecode
253 e2fsprogs
254 py27-Jinja2
255@@ -16,7 +17,7 @@ pkgs="
256 py27-configobj
257 py27-jsonpatch
258 py27-jsonpointer
259- py27-oauth
260+ py27-oauthlib
261 py27-prettytable
262 py27-requests
263 py27-serial
264@@ -35,9 +36,6 @@ touch $depschecked
265 python setup.py build
266 python setup.py install -O1 --skip-build --prefix /usr/local/ --init-system sysvinit_freebsd
267
268-# Install the correct config file:
269-cp config/cloud.cfg-freebsd /etc/cloud/cloud.cfg
270-
271 # Enable cloud-init in /etc/rc.conf:
272 sed -i.bak -e "/cloudinit_enable=.*/d" /etc/rc.conf
273 echo 'cloudinit_enable="YES"' >> /etc/rc.conf

Subscribers

People subscribed via source and target branches