Merge ~alexmurray/ubuntu-security-tools:cmd-qrt into ubuntu-security-tools:master
- Git
- lp:~alexmurray/ubuntu-security-tools
- cmd-qrt
- Merge into master
Status: | Merged |
---|---|
Merged at revision: | aa982b0163d74e9bf82634768a2abf2b5796cbcf |
Proposed branch: | ~alexmurray/ubuntu-security-tools:cmd-qrt |
Merge into: | ubuntu-security-tools:master |
Diff against target: |
288 lines (+200/-15) 2 files modified
build-tools/umt (+199/-14) build-tools/umt-completion.bash (+1/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Emilia Torino | Approve | ||
Eduardo Barretto | Approve | ||
Review via email: mp+376398@code.launchpad.net |
Commit message
Description of the change
Marc Deslauriers (mdeslaur) : | # |
Marc Deslauriers (mdeslaur) : | # |
Alex Murray (alexmurray) wrote : | # |
Emilia Torino (emitorino) wrote : | # |
This is great Alex! I added 2 inline comments with just refactoring suggestions + one question. Thanks!
Eduardo Barretto (ebarretto) wrote : | # |
Adding a comment to Emi's suggestion as discussed on IRC.
Eduardo Barretto (ebarretto) wrote : | # |
sorry for the spam, my comment didn't work, commenting again!
Alex Murray (alexmurray) wrote : | # |
emitorino + ebarretto - I just pushed https:/
Alex Murray (alexmurray) wrote : | # |
Whoops - accidentally left in too much old code - fixed in subsequent commit https:/
Steve Beattie (sbeattie) wrote : | # |
I haven't looked at the implementation, so no real comment on that, but I did look briefly and tried to guess how it would be used; can we call the option 'test' rather than 'qrt', since the qrt tree is an implementation detail?
Emilia Torino (emitorino) : | # |
Alex Murray (alexmurray) wrote : | # |
> I haven't looked at the implementation, so no real comment on that, but I did
> look briefly and tried to guess how it would be used; can we call the option
> 'test' rather than 'qrt', since the qrt tree is an implementation detail?
Well I want to add autopkgtest support as well and so was thinking it was easier to add both as separate sub-commands, otherwise if there was a single 'test' command this would have to have multiple mutually incompatible options depending on whether it ran qrt or autopkgtest etc - so to keep it simpler, I figure it is better to have separate qrt and autopkgtest commands.
Preview Diff
1 | diff --git a/build-tools/umt b/build-tools/umt | |||
2 | index 5aea099..c8939bc 100755 | |||
3 | --- a/build-tools/umt | |||
4 | +++ b/build-tools/umt | |||
5 | @@ -36,6 +36,7 @@ binary_dest = '../binary' | |||
6 | 36 | reports_dest = '../reports' | 36 | reports_dest = '../reports' |
7 | 37 | previous_dest = '../previous' | 37 | previous_dest = '../previous' |
8 | 38 | coverity_dest = '../coverity' | 38 | coverity_dest = '../coverity' |
9 | 39 | tests_dest = '../tests' | ||
10 | 39 | 40 | ||
11 | 40 | # Per-package overrides for the sbuild resolver | 41 | # Per-package overrides for the sbuild resolver |
12 | 41 | # <sbuild resolver> = sbuild_dep_resolver_overrides[<srcpkg>][<ubuntu release>] | 42 | # <sbuild resolver> = sbuild_dep_resolver_overrides[<srcpkg>][<ubuntu release>] |
13 | @@ -1364,19 +1365,11 @@ Acquire::Languages "none"; | |||
14 | 1364 | recursive_rm(tempdir) | 1365 | recursive_rm(tempdir) |
15 | 1365 | 1366 | ||
16 | 1366 | 1367 | ||
18 | 1367 | def cmd_repo(): | 1368 | def copy_to_repo(opt, details, quiet=False): |
19 | 1368 | '''Copy all built packages into local repository''' | 1369 | '''Copy all built packages into local repository''' |
20 | 1369 | parser = umt_optparse("usage: %prog repo [options]") | ||
21 | 1370 | |||
22 | 1371 | parser.add_option("-r", "--release", dest="release", default=False, | ||
23 | 1372 | help="specify repository to copy to") | ||
24 | 1373 | parser.add_option("--purge", dest="purge", default=False, action='store_true', | ||
25 | 1374 | help="purge repository of all packages before copying") | ||
26 | 1375 | |||
27 | 1376 | (opt, args) = parser.parse_args() | ||
28 | 1377 | repo_base = ust['package_tools_repo_base'] | 1370 | repo_base = ust['package_tools_repo_base'] |
31 | 1378 | details = parse_package_details(release = opt.release, skip_sanity=True) | 1371 | if not quiet: |
32 | 1379 | print_details(details) | 1372 | print_details(details) |
33 | 1380 | 1373 | ||
34 | 1381 | repo_dest = os.path.join(repo_base, details['release']) | 1374 | repo_dest = os.path.join(repo_base, details['release']) |
35 | 1382 | 1375 | ||
36 | @@ -1404,7 +1397,8 @@ def cmd_repo(): | |||
37 | 1404 | err("Could not find 'update_repo' (is '$UST' set?). Aborting.") | 1397 | err("Could not find 'update_repo' (is '$UST' set?). Aborting.") |
38 | 1405 | sys.exit(1) | 1398 | sys.exit(1) |
39 | 1406 | 1399 | ||
41 | 1407 | print("Repository: %s" % (repo_dest)) | 1400 | if not quiet: |
42 | 1401 | print("Repository: %s" % (repo_dest)) | ||
43 | 1408 | 1402 | ||
44 | 1409 | # Copy the source files, ignoring stuff we don't want | 1403 | # Copy the source files, ignoring stuff we don't want |
45 | 1410 | files = os.listdir(source_dest) | 1404 | files = os.listdir(source_dest) |
46 | @@ -1412,7 +1406,8 @@ def cmd_repo(): | |||
47 | 1412 | (root, ext) = os.path.splitext(f) | 1406 | (root, ext) = os.path.splitext(f) |
48 | 1413 | if ext in [ ".build", ".upload", ".debdiff" ]: | 1407 | if ext in [ ".build", ".upload", ".debdiff" ]: |
49 | 1414 | continue | 1408 | continue |
51 | 1415 | print("Copying '%s'..." % (f)) | 1409 | if not quiet: |
52 | 1410 | print("Copying '%s'..." % (f)) | ||
53 | 1416 | shutil.copy(os.path.join(source_dest, f), repo_dest) | 1411 | shutil.copy(os.path.join(source_dest, f), repo_dest) |
54 | 1417 | 1412 | ||
55 | 1418 | # Copy the files, as we may have built multiple architectures and the | 1413 | # Copy the files, as we may have built multiple architectures and the |
56 | @@ -1420,7 +1415,8 @@ def cmd_repo(): | |||
57 | 1420 | files = os.listdir(binary_dest) | 1415 | files = os.listdir(binary_dest) |
58 | 1421 | for f in files: | 1416 | for f in files: |
59 | 1422 | if f.endswith("deb"): | 1417 | if f.endswith("deb"): |
61 | 1423 | print("Copying '%s'..." % (f)) | 1418 | if not quiet: |
62 | 1419 | print("Copying '%s'..." % (f)) | ||
63 | 1424 | shutil.copy(os.path.join(binary_dest, f), repo_dest) | 1420 | shutil.copy(os.path.join(binary_dest, f), repo_dest) |
64 | 1425 | 1421 | ||
65 | 1426 | (rc, report) = runcmd([update_repo, details['release']], stdin=sys.stdin, stdout=None) | 1422 | (rc, report) = runcmd([update_repo, details['release']], stdin=sys.stdin, stdout=None) |
66 | @@ -1428,6 +1424,22 @@ def cmd_repo(): | |||
67 | 1428 | err("failure running '%s':\n%s" % (update_repo, report)) | 1424 | err("failure running '%s':\n%s" % (update_repo, report)) |
68 | 1429 | sys.exit(1) | 1425 | sys.exit(1) |
69 | 1430 | 1426 | ||
70 | 1427 | |||
71 | 1428 | def add_repo_arguments(parser): | ||
72 | 1429 | '''Add the command-line arguments used by repo''' | ||
73 | 1430 | |||
74 | 1431 | parser.add_option("-r", "--release", dest="release", default=False, | ||
75 | 1432 | help="specify repository to copy to") | ||
76 | 1433 | parser.add_option("--purge", dest="purge", default=False, action='store_true', | ||
77 | 1434 | help="purge repository of all packages before copying") | ||
78 | 1435 | |||
79 | 1436 | def cmd_repo(): | ||
80 | 1437 | '''Copy all built packages into local repository''' | ||
81 | 1438 | parser = umt_optparse("usage: %prog repo [options]") | ||
82 | 1439 | add_repo_arguments(parser) | ||
83 | 1440 | (opt, args) = parser.parse_args() | ||
84 | 1441 | details = parse_package_details(release = opt.release, skip_sanity=True) | ||
85 | 1442 | copy_to_repo(opt, details) | ||
86 | 1431 | success("SUCCESS") | 1443 | success("SUCCESS") |
87 | 1432 | 1444 | ||
88 | 1433 | def cmd_upload(): | 1445 | def cmd_upload(): |
89 | @@ -1556,6 +1568,178 @@ def cmd_search(): | |||
90 | 1556 | print("%s: %s, Pocket: %s, Component: %s" % (release, | 1568 | print("%s: %s, Pocket: %s, Component: %s" % (release, |
91 | 1557 | pkg_list[release][0], pkg_list[release][1], pkg_list[release][2])) | 1569 | pkg_list[release][0], pkg_list[release][1], pkg_list[release][2])) |
92 | 1558 | 1570 | ||
93 | 1571 | def cmd_qrt(): | ||
94 | 1572 | '''Run QRT tests for the package in the current directory of unpacked source''' | ||
95 | 1573 | parser = umt_optparse("usage: %prog qrt [options]") | ||
96 | 1574 | add_repo_arguments(parser) | ||
97 | 1575 | parser.add_option("-s", "--sudo", dest="sudo", default=False, action='store_true', | ||
98 | 1576 | help="Whether to run the QRT tests via sudo as a regular user") | ||
99 | 1577 | parser.add_option("-u", "--user", dest="user", default=os.getlogin(), metavar='USER', | ||
100 | 1578 | help="The username to run the QRT tests as (default: current user)") | ||
101 | 1579 | parser.add_option("-q", "--qrt-path", dest="qrt_path", default=os.getenv("QRT", ""), metavar='QRT_PATH', | ||
102 | 1580 | help="Path to the local copy of the qa-regression-tools (QRT) repo") | ||
103 | 1581 | parser.add_option("--no-update", dest="no_update", default=False, action='store_true', | ||
104 | 1582 | help="Don't update UVT VM before initial test run") | ||
105 | 1583 | parser.add_option("--no-snapshot", dest="no_snapshot", default=False, action='store_true', | ||
106 | 1584 | help="Don't revert or update snapshot of UVT VM on update") | ||
107 | 1585 | parser.add_option("-n", "--dry-run", dest="dry_run", default=False, action='store_true', | ||
108 | 1586 | help="Don't actually execute tests, instead print what would be run") | ||
109 | 1587 | parser.add_option("-p", "--uvt-prefix", dest="uvt_prefix", default="sec", | ||
110 | 1588 | help="Prefix to use for UVT VM names (default: sec)") | ||
111 | 1589 | parser.add_option("-v", "--uvt-vm", dest="uvt_vm", default=None, | ||
112 | 1590 | help="VM use for UVT tests (default: use {uvt-prefix}-release-{arch})") | ||
113 | 1591 | parser.add_option("--debug", default=False, action='store_true', | ||
114 | 1592 | help="Report additional debug details") | ||
115 | 1593 | parser.add_option("-f", "--force", dest="force", default=False, action='store_true', | ||
116 | 1594 | help="force deletion of ../tests before running") | ||
117 | 1595 | (opt, args) = parser.parse_args() | ||
118 | 1596 | |||
119 | 1597 | validate_toplevel() | ||
120 | 1598 | details = parse_package_details(skip_sanity=True) | ||
121 | 1599 | run_qrt_tests(opt, details) | ||
122 | 1600 | # also alert if there appear to be other bits in QRT worth looking at | ||
123 | 1601 | for d in ['build_testing', 'notes_testing']: | ||
124 | 1602 | testing_dir = os.path.join(opt.qrt_path, d, details["package"]) | ||
125 | 1603 | try: | ||
126 | 1604 | files = os.listdir(testing_dir) | ||
127 | 1605 | except: | ||
128 | 1606 | files = [] | ||
129 | 1607 | finally: | ||
130 | 1608 | if len(files) > 0: | ||
131 | 1609 | warn("QRT also contains the following relevant files for " + details["package"] + ":") | ||
132 | 1610 | for f in files: | ||
133 | 1611 | warn(" " + os.path.join(d, details["package"], f)) | ||
134 | 1612 | warn("Please ensure you consult these") | ||
135 | 1613 | |||
136 | 1614 | |||
137 | 1615 | def runcmdopt(cmd, opt, okrc=[0]): | ||
138 | 1616 | output = "" | ||
139 | 1617 | if opt.debug: | ||
140 | 1618 | print("[" + " ".join(cmd) + "]") | ||
141 | 1619 | if not opt.dry_run: | ||
142 | 1620 | (rc, output) = runcmd(cmd) | ||
143 | 1621 | if rc not in okrc: | ||
144 | 1622 | raise Exception("Failed to execute command: " + " ".join(cmd) + ": " + output) | ||
145 | 1623 | if opt.debug: | ||
146 | 1624 | print(output) | ||
147 | 1625 | return output | ||
148 | 1626 | |||
149 | 1627 | |||
150 | 1628 | def run_qrt_tests(opt, details): | ||
151 | 1629 | qrt_test = os.path.join(opt.qrt_path, "scripts", "test-" + details["package"] + ".py") | ||
152 | 1630 | if not os.path.exists(qrt_test): | ||
153 | 1631 | print("No QRT test found for " + details["package"] + ": expected to find in " + qrt_test) | ||
154 | 1632 | return | ||
155 | 1633 | qrt_tests_dest = os.path.join(tests_dest, "qrt") | ||
156 | 1634 | prepare_dir(qrt_tests_dest, opt.force) | ||
157 | 1635 | user = opt.user | ||
158 | 1636 | sudo = "" | ||
159 | 1637 | if opt.sudo: | ||
160 | 1638 | sudo = 'echo ubuntu | sudo -S ' | ||
161 | 1639 | print("Running QRT test " + qrt_test + " for " + details["package"] + " as " + user + (" via sudo" if opt.sudo else "")) | ||
162 | 1640 | if opt.uvt_vm is not None: | ||
163 | 1641 | vm = opt.uvt_vm | ||
164 | 1642 | else: | ||
165 | 1643 | # TODO suport different arch's | ||
166 | 1644 | vm = opt.uvt_prefix + "-" + details["release"] + "-amd64" | ||
167 | 1645 | # find path to uvt binary - first see if UQT is defined, otherwise try | ||
168 | 1646 | # in relative location to the current script | ||
169 | 1647 | uqt = os.getenv("UQT") | ||
170 | 1648 | if uqt != None and os.path.exists(os.path.join(uqt, "vm-tools", "uvt")): | ||
171 | 1649 | uvt = os.path.join(uqt, "vm-tools", "uvt") | ||
172 | 1650 | elif os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))), | ||
173 | 1651 | "ubuntu-qa-tools", "vm-tools", "uvt"): | ||
174 | 1652 | uvt = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))), | ||
175 | 1653 | "ubuntu-qa-tools", "vm-tools", "uvt") | ||
176 | 1654 | else: | ||
177 | 1655 | err("Unable to find uvt - please define QRT as the path to ubuntu-qa-tools repo") | ||
178 | 1656 | # catch any exception which occurred during execution | ||
179 | 1657 | try: | ||
180 | 1658 | if not opt.no_update: | ||
181 | 1659 | # ensure vm is stopped first - unless no_snapshot we can stop | ||
182 | 1660 | # it by force | ||
183 | 1661 | cmd = [uvt, "stop", vm] | ||
184 | 1662 | if not opt.no_snapshot: | ||
185 | 1663 | cmd.append("-f") | ||
186 | 1664 | print("Ensuring uvt VM " + vm + " is stopped...") | ||
187 | 1665 | runcmdopt(cmd, opt) | ||
188 | 1666 | print("Updating uvt VM " + vm + "...") | ||
189 | 1667 | cmd = [uvt, "update", "-f", vm] | ||
190 | 1668 | if opt.no_snapshot: | ||
191 | 1669 | cmd.append("--nosnapshot") | ||
192 | 1670 | runcmdopt(cmd, opt) | ||
193 | 1671 | print("Launching uvt VM " + vm + "...") | ||
194 | 1672 | # run headless since we don't use the GUI and wait for ssh | ||
195 | 1673 | # availability | ||
196 | 1674 | runcmdopt([uvt, "start", "-v", "-w", vm], opt) | ||
197 | 1675 | print("Packaging QRT test...") | ||
198 | 1676 | runcmdopt([os.path.join(opt.qrt_path, "scripts", "make-test-tarball"), | ||
199 | 1677 | qrt_test], opt) | ||
200 | 1678 | print("Deploying QRT test...") | ||
201 | 1679 | runcmdopt(["/usr/bin/scp", | ||
202 | 1680 | "/tmp/qrt-test-" + details["package"] + ".tar.gz", | ||
203 | 1681 | user + "@" + vm + ":"], opt) | ||
204 | 1682 | print("Extracting QRT test...") | ||
205 | 1683 | runcmdopt(["/usr/bin/ssh", "-T", user + "@" + vm, | ||
206 | 1684 | "tar -xvf qrt-test-" + details["package"] + ".tar.gz" ], | ||
207 | 1685 | opt) | ||
208 | 1686 | print("Installing packages for QRT test...") | ||
209 | 1687 | # always install packages via sudo | ||
210 | 1688 | runcmdopt(["/usr/bin/ssh", "-T", user + "@" + vm, | ||
211 | 1689 | "cd ./qrt-test-" + details["package"] + "; " + | ||
212 | 1690 | "echo ubuntu | sudo -S ./install-packages ./test-" + details["package"] + ".py"], | ||
213 | 1691 | opt) | ||
214 | 1692 | print("Executing QRT baseline test...") | ||
215 | 1693 | report = runcmdopt(["/usr/bin/ssh", "-T", user + "@" + vm, | ||
216 | 1694 | "cd ./qrt-test-" + details["package"] + ";" + | ||
217 | 1695 | sudo + " ./test-" + details["package"] + ".py -v"], | ||
218 | 1696 | opt) | ||
219 | 1697 | # save the report so we can compare it later | ||
220 | 1698 | path = os.path.join(qrt_tests_dest, "qrt-test-" + details["package"] + '-orig.txt') | ||
221 | 1699 | with open(path,"w+") as handle: | ||
222 | 1700 | handle.write(report) | ||
223 | 1701 | handle.flush() | ||
224 | 1702 | |||
225 | 1703 | print("Copying packages to umt repo...") | ||
226 | 1704 | if not opt.dry_run: | ||
227 | 1705 | copy_to_repo(opt, details, quiet=not opt.debug) | ||
228 | 1706 | |||
229 | 1707 | print("Enabling repo for uvt VM...") | ||
230 | 1708 | runcmdopt([uvt, "repo", "-e", vm], opt) | ||
231 | 1709 | |||
232 | 1710 | print("Upgrading packages in uvt VM...") | ||
233 | 1711 | runcmdopt(["/usr/bin/ssh", "-T", "root@" + vm, | ||
234 | 1712 | "apt-get dist-upgrade -y --force-yes"], opt) | ||
235 | 1713 | |||
236 | 1714 | print("Re-executing QRT test...") | ||
237 | 1715 | report = runcmdopt(["/usr/bin/ssh", "-T", user + "@" + vm, | ||
238 | 1716 | "cd ./qrt-test-" + details["package"] + ";" + | ||
239 | 1717 | sudo + " ./test-" + details["package"] + ".py -v"], opt) | ||
240 | 1718 | # save the report so we can compare it later | ||
241 | 1719 | path = os.path.join(qrt_tests_dest, "qrt-test-" + details["package"] + '.txt') | ||
242 | 1720 | with open(path,"w+") as handle: | ||
243 | 1721 | handle.write(report) | ||
244 | 1722 | handle.flush() | ||
245 | 1723 | |||
246 | 1724 | print("Generating diff of QRT tests output...") | ||
247 | 1725 | report = runcmdopt(["/usr/bin/diff", "-u", | ||
248 | 1726 | os.path.join(qrt_tests_dest, "qrt-test-" + details["package"] + '-orig.txt'), | ||
249 | 1727 | os.path.join(qrt_tests_dest, "qrt-test-" + details["package"] + '.txt')], | ||
250 | 1728 | # diff returns 1 if different and 0 if same - so | ||
251 | 1729 | # both are valid ok return codes | ||
252 | 1730 | opt, [0, 1]) | ||
253 | 1731 | # save the diff | ||
254 | 1732 | path = os.path.join(qrt_tests_dest, "qrt-test-" + details["package"] + '.diff') | ||
255 | 1733 | with open(path,"w+") as handle: | ||
256 | 1734 | handle.write(report) | ||
257 | 1735 | handle.flush() | ||
258 | 1736 | print("QRT test run diff in " + path) | ||
259 | 1737 | |||
260 | 1738 | except Exception as e: | ||
261 | 1739 | err(str(e)) | ||
262 | 1740 | print("Stopping uvt VM " + vm + "...") | ||
263 | 1741 | runcmdopt([uvt, "stop", vm], opt) | ||
264 | 1742 | |||
265 | 1559 | # | 1743 | # |
266 | 1560 | # Misc functions | 1744 | # Misc functions |
267 | 1561 | # | 1745 | # |
268 | @@ -3273,6 +3457,7 @@ commands = { | |||
269 | 3273 | 'compare-bin' : cmd_compare_bin, | 3457 | 'compare-bin' : cmd_compare_bin, |
270 | 3274 | 'repo' : cmd_repo, | 3458 | 'repo' : cmd_repo, |
271 | 3275 | 'upload' : cmd_upload, | 3459 | 'upload' : cmd_upload, |
272 | 3460 | 'qrt' : cmd_qrt, | ||
273 | 3276 | 'open' : cmd_open, | 3461 | 'open' : cmd_open, |
274 | 3277 | 'read' : cmd_read, | 3462 | 'read' : cmd_read, |
275 | 3278 | 'help' : cmd_help | 3463 | 'help' : cmd_help |
276 | diff --git a/build-tools/umt-completion.bash b/build-tools/umt-completion.bash | |||
277 | index 7935679..bd73861 100644 | |||
278 | --- a/build-tools/umt-completion.bash | |||
279 | +++ b/build-tools/umt-completion.bash | |||
280 | @@ -12,7 +12,7 @@ _umt_complete() | |||
281 | 12 | COMPREPLY=( $( compgen -W "$( grep Package: /var/lib/apt/lists/*Sources | awk '{print $2}' | sort | uniq )" "$cur") ) | 12 | COMPREPLY=( $( compgen -W "$( grep Package: /var/lib/apt/lists/*Sources | awk '{print $2}' | sort | uniq )" "$cur") ) |
282 | 13 | } | 13 | } |
283 | 14 | 14 | ||
285 | 15 | umt_commands="search download changelog source binary build build-orig compare-log compare-bin sign check upload" | 15 | umt_commands="search download changelog source binary build build-orig compare-log compare-bin sign check upload qrt" |
286 | 16 | 16 | ||
287 | 17 | if [ $COMP_CWORD -eq 1 ]; then | 17 | if [ $COMP_CWORD -eq 1 ]; then |
288 | 18 | COMPREPLY=( $(compgen -W "$umt_commands" -- ${COMP_WORDS[COMP_CWORD]}) ) | 18 | COMPREPLY=( $(compgen -W "$umt_commands" -- ${COMP_WORDS[COMP_CWORD]}) ) |
Thanks for the review Marc - I have pushed extra commits to address your comments - see https:/ /git.launchpad. net/~alexmurray /ubuntu- security- tools/commit/ ?id=d8b061e61a9 6bce2d22829c41f b4bf39302af080 which removes the revert part - and https:/ /git.launchpad. net/~alexmurray /ubuntu- security- tools/commit/ ?id=2ebdc31cb89 e21acd4f35d7881 6823fe13e00f84 which always adds -v when running tests so we always use verbose output. Perhaps in the future we might want a way to turn this off if the diffs get too messy with it but I think for now this an improvement.