Skip to content

Commit

Permalink
Ko: disable compiler optimizations for debug (#6595)
Browse files Browse the repository at this point in the history
Disable Go compiler optimizations when building an artifact with the ko
builder, and the Skaffold run mode is debug.

**Tracking**: #6041
**Related**: #6563, #6569
  • Loading branch information
halvards authored Sep 20, 2021
1 parent 85a9c67 commit 8f040de
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 16 deletions.
2 changes: 1 addition & 1 deletion docs/design_proposals/ko-builder.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ Ko provides the
[`DisableOptimizations`](https://github.com/google/ko/blob/780c2812926cd706423e2ba65aeb1beb842c04af/pkg/commands/options/build.go#L34)
build option to
[set `gcflags` to disable optimizations and inlining](https://github.com/google/ko/blob/335c1ac8a6fdcc5eb0bb26579e4b44b4c62a9565/pkg/build/gobuild.go#L709-L712).
The ko builder will set `DisableOptimizations` to `true` when the Skaffold
The ko builder sets `DisableOptimizations` to `true` when the Skaffold
`runMode` is `Debug`.

Skaffold can
Expand Down
5 changes: 3 additions & 2 deletions pkg/skaffold/build/ko/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (

// latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/ko/schema"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/testutil"
)
Expand Down Expand Up @@ -149,7 +150,7 @@ func Test_getImportPath(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
b := NewArtifactBuilder(nil, false)
b := NewArtifactBuilder(nil, false, config.RunModes.Build)
koBuilder, err := b.newKoBuilder(context.Background(), test.artifact)
t.CheckNoError(err)

Expand Down Expand Up @@ -210,7 +211,7 @@ func Test_getImageIdentifier(t *testing.T) {
func stubKoArtifactBuilder(ref string, imageID string, pushImages bool, importpath string) *Builder {
api := (&testutil.FakeAPIClient{}).Add(ref, imageID)
localDocker := fakeLocalDockerDaemon(api)
b := NewArtifactBuilder(localDocker, pushImages)
b := NewArtifactBuilder(localDocker, pushImages, config.RunModes.Build)

// Fake implementation of the `publishImages` function.
// Returns a map with one entry: importpath -> ref
Expand Down
14 changes: 8 additions & 6 deletions pkg/skaffold/build/ko/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@ import (

// latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/ko/schema"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
)

func (b *Builder) newKoBuilder(ctx context.Context, a *latestV1.Artifact) (build.Interface, error) {
bo := buildOptions(a)
bo := buildOptions(a, b.runMode)
return commands.NewBuilder(ctx, bo)
}

func buildOptions(a *latestV1.Artifact) *options.BuildOptions {
func buildOptions(a *latestV1.Artifact, runMode config.RunMode) *options.BuildOptions {
workingDirectory := filepath.Join(a.Workspace, a.KoArtifact.Dir)
return &options.BuildOptions{
BaseImage: a.KoArtifact.BaseImage,
Expand All @@ -52,9 +53,10 @@ func buildOptions(a *latestV1.Artifact) *options.BuildOptions {
Main: a.KoArtifact.Target,
},
},
ConcurrentBuilds: 1,
Platform: strings.Join(a.KoArtifact.Platforms, ","),
UserAgent: version.UserAgentWithClient(),
WorkingDirectory: workingDirectory,
ConcurrentBuilds: 1,
DisableOptimizations: runMode == config.RunModes.Debug,
Platform: strings.Join(a.KoArtifact.Platforms, ","),
UserAgent: version.UserAgentWithClient(),
WorkingDirectory: workingDirectory,
}
}
25 changes: 20 additions & 5 deletions pkg/skaffold/build/ko/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,21 @@ import (
"path/filepath"
"testing"

// latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/ko/schema"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestBuildOptions(t *testing.T) {
tests := []struct {
description string
artifact latestV1.Artifact
wantPlatform string
wantWorkingDirectory string
description string
artifact latestV1.Artifact
runMode config.RunMode
wantPlatform string
wantWorkingDirectory string
wantDisableOptimizations bool
}{
{
description: "all zero value",
Expand Down Expand Up @@ -107,17 +111,28 @@ func TestBuildOptions(t *testing.T) {
},
wantWorkingDirectory: "my-app-subdirectory" + string(filepath.Separator) + "my-go-mod-is-here",
},
{
description: "disable compiler optimizations for debug",
artifact: latestV1.Artifact{
ArtifactType: latestV1.ArtifactType{
KoArtifact: &latestV1.KoArtifact{},
},
},
runMode: config.RunModes.Debug,
wantDisableOptimizations: true,
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
bo := buildOptions(&test.artifact)
bo := buildOptions(&test.artifact, test.runMode)
t.CheckDeepEqual(test.artifact.KoArtifact.BaseImage, bo.BaseImage)
if bo.ConcurrentBuilds < 1 {
t.Errorf("ConcurrentBuilds must always be >= 1 for the ko builder")
}
t.CheckDeepEqual(test.wantPlatform, bo.Platform)
t.CheckDeepEqual(version.UserAgentWithClient(), bo.UserAgent)
t.CheckDeepEqual(test.wantWorkingDirectory, bo.WorkingDirectory)
t.CheckDeepEqual(test.wantDisableOptimizations, bo.DisableOptimizations)
})
}
}
5 changes: 4 additions & 1 deletion pkg/skaffold/build/ko/ko.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,26 @@ import (
"github.com/google/ko/pkg/commands"
"github.com/google/ko/pkg/publish"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
)

// Builder is an artifact builder that uses ko
type Builder struct {
localDocker docker.LocalDaemon
pushImages bool
runMode config.RunMode

// publishImages can be overridden for unit testing purposes.
publishImages func(context.Context, []string, publish.Interface, build.Interface) (map[string]name.Reference, error)
}

// NewArtifactBuilder returns a new ko artifact builder
func NewArtifactBuilder(localDocker docker.LocalDaemon, pushImages bool) *Builder {
func NewArtifactBuilder(localDocker docker.LocalDaemon, pushImages bool, runMode config.RunMode) *Builder {
return &Builder{
localDocker: localDocker,
pushImages: pushImages,
runMode: runMode,
publishImages: commands.PublishImages,
}
}
4 changes: 3 additions & 1 deletion pkg/skaffold/build/ko/ko_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ package ko

import (
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
)

func TestNewArtifactBuilderCanPublishImages(t *testing.T) {
b := NewArtifactBuilder(nil, true)
b := NewArtifactBuilder(nil, true, config.RunModes.Build)
if b.publishImages == nil {
t.Errorf("constructor function should populate publishImages func")
}
Expand Down

0 comments on commit 8f040de

Please sign in to comment.