Skip to content
This repository was archived by the owner on Jul 18, 2025. It is now read-only.

Commit c10b1cf

Browse files
Fix invocation container always printing on stdout/stderr:
* the stderr was printed before the command top level error, so it was confusing * the stdout needs to be captured by the render command to put it into a file if specified Signed-off-by: Silvin Lubecki <[email protected]>
1 parent 3e25c6e commit c10b1cf

File tree

9 files changed

+60
-32
lines changed

9 files changed

+60
-32
lines changed

e2e/cnab_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestCallCustomStatusAction(t *testing.T) {
2828
{
2929
name: "missingCustomStatusAction",
3030
exitCode: 1,
31-
expectedOutput: "Status failed: action not defined for bundle",
31+
expectedOutput: "status failed: action not defined for bundle",
3232
cnab: "cnab-without-status",
3333
},
3434
}

e2e/commands_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ func testRenderApp(appPath string, env ...string) func(*testing.T) {
5353
return func(t *testing.T) {
5454
cmd, cleanup := dockerCli.createTestCmd()
5555
defer cleanup()
56+
dir := fs.NewDir(t, "")
57+
defer dir.Remove()
5658

5759
envParameters := map[string]string{}
5860
data, err := ioutil.ReadFile(filepath.Join(appPath, "env.yml"))
@@ -64,8 +66,14 @@ func testRenderApp(appPath string, env ...string) func(*testing.T) {
6466
}
6567
cmd.Command = args
6668
cmd.Env = append(cmd.Env, env...)
69+
// Check rendering to stdout
6770
result := icmd.RunCmd(cmd).Assert(t, icmd.Success)
6871
assert.Assert(t, is.Equal(readFile(t, filepath.Join(appPath, "expected.txt")), result.Stdout()), "rendering mismatch")
72+
// Checks rendering to a file
73+
cmd.Command = append(cmd.Command, "--output="+dir.Join("actual.yaml"))
74+
result = icmd.RunCmd(cmd).Assert(t, icmd.Success)
75+
76+
assert.Assert(t, is.Equal(readFile(t, filepath.Join(appPath, "expected.txt")), readFile(t, dir.Join("actual.yaml"))), "rendering mismatch")
6977
}
7078
}
7179

internal/commands/cnab.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package commands
22

33
import (
4+
"bytes"
45
"fmt"
6+
"io"
57
"io/ioutil"
68
"os"
79
"path/filepath"
@@ -90,13 +92,18 @@ func duffleHome() home.Home {
9092
}
9193

9294
// prepareDriver prepares a driver per the user's request.
93-
func prepareDriver(dockerCli command.Cli, bindMount bindMount) (driver.Driver, error) {
95+
func prepareDriver(dockerCli command.Cli, bindMount bindMount, stdout io.Writer) (driver.Driver, *bytes.Buffer, error) {
9496
driverImpl, err := driver.Lookup("docker")
9597
if err != nil {
96-
return driverImpl, err
98+
return driverImpl, nil, err
9799
}
100+
errBuf := bytes.NewBuffer(nil)
98101
if d, ok := driverImpl.(*driver.DockerDriver); ok {
99102
d.SetDockerCli(dockerCli)
103+
if stdout != nil {
104+
d.SetContainerOut(stdout)
105+
}
106+
d.SetContainerErr(errBuf)
100107
if bindMount.required {
101108
d.AddConfigurationOptions(func(config *container.Config, hostConfig *container.HostConfig) error {
102109
config.User = "0:0"
@@ -122,7 +129,7 @@ func prepareDriver(dockerCli command.Cli, bindMount bindMount) (driver.Driver, e
122129
configurable.SetConfig(driverCfg)
123130
}
124131

125-
return driverImpl, err
132+
return driverImpl, errBuf, err
126133
}
127134

128135
func getAppNameKind(name string) (string, nameKind) {
@@ -243,21 +250,20 @@ func isDockerHostLocal(host string) bool {
243250
return host == "" || strings.HasPrefix(host, "unix://") || strings.HasPrefix(host, "npipe://")
244251
}
245252

246-
func prepareCustomAction(actionName string, dockerCli command.Cli, appname string,
247-
registryOpts registryOptions, pullOpts pullOptions, paramsOpts parametersOptions) (*action.RunCustom, *claim.Claim, error) {
248-
defer muteDockerCli(dockerCli)()
253+
func prepareCustomAction(actionName string, dockerCli command.Cli, appname string, stdout io.Writer,
254+
registryOpts registryOptions, pullOpts pullOptions, paramsOpts parametersOptions) (*action.RunCustom, *claim.Claim, *bytes.Buffer, error) {
249255

250256
c, err := claim.New(actionName)
251257
if err != nil {
252-
return nil, nil, err
258+
return nil, nil, nil, err
253259
}
254-
driverImpl, err := prepareDriver(dockerCli, bindMount{})
260+
driverImpl, errBuf, err := prepareDriver(dockerCli, bindMount{}, stdout)
255261
if err != nil {
256-
return nil, nil, err
262+
return nil, nil, nil, err
257263
}
258264
bundle, err := resolveBundle(dockerCli, appname, pullOpts.pull, registryOpts.insecureRegistries)
259265
if err != nil {
260-
return nil, nil, err
266+
return nil, nil, nil, err
261267
}
262268
c.Bundle = bundle
263269

@@ -266,13 +272,13 @@ func prepareCustomAction(actionName string, dockerCli command.Cli, appname strin
266272
withCommandLineParameters(paramsOpts.overrides),
267273
)
268274
if err != nil {
269-
return nil, nil, err
275+
return nil, nil, nil, err
270276
}
271277
c.Parameters = parameters
272278

273279
a := &action.RunCustom{
274280
Action: internal.Namespace + actionName,
275281
Driver: driverImpl,
276282
}
277-
return a, c, nil
283+
return a, c, errBuf, nil
278284
}

internal/commands/inspect.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package commands
22

33
import (
4+
"fmt"
5+
46
"github.com/docker/cli/cli"
57
"github.com/docker/cli/cli/command"
6-
"github.com/pkg/errors"
78
"github.com/spf13/cobra"
89
)
910

@@ -30,9 +31,13 @@ func inspectCmd(dockerCli command.Cli) *cobra.Command {
3031
}
3132

3233
func runInspect(dockerCli command.Cli, appname string, opts inspectOptions) error {
33-
a, c, err := prepareCustomAction("inspect", dockerCli, appname, opts.registryOptions, opts.pullOptions, opts.parametersOptions)
34+
defer muteDockerCli(dockerCli)()
35+
a, c, errBuf, err := prepareCustomAction("inspect", dockerCli, appname, nil, opts.registryOptions, opts.pullOptions, opts.parametersOptions)
3436
if err != nil {
3537
return err
3638
}
37-
return errors.Wrap(a.Run(c, nil, dockerCli.Out()), "Inspect failed")
39+
if err := a.Run(c, nil, nil); err != nil {
40+
return fmt.Errorf("inspect failed: %s", errBuf)
41+
}
42+
return nil
3843
}

internal/commands/install.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func runInstall(dockerCli command.Cli, appname string, opts installOptions) erro
9898
return err
9999
}
100100

101-
driverImpl, err := prepareDriver(dockerCli, bind)
101+
driverImpl, errBuf, err := prepareDriver(dockerCli, bind, nil)
102102
if err != nil {
103103
return err
104104
}
@@ -129,7 +129,7 @@ func runInstall(dockerCli command.Cli, appname string, opts installOptions) erro
129129
// so any installation needs a clean uninstallation.
130130
err2 := claimStore.Store(*c)
131131
if err != nil {
132-
return fmt.Errorf("install failed: %v", err)
132+
return fmt.Errorf("install failed: %s", errBuf)
133133
}
134134
return err2
135135
}

internal/commands/render.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
package commands
22

33
import (
4+
"bytes"
5+
"fmt"
46
"io"
57
"os"
68

79
"github.com/docker/app/internal"
810
"github.com/docker/cli/cli"
911
"github.com/docker/cli/cli/command"
10-
"github.com/pkg/errors"
1112
"github.com/spf13/cobra"
1213
)
1314

@@ -41,21 +42,26 @@ func renderCmd(dockerCli command.Cli) *cobra.Command {
4142
}
4243

4344
func runRender(dockerCli command.Cli, appname string, opts renderOptions) error {
44-
a, c, err := prepareCustomAction("render", dockerCli, appname, opts.registryOptions, opts.pullOptions, opts.parametersOptions)
45+
defer muteDockerCli(dockerCli)()
46+
outBuf := bytes.NewBuffer(nil)
47+
a, c, errBuf, err := prepareCustomAction("render", dockerCli, appname, outBuf, opts.registryOptions, opts.pullOptions, opts.parametersOptions)
4548
if err != nil {
4649
return err
4750
}
4851
c.Parameters[internal.Namespace+"render-format"] = opts.formatDriver
4952

50-
var writer io.Writer = dockerCli.Out()
53+
if err := a.Run(c, nil, nil); err != nil {
54+
return fmt.Errorf("render failed: %s", errBuf)
55+
}
56+
var w io.Writer = os.Stdout
5157
if opts.renderOutput != "-" {
5258
f, err := os.Create(opts.renderOutput)
5359
if err != nil {
5460
return err
5561
}
5662
defer f.Close()
57-
writer = f
63+
w = f
5864
}
59-
err = a.Run(c, nil, writer)
60-
return errors.Wrap(err, "Render failed")
65+
fmt.Fprintf(w, outBuf.String())
66+
return nil
6167
}

internal/commands/status.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
package commands
22

33
import (
4+
"fmt"
5+
46
"github.com/deislabs/duffle/pkg/action"
57
"github.com/deislabs/duffle/pkg/claim"
68
"github.com/deislabs/duffle/pkg/credentials"
79
"github.com/deislabs/duffle/pkg/utils/crud"
810
"github.com/docker/app/internal"
911
"github.com/docker/cli/cli"
1012
"github.com/docker/cli/cli/command"
11-
"github.com/pkg/errors"
1213
"github.com/spf13/cobra"
1314
)
1415

@@ -42,7 +43,7 @@ func runStatus(dockerCli command.Cli, claimName string, opts credentialOptions)
4243
if err != nil {
4344
return err
4445
}
45-
driverImpl, err := prepareDriver(dockerCli, bind)
46+
driverImpl, errBuf, err := prepareDriver(dockerCli, bind, nil)
4647
if err != nil {
4748
return err
4849
}
@@ -57,6 +58,8 @@ func runStatus(dockerCli command.Cli, claimName string, opts credentialOptions)
5758
Action: internal.Namespace + "status",
5859
Driver: driverImpl,
5960
}
60-
err = status.Run(&c, creds, dockerCli.Out())
61-
return errors.Wrap(err, "Status failed")
61+
if err := status.Run(&c, creds, dockerCli.Out()); err != nil {
62+
return fmt.Errorf("status failed: %s\n%s", err, errBuf)
63+
}
64+
return nil
6265
}

internal/commands/uninstall.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func runUninstall(dockerCli command.Cli, claimName string, opts credentialOption
4242
if err != nil {
4343
return err
4444
}
45-
driverImpl, err := prepareDriver(dockerCli, bind)
45+
driverImpl, errBuf, err := prepareDriver(dockerCli, bind, nil)
4646
if err != nil {
4747
return err
4848
}
@@ -63,5 +63,5 @@ func runUninstall(dockerCli command.Cli, claimName string, opts credentialOption
6363
if err2 := claimStore.Store(c); err2 != nil {
6464
fmt.Fprintf(dockerCli.Err(), "failed to update claim: %s\n", err2)
6565
}
66-
return err
66+
return fmt.Errorf("uninstall failed: %s", errBuf)
6767
}

internal/commands/upgrade.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func runUpgrade(dockerCli command.Cli, installationName string, opts upgradeOpti
6767
if err != nil {
6868
return err
6969
}
70-
driverImpl, err := prepareDriver(dockerCli, bind)
70+
driverImpl, errBuf, err := prepareDriver(dockerCli, bind, nil)
7171
if err != nil {
7272
return err
7373
}
@@ -84,7 +84,7 @@ func runUpgrade(dockerCli command.Cli, installationName string, opts upgradeOpti
8484
err = u.Run(&c, creds, dockerCli.Out())
8585
err2 := claimStore.Store(c)
8686
if err != nil {
87-
return fmt.Errorf("upgrade failed: %v", err)
87+
return fmt.Errorf("upgrade failed: %s", errBuf)
8888
}
8989
return err2
9090
}

0 commit comments

Comments
 (0)