Merge pull request #1689 from jessvalarezo/ctr-refactor-third-pass

ctr: move signals, resolver utility functions to commands package

cmd/ctr/attach.go

@@ -55,8 +55,8 @@ var taskAttachCommand = cli.Command{
 				logrus.WithError(err).Error("console resize")
 			}
 		} else {
-			sigc := forwardAllSignals(ctx, task)
+			sigc := commands.ForwardAllSignals(ctx, task)
-			defer stopCatch(sigc)
+			defer commands.StopCatch(sigc)
 		}
 
 		ec := <-statusC

cmd/ctr/commands/resolver.go

@@ -0,0 +1,92 @@
+package commands
+
+import (
+	"bufio"
+	gocontext "context"
+	"crypto/tls"
+	"fmt"
+	"net"
+	"net/http"
+	"strings"
+	"time"
+
+	"github.com/containerd/console"
+	"github.com/containerd/containerd/remotes"
+	"github.com/containerd/containerd/remotes/docker"
+	"github.com/pkg/errors"
+	"github.com/urfave/cli"
+)
+
+// PushTracker returns a new InMemoryTracker which tracks the ref status
+var PushTracker = docker.NewInMemoryTracker()
+
+func passwordPrompt() (string, error) {
+	c := console.Current()
+	defer c.Reset()
+
+	if err := c.DisableEcho(); err != nil {
+		return "", errors.Wrap(err, "failed to disable echo")
+	}
+
+	line, _, err := bufio.NewReader(c).ReadLine()
+	if err != nil {
+		return "", errors.Wrap(err, "failed to read line")
+	}
+	return string(line), nil
+}
+
+// GetResolver prepares the resolver from the environment and options
+func GetResolver(ctx gocontext.Context, clicontext *cli.Context) (remotes.Resolver, error) {
+	username := clicontext.String("user")
+	var secret string
+	if i := strings.IndexByte(username, ':'); i > 0 {
+		secret = username[i+1:]
+		username = username[0:i]
+	}
+	options := docker.ResolverOptions{
+		PlainHTTP: clicontext.Bool("plain-http"),
+		Tracker:   PushTracker,
+	}
+	if username != "" {
+		if secret == "" {
+			fmt.Printf("Password: ")
+
+			var err error
+			secret, err = passwordPrompt()
+			if err != nil {
+				return nil, err
+			}
+
+			fmt.Print("\n")
+		}
+	} else if rt := clicontext.String("refresh"); rt != "" {
+		secret = rt
+	}
+
+	options.Credentials = func(host string) (string, string, error) {
+		// Only one host
+		return username, secret, nil
+	}
+
+	tr := &http.Transport{
+		Proxy: http.ProxyFromEnvironment,
+		DialContext: (&net.Dialer{
+			Timeout:   30 * time.Second,
+			KeepAlive: 30 * time.Second,
+			DualStack: true,
+		}).DialContext,
+		MaxIdleConns:        10,
+		IdleConnTimeout:     30 * time.Second,
+		TLSHandshakeTimeout: 10 * time.Second,
+		TLSClientConfig: &tls.Config{
+			InsecureSkipVerify: clicontext.Bool("insecure"),
+		},
+		ExpectContinueTimeout: 5 * time.Second,
+	}
+
+	options.Client = &http.Client{
+		Transport: tr,
+	}
+
+	return docker.NewResolver(options), nil
+}

cmd/ctr/commands/signals.go

@@ -0,0 +1,59 @@
+package commands
+
+import (
+	gocontext "context"
+	"fmt"
+	"os"
+	"os/signal"
+	"strconv"
+	"strings"
+	"syscall"
+
+	"github.com/containerd/containerd"
+	"github.com/sirupsen/logrus"
+)
+
+type killer interface {
+	Kill(gocontext.Context, syscall.Signal, ...containerd.KillOpts) error
+}
+
+// ForwardAllSignals forwards signals
+func ForwardAllSignals(ctx gocontext.Context, task killer) chan os.Signal {
+	sigc := make(chan os.Signal, 128)
+	signal.Notify(sigc)
+	go func() {
+		for s := range sigc {
+			logrus.Debug("forwarding signal ", s)
+			if err := task.Kill(ctx, s.(syscall.Signal)); err != nil {
+				logrus.WithError(err).Errorf("forward signal %s", s)
+			}
+		}
+	}()
+	return sigc
+}
+
+// StopCatch stops and closes a channel
+func StopCatch(sigc chan os.Signal) {
+	signal.Stop(sigc)
+	close(sigc)
+}
+
+// ParseSignal parses a given string into a syscall.Signal
+// it checks that the signal exists in the platform-appropriate signalMap
+func ParseSignal(rawSignal string) (syscall.Signal, error) {
+	s, err := strconv.Atoi(rawSignal)
+	if err == nil {
+		sig := syscall.Signal(s)
+		for _, msig := range signalMap {
+			if sig == msig {
+				return sig, nil
+			}
+		}
+		return -1, fmt.Errorf("unknown signal %q", rawSignal)
+	}
+	signal, ok := signalMap[strings.TrimPrefix(strings.ToUpper(rawSignal), "SIG")]
+	if !ok {
+		return -1, fmt.Errorf("unknown signal %q", rawSignal)
+	}
+	return signal, nil
+}

cmd/ctr/commands/signals_linux.go

@@ -1,4 +1,4 @@
-package main
+package commands
 
 import (
 	"syscall"

cmd/ctr/commands/signals_unix.go

@@ -1,6 +1,6 @@
 // +build darwin freebsd solaris
 
-package main
+package commands
 
 import (
 	"syscall"

cmd/ctr/commands/signals_windows.go

@@ -0,0 +1,23 @@
+package commands
+
+import (
+	"syscall"
+
+	"golang.org/x/sys/windows"
+)
+
+var signalMap = map[string]syscall.Signal{
+	"HUP":    syscall.Signal(windows.SIGHUP),
+	"INT":    syscall.Signal(windows.SIGINT),
+	"QUIT":   syscall.Signal(windows.SIGQUIT),
+	"SIGILL": syscall.Signal(windows.SIGILL),
+	"TRAP":   syscall.Signal(windows.SIGTRAP),
+	"ABRT":   syscall.Signal(windows.SIGABRT),
+	"BUS":    syscall.Signal(windows.SIGBUS),
+	"FPE":    syscall.Signal(windows.SIGFPE),
+	"KILL":   syscall.Signal(windows.SIGKILL),
+	"SEGV":   syscall.Signal(windows.SIGSEGV),
+	"PIPE":   syscall.Signal(windows.SIGPIPE),
+	"ALRM":   syscall.Signal(windows.SIGALRM),
+	"TERM":   syscall.Signal(windows.SIGTERM),
+}

cmd/ctr/exec.go

@@ -10,6 +10,7 @@ import (
 	"github.com/urfave/cli"
 )
 
+//TODO:(jessvalarezo) exec-id is optional here, update to required arg
 var taskExecCommand = cli.Command{
 	Name:      "exec",
 	Usage:     "execute additional processes in an existing container",
@@ -87,8 +88,8 @@ var taskExecCommand = cli.Command{
 				logrus.WithError(err).Error("console resize")
 			}
 		} else {
-			sigc := forwardAllSignals(ctx, process)
+			sigc := commands.ForwardAllSignals(ctx, process)
-			defer stopCatch(sigc)
+			defer commands.StopCatch(sigc)
 		}
 
 		if err := process.Start(ctx); err != nil {

cmd/ctr/fetch.go

@@ -57,7 +57,7 @@ func fetch(ref string, cliContext *cli.Context) (containerd.Image, error) {
 	}
 	defer cancel()
 
-	resolver, err := getResolver(ctx, cliContext)
+	resolver, err := commands.GetResolver(ctx, cliContext)
 	if err != nil {
 		return nil, err
 	}

cmd/ctr/fetchobject.go

@@ -26,7 +26,7 @@ var fetchObjectCommand = cli.Command{
 		ctx, cancel := commands.AppContext(context)
 		defer cancel()
 
-		resolver, err := getResolver(ctx, context)
+		resolver, err := commands.GetResolver(ctx, context)
 		if err != nil {
 			return err
 		}

cmd/ctr/kill.go

@@ -6,6 +6,8 @@ import (
 	"github.com/urfave/cli"
 )
 
+// TODO:(jessvalarezo) the pid flag is not used here
+// update to be able to signal given pid
 var taskKillCommand = cli.Command{
 	Name:      "kill",
 	Usage:     "signal a container (default: SIGTERM)",
@@ -31,7 +33,7 @@ var taskKillCommand = cli.Command{
 		if id == "" {
 			return errors.New("container id must be provided")
 		}
-		signal, err := parseSignal(context.String("signal"))
+		signal, err := commands.ParseSignal(context.String("signal"))
 		if err != nil {
 			return err
 		}

cmd/ctr/push.go

@@ -21,10 +21,6 @@ import (
 	"golang.org/x/sync/errgroup"
 )
 
-var (
-	pushTracker = docker.NewInMemoryTracker()
-)
-
 var pushCommand = cli.Command{
 	Name:      "push",
 	Usage:     "push an image to a remote",
@@ -74,11 +70,11 @@ var pushCommand = cli.Command{
 			desc = img.Target
 		}
 
-		resolver, err := getResolver(ctx, context)
+		resolver, err := commands.GetResolver(ctx, context)
 		if err != nil {
 			return err
 		}
-		ongoing := newPushJobs(pushTracker)
+		ongoing := newPushJobs(commands.PushTracker)
 
 		eg, ctx := errgroup.WithContext(ctx)
 

cmd/ctr/pushobject.go

@@ -33,7 +33,7 @@ var pushObjectCommand = cli.Command{
 		}
 		defer cancel()
 
-		resolver, err := getResolver(ctx, context)
+		resolver, err := commands.GetResolver(ctx, context)
 		if err != nil {
 			return err
 		}

cmd/ctr/run.go

@@ -4,7 +4,6 @@ import (
 	gocontext "context"
 	"fmt"
 	"runtime"
-	"syscall"
 
 	"github.com/containerd/console"
 	"github.com/containerd/containerd"
@@ -20,10 +19,6 @@ type resizer interface {
 	Resize(ctx gocontext.Context, w, h uint32) error
 }
 
-type killer interface {
-	Kill(gocontext.Context, syscall.Signal, ...containerd.KillOpts) error
-}
-
 func withEnv(context *cli.Context) containerd.SpecOpts {
 	return func(_ gocontext.Context, _ *containerd.Client, _ *containers.Container, s *specs.Spec) error {
 		env := context.StringSlice("env")
@@ -160,8 +155,8 @@ var runCommand = cli.Command{
 				logrus.WithError(err).Error("console resize")
 			}
 		} else {
-			sigc := forwardAllSignals(ctx, task)
+			sigc := commands.ForwardAllSignals(ctx, task)
-			defer stopCatch(sigc)
+			defer commands.StopCatch(sigc)
 		}
 		status := <-statusC
 		code, _, err := status.Result()

cmd/ctr/start.go

@@ -70,8 +70,8 @@ var taskStartCommand = cli.Command{
 				logrus.WithError(err).Error("console resize")
 			}
 		} else {
-			sigc := forwardAllSignals(ctx, task)
+			sigc := commands.ForwardAllSignals(ctx, task)
-			defer stopCatch(sigc)
+			defer commands.StopCatch(sigc)
 		}
 
 		status := <-statusC

cmd/ctr/utils.go

@@ -1,137 +1,13 @@
 package main
 
 import (
-	"bufio"
-	gocontext "context"
-	"crypto/tls"
 	"encoding/csv"
 	"fmt"
-	"net"
-	"net/http"
-	"os"
-	"os/signal"
-	"strconv"
 	"strings"
-	"syscall"
-	"time"
 
-	"github.com/containerd/console"
-	"github.com/containerd/containerd/remotes"
-	"github.com/containerd/containerd/remotes/docker"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
-	"github.com/urfave/cli"
 )
 
-func passwordPrompt() (string, error) {
-	c := console.Current()
-	defer c.Reset()
-
-	if err := c.DisableEcho(); err != nil {
-		return "", errors.Wrap(err, "failed to disable echo")
-	}
-
-	line, _, err := bufio.NewReader(c).ReadLine()
-	if err != nil {
-		return "", errors.Wrap(err, "failed to read line")
-	}
-	return string(line), nil
-}
-
-// getResolver prepares the resolver from the environment and options.
-func getResolver(ctx gocontext.Context, clicontext *cli.Context) (remotes.Resolver, error) {
-	username := clicontext.String("user")
-	var secret string
-	if i := strings.IndexByte(username, ':'); i > 0 {
-		secret = username[i+1:]
-		username = username[0:i]
-	}
-	options := docker.ResolverOptions{
-		PlainHTTP: clicontext.Bool("plain-http"),
-		Tracker:   pushTracker,
-	}
-	if username != "" {
-		if secret == "" {
-			fmt.Printf("Password: ")
-
-			var err error
-			secret, err = passwordPrompt()
-			if err != nil {
-				return nil, err
-			}
-
-			fmt.Print("\n")
-		}
-	} else if rt := clicontext.String("refresh"); rt != "" {
-		secret = rt
-	}
-
-	options.Credentials = func(host string) (string, string, error) {
-		// Only one host
-		return username, secret, nil
-	}
-
-	tr := &http.Transport{
-		Proxy: http.ProxyFromEnvironment,
-		DialContext: (&net.Dialer{
-			Timeout:   30 * time.Second,
-			KeepAlive: 30 * time.Second,
-			DualStack: true,
-		}).DialContext,
-		MaxIdleConns:        10,
-		IdleConnTimeout:     30 * time.Second,
-		TLSHandshakeTimeout: 10 * time.Second,
-		TLSClientConfig: &tls.Config{
-			InsecureSkipVerify: clicontext.Bool("insecure"),
-		},
-		ExpectContinueTimeout: 5 * time.Second,
-	}
-
-	options.Client = &http.Client{
-		Transport: tr,
-	}
-
-	return docker.NewResolver(options), nil
-}
-
-func forwardAllSignals(ctx gocontext.Context, task killer) chan os.Signal {
-	sigc := make(chan os.Signal, 128)
-	signal.Notify(sigc)
-	go func() {
-		for s := range sigc {
-			logrus.Debug("forwarding signal ", s)
-			if err := task.Kill(ctx, s.(syscall.Signal)); err != nil {
-				logrus.WithError(err).Errorf("forward signal %s", s)
-			}
-		}
-	}()
-	return sigc
-}
-
-func parseSignal(rawSignal string) (syscall.Signal, error) {
-	s, err := strconv.Atoi(rawSignal)
-	if err == nil {
-		sig := syscall.Signal(s)
-		for _, msig := range signalMap {
-			if sig == msig {
-				return sig, nil
-			}
-		}
-		return -1, fmt.Errorf("unknown signal %q", rawSignal)
-	}
-	signal, ok := signalMap[strings.TrimPrefix(strings.ToUpper(rawSignal), "SIG")]
-	if !ok {
-		return -1, fmt.Errorf("unknown signal %q", rawSignal)
-	}
-	return signal, nil
-}
-
-func stopCatch(sigc chan os.Signal) {
-	signal.Stop(sigc)
-	close(sigc)
-}
-
 // parseMountFlag parses a mount string in the form "type=foo,source=/path,destination=/target,options=rbind:rw"
 func parseMountFlag(m string) (specs.Mount, error) {
 	mount := specs.Mount{}

cmd/ctr/utils_windows.go

@@ -5,12 +5,10 @@ import (
 	"net"
 	"os"
 	"sync"
-	"syscall"
 
 	"github.com/Microsoft/go-winio"
 	clog "github.com/containerd/containerd/log"
 	"github.com/pkg/errors"
-	"golang.org/x/sys/windows"
 )
 
 func prepareStdio(stdin, stdout, stderr string, console bool) (*sync.WaitGroup, error) {
@@ -91,19 +89,3 @@ func prepareStdio(stdin, stdout, stderr string, console bool) (*sync.WaitGroup,
 
 	return &wg, nil
 }
-
-var signalMap = map[string]syscall.Signal{
-	"HUP":    syscall.Signal(windows.SIGHUP),
-	"INT":    syscall.Signal(windows.SIGINT),
-	"QUIT":   syscall.Signal(windows.SIGQUIT),
-	"SIGILL": syscall.Signal(windows.SIGILL),
-	"TRAP":   syscall.Signal(windows.SIGTRAP),
-	"ABRT":   syscall.Signal(windows.SIGABRT),
-	"BUS":    syscall.Signal(windows.SIGBUS),
-	"FPE":    syscall.Signal(windows.SIGFPE),
-	"KILL":   syscall.Signal(windows.SIGKILL),
-	"SEGV":   syscall.Signal(windows.SIGSEGV),
-	"PIPE":   syscall.Signal(windows.SIGPIPE),
-	"ALRM":   syscall.Signal(windows.SIGALRM),
-	"TERM":   syscall.Signal(windows.SIGTERM),
-}