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

chore: additional tidying and use ctx propogation to enable docker build cancellation #971

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
17 changes: 11 additions & 6 deletions builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
package builder

import (
"context"
"crypto/md5"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io"
"io/ioutil"
Expand All @@ -18,7 +20,7 @@ import (
"sort"
"strings"

v1execute "github.com/alexellis/go-execute/pkg/v1"
"github.com/openfaas/faas-cli/execute"
"github.com/openfaas/faas-cli/schema"
"github.com/openfaas/faas-cli/stack"
vcs "github.com/openfaas/faas-cli/versioncontrol"
Expand All @@ -30,7 +32,7 @@ const AdditionalPackageBuildArg = "ADDITIONAL_PACKAGE"

// BuildImage construct Docker image from function parameters
// TODO: refactor signature to a struct to simplify the length of the method header
func BuildImage(image string, handler string, functionName string, language string, nocache bool, squash bool, shrinkwrap bool, buildArgMap map[string]string, buildOptions []string, tagFormat schema.BuildFormat, buildLabelMap map[string]string, quietBuild bool, copyExtraPaths []string, remoteBuilder, payloadSecretPath string) error {
func BuildImage(ctx context.Context, image string, handler string, functionName string, language string, nocache bool, squash bool, shrinkwrap bool, buildArgMap map[string]string, buildOptions []string, tagFormat schema.BuildFormat, buildLabelMap map[string]string, quietBuild bool, copyExtraPaths []string, remoteBuilder, payloadSecretPath string) error {

if stack.IsValidTemplate(language) {
pathToTemplateYAML := fmt.Sprintf("./template/%s/template.yml", language)
Expand Down Expand Up @@ -135,22 +137,25 @@ func BuildImage(image string, handler string, functionName string, language stri
envs = append(envs, "DOCKER_BUILDKIT=1")
}

task := v1execute.ExecTask{
task := execute.ExecTask{
Cwd: tempPath,
Command: command,
Args: args,
StreamStdio: !quietBuild,
Env: envs,
}

res, err := task.Execute()

res, err := task.Execute(ctx)
if err != nil {
return err
}

if res.ExitCode == -1 && errors.Is(ctx.Err(), context.Canceled) {
return ctx.Err()
}

if res.ExitCode != 0 {
return fmt.Errorf("[%s] received non-zero exit code from build, error: %s", functionName, res.Stderr)
return fmt.Errorf("[%s] received non-zero exit code %d from build, error: %s", functionName, res.ExitCode, res.Stderr)
}

fmt.Printf("Image: %s built.\n", imageName)
Expand Down
30 changes: 20 additions & 10 deletions commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package commands

import (
"context"
"errors"
"fmt"
"log"
"os"
Expand Down Expand Up @@ -151,7 +153,7 @@ func parseBuildArgs(args []string) (map[string]string, error) {
}

func runBuild(cmd *cobra.Command, args []string) error {

ctx := cmd.Context()
var services stack.Services
if len(yamlFile) > 0 {
parsedServices, err := stack.ParseYAMLFile(yamlFile, regex, filter, envsubst)
Expand Down Expand Up @@ -192,7 +194,9 @@ func runBuild(cmd *cobra.Command, args []string) error {
return fmt.Errorf("please provide the deployed --name of your function")
}

if err := builder.BuildImage(image,
if err := builder.BuildImage(
ctx,
image,
handler,
functionName,
language,
Expand All @@ -214,7 +218,12 @@ func runBuild(cmd *cobra.Command, args []string) error {
return nil
}

errors := build(&services, parallel, shrinkwrap, quietBuild)
// a proper multi-error such as https://github.com/hashicorp/go-multierror
// would be nice here. In the current implementation we are unable to inspect
// the cause of the error, so we opted to just hide context cancel errors,
// but this makes it harder to detect this case upstream in the call stack.

errors := build(ctx, &services, parallel, shrinkwrap, quietBuild)
if len(errors) > 0 {
errorSummary := "Errors received during build:\n"
for _, err := range errors {
Expand All @@ -226,10 +235,10 @@ func runBuild(cmd *cobra.Command, args []string) error {
return nil
}

func build(services *stack.Services, queueDepth int, shrinkwrap, quietBuild bool) []error {
func build(ctx context.Context, services *stack.Services, queueDepth int, shrinkwrap, quietBuild bool) []error {
startOuter := time.Now()

errors := []error{}
buildErrors := []error{}

wg := sync.WaitGroup{}

Expand All @@ -248,7 +257,9 @@ func build(services *stack.Services, queueDepth int, shrinkwrap, quietBuild bool
combinedBuildOptions := combineBuildOpts(function.BuildOptions, buildOptions)
combinedBuildArgMap := util.MergeMap(function.BuildArgs, buildArgMap)
combinedExtraPaths := util.MergeSlice(services.StackConfiguration.CopyExtraPaths, copyExtra)
err := builder.BuildImage(function.Image,
err := builder.BuildImage(
ctx,
function.Image,
function.Handler,
function.Name,
function.Language,
Expand All @@ -264,9 +275,8 @@ func build(services *stack.Services, queueDepth int, shrinkwrap, quietBuild bool
remoteBuilder,
payloadSecretPath,
)

if err != nil {
errors = append(errors, err)
if err != nil && !errors.Is(err, context.Canceled) {
buildErrors = append(buildErrors, err)
}
}

Expand Down Expand Up @@ -295,7 +305,7 @@ func build(services *stack.Services, queueDepth int, shrinkwrap, quietBuild bool

duration := time.Since(startOuter)
fmt.Printf("\n%s\n", aec.Apply(fmt.Sprintf("Total build time: %1.2fs", duration.Seconds()), aec.YellowF))
return errors
return buildErrors
}

// pullTemplates pulls templates from specified git remote. templateURL may be a pinned repository.
Expand Down
7 changes: 6 additions & 1 deletion commands/faas.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package commands

import (
"context"
"fmt"
"log"
"os"
Expand All @@ -12,6 +13,7 @@ import (
"syscall"

"github.com/moby/term"
"github.com/openfaas/faas-cli/contexts"
"github.com/openfaas/faas-cli/version"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -74,6 +76,9 @@ func init() {
func Execute(customArgs []string) {
checkAndSetDefaultYaml()

ctx, cancel := contexts.WithSignals(context.Background(), os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
defer cancel()

faasCmd.SilenceUsage = true
faasCmd.SilenceErrors = true
faasCmd.SetArgs(customArgs[1:])
Expand Down Expand Up @@ -105,7 +110,7 @@ func Execute(customArgs []string) {
}
}

if err := faasCmd.Execute(); err != nil {
if err := faasCmd.ExecuteContext(ctx); err != nil {
e := err.Error()
fmt.Println(strings.ToUpper(e[:1]) + e[1:])
os.Exit(1)
Expand Down
83 changes: 31 additions & 52 deletions commands/local_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@ import (
"os"
"path/filepath"
"strings"
"syscall"

"os/exec"
"os/signal"

"github.com/openfaas/faas-cli/builder"
"github.com/openfaas/faas-cli/logger"
"github.com/openfaas/faas-cli/schema"
"github.com/openfaas/faas-cli/stack"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
)

const localSecretsDir = ".secrets"
Expand Down Expand Up @@ -102,15 +100,20 @@ func runLocalRunE(cmd *cobra.Command, args []string) error {
return watchLoop(cmd, args, localRunExec)
}

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

return localRunExec(cmd, args, ctx)
return localRunExec(cmd, args)
}

func localRunExec(cmd *cobra.Command, args []string, ctx context.Context) error {
func localRunExec(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
if opts.build {
log.Printf("[Local-Run] Building function")
if err := localBuild(cmd, args); err != nil {
if err == context.Canceled {
log.Printf("[Local-Run] Context cancelled, build cancelled")
return nil
}

logger.Debugf("[Local-Run] Error building function: %s", err.Error())
return err
}
}
Expand All @@ -123,6 +126,13 @@ func localRunExec(cmd *cobra.Command, args []string, ctx context.Context) error
name = args[0]
}

// In watch mode, it is possible that runFunction might be invoked after a cancelled build.
if ctx.Err() != nil {
log.Printf("[Local-Run] Context cancelled, skipping run")
return nil
}

logger.Debugf("[Local-Run] Starting execution: %s", name)
return runFunction(ctx, name, opts, args)

}
Expand Down Expand Up @@ -204,63 +214,32 @@ func runFunction(ctx context.Context, name string, opts runOptions, args []strin
fmt.Fprintf(opts.output, "%s\n", cmd.String())
return nil
}

sigs := make(chan os.Signal, 1)
signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)

cmd.Stdout = opts.output
cmd.Stderr = opts.err

fmt.Printf("Starting local-run for: %s on: http://0.0.0.0:%d\n\n", name, opts.port)
grpContext := context.Background()
grpContext, cancel := context.WithCancel(grpContext)
defer cancel()

errGrp, _ := errgroup.WithContext(grpContext)

errGrp.Go(func() error {
if err = cmd.Start(); err != nil {
return err
}

if err := cmd.Wait(); err != nil {
if strings.Contains(err.Error(), "signal: killed") {
return nil
} else if strings.Contains(err.Error(), "os: process already finished") {
return nil
}
// Always try to remove the container
defer removeContainer(name)

return err
}
return nil
})
if err = cmd.Start(); err != nil {
return err
}

// Always try to remove the container
defer func() {
removeContainer(name)
}()

errGrp.Go(func() error {

select {
case <-sigs:
log.Printf("Caught signal, exiting")
cancel()
case <-ctx.Done():
log.Printf("Context cancelled, exiting..")
cancel()
if err := cmd.Wait(); err != nil {
if strings.Contains(err.Error(), "signal: killed") {
return nil
} else if strings.Contains(err.Error(), "os: process already finished") {
return nil
}
return nil
})

return errGrp.Wait()
return err
}
return nil
}

func removeContainer(name string) {

runDockerRm := exec.Command("docker", "rm", "-f", name)
runDockerRm.Run()

}

// buildDockerRun constructs a exec.Cmd from the given stack Function
Expand Down
10 changes: 4 additions & 6 deletions commands/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package commands

import (
"bufio"
"context"
"fmt"
"os"
"strings"
Expand Down Expand Up @@ -89,21 +88,20 @@ func preRunUp(cmd *cobra.Command, args []string) error {

func upHandler(cmd *cobra.Command, args []string) error {
if watch {
return watchLoop(cmd, args, func(cmd *cobra.Command, args []string, ctx context.Context) error {
return watchLoop(cmd, args, func(cmd *cobra.Command, args []string) error {

if err := upRunner(cmd, args, ctx); err != nil {
if err := upRunner(cmd, args); err != nil {
return err
}
fmt.Println("[Watch] Change a file to trigger a rebuild...")
return nil
})
}

ctx := context.Background()
return upRunner(cmd, args, ctx)
return upRunner(cmd, args)
}

func upRunner(cmd *cobra.Command, args []string, ctx context.Context) error {
func upRunner(cmd *cobra.Command, args []string) error {
if usePublish {
if err := runPublish(cmd, args); err != nil {
return err
Expand Down
Loading