-
Notifications
You must be signed in to change notification settings - Fork 879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
printing the error stream of the dhclient process before killing it #369
printing the error stream of the dhclient process before killing it #369
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of suggested improvements, and one higher-level question about the change.
(Sorry for the delay in reviewing this!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
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 |
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: