duffle driver ignores operation writer, writes directly to stdout

[carolynvs]

The docker driver is passed on the Operation struct a writer for any output, but instead it is writing directly to stdout.

As part of Porter we are implementing a verbose flag and would like to be able to show/hide this output, but it is printing directly so we can't control it. Would you be up for a pull request that switches the driver over to using op.Out?

[glyn]

Seems reasonable to me.

[benpatt]

The docker driver is passed on the Operation struct a writer for any output, but instead it is writing directly to stdout.

Is this still the case? If so, can you give a pointer?

[carolynvs]

Yup, it's still the case. 😀 You feel like tackling it?

The driver implements the common interface for a driver, but in it's exec function it ignores the writer field coming in on the op argument to exec.

https://github.com/deislabs/duffle/blob/08636d6a7f3651ee05cba1f2fb2cdaaa3d405fae/pkg/driver/docker_driver.go#L202

[glyn]

When discussing this issue with @benpatt, I wondered if there was a distinction between the output stream for the logs from the docker driver and from the container that the docker drive runs. @carolynvs would you say that all the logs from the driver itself and from the container should be sent to the same output stream (the one in the Operation struct)?

[carolynvs]

The driver doesn't seem to print much (just 1 error message about pulling the image). Outputting everything to the Operation writer would be just fine by me.

[silvin-lubecki]

As said on this PR review cnabio/duffle#808 (review)

Well we are actually using those 2 methods here in docker/app
https://github.com/docker/app/blob/master/internal/commands/cnab.go#L191
If I remember correctly we are using the container output stdout and stderr for our custom actions (ex: our render action which can fail or print a result), but we still need to mute the driver itself using the dockerCli stdout and stderr pointing to ioutil.Discard.
By the way, I'm wondering why we are still using the drivers here, as they were moved to cnab-go 🤔
https://github.com/deislabs/cnab-go/blob/master/driver/docker/docker.go

[glyn]

As said on this PR review #808 (review)

Well we are actually using those 2 methods here in docker/app
https://github.com/docker/app/blob/master/internal/commands/cnab.go#L191
If I remember correctly we are using the container output stdout and stderr for our custom actions (ex: our render action which can fail or print a result), but we still need to mute the driver itself using the dockerCli stdout and stderr pointing to ioutil.Discard.

It would be good to add a driver test to tie down the behaviour. Otherwise, it is bound to break docker/app in future.

By the way, I'm wondering why we are still using the drivers here, as they were moved to cnab-go 🤔
https://github.com/deislabs/cnab-go/blob/master/driver/docker/docker.go

Good catch. Raised cnabio/duffle#809 to delete the old code. We also need to move this issue to cnab-go which is now its proper home.