Skip to content

Commit

Permalink
Add ctx everywhere (#268)
Browse files Browse the repository at this point in the history
* Add ctx to publish.Interface

I noticed that hitting ctrl-C didn't work when pushing images, this
should fix that.

* Use context everywhere that makes sense
  • Loading branch information
jonjohnsonjr authored Dec 21, 2020
1 parent 552c3d4 commit 522c37c
Show file tree
Hide file tree
Showing 28 changed files with 88 additions and 65 deletions.
12 changes: 6 additions & 6 deletions pkg/build/gobuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const (
)

// GetBase takes an importpath and returns a base image.
type GetBase func(string) (Result, error)
type GetBase func(context.Context, string) (Result, error)

type builder func(context.Context, string, v1.Platform, bool) (string, error)

Expand Down Expand Up @@ -105,14 +105,14 @@ type modInfo struct {
// using go modules, otherwise returns nil.
//
// Related: https://github.com/golang/go/issues/26504
func moduleInfo() (*modules, error) {
func moduleInfo(ctx context.Context) (*modules, error) {
modules := modules{
deps: make(map[string]*modInfo),
}

// TODO we read all the output as a single byte array - it may
// be possible & more efficient to stream it
output, err := exec.Command("go", "list", "-mod=readonly", "-json", "-m", "all").Output()
output, err := exec.CommandContext(ctx, "go", "list", "-mod=readonly", "-json", "-m", "all").Output()
if err != nil {
return nil, nil
}
Expand Down Expand Up @@ -149,8 +149,8 @@ func moduleInfo() (*modules, error) {
// NewGo returns a build.Interface implementation that:
// 1. builds go binaries named by importpath,
// 2. containerizes the binary on a suitable base,
func NewGo(options ...Option) (Interface, error) {
module, err := moduleInfo()
func NewGo(ctx context.Context, options ...Option) (Interface, error) {
module, err := moduleInfo(ctx)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -579,7 +579,7 @@ func updatePath(cf *v1.ConfigFile) {
// Build implements build.Interface
func (g *gobuild) Build(ctx context.Context, s string) (Result, error) {
// Determine the appropriate base image for this import path.
base, err := g.getBase(s)
base, err := g.getBase(ctx, s)
if err != nil {
return nil, err
}
Expand Down
18 changes: 11 additions & 7 deletions pkg/build/gobuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestGoBuildIsSupportedRef(t *testing.T) {
t.Fatalf("random.Image() = %v", err)
}

ng, err := NewGo(WithBaseImages(func(string) (Result, error) { return base, nil }))
ng, err := NewGo(context.Background(), WithBaseImages(func(context.Context, string) (Result, error) { return base, nil }))
if err != nil {
t.Fatalf("NewGo() = %v", err)
}
Expand Down Expand Up @@ -88,7 +88,7 @@ func TestGoBuildIsSupportedRefWithModules(t *testing.T) {
}

opts := []Option{
WithBaseImages(func(string) (Result, error) { return base, nil }),
WithBaseImages(func(context.Context, string) (Result, error) { return base, nil }),
withModuleInfo(mods),
withBuildContext(stubBuildContext{
// make all referenced deps commands
Expand All @@ -99,7 +99,7 @@ func TestGoBuildIsSupportedRefWithModules(t *testing.T) {
}),
}

ng, err := NewGo(opts...)
ng, err := NewGo(context.Background(), opts...)
if err != nil {
t.Fatalf("NewGo() = %v", err)
}
Expand Down Expand Up @@ -158,8 +158,9 @@ func TestGoBuildNoKoData(t *testing.T) {

creationTime := v1.Time{Time: time.Unix(5000, 0)}
ng, err := NewGo(
context.Background(),
WithCreationTime(creationTime),
WithBaseImages(func(string) (Result, error) { return base, nil }),
WithBaseImages(func(context.Context, string) (Result, error) { return base, nil }),
withBuilder(writeTempFile),
)
if err != nil {
Expand Down Expand Up @@ -399,8 +400,9 @@ func TestGoBuild(t *testing.T) {

creationTime := v1.Time{Time: time.Unix(5000, 0)}
ng, err := NewGo(
context.Background(),
WithCreationTime(creationTime),
WithBaseImages(func(string) (Result, error) { return base, nil }),
WithBaseImages(func(context.Context, string) (Result, error) { return base, nil }),
withBuilder(writeTempFile),
)
if err != nil {
Expand Down Expand Up @@ -453,8 +455,9 @@ func TestGoBuildIndex(t *testing.T) {

creationTime := v1.Time{Time: time.Unix(5000, 0)}
ng, err := NewGo(
context.Background(),
WithCreationTime(creationTime),
WithBaseImages(func(string) (Result, error) { return base, nil }),
WithBaseImages(func(context.Context, string) (Result, error) { return base, nil }),
withBuilder(writeTempFile),
)
if err != nil {
Expand Down Expand Up @@ -523,8 +526,9 @@ func TestNestedIndex(t *testing.T) {

creationTime := v1.Time{Time: time.Unix(5000, 0)}
ng, err := NewGo(
context.Background(),
WithCreationTime(creationTime),
WithBaseImages(func(string) (Result, error) { return nestedBase, nil }),
WithBaseImages(func(context.Context, string) (Result, error) { return nestedBase, nil }),
withBuilder(writeTempFile),
)
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions pkg/commands/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ func addApply(topLevel *cobra.Command) {
return
}

builder, err := makeBuilder(bo)
// Cancel on signals.
ctx := createCancellableContext()

builder, err := makeBuilder(ctx, bo)
if err != nil {
log.Fatalf("error creating builder: %v", err)
}
Expand Down Expand Up @@ -97,7 +100,7 @@ func addApply(topLevel *cobra.Command) {
// to which we will pipe the resolved files.
argv := []string{"apply", "-f", "-"}
argv = append(argv, kubectlFlags...)
kubectlCmd := exec.Command("kubectl", argv...)
kubectlCmd := exec.CommandContext(ctx, "kubectl", argv...)

// Pass through our environment
kubectlCmd.Env = os.Environ()
Expand All @@ -111,9 +114,6 @@ func addApply(topLevel *cobra.Command) {
log.Fatalf("error piping to 'kubectl apply': %v", err)
}

// Cancel on signals.
ctx := createCancellableContext()

// Make sure builds are cancelled if kubectl apply fails.
g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
Expand Down
3 changes: 2 additions & 1 deletion pkg/commands/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func getBaseImage(platform string) build.GetBase {
}
}

return func(s string) (build.Result, error) {
return func(ctx context.Context, s string) (build.Result, error) {
s = strings.TrimPrefix(s, build.StrictScheme)
// Viper configuration file keys are case insensitive, and are
// returned as all lowercase. This means that import paths with
Expand All @@ -66,6 +66,7 @@ func getBaseImage(platform string) build.GetBase {
ropt := []remote.Option{
remote.WithAuthFromKeychain(authn.DefaultKeychain),
remote.WithTransport(defaultTransport()),
remote.WithContext(ctx),
}

// Using --platform=all will use an image index for the base,
Expand Down
10 changes: 5 additions & 5 deletions pkg/commands/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ func addCreate(topLevel *cobra.Command) {
return
}

builder, err := makeBuilder(bo)
// Cancel on signals.
ctx := createCancellableContext()

builder, err := makeBuilder(ctx, bo)
if err != nil {
log.Fatalf("error creating builder: %v", err)
}
Expand Down Expand Up @@ -97,7 +100,7 @@ func addCreate(topLevel *cobra.Command) {
// to which we will pipe the resolved files.
argv := []string{"create", "-f", "-"}
argv = append(argv, kubectlFlags...)
kubectlCmd := exec.Command("kubectl", argv...)
kubectlCmd := exec.CommandContext(ctx, "kubectl", argv...)

// Pass through our environment
kubectlCmd.Env = os.Environ()
Expand All @@ -111,9 +114,6 @@ func addCreate(topLevel *cobra.Command) {
log.Fatalf("error piping to 'kubectl create': %v", err)
}

// Cancel on signals.
ctx := createCancellableContext()

// Make sure builds are cancelled if kubectl create fails.
g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
Expand Down
5 changes: 4 additions & 1 deletion pkg/commands/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ func passthru(command string) runCmd {
return
}

// Cancel on signals.
ctx := createCancellableContext()

// Start building a command line invocation by passing
// through our arguments to command's CLI.
cmd := exec.Command(command, os.Args[1:]...)
cmd := exec.CommandContext(ctx, command, os.Args[1:]...)

// Pass through our environment
cmd.Env = os.Environ()
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ func addPublish(topLevel *cobra.Command) {
ko publish --local github.com/foo/bar/cmd/baz github.com/foo/bar/cmd/blah`,
Args: cobra.MinimumNArgs(1),
Run: func(_ *cobra.Command, args []string) {
builder, err := makeBuilder(bo)
ctx := createCancellableContext()
builder, err := makeBuilder(ctx, bo)
if err != nil {
log.Fatalf("error creating builder: %v", err)
}
Expand All @@ -67,7 +68,6 @@ func addPublish(topLevel *cobra.Command) {
log.Fatalf("error creating publisher: %v", err)
}
defer publisher.Close()
ctx := createCancellableContext()
images, err := publishImages(ctx, args, publisher, builder)
if err != nil {
log.Fatalf("failed to publish images: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func publishImages(ctx context.Context, importpaths []string, pub publish.Interf
if err != nil {
return nil, fmt.Errorf("error building %q: %v", importpath, err)
}
ref, err := pub.Publish(img, importpath)
ref, err := pub.Publish(ctx, img, importpath)
if err != nil {
return nil, fmt.Errorf("error publishing %s: %v", importpath, err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ func addResolve(topLevel *cobra.Command) {
ko resolve --local -f config/`,
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, args []string) {
builder, err := makeBuilder(bo)
ctx := createCancellableContext()
builder, err := makeBuilder(ctx, bo)
if err != nil {
log.Fatalf("error creating builder: %v", err)
}
Expand All @@ -65,7 +66,6 @@ func addResolve(topLevel *cobra.Command) {
log.Fatalf("error creating publisher: %v", err)
}
defer publisher.Close()
ctx := createCancellableContext()
if err := resolveFilesToWriter(ctx, builder, publisher, fo, so, sto, os.Stdout); err != nil {
log.Fatal(err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/commands/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ func gobuildOptions(bo *options.BuildOptions) ([]build.Option, error) {
return opts, nil
}

func makeBuilder(bo *options.BuildOptions) (*build.Caching, error) {
func makeBuilder(ctx context.Context, bo *options.BuildOptions) (*build.Caching, error) {
opt, err := gobuildOptions(bo)
if err != nil {
return nil, fmt.Errorf("error setting up builder options: %v", err)
}
innerBuilder, err := build.NewGo(opt...)
innerBuilder, err := build.NewGo(ctx, opt...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -188,7 +188,7 @@ type nopPublisher struct {
namer publish.Namer
}

func (n nopPublisher) Publish(br build.Result, s string) (name.Reference, error) {
func (n nopPublisher) Publish(_ context.Context, br build.Result, s string) (name.Reference, error) {
h, err := br.Digest()
if err != nil {
return nil, err
Expand Down
7 changes: 4 additions & 3 deletions pkg/commands/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ func addRun(topLevel *cobra.Command) {
# You can also supply args and flags to the command.
ko run ./cmd/baz -- -v arg1 arg2 --yes`,
Run: func(cmd *cobra.Command, args []string) {
ctx := createCancellableContext()

// Args after -- are for kubectl, so only consider importPaths before it.
importPaths := args
dashes := cmd.Flags().ArgsLenAtDash()
Expand All @@ -63,7 +65,7 @@ func addRun(topLevel *cobra.Command) {
kubectlArgs = os.Args[dashes:]
}

builder, err := makeBuilder(bo)
builder, err := makeBuilder(ctx, bo)
if err != nil {
log.Fatalf("error creating builder: %v", err)
}
Expand All @@ -80,7 +82,6 @@ func addRun(topLevel *cobra.Command) {
if strings.HasPrefix(ip, "-") {
log.Fatalf("expected first arg to be positional, got %q", ip)
}
ctx := createCancellableContext()
imgs, err := publishImages(ctx, importPaths, publisher, builder)
if err != nil {
log.Fatalf("failed to publish images: %v", err)
Expand Down Expand Up @@ -115,7 +116,7 @@ func addRun(topLevel *cobra.Command) {
argv = append([]string{"run", pod}, argv...)

log.Printf("$ kubectl %s", strings.Join(argv, " "))
kubectlCmd := exec.Command("kubectl", argv...)
kubectlCmd := exec.CommandContext(ctx, "kubectl", argv...)

// Pass through our environment
kubectlCmd.Env = os.Environ()
Expand Down
2 changes: 1 addition & 1 deletion pkg/internal/testing/fixed.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func NewFixedPublish(base name.Repository, entries map[string]v1.Hash) publish.I
}

// Publish implements publish.Interface
func (f *fixedPublish) Publish(_ build.Result, s string) (name.Reference, error) {
func (f *fixedPublish) Publish(_ context.Context, _ build.Result, s string) (name.Reference, error) {
s = strings.TrimPrefix(s, build.StrictScheme)
h, ok := f.entries[s]
if !ok {
Expand Down
6 changes: 3 additions & 3 deletions pkg/internal/testing/fixed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,23 @@ func TestFixedPublish(t *testing.T) {
},
})

fooDigest, err := f.Publish(nil, "foo")
fooDigest, err := f.Publish(context.Background(), nil, "foo")
if err != nil {
t.Errorf("Publish(foo) = %v", err)
}
if got, want := fooDigest.String(), "gcr.io/asdf/foo@sha256:"+hex1; got != want {
t.Errorf("Publish(foo) = %q, want %q", got, want)
}

barDigest, err := f.Publish(nil, "bar")
barDigest, err := f.Publish(context.Background(), nil, "bar")
if err != nil {
t.Errorf("Publish(bar) = %v", err)
}
if got, want := barDigest.String(), "gcr.io/asdf/bar@sha256:"+hex2; got != want {
t.Errorf("Publish(bar) = %q, want %q", got, want)
}

d, err := f.Publish(nil, "baz")
d, err := f.Publish(context.Background(), nil, "baz")
if err == nil {
t.Errorf("Publish(baz) = %v, want error", d)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/publish/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package publish

import (
"context"
"fmt"
"log"
"os"
Expand Down Expand Up @@ -43,7 +44,7 @@ func NewDaemon(namer Namer, tags []string) Interface {
}

// Publish implements publish.Interface
func (d *demon) Publish(br build.Result, s string) (name.Reference, error) {
func (d *demon) Publish(_ context.Context, br build.Result, s string) (name.Reference, error) {
s = strings.TrimPrefix(s, build.StrictScheme)
// https://github.com/google/go-containerregistry/issues/212
s = strings.ToLower(s)
Expand Down
4 changes: 2 additions & 2 deletions pkg/publish/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestDaemon(t *testing.T) {
}

def := NewDaemon(md5Hash, []string{})
if d, err := def.Publish(img, importpath); err != nil {
if d, err := def.Publish(context.Background(), img, importpath); err != nil {
t.Errorf("Publish() = %v", err)
} else if got, want := d.String(), md5Hash("ko.local", importpath); !strings.HasPrefix(got, want) {
t.Errorf("Publish() = %v, wanted prefix %v", got, want)
Expand All @@ -72,7 +72,7 @@ func TestDaemonTags(t *testing.T) {
}

def := NewDaemon(md5Hash, []string{"v2.0.0", "v1.2.3", "production"})
if d, err := def.Publish(img, importpath); err != nil {
if d, err := def.Publish(context.Background(), img, importpath); err != nil {
t.Errorf("Publish() = %v", err)
} else if got, want := d.String(), md5Hash("ko.local", importpath); !strings.HasPrefix(got, want) {
t.Errorf("Publish() = %v, wanted prefix %v", got, want)
Expand Down
Loading

0 comments on commit 522c37c

Please sign in to comment.