printing the error stream of the dhclient process before killing it

[Moustafa-Moustafa]

Logging the error stream of the dhclient process is very handy when debugging the dhclient issues since it shows different DHCP signals. Here's a sample from the cloud-init.log of how this could look like:

2020-05-15 22:18:04,599 - dhcp.py[DEBUG]: dhclient error stream: Internet Systems Consortium DHCP Client 4.3.5
Copyright 2004-2016 Internet Systems Consortium.
All rights reserved.
For info, please visit https://www.isc.org/software/dhcp/

Listening on LPF/eth0/00:0d:3a:6e:d4:52
Sending on   LPF/eth0/00:0d:3a:6e:d4:52
Sending on   Socket/fallback
DHCPDISCOVER on eth0 to 255.255.255.255 port 67 interval 3 (xid=0xc988334c)
DHCPREQUEST of 10.0.0.22 on eth0 to 255.255.255.255 port 67 (xid=0x4c3388c9)
DHCPOFFER of 10.0.0.22 from 168.63.129.16
DHCPACK of 10.0.0.22 from 168.63.129.16
bound to 10.0.0.22 -- renewal in 4294967295 seconds.

[OddBloke]

A couple of suggested improvements, and one higher-level question about the change.

(Sorry for the delay in reviewing this!)

[OddBloke]

OK, this change looks good now, thanks! There are conflicts which need resolving, however, I'll give this a final review once those conflicts are fixed.

[OddBloke]

LGTM, thanks!

[OddBloke]

On Thu, Oct 15, 2020 at 04:50:08AM -0700, Eduardo Otubo wrote: I know this PR was landed months ago, and this comment will probably be ignored, but I would like to question the choice of the word "dhclient *error* stream" that even on a successful DHCP lease will printout "error". I got this while running some tests and left us with some question in mind.

Would changing "error stream" to "stderr" address this? That would seem like a reasonable change to propose, IMO.

[Moustafa-Moustafa]

On Thu, Oct 15, 2020 at 04:50:08AM -0700, Eduardo Otubo wrote: I know this PR was landed months ago, and this comment will probably be ignored, but I would like to question the choice of the word "dhclient error stream" that even on a successful DHCP lease will printout "error". I got this while running some tests and left us with some question in mind.
Would changing "error stream" to "stderr" address this? That would seem like a reasonable change to propose, IMO.

LGTM