Merge ~pieq/bugit/+git/qabro:fix-1816437-fwts-logs into bugit:master

Proposed by Pierre Equoy
Status: Merged
Approved by: Pierre Equoy
Approved revision: 8d1cb78f71bef16e1510c665fbe5e64e4803fda9
Merged at revision: a913710ecdf5f2fc1a8541ec885c6dcd027625a8
Proposed branch: ~pieq/bugit/+git/qabro:fix-1816437-fwts-logs
Merge into: bugit:master
Diff against target: 158 lines (+46/-60)
2 files modified
qabro/bug_assistant.py (+46/-35)
snap/snapcraft.yaml (+0/-25)
Reviewer Review Type Date Requested Status
Maciej Kisielewski (community) Approve
Pierre Equoy Needs Resubmitting
Review via email: mp+373026@code.launchpad.net

Description of the change

Handles LP issues #1807359 and #1816437. In a nutshell:

- don't run FWTS from qabro anymore
- only rely on FWTS logs generated by Checkbox (from the latest active session)
- remove FWTS binaries from qabro snap

See commit messages for more explanation.

Tested as follow:

0. In a 18.04.3 LXC container, install Checkbox from PPA, as well as checkbox-snappy from Snap store.
1. Build qabro snap from this branch (run `snapcraft` from within the git repo), copy the generated snap to the container and install it:
    (inside container) $ sudo snap install qabro_0.11dev_amd64.snap --devmode
2. Run checkbox-cli (Checkbox from PPA isntall), select 18.04 cert tests, and run tests from the Firmware category. This will create a Checkbox session in ~/.cache/plainbox/sessions/ that contains "fwts*.log" files.
3. Run qabro (while pointing at Staging Launchpad to avoid spamming the real Launchpad):
    $ APPORT_LAUNCHPAD_INSTANCE=staging qabro
4. Enter title, select "qabro" for project, then "Firmware" for category, and press Alt+Enter to submit the bug.
5. Check that qabro output contains the following:
    Uploading attachment fwts_desktop_diagnosis_results.log...
    Uploading attachment fwts_uefirtvariable.log...
6. Check that the generated LP issue contains the two above logs as attachments
7. Repeat steps 2~6 using checkbox-snappy.checkbox-cli instead of checkbox-cli to make sure things work fine with snap-based Checkbox sessions
8. Run checkbox-cli again, but this time only run tests that do not contain FWTS jobs (you can for instance run the 18.04 cert test plan and deselect everything before starting the test).
9. Repeat step 3~4, and check that qabro output contains the following:
    No FWTS logs found in Checkbox session!
This is expected since the last active Checkbox session found has not run FWTS-related jobs.

To post a comment you must log in.
Revision history for this message
Maciej Kisielewski (kissiel) wrote :

Some improvement suggestions below.
Landable as is. +1

review: Approve
Revision history for this message
Pierre Equoy (pieq) wrote :

Thanks for the feedback Maciek!

I'll resubmit this, rebased. I added a few inline answers to your comments.

Revision history for this message
Pierre Equoy (pieq) wrote :

Oops, I realized $HOME and /home/$USER were here for a reason (see comment below).

Revision history for this message
Pierre Equoy (pieq) wrote :

Resubmitted with minor changes (PEP257, ...).

review: Needs Resubmitting
Revision history for this message
Maciej Kisielewski (kissiel) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/qabro/bug_assistant.py b/qabro/bug_assistant.py
index 12f6df4..d8248d5 100644
--- a/qabro/bug_assistant.py
+++ b/qabro/bug_assistant.py
@@ -1,4 +1,5 @@
1import getpass1import getpass
2from glob import glob
2import os3import os
3import re4import re
4import subprocess5import subprocess
@@ -263,59 +264,69 @@ class AttachmentAssistant:
263 snap["confinement"]) + "\n"264 snap["confinement"]) + "\n"
264 self.attachments["snap_list.log"] = out.encode()265 self.attachments["snap_list.log"] = out.encode()
265266
266 def attach_last_checkbox_session(self):267 def _get_last_checkbox_session_path(self):
267 """Archive the most recent Plainbox session available and add it to268 """Return path of the most recent Checkbox session directory."""
268 the attachments.269 rootpath = ''
269 """270 latest_session_dir = ''
270 session_tgz = os.path.expandvars('$HOME/checkbox-session.tgz')
271 if os.path.exists(session_tgz):
272 os.remove(session_tgz)
273
274 # Determine if we use a classic install or a snap install271 # Determine if we use a classic install or a snap install
275 classic_path = os.path.expandvars('/home/$USER/.cache/plainbox/sessions')272 classic_path = os.path.expandvars('/home/$USER/.cache/plainbox/sessions')
276 rootpath = ''
277 if os.path.exists(classic_path):273 if os.path.exists(classic_path):
278 rootpath = classic_path274 rootpath = classic_path
279 else:275 else:
280 ss = snapdsocket()276 ss = snapdsocket()
281 snap_list = [e for e in ss.get_snaps() if 'checkbox' in e['name']]277 snap_list = [e['title'] for e in ss.get_snaps() if 'checkbox-' in e['name']]
282 if snap_list:278 for snap_name in snap_list:
283 snap_name = snap_list[0]["name"]279 sp = ["/home/$USER/snap/{}/current/.cache/plainbox/sessions",
284 sp = "/home/$USER/snap/{}/current/plainbox/sessions"280 "/root/snap/{}/current/.cache/plainbox/sessions"]
285 snap_path = os.path.expandvars(sp.format(snap_name))281 for p in sp:
286 if os.path.exists(snap_path):282 snap_path = os.path.expandvars(p.format(snap_name))
287 rootpath = snap_path283 if os.path.exists(snap_path):
288284 rootpath = snap_path
289 # Get the most recent Checkbox session and add it to the attachments285 # Get path of the most recent Checkbox session
290 if rootpath:286 if rootpath:
291 sessions_dirs = [os.path.join(rootpath, f) for f in os.listdir(rootpath)287 sessions_dirs = [os.path.join(rootpath, f) for f in os.listdir(rootpath)
292 if os.path.isdir(os.path.join(rootpath, f))]288 if os.path.isdir(os.path.join(rootpath, f))]
293 if sessions_dirs:289 if sessions_dirs:
294 latest_session_dir = max(sessions_dirs, key=lambda x: os.stat(x).st_mtime)290 latest_session_dir = max(sessions_dirs, key=lambda x: os.stat(x).st_mtime)
295 with tarfile.open(session_tgz, 'w:gz') as tar:
296 tar.add(latest_session_dir)
297 with open(session_tgz, 'rb') as f:
298 data = f.read()
299 self.attachments['checkbox-session.tgz'] = data
300 else:291 else:
301 print('There does not seem to be any Checkbox session here...')292 print('There does not seem to be any Checkbox session here...')
293 return latest_session_dir
294
295 def attach_last_checkbox_session(self):
296 """Archive and attach the most recent Checkbox session available."""
297 latest_session_dir = self._get_last_checkbox_session_path()
298 if latest_session_dir:
299 session_tgz = os.path.expandvars('$HOME/checkbox-session.tgz')
300 if os.path.exists(session_tgz):
301 os.remove(session_tgz)
302 with tarfile.open(session_tgz, 'w:gz') as tar:
303 tar.add(latest_session_dir)
304 with open(session_tgz, 'rb') as f:
305 data = f.read()
306 self.attachments['checkbox-session.tgz'] = data
302307
303 def attach_fwts_logs(self):308 def attach_fwts_logs(self):
309 """Check for FWTS-related log files in the most recent Checkbox session
310 and attach them.
311 """
304 p = os.getenv("PATH")312 p = os.getenv("PATH")
305 lp = os.getenv("LD_LIBRARY_PATH")313 lp = os.getenv("LD_LIBRARY_PATH")
306 logfile = os.path.expandvars("$HOME/fwts_results.log")314 latest_session_dir = self._get_last_checkbox_session_path()
307 if p and lp:315 if p and lp:
308 print("Running FWTS...")316 if latest_session_dir:
309 command = ["sudo", "env", "PATH={}".format(p),317 logpath = os.path.join(latest_session_dir, "CHECKBOX_DATA",
310 "LD_LIBRARY_PATH={}".format(lp), "fwts", "-q", "-r",318 "fwts*.log")
311 logfile, "version", "cpufreq", "maxfreq", "msr", "mtrr",319 logfiles = glob(logpath)
312 "nx", "virt", "aspm", "dmicheck", "apicedge", "klog",320 if logfiles:
313 "oops", "esrt", "uefibootpath", "uefirtvariable",321 for log in logfiles:
314 "uefirttime", "uefirtmisc", "--acpitests", "--log-level=high"]322 with open(log, 'rb') as f:
315 subprocess.call(command)323 content = f.read()
316 with open(logfile, 'rb') as f:324 self.attachments[os.path.basename(log)] = content
317 content = f.read()325 else:
318 self.attachments['fwts_results.log'] = content326 print("No FWTS logs found in Checkbox session!")
327 else:
328 print(("Cannot find Checkbox session where FWTS logs could be "
329 "located!"))
319 else:330 else:
320 print("$PATH and/or $LD_LIBRARY_PATH missing. Cannot run FWTS!")331 print("$PATH and/or $LD_LIBRARY_PATH missing. Cannot run FWTS!")
321332
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index 2c9982d..41a00ee 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -17,9 +17,6 @@ apps:
17 sosreport:17 sosreport:
18 command: sosreport --no-report --batch --tmp-dir $SNAP_USER_COMMON -z gzip18 command: sosreport --no-report --batch --tmp-dir $SNAP_USER_COMMON -z gzip
1919
20 fwts:
21 command: bin/fwts
22
23 lspci:20 lspci:
24 command: lspci21 command: lspci
2522
@@ -57,28 +54,6 @@ parts:
57 # variable used by sosreport to call other commands.54 # variable used by sosreport to call other commands.
58 patch $SNAPCRAFT_PART_INSTALL/usr/share/sosreport/sos/policies/ubuntu.py $SNAPCRAFT_STAGE/patches/sosreport-env-path-snap.patch55 patch $SNAPCRAFT_PART_INSTALL/usr/share/sosreport/sos/policies/ubuntu.py $SNAPCRAFT_STAGE/patches/sosreport-env-path-snap.patch
5956
60 fwts:
61 plugin: autotools
62 source: git://kernel.ubuntu.com/hwe/fwts
63 source-tag: V19.01.00
64 stage-packages:
65 - libfdt1
66 - libbsd0
67 - libpci3
68 build-packages:
69 - gcc
70 - make
71 - autoconf
72 - automake
73 - libtool
74 - libjson-c-dev
75 - flex
76 - bison
77 - dh-autoreconf
78 - libglib2.0-dev
79 - libfdt-dev
80 - libbsd-dev
81
82 lspci:57 lspci:
83 plugin: nil58 plugin: nil
84 stage-packages:59 stage-packages:

Subscribers

People subscribed via source and target branches

to all changes: