Redirecting stderr to stdout is suseptible to breakage due to warning output to stderr

[David Ames]

As LP Bug#1868322 [0] showed many charm helpers are susceptible to breakage when warning messages are sent to stderr.

The pattern of redirecting stderr to stdout for subprocess, stderr=subprocess.STDOUT, is the root cause. This pattern is found in many charm helpers like the network_get_primary helper:

def network_get_primary_address(binding):
...
    cmd = ['network-get', '--primary-address', binding]
    try:
        response = subprocess.check_output(
            cmd,
            stderr=subprocess.STDOUT).decode('UTF-8').strip()
    except CalledProcessError as e:
        if 'no network config found for binding' in e.output.decode('UTF-8'):
            raise NoNetworkBinding("No network binding for {}"
                                   .format(binding))
...

Per the subprocess documentation [1] we should use e.stderr for exception handling rather than redirecting stderr to stdout:

    try:
        response = subprocess.check_output(cmd).decode('UTF-8').strip()
    except CalledProcessError as e:
        if 'no network config found for binding' in e.stderr.decode('UTF-8'):
            raise NoNetworkBinding("No network binding for {}"
                                   .format(binding))

[0] https://bugs.launchpad.net/juju/+bug/1868322
[1] https://docs.python.org/3/library/subprocess.html#subprocess.CalledProcessError

[Alex Kavanagh (ajkavanagh)]

A quick and dirty grep of the charm-helpers code reveals 25ish sites that need looking at in the charm-helpers code to make charm-helpers less brittle to this issue:

charmhelpers/payload/execd.py:50:def execd_run(command, execd_dir=None, die_on_error=True, stderr=subprocess.STDOUT):
charmhelpers/payload/execd.py:54: subprocess.check_output(submodule_path, stderr=stderr,
charmhelpers/core/host.py:294: cmd, stderr=subprocess.STDOUT).decode('UTF-8')
charmhelpers/core/host.py:502: return subprocess.check_output(cmd, stderr=subprocess.STDOUT).decode('UTF-8').strip()
charmhelpers/core/host_factory/ubuntu.py:48: stderr=subprocess.STDOUT).decode('UTF-8')
charmhelpers/core/hookenv.py:1340: stderr=subprocess.STDOUT).decode('UTF-8').strip()
charmhelpers/core/hookenv.py:1370: stderr=subprocess.STDOUT).decode('UTF-8').strip()
charmhelpers/fetch/bzrurl.py:58: check_output(cmd, stderr=STDOUT)
charmhelpers/fetch/giturl.py:53: check_output(cmd, stderr=STDOUT)
charmhelpers/fetch/ubuntu.py:433: stderr=subprocess.PIPE,
charmhelpers/fetch/ubuntu.py:492: stderr=subprocess.PIPE,
charmhelpers/fetch/ubuntu_apt_pkg.py:111: stderr=subprocess.STDOUT,
charmhelpers/fetch/ubuntu_apt_pkg.py:160: stderr=subprocess.STDOUT,
charmhelpers/fetch/ubuntu_apt_pkg.py:197: stderr=subprocess.STDOUT,
charmhelpers/fetch/ubuntu_apt_pkg.py:257: stderr=subprocess.STDOUT,
charmhelpers/contrib/hahelpers/cluster.py:128: status = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
charmhelpers/contrib/hahelpers/cluster.py:161: status = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
charmhelpers/contrib/charmsupport/nrpe.py:438: stderr=subprocess.STDOUT
charmhelpers/contrib/network/ip.py:252: stderr=subprocess.STDOUT,
charmhelpers/contrib/network/ufw.py:369: stderr=subprocess.STDOUT,
charmhelpers/contrib/hardening/host/checks/suid_sgid.py:127: p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
charmhelpers/contrib/ssl/service.py:115: output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
charmhelpers/contrib/ssl/service.py:164: subprocess.check_call(cmd, stderr=subprocess.PIPE)
charmhelpers/contrib/ssl/service.py:166: subprocess.check_call(cmd, stderr=subprocess.PIPE)
charmhelpers/contrib/ssl/service.py:180: subprocess.check_call(cmd, stderr=subprocess.PIPE)

One solution is to use the fact that the subprocess.CalledProcessError class has a 'stderr' member with the output of STDERR when an exception occurs. This is available in py3.5 (xenial) but NOT in py3.4 (trusty which currently has Python 3.4.3). Obviously, that presents a slight annoyance!