Skip to content

Commit

Permalink
Merge pull request getporter#2243 from carolynvs/always-show-docker-b…
Browse files Browse the repository at this point in the history
…uild-output

Remove build --verbose, always print docker build output
  • Loading branch information
carolynvs authored Jul 18, 2022
2 parents 7cdb709 + 4c10f7d commit 5488a8b
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 35 deletions.
1 change: 0 additions & 1 deletion cmd/porter/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ The docker driver builds the bundle image using the local Docker host. To use a

f := cmd.Flags()
f.BoolVar(&opts.NoLint, "no-lint", false, "Do not run the linter")
f.BoolVarP(&opts.Verbose, "verbose", "v", false, "Enable verbose logging")
f.StringVar(&opts.Name, "name", "", "Override the bundle name")
f.StringVar(&opts.Version, "version", "", "Override the bundle version")
f.StringVarP(&opts.File, "file", "f", "",
Expand Down
1 change: 0 additions & 1 deletion docs/content/cli/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ porter build [flags]
--no-lint Do not run the linter
--secret stringArray Secret file to expose to the build (format: id=mysecret,src=/local/secret). Custom values are assessible as build arguments in the template Dockerfile and in the manifest using template variables. May be specified multiple times.
--ssh stringArray SSH agent socket or keys to expose to the build (format: default|<id>[=<socket>|<key>[,<key>]]). May be specified multiple times.
-v, --verbose Enable verbose logging
--version string Override the bundle version
```

Expand Down
1 change: 0 additions & 1 deletion docs/content/cli/bundles_build.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ porter bundles build [flags]
--no-lint Do not run the linter
--secret stringArray Secret file to expose to the build (format: id=mysecret,src=/local/secret). Custom values are assessible as build arguments in the template Dockerfile and in the manifest using template variables. May be specified multiple times.
--ssh stringArray SSH agent socket or keys to expose to the build (format: default|<id>[=<socket>|<key>[,<key>]]). May be specified multiple times.
-v, --verbose Enable verbose logging
--version string Override the bundle version
```

Expand Down
13 changes: 2 additions & 11 deletions pkg/build/buildkit/buildx.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"context"
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -133,16 +132,8 @@ func (b *Builder) BuildInvocationImage(ctx context.Context, manifest *manifest.M
},
}

out := ioutil.Discard
mode := progress.PrinterModeQuiet
if b.IsVerbose() {
mode = progress.PrinterModeAuto // Auto writes to stderr regardless of what you pass in

ctx, log = log.StartSpanWithName("buildkit", attribute.String("source", "porter.build.buildkit"))
defer log.EndSpan()
out = unstructuredLogger{log}
}

mode := progress.PrinterModeAuto // Auto writes to stderr regardless of what you pass in
out := unstructuredLogger{log}
printer := progress.NewPrinter(ctx, out, os.Stderr, mode)
_, buildErr := buildx.Build(ctx, drivers, buildxOpts, dockerToBuildx{cli}, confutil.ConfigDir(cli), printer)
printErr := printer.Wait()
Expand Down
22 changes: 9 additions & 13 deletions pkg/build/dockerfile-generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,28 +42,30 @@ func NewDockerfileGenerator(config *config.Config, m *manifest.Manifest, tmpl *t
}

func (g *DockerfileGenerator) GenerateDockerFile(ctx context.Context) error {
ctx, span := tracing.StartSpan(ctx)
defer span.EndSpan()

lines, err := g.buildDockerfile(ctx)
if err != nil {
return fmt.Errorf("error generating the Dockerfile: %w", err)
return span.Error(fmt.Errorf("error generating the Dockerfile: %w", err))
}

fmt.Fprintf(g.Out, "\nWriting Dockerfile =======>\n")
contents := strings.Join(lines, "\n")

if g.IsVerbose() {
fmt.Fprintln(g.Out, contents)
}
// Output the generated dockerfile
span.Debug(contents)

err = g.FileSystem.WriteFile(DOCKER_FILE, []byte(contents), pkg.FileModeWritable)
if err != nil {
return fmt.Errorf("couldn't write the Dockerfile: %w", err)
return span.Error(fmt.Errorf("couldn't write the Dockerfile: %w", err))
}

return nil
}

func (g *DockerfileGenerator) buildDockerfile(ctx context.Context) ([]string, error) {
fmt.Fprintf(g.Out, "\nGenerating Dockerfile =======>\n")
log := tracing.LoggerFromContext(ctx)
log.Debug("Generating Dockerfile")

lines, err := g.getBaseDockerfile(ctx)
if err != nil {
Expand All @@ -75,12 +77,6 @@ func (g *DockerfileGenerator) buildDockerfile(ctx context.Context) ([]string, er
lines = append(lines, g.buildWORKDIRSection())
lines = append(lines, g.buildCMDSection())

if g.IsVerbose() {
for _, line := range lines {
fmt.Fprintln(g.Out, line)
}
}

return lines, nil
}

Expand Down
22 changes: 14 additions & 8 deletions pkg/build/dockerfile-generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"get.porter.sh/porter/pkg/mixin"
"get.porter.sh/porter/pkg/templates"
"get.porter.sh/porter/pkg/test"
"get.porter.sh/porter/tests"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -111,32 +112,37 @@ COPY mybin /cnab/app/
func TestPorter_generateDockerfile(t *testing.T) {
t.Parallel()

ctx := context.Background()
c := config.NewTestConfig(t)
defer c.Close()

// Start a span so we can capture the output
ctx, log := c.StartRootSpan(ctx, t.Name())
defer log.Close()

tmpl := templates.NewTemplates(c.Config)
configTpl, err := tmpl.GetManifest()
require.Nil(t, err)
c.TestContext.AddTestFileContents(configTpl, config.Name)

m, err := manifest.LoadManifestFrom(context.Background(), c.Config, config.Name)
m, err := manifest.LoadManifestFrom(ctx, c.Config, config.Name)
require.NoError(t, err, "could not load manifest")

// ignore mixins in the unit tests
m.Mixins = []manifest.MixinDeclaration{}

mp := mixin.NewTestMixinProvider()
g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
err = g.GenerateDockerFile(context.Background())
err = g.GenerateDockerFile(ctx)
require.NoError(t, err)

wantDockerfilePath := ".cnab/Dockerfile"
dockerfileExists, err := c.FileSystem.Exists(wantDockerfilePath)
gotDockerfile, err := c.FileSystem.ReadFile(wantDockerfilePath)
require.NoError(t, err)
require.True(t, dockerfileExists, "Dockerfile wasn't written")

f, _ := c.FileSystem.Stat(wantDockerfilePath)
if f.Size() == 0 {
t.Fatalf("Dockerfile is empty")
}
// Verify that we logged the dockerfile contents
tests.RequireOutputContains(t, c.TestContext.GetError(), string(gotDockerfile), "expected the dockerfile to be printed to the logs")
test.CompareGoldenFile(t, "testdata/buildkit.Dockerfile", string(gotDockerfile))

// Verify that we didn't generate a Dockerfile at the root of the bundle dir
oldDockerfilePathExists, _ := c.FileSystem.Exists("Dockerfile")
Expand Down

0 comments on commit 5488a8b

Please sign in to comment.