Merge ~smoser/cloud-init:feature/ds-identify-test into cloud-init:master

Proposed by Scott Moser on 2017-04-24
Status: Merged
Merged at revision: 370a04e8d7b530c1ef8280e15eb628ff6880c736
Proposed branch: ~smoser/cloud-init:feature/ds-identify-test
Merge into: cloud-init:master
Diff against target: 384 lines (+313/-8)
3 files modified
tests/unittests/helpers.py (+7/-0)
tests/unittests/test_ds_identify.py (+290/-0)
tools/ds-identify (+16/-8)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve on 2017-05-10
Chad Smith Approve on 2017-05-09
cloud-init commiters 2017-04-24 Pending
Review via email: mp+323059@code.launchpad.net

Commit Message

Add unit tests for ds-identify, fix Ec2 bug found.

This adds several unit tests for ds-identify, and fixes a bug
in Ec2 detection that I found while writing these tests.

The method of testing is to use the ds-identify code as a shell
library. The TestDsIdentify:call basically does:

  * populate a (temp) directory with files that represent what
    ds-identify would see in /sys or other locations it reads.
  * create a file '_shwrap' that replaces the 3 programs that are executed
    in ds-identify code path. It supports setting their stdout, stderr,
    and exit code.
  * set the default policies explicitly (DI_DEFAULT_POLICY) so we can
    support testing different builtins. This is necessary because the
    Ubuntu branches patch the builtin value. If we did not explicilty set
    it, then testing there would fail.
  * execute sh to source the script and call its main.

To post a comment you must log in.
review: Needs Fixing (continuous-integration)
review: Needs Fixing (continuous-integration)
review: Needs Fixing (continuous-integration)

FAILED: Continuous integration, rev:9e2de035ab51831335f5c1c9b21058a6b5875c74
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/323059/+edit-commit-message

https://jenkins.ubuntu.com/server/job/cloud-init-ci/305/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/305/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/305/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/305/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/305/console

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

review: Needs Fixing (continuous-integration)

FAILED: Continuous integration, rev:ee51740e55f20b87e3f295ddb3249e6b1dbac94a
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/323059/+edit-commit-message

https://jenkins.ubuntu.com/server/job/cloud-init-ci/307/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/307
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/307
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/307
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/307/console

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

review: Needs Fixing (continuous-integration)
Ryan Harper (raharper) wrote :

This looks really good.

Would you be opposed to putting the infrastructure in one of the helper files and have only the inputs/unittests in the test_dsidentify class?

Scott Moser (smoser) :
Scott Moser (smoser) wrote :

with respect to the infrastructure to test this being elsewhere... most of it is specific to ds-identify, so I dont think it makes a lot of sense to hide it all away from the testing of ds-identify.

Chad Smith (chad.smith) wrote :

Initial comments added

Chad Smith (chad.smith) :
Scott Moser (smoser) :
Chad Smith (chad.smith) :
Chad Smith (chad.smith) wrote :

+1 thanks for the rework + docstrings.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
2index 90e2431..a711404 100644
3--- a/tests/unittests/helpers.py
4+++ b/tests/unittests/helpers.py
5@@ -3,6 +3,7 @@
6 from __future__ import print_function
7
8 import functools
9+import json
10 import os
11 import shutil
12 import sys
13@@ -280,6 +281,12 @@ def dir2dict(startdir, prefix=None):
14 return flist
15
16
17+def json_dumps(data):
18+ # print data in nicely formatted json.
19+ return json.dumps(data, indent=1, sort_keys=True,
20+ separators=(',', ': '))
21+
22+
23 def wrap_and_call(prefix, mocks, func, *args, **kwargs):
24 """
25 call func(args, **kwargs) with mocks applied, then unapplies mocks
26diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
27new file mode 100644
28index 0000000..9e14885
29--- /dev/null
30+++ b/tests/unittests/test_ds_identify.py
31@@ -0,0 +1,290 @@
32+# This file is part of cloud-init. See LICENSE file for license information.
33+
34+import copy
35+import os
36+from uuid import uuid4
37+
38+from cloudinit import safeyaml
39+from cloudinit import util
40+from .helpers import CiTestCase, dir2dict, json_dumps, populate_dir
41+
42+UNAME_MYSYS = ("Linux bart 4.4.0-62-generic #83-Ubuntu "
43+ "SMP Wed Jan 18 14:10:15 UTC 2017 x86_64 GNU/Linux")
44+BLKID_EFI_ROOT = """
45+DEVNAME=/dev/sda1
46+UUID=8B36-5390
47+TYPE=vfat
48+PARTUUID=30d7c715-a6ae-46ee-b050-afc6467fc452
49+
50+DEVNAME=/dev/sda2
51+UUID=19ac97d5-6973-4193-9a09-2e6bbfa38262
52+TYPE=ext4
53+PARTUUID=30c65c77-e07d-4039-b2fb-88b1fb5fa1fc
54+"""
55+
56+DI_DEFAULT_POLICY = "search,found=all,maybe=all,notfound=enabled"
57+DI_DEFAULT_POLICY_NO_DMI = "search,found=all,maybe=all,notfound=disabled"
58+
59+SHELL_MOCK_TMPL = """\
60+%(name)s() {
61+ local out='%(out)s' err='%(err)s' r='%(ret)s' RET='%(RET)s'
62+ [ "$out" = "_unset" ] || echo "$out"
63+ [ "$err" = "_unset" ] || echo "$err" 2>&1
64+ [ "$RET" = "_unset" ] || _RET="$RET"
65+ return $r
66+}
67+"""
68+
69+RC_FOUND = 0
70+RC_NOT_FOUND = 1
71+DS_NONE = 'None'
72+
73+P_PRODUCT_NAME = "sys/class/dmi/id/product_name"
74+P_PRODUCT_SERIAL = "sys/class/dmi/id/product_serial"
75+P_PRODUCT_UUID = "sys/class/dmi/id/product_uuid"
76+P_DSID_CFG = "etc/cloud/ds-identify.cfg"
77+
78+MOCK_VIRT_IS_KVM = {'name': 'detect_virt', 'RET': 'kvm', 'ret': 0}
79+
80+
81+class TestDsIdentify(CiTestCase):
82+ dsid_path = os.path.realpath('tools/ds-identify')
83+
84+ def call(self, rootd=None, mocks=None, args=None, files=None,
85+ policy_dmi=DI_DEFAULT_POLICY,
86+ policy_nodmi=DI_DEFAULT_POLICY_NO_DMI):
87+ if args is None:
88+ args = []
89+ if mocks is None:
90+ mocks = []
91+
92+ if files is None:
93+ files = {}
94+
95+ if rootd is None:
96+ rootd = self.tmp_dir()
97+
98+ unset = '_unset'
99+ wrap = self.tmp_path(path="_shwrap", dir=rootd)
100+ populate_dir(rootd, files)
101+
102+ # DI_DEFAULT_POLICY* are declared always as to not rely
103+ # on the default in the code. This is because SRU releases change
104+ # the value in the code, and thus tests would fail there.
105+ head = [
106+ "DI_MAIN=noop",
107+ "DEBUG_LEVEL=2",
108+ "DI_LOG=stderr",
109+ "PATH_ROOT='%s'" % rootd,
110+ ". " + self.dsid_path,
111+ 'DI_DEFAULT_POLICY="%s"' % policy_dmi,
112+ 'DI_DEFAULT_POLICY_NO_DMI="%s"' % policy_nodmi,
113+ ""
114+ ]
115+
116+ def write_mock(data):
117+ ddata = {'out': None, 'err': None, 'ret': 0, 'RET': None}
118+ ddata.update(data)
119+ for k in ddata:
120+ if ddata[k] is None:
121+ ddata[k] = unset
122+ return SHELL_MOCK_TMPL % ddata
123+
124+ mocklines = []
125+ defaults = [
126+ {'name': 'detect_virt', 'RET': 'none', 'ret': 1},
127+ {'name': 'uname', 'out': UNAME_MYSYS},
128+ {'name': 'blkid', 'out': BLKID_EFI_ROOT},
129+ ]
130+
131+ written = [d['name'] for d in mocks]
132+ for data in mocks:
133+ mocklines.append(write_mock(data))
134+ for d in defaults:
135+ if d['name'] not in written:
136+ mocklines.append(write_mock(d))
137+
138+ endlines = [
139+ 'main %s' % ' '.join(['"%s"' % s for s in args])
140+ ]
141+
142+ with open(wrap, "w") as fp:
143+ fp.write('\n'.join(head + mocklines + endlines) + "\n")
144+
145+ rc = 0
146+ try:
147+ out, err = util.subp(['sh', '-c', '. %s' % wrap], capture=True)
148+ except util.ProcessExecutionError as e:
149+ rc = e.exit_code
150+ out = e.stdout
151+ err = e.stderr
152+
153+ cfg = None
154+ cfg_out = os.path.join(rootd, 'run/cloud-init/cloud.cfg')
155+ if os.path.exists(cfg_out):
156+ contents = util.load_file(cfg_out)
157+ try:
158+ cfg = safeyaml.load(contents)
159+ except Exception as e:
160+ cfg = {"_INVALID_YAML": contents,
161+ "_EXCEPTION": str(e)}
162+
163+ return rc, out, err, cfg, dir2dict(rootd)
164+
165+ def _call_via_dict(self, data, rootd=None, **kwargs):
166+ # return output of self.call with a dict input like VALID_CFG[item]
167+ xwargs = {'rootd': rootd}
168+ for k in ('mocks', 'args', 'policy_dmi', 'policy_nodmi', 'files'):
169+ if k in data:
170+ xwargs[k] = data[k]
171+ if k in kwargs:
172+ xwargs[k] = kwargs[k]
173+
174+ return self.call(**xwargs)
175+
176+ def _test_ds_found(self, name):
177+ data = copy.deepcopy(VALID_CFG[name])
178+ return self._check_via_dict(
179+ data, RC_FOUND, dslist=[data.get('ds'), DS_NONE])
180+
181+ def _check_via_dict(self, data, rc, dslist=None, **kwargs):
182+ found_rc, out, err, cfg, files = self._call_via_dict(data, **kwargs)
183+ good = False
184+ try:
185+ self.assertEqual(rc, found_rc)
186+ if dslist is not None:
187+ self.assertEqual(dslist, cfg['datasource_list'])
188+ good = True
189+ finally:
190+ if not good:
191+ _print_run_output(rc, out, err, cfg, files)
192+ return rc, out, err, cfg, files
193+
194+ def test_aws_ec2_hvm(self):
195+ """EC2: hvm instances use dmi serial and uuid starting with 'ec2'."""
196+ self._test_ds_found('Ec2-hvm')
197+
198+ def test_aws_ec2_xen(self):
199+ """EC2: sys/hypervisor/uuid starts with ec2."""
200+ self._test_ds_found('Ec2-xen')
201+
202+ def test_brightbox_is_ec2(self):
203+ """EC2: product_serial ends with 'brightbox.com'"""
204+ self._test_ds_found('Ec2-brightbox')
205+
206+ def test_gce_by_product_name(self):
207+ """GCE identifies itself with product_name."""
208+ self._test_ds_found('GCE')
209+
210+ def test_gce_by_serial(self):
211+ """Older gce compute instances must be identified by serial."""
212+ self._test_ds_found('GCE-serial')
213+
214+ def test_config_drive(self):
215+ """ConfigDrive datasource has a disk with LABEL=config-2."""
216+ self._test_ds_found('ConfigDrive')
217+ return
218+
219+ def test_policy_disabled(self):
220+ """A Builtin policy of 'disabled' should return not found.
221+
222+ Even though a search would find something, the builtin policy of
223+ disabled should cause the return of not found."""
224+ mydata = copy.deepcopy(VALID_CFG['Ec2-hvm'])
225+ self._check_via_dict(mydata, rc=RC_NOT_FOUND, policy_dmi="disabled")
226+
227+ def test_policy_config_disable_overrides_builtin(self):
228+ """explicit policy: disabled in config file should cause not found."""
229+ mydata = copy.deepcopy(VALID_CFG['Ec2-hvm'])
230+ mydata['files'][P_DSID_CFG] = '\n'.join(['policy: disabled', ''])
231+ self._check_via_dict(mydata, rc=RC_NOT_FOUND)
232+
233+ def test_single_entry_defines_datasource(self):
234+ """If config has a single entry in datasource_list, that is used.
235+
236+ Test the valid Ec2-hvm, but provide a config file that specifies
237+ a single entry in datasource_list. The configured value should
238+ be used."""
239+ mydata = copy.deepcopy(VALID_CFG['Ec2-hvm'])
240+ cfgpath = 'etc/cloud/cloud.cfg.d/myds.cfg'
241+ mydata['files'][cfgpath] = 'datasource_list: ["NoCloud"]\n'
242+ self._check_via_dict(mydata, rc=RC_FOUND, dslist=['NoCloud', DS_NONE])
243+
244+
245+def blkid_out(disks=None):
246+ """Convert a list of disk dictionaries into blkid content."""
247+ if disks is None:
248+ disks = []
249+ lines = []
250+ for disk in disks:
251+ if not disk["DEVNAME"].startswith("/dev/"):
252+ disk["DEVNAME"] = "/dev/" + disk["DEVNAME"]
253+ for key in disk:
254+ lines.append("%s=%s" % (key, disk[key]))
255+ lines.append("")
256+ return '\n'.join(lines)
257+
258+
259+def _print_run_output(rc, out, err, cfg, files):
260+ """A helper to print return of TestDsIdentify.
261+
262+ _print_run_output(self.call())"""
263+ print('\n'.join([
264+ '-- rc = %s --' % rc,
265+ '-- out --', str(out),
266+ '-- err --', str(err),
267+ '-- cfg --', json_dumps(cfg)]))
268+ print('-- files --')
269+ for k, v in files.items():
270+ if "/_shwrap" in k:
271+ continue
272+ print(' === %s ===' % k)
273+ for line in v.splitlines():
274+ print(" " + line)
275+
276+
277+VALID_CFG = {
278+ 'Ec2-hvm': {
279+ 'ds': 'Ec2',
280+ 'mocks': [{'name': 'detect_virt', 'RET': 'kvm', 'ret': 0}],
281+ 'files': {
282+ P_PRODUCT_SERIAL: 'ec23aef5-54be-4843-8d24-8c819f88453e\n',
283+ P_PRODUCT_UUID: 'EC23AEF5-54BE-4843-8D24-8C819F88453E\n',
284+ }
285+ },
286+ 'Ec2-xen': {
287+ 'ds': 'Ec2',
288+ 'mocks': [{'name': 'detect_virt', 'RET': 'xen', 'ret': 0}],
289+ 'files': {
290+ 'sys/hypervisor/uuid': 'ec2c6e2f-5fac-4fc7-9c82-74127ec14bbb\n'
291+ },
292+ },
293+ 'Ec2-brightbox': {
294+ 'ds': 'Ec2',
295+ 'files': {P_PRODUCT_SERIAL: 'facc6e2f.brightbox.com\n'},
296+ },
297+ 'GCE': {
298+ 'ds': 'GCE',
299+ 'files': {P_PRODUCT_NAME: 'Google Compute Engine\n'},
300+ 'mocks': [MOCK_VIRT_IS_KVM],
301+ },
302+ 'GCE-serial': {
303+ 'ds': 'GCE',
304+ 'files': {P_PRODUCT_SERIAL: 'GoogleCloud-8f2e88f\n'},
305+ 'mocks': [MOCK_VIRT_IS_KVM],
306+ },
307+ 'ConfigDrive': {
308+ 'ds': 'ConfigDrive',
309+ 'mocks': [
310+ {'name': 'blkid', 'ret': 0,
311+ 'out': blkid_out(
312+ [{'DEVNAME': 'vda1', 'TYPE': 'vfat', 'PARTUUID': uuid4()},
313+ {'DEVNAME': 'vda2', 'TYPE': 'ext4',
314+ 'LABEL': 'cloudimg-rootfs', 'PARTUUID': uuid4()},
315+ {'DEVNAME': 'vdb', 'TYPE': 'vfat', 'LABEL': 'config-2'}])
316+ },
317+ ],
318+ },
319+}
320+
321+# vi: ts=4 expandtab
322diff --git a/tools/ds-identify b/tools/ds-identify
323index a43b129..aff26eb 100755
324--- a/tools/ds-identify
325+++ b/tools/ds-identify
326@@ -216,9 +216,8 @@ has_cdrom() {
327 [ -e "${PATH_ROOT}/dev/cdrom" ]
328 }
329
330-read_virt() {
331- cached "$DI_VIRT" && return 0
332- local out="" r="" virt="${UNAVAILABLE}"
333+detect_virt() {
334+ local virt="${UNAVAILABLE}" r="" out=""
335 if [ -d /run/systemd ]; then
336 out=$(systemd-detect-virt 2>&1)
337 r=$?
338@@ -226,7 +225,13 @@ read_virt() {
339 virt="$out"
340 fi
341 fi
342- DI_VIRT=$virt
343+ _RET="$virt"
344+}
345+
346+read_virt() {
347+ cached "$DI_VIRT" && return 0
348+ detect_virt
349+ DI_VIRT=${_RET}
350 }
351
352 is_container() {
353@@ -318,8 +323,11 @@ parse_yaml_array() {
354 # ['1'] or [1]
355 # '1', '2'
356 local val="$1" oifs="$IFS" ret="" tok=""
357- val=${val#[}
358- val=${val%]}
359+ # i386/14.04 (dash=0.5.7-4ubuntu1): the following outputs "[foo"
360+ # sh -c 'n="$1"; echo ${n#[}' -- "[foo"
361+ # the fix was to quote the open bracket (val=${val#"["}) (LP: #1689648)
362+ val=${val#"["}
363+ val=${val%"]"}
364 IFS=","; set -- $val; IFS="$oifs"
365 for tok in "$@"; do
366 trim "$tok"
367@@ -714,7 +722,7 @@ ec2_identify_platform() {
368
369 # AWS http://docs.aws.amazon.com/AWSEC2/
370 # latest/UserGuide/identify_ec2_instances.html
371- local uuid="" hvuuid="$PATH_ROOT/sys/hypervisor/uuid"
372+ local uuid="" hvuuid="${PATH_SYS_HYPERVISOR}/uuid"
373 # if the (basically) xen specific /sys/hypervisor/uuid starts with 'ec2'
374 if [ -r "$hvuuid" ] && read uuid < "$hvuuid" &&
375 [ "${uuid#ec2}" != "$uuid" ]; then
376@@ -725,7 +733,7 @@ ec2_identify_platform() {
377 # product uuid and product serial start with case insensitive
378 local uuid="${DI_DMI_PRODUCT_UUID}"
379 case "$uuid:$serial" in
380- [Ee][Cc]2*:[Ee][Cc]2)
381+ [Ee][Cc]2*:[Ee][Cc]2*)
382 # both start with ec2, now check for case insenstive equal
383 nocase_equal "$uuid" "$serial" &&
384 { _RET="AWS"; return 0; };;

Subscribers

People subscribed via source and target branches

to all changes: