Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait for parallel builds to be cancelled on error #2424

Merged
merged 2 commits into from
Jul 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/skaffold/build/custom/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,17 @@ func NewArtifactBuilder(pushImages bool, additionalEnv []string) *ArtifactBuilde
// Build builds a custom artifact
// It returns true if the image is expected to exist remotely, or false if it is expected to exist locally
func (b *ArtifactBuilder) Build(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) error {
cmd, err := b.retrieveCmd(a, tag)
cmd, err := b.retrieveCmd(ctx, a, tag)
if err != nil {
return errors.Wrap(err, "retrieving cmd")
}
return cmd.Run()
}

func (b *ArtifactBuilder) retrieveCmd(a *latest.Artifact, tag string) (*exec.Cmd, error) {
func (b *ArtifactBuilder) retrieveCmd(ctx context.Context, a *latest.Artifact, tag string) (*exec.Cmd, error) {
artifact := a.CustomArtifact
split := strings.Split(artifact.BuildCommand, " ")
cmd := exec.Command(split[0], split[1:]...)
cmd := exec.CommandContext(ctx, split[0], split[1:]...)
env, err := b.retrieveEnv(a, tag)
if err != nil {
return nil, errors.Wrapf(err, "retrieving env variables for %s", a.ImageName)
Expand Down
5 changes: 3 additions & 2 deletions pkg/skaffold/build/custom/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package custom

import (
"context"
"os"
"os/exec"
"reflect"
Expand Down Expand Up @@ -115,7 +116,7 @@ func TestRetrieveCmd(t *testing.T) {
t.Override(&buildContext, func(string) (string, error) { return test.artifact.Workspace, nil })

builder := NewArtifactBuilder(false, nil)
cmd, err := builder.retrieveCmd(test.artifact, test.tag)
cmd, err := builder.retrieveCmd(context.Background(), test.artifact, test.tag)

t.CheckNoError(err)
// cmp.Diff cannot access unexported fields in *exec.Cmd, so use reflect.DeepEqual here directly
Expand All @@ -127,7 +128,7 @@ func TestRetrieveCmd(t *testing.T) {
}

func expectedCmd(buildCommand, dir string, args, env []string) *exec.Cmd {
cmd := exec.Command(buildCommand, args...)
cmd := exec.CommandContext(context.Background(), buildCommand, args...)
cmd.Dir = dir
cmd.Env = env
cmd.Stdout = os.Stdout
Expand Down
9 changes: 8 additions & 1 deletion pkg/skaffold/build/parallel.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,28 @@ func InParallel(ctx context.Context, out io.Writer, tags tag.ImageTags, artifact
return runInSequence(ctx, out, tags, artifacts, buildArtifact)
}

var wg sync.WaitGroup
defer wg.Wait()

ctx, cancel := context.WithCancel(ctx)
defer cancel()

results := new(sync.Map)
outputs := make([]chan []byte, len(artifacts))

// Run builds in //
wg.Add(len(artifacts))
for i := range artifacts {
outputs[i] = make(chan []byte, buffSize)
r, w := io.Pipe()
cw := setUpColorWriter(w, out)

// Run build and write output/logs to piped writer and store build result in
// sync.Map
go runBuild(ctx, cw, tags, artifacts[i], results, buildArtifact)
go func(i int) {
runBuild(ctx, cw, tags, artifacts[i], results, buildArtifact)
wg.Done()
}(i)
// Read build output/logs and write to buffered channel
go readOutputAndWriteToChannel(r, outputs[i])
}
Expand Down