Fix STDOUT and STDERR direction based on Log.* calls

[nathanleclaire]

Closes #2587

It's a big issue. create, for instance, logs everything to STDERR rather than STDOUT in v0.5.3 today. You can easily verify this with a shell function like this:

stderrcolor () {
    "$@" 2> >(while read line; do echo -e "\e[01;31m$line\e[0m" >&2; done)
}

Since we're not using the Bugsnag hook for logrus at all, my feeling is that we should just remove the use of logrus altogether (I can't see any particular advantages) and use only fmt logger. That's what I've implemented here. AFAICT there's no way to get logrus to change which stream it logs to based on logging levels. I can followup PR to remove the vendored code for this.

During standup we need to discuss whether we feel like this merits a 0.5.4 release ASAP. In my opinion it does, since the accidental regression is so large in scope, and could potentially affect a lot of downstream users and/or existing scripts for Docker Machine.

The bundled tests are a step in the right direction to ensure something like this doesn't happen again in the future. More would be better, of course.

cc @dgageot @jeanlaurent

Signed-off-by: Nathan LeClaire nathan.leclaire@gmail.com

[jeanlaurent]

Well, this is doing two things at the same time, fixing the issue and removing logrus. I usually really don't think this is a good idea especially when we have an outstanding issue to fix. I would prefer we fix the issue, release 0.5.4, discuss the logrus case afterwards, one thing after the other.

I spent 1/2 hour fixing the issue while keeping logrus this give this code,
jeanlaurent@6e09858.

There is two major points of interest with logrus : It handle the log levels and has a hook system, which allow in theory to communicate logs to an external system.

To try to prove we still need logrus I tried to log everything that is logged to a file. I did it in jeanlaurent@7038122. And this is failing badly, because logrus as only one LogLevel used as a threshold to decide to keep the log or not, ( I was accustomed to java logging library that handle that very well) . If the logger is set at the info level, all debug logs are not send nor recorded, they are lost. The hook system does not even receive the event.

So for our usecases, the hook system is useless, we have to listen to log event, before they are actually sent to logrus to bypass the one loglevel limitation. That what we do with the various record functions.

So the logrus usage boils down to library popularity and... standard logging.

So the PR which keeps logrus ( jeanlaurent@6e09858. ) actually add even more code and a bit of complexity than the current PR does.

So I'm going to LGTM this one, and give a good "farewell" to logrus.

There is a couple of points which needs some polish, that will be dealt in a followup PR.

[generalov]

I look at the https://github.com/docker/machine/blob/19ce7b79bd8/commands/status.go#L8 . I think it was fogiven to output the status to user. This function just writes it to log.

I suppose there are two different architectural things in this case: the status as anwer of command line UI to user, and status as 'technical' message writed by docker-machine to logging subsystem.

I think, the logging messages are totally "technical" aspect and is a subject of configuration by sysadmins. For example logging messages could be disabled, routed to syslog, to another log agregation host and etc.

But answers of tool is aspect of its user interface. This messages are could be formatted, coloured, translated to languages.

As for me its a strange trick to use logging subsystem here to output the anwer (status) to user here. I think the logrus is a good tool for logging (to deal with technical messages), but it is a bad place for answers from CLI.

sorry for my silly english.