duffle driver ignores operation writer, writes directly to stdout about cnab-go HOT 7 CLOSED

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.

from cnab-go.

Seems reasonable to me.

from cnab-go.

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?

from cnab-go.

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

from cnab-go.

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)?

from cnab-go.

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

from cnab-go.

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.

from cnab-go.