unmount: disabled causes installation failure to exit silently

[Michael Hudson-Doyle]

from cmd_install:

    finally:
        log_target_path = instcfg.get('save_install_log', SAVE_INSTALL_LOG)
        if log_target_path:
            copy_install_log(logfile, workingd.target, log_target_path)

        if instcfg.get('unmount', "") == "disabled":
            LOG.info('Skipping unmount: config disabled target unmounting')
            return

        # unmount everything (including iscsi disks)
        util.do_umount(workingd.target, recursive=True)

that 'return' means the function returns normally even in an error case. I think this needs to be changed into an if/else.

[Ryan Harper (raharper)]

My reading of the python3 docs suggests that this analysis is correct. If we issue a return, then the saved exception is discarded.

If finally is present, it specifies a ‘cleanup’ handler. The try clause is executed, including any except and else clauses. If an exception occurs in any of the clauses and is not handled, the exception is temporarily saved. The finally clause is executed. If there is a saved exception it is re-raised at the end of the finally clause. If the finally clause raises another exception, the saved exception is set as the context of the new exception. If the finally clause executes a return or break statement, the saved exception is discarded:

>>>
>>> def f():
... try:
... 1/0
... finally:
... return 42
...
>>> f()
42

https://docs.python.org/3/reference/compound_stmts.html#try

[Scott Moser (smoser)]

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/curtin/commit/?id=1d4d17ce

[Launchpad Janitor (janitor)]

This bug was fixed in the package curtin - 18.1-5-g572ae5d6-0ubuntu1

---------------
curtin (18.1-5-g572ae5d6-0ubuntu1) bionic; urgency=medium

  * New upstream snapshot.
    - clear-holders: fix lvm name use when shutting down (LP: #1764602)
    - install: prevent unmount: disabled from swallowing installation failures
      (LP: #1764210)
    - vmtest: bionic images no longer use the vlan package
    - pycodestyle: Fix invalid escape sequences in string literals.

 -- Ryan Harper <email address hidden> Wed, 18 Apr 2018 10:15:46 -0500

[Ryan Harper (raharper)]

This bug is believed to be fixed in curtin in version 18.2. If this is still a problem for you, please make a comment and set the state back to New

Thank you.