From 8f040de82c71d7608c9b214b14ecdb99274ebacb Mon Sep 17 00:00:00 2001 From: Halvard Skogsrud Date: Tue, 21 Sep 2021 03:53:34 +1000 Subject: [PATCH] Ko: disable compiler optimizations for debug (#6595) Disable Go compiler optimizations when building an artifact with the ko builder, and the Skaffold run mode is debug. **Tracking**: #6041 **Related**: #6563, #6569 --- docs/design_proposals/ko-builder.md | 2 +- pkg/skaffold/build/ko/build_test.go | 5 +++-- pkg/skaffold/build/ko/builder.go | 14 ++++++++------ pkg/skaffold/build/ko/builder_test.go | 25 ++++++++++++++++++++----- pkg/skaffold/build/ko/ko.go | 5 ++++- pkg/skaffold/build/ko/ko_test.go | 4 +++- 6 files changed, 39 insertions(+), 16 deletions(-) diff --git a/docs/design_proposals/ko-builder.md b/docs/design_proposals/ko-builder.md index a57ec449a9f..0fddf950b42 100644 --- a/docs/design_proposals/ko-builder.md +++ b/docs/design_proposals/ko-builder.md @@ -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 diff --git a/pkg/skaffold/build/ko/build_test.go b/pkg/skaffold/build/ko/build_test.go index 9442183f564..8973052d4bf 100644 --- a/pkg/skaffold/build/ko/build_test.go +++ b/pkg/skaffold/build/ko/build_test.go @@ -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" ) @@ -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) @@ -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 diff --git a/pkg/skaffold/build/ko/builder.go b/pkg/skaffold/build/ko/builder.go index 37dd0c37122..e68ec4f3a16 100644 --- a/pkg/skaffold/build/ko/builder.go +++ b/pkg/skaffold/build/ko/builder.go @@ -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, @@ -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, } } diff --git a/pkg/skaffold/build/ko/builder_test.go b/pkg/skaffold/build/ko/builder_test.go index 65e8419148a..8ef2d8c37c7 100644 --- a/pkg/skaffold/build/ko/builder_test.go +++ b/pkg/skaffold/build/ko/builder_test.go @@ -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", @@ -107,10 +111,20 @@ 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") @@ -118,6 +132,7 @@ func TestBuildOptions(t *testing.T) { t.CheckDeepEqual(test.wantPlatform, bo.Platform) t.CheckDeepEqual(version.UserAgentWithClient(), bo.UserAgent) t.CheckDeepEqual(test.wantWorkingDirectory, bo.WorkingDirectory) + t.CheckDeepEqual(test.wantDisableOptimizations, bo.DisableOptimizations) }) } } diff --git a/pkg/skaffold/build/ko/ko.go b/pkg/skaffold/build/ko/ko.go index e53db9fd410..0839aabd8c7 100644 --- a/pkg/skaffold/build/ko/ko.go +++ b/pkg/skaffold/build/ko/ko.go @@ -24,6 +24,7 @@ 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" ) @@ -31,16 +32,18 @@ import ( 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, } } diff --git a/pkg/skaffold/build/ko/ko_test.go b/pkg/skaffold/build/ko/ko_test.go index 566a1a97bac..a6b0250d6f1 100644 --- a/pkg/skaffold/build/ko/ko_test.go +++ b/pkg/skaffold/build/ko/ko_test.go @@ -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") }