[Merge] ~cpete/ubuntu/+source/apport:updated-subiquity-hook into ~ubuntu-core-dev/ubuntu/+source/apport:ubuntu/devel

[Chris Peterson]

Thanks for the review, I'll update with the two fixes in a moment.

Diff comments:

> diff --git a/debian/package-hooks/subiquity.py b/debian/package-hooks/subiquity.py
> index 9519b1f..fce04df 100644
> --- a/debian/package-hooks/subiquity.py
> +++ b/debian/package-hooks/subiquity.py
> @@ -1,24 +1,46 @@
>  """Send reports about subiquity to the correct Launchpad project."""
>  
>  import os
> +import re
>  
>  from apport import hookutils
>  
>  
> +def get_revision() -> str:
> +    """Get subiquity revision.
> +
> +    Returns revision found (e.g. "1234") or "unknown" if not found.
> +    """
> +
> +    # Parse subiquity version from log file
> +    logfile = os.path.realpath("/var/log/installer/subiquity-server-debug.log")
> +    log_contents = hookutils.read_file(logfile)

Well it will return a text representation of the error, something like "Error: could not find file". Then the revision will end up as "unknown" which is okay I think.

> +    first_line = log_contents.splitlines()[0]
> +    marker = "Starting Subiquity server revision"
> +    if marker in first_line:
> +        revision = first_line.split(marker)[1].strip()
> +    else:
> +        revision = "unknown"
> +
> +    return revision
> +
> +
>  def add_info(report, unused_ui):
> -    """Send reports about subiquity to the correct Launchpad project."""

It's the same as the module docstring and I felt that a docstring for the add_info function wasn't necessary. I can update it instead though.

> -    # TODO: read the version from the log file?
> -    logfile = os.path.realpath("/var/log/installer/subiquity-debug.log")
> -    revision = "unknown"
> -    if os.path.exists(logfile):
> -        hookutils.attach_file(report, "logfile", "InstallerLog")
> -        with open(logfile, encoding="utf-8") as log_fp:
> -            first_line = log_fp.readline()
> -        marker = "Starting Subiquity revision"
> -        if marker in first_line:
> -            revision = first_line.split(marker)[1].strip()
> -    report["Package"] = f"subiquity ({revision})"
> +
> +    revision = get_revision()
> +    report["Package"] = f"subiquity ({revision})"  # Leave revision in for UI prompt
>      report["SourcePackage"] = "subiquity"
> +
> +    # Package and SourcePackage keys are deleted when the Snap is chosen,
> +    # so make a separate key to keep track of the revision
> +    report["SnapVersion"] = revision
> +
> +    # Check if the snap was updated
> +    # TODO: Add logic to support this outside of the live environment.
> +    #       It may be possible someone wants to report a bug against the
> +    #       installer after first boot.
> +    report["SnapUpdated"] = str(os.path.exists("/run/subiquity/updating"))
> +
>      # rewrite this section so the report goes to the project in Launchpad
>      report[
>          "CrashDB"
> @@ -31,22 +53,60 @@ def add_info(report, unused_ui):
>  }
>  """
>  
> -    # add in subiquity stuff
> -    hookutils.attach_file_if_exists(
> -        report, "/var/log/installer/subiquity-curtin-install.conf", "CurtinConfig"
> -    )
> -    hookutils.attach_file_if_exists(
> -        report, "/var/log/installer/curtin-install.log", "CurtinLog"
> -    )
> -    hookutils.attach_file_if_exists(
> -        report, "/var/log/installer/block/probe-data.json", "ProbeData"
> -    )
> -
> -    # collect desktop installer details if available
> -    desktoplog = os.path.realpath("/var/log/installer/ubuntu_bootstrap.log")
> -    if os.path.exists(desktoplog):
> -        hookutils.attach_file(report, desktoplog, "DesktopInstallerLog")
> +    # add in hardware information
> +    hookutils.attach_hardware(report)
> +
> +    # static subiquity generated logs
> +    log_map = {
> +        "InstallerServerLog": "/var/log/installer/subiquity-server-debug.log",
> +        "InstallerServerLogInfo": "/var/log/installer/subiquity-server-info.log",
> +        "InstallerClientLog": "/var/log/installer/subiquity-client-debug.log",
> +        "InstallerClientLogInfo": "/var/log/installer/subiquity-client-info.log",
> +        "CurtinLog": "/var/log/installer/curtin-install.log",
> +        "CurtinAptConfig": "var/log/installer/curtin-install/subiquity-curtin-apt.conf",
> +        "CurtinCurthooksConfig": "/var/log/installer/curtin-install/subiquity-curthooks.conf",
> +        "CurtinExtractConfig": "/var/log/installer/curtin-install/subiquity-extract.conf",
> +        "CurtinInitialConfig": "/var/log/installer/curtin-install/subiquity-initial.conf",
> +        "CurtinPartitioningConfig": "/var/log/installer/curtin-install/subiquity-partitioning.conf",
> +        "ProbeData": "/var/log/installer/block/probe-data.json",
> +        "Traceback": "/var/log/installer/subiquity-traceback.txt",
> +        "NetplanInstallerConfig": "/etc/netplan/00-installer-config.yaml",
> +        "NetplanSnapdConfig": "/etc/netplan/00-snapd-config.yaml",
> +    }
> +
> +    # Add journal log - from the installer if it exists or from the system
> +    if "InstallerJournal" not in report:

Thanks for the catch. This is remnant from an older revision that didn't get cleaned up. I'll update this and put it into another function.

> +        installer_journal = "/var/log/installer/installer-journal.txt"
> +        if os.path.exists(installer_journal):
> +            log_map["InstallerJournal"] = installer_journal
> +        else:
> +            report["SystemJournal"] = hookutils.recent_syslog(re.compile("."))
> +
> +    # Add ubuntu-desktop-bootstrap information if available
> +    udb_log = os.path.realpath("/var/log/installer/ubuntu_bootstrap.log")
> +    if os.path.exists(udb_log):
> +        log_map["BoostrapLog"] = udb_log
>          report.add_tags(["ubuntu-desktop-bootstrap"])
> -        snapdir = os.path.realpath("/snap/ubuntu-desktop-bootstrap/current")
> -        if os.path.exists(snapdir):
> -            report["DesktopInstallerRev"] = os.path.basename(snapdir)
> +        # check for udb snap version
> +        udb_snap = os.path.realpath("/snap/ubuntu-desktop-bootstrap/current")
> +        if os.path.exists(udb_snap):
> +            # "current" is a symlink to the current revision and
> +            # os.path.realpath should have resolved it
> +            report["DesktopInstallerRev"] = os.path.basename(udb_snap)
> +
> +    # Attach logs if they exist
> +    # The conventional way to attach logs would be to use the
> +    # hookutils.attach_file_if_exists method, but since subiquity logs
> +    # are mostly locked down with root r/w only then we will get a permission
> +    # error if the caller does not have permissions. Ask for elevated
> +    # permissions instead of requiring users know to run with sudo or similar
> +    command_mapping = {}
> +    for name, path in log_map.items():
> +        if os.path.exists(path):
> +            real_path = os.path.realpath(path)
> +            command_mapping[name] = f"cat {real_path}"
> +
> +    hookutils.attach_root_command_outputs(report, command_mapping)
> +
> +    # Always set reports to private since we might collect sensitive data
> +    report["LaunchpadPrivate"] = "1"


-- 
https://code.launchpad.net/~cpete/ubuntu/+source/apport/+git/apport/+merge/454563
Your team Ubuntu Core Development Team is subscribed to branch ~ubuntu-core-dev/ubuntu/+source/apport:ubuntu/devel.