Add global handler for Interrupt signal

Fix Ctrl+C not working with "crictl logs -f" command.
Switch to the global signal handler in following commands:
  * crictl port-forward
  * crictl stats -w

cmd/crictl/logs.go

@@ -67,7 +67,7 @@ var logsCommand = &cli.Command{
 			Usage:   "Show timestamps",
 		},
 	},
-	Action: func(ctx *cli.Context) error {
+	Action: func(ctx *cli.Context) (retErr error) {
 		runtimeService, err := getRuntimeService(ctx)
 		if err != nil {
 			return err
@@ -108,7 +108,26 @@ var logsCommand = &cli.Command{
 			logPath = fmt.Sprintf("%s%s%s", logPath[:strings.LastIndex(logPath, "/")+1], fmt.Sprint(containerAttempt-1),
 				logPath[strings.LastIndex(logPath, "."):])
 		}
-		return logs.ReadLogs(context.Background(), logPath, status.GetId(), logOptions, runtimeService, os.Stdout, os.Stderr)
+		// build a WithCancel context based on cli.context
+		readLogCtx, cancelFn := context.WithCancel(ctx.Context)
+		go func() {
+			<-SetupInterruptSignalHandler()
+			// cancel readLogCtx when Interrupt signal received
+			cancelFn()
+		}()
+		defer func() {
+			// We can not use the typed error "context.Canceled" here
+			// because the upstream K8S dependency explicitly returns a fmt.Errorf("context cancelled").
+			// So we need to compare the error in string.
+			if retErr != nil && retErr.Error() == "context cancelled" {
+				// Silent the "context cancelled" error.
+				// In order to prevent the error msg when user hit Ctrl+C.
+				retErr = nil
+			}
+			// Ensure no context leak
+			cancelFn()
+		}()
+		return logs.ReadLogs(readLogCtx, logPath, status.GetId(), logOptions, runtimeService, os.Stdout, os.Stderr)
 	},
 }
 

cmd/crictl/main_unix.go

@@ -18,8 +18,15 @@ limitations under the License.
 
 package main
 
+import (
+	"os"
+	"syscall"
+)
+
 const (
 	defaultConfigPath = "/etc/crictl.yaml"
 )
 
 var defaultRuntimeEndpoints = []string{"unix:///var/run/dockershim.sock", "unix:///run/containerd/containerd.sock", "unix:///run/crio/crio.sock"}
+
+var shutdownSignals = []os.Signal{os.Interrupt, syscall.SIGTERM}

cmd/crictl/main_windows.go

@@ -26,6 +26,8 @@ import (
 var defaultRuntimeEndpoints = []string{"npipe:////./pipe/dockershim", "npipe:////./pipe/containerd", "npipe:////./pipe/crio"}
 var defaultConfigPath string
 
+var shutdownSignals = []os.Signal{os.Interrupt}
+
 func init() {
 	defaultConfigPath = filepath.Join(os.Getenv("USERPROFILE"), ".crictl", "crictl.yaml")
 }

cmd/crictl/portforward.go

@@ -21,14 +21,13 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"os/signal"
 
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli/v2"
 	"golang.org/x/net/context"
 	restclient "k8s.io/client-go/rest"
-	portforward "k8s.io/client-go/tools/portforward"
+	"k8s.io/client-go/tools/portforward"
 	"k8s.io/client-go/transport/spdy"
 	pb "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 )
@@ -100,21 +99,10 @@ func PortForward(client pb.RuntimeServiceClient, opts portforwardOptions) error
 	}
 	dialer := spdy.NewDialer(upgrader, &http.Client{Transport: transport}, "POST", URL)
 
-	stopChan := make(chan struct{}, 1)
 	readyChan := make(chan struct{})
 
-	signals := make(chan os.Signal, 1)
-	signal.Notify(signals, os.Interrupt)
-	defer signal.Stop(signals)
-
-	go func() {
-		<-signals
-		if stopChan != nil {
-			close(stopChan)
-		}
-	}()
 	logrus.Debugf("Ports to forword: %v", opts.ports)
-	pf, err := portforward.New(dialer, opts.ports, stopChan, readyChan, os.Stdout, os.Stderr)
+	pf, err := portforward.New(dialer, opts.ports, SetupInterruptSignalHandler(), readyChan, os.Stdout, os.Stderr)
 	if err != nil {
 		return err
 	}

cmd/crictl/stats.go

@@ -19,7 +19,6 @@ package main
 import (
 	"fmt"
 	"os"
-	"os/signal"
 	"sort"
 	"time"
 
@@ -154,10 +153,13 @@ func ContainerStats(client pb.RuntimeServiceClient, opts statsOptions) error {
 			return err
 		}
 	} else {
-		s := make(chan os.Signal)
-		signal.Notify(s, os.Interrupt)
 		go func() {
-			<-s
+			<-SetupInterruptSignalHandler()
+			// TODO(povsister): Safely return nil in main goroutine
+			// instead of calling os.Exit(0) when user hit Ctrl+C.
+			// It help preserve the "Post Run" behavior defined in
+			// "AfterFunc" with the CLI framework.
+			// (Though there is no "AfterFunc" yet.)
 			os.Exit(0)
 		}()
 		for range time.Tick(500 * time.Millisecond) {

cmd/crictl/util.go

@@ -22,10 +22,12 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"os/signal"
 	"reflect"
 	"regexp"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/golang/protobuf/jsonpb"
@@ -43,6 +45,32 @@ const (
 	truncatedIDLen = 13
 )
 
+var (
+	// The global stopCh for monitoring Interrupt signal.
+	// DO NOT use it directly. Use SetupInterruptSignalHandler() to get it.
+	signalIntStopCh chan struct{}
+	// only setup stopCh once
+	signalIntSetupOnce = &sync.Once{}
+)
+
+// SetupInterruptSignalHandler setup a global signal handler monitoring Interrupt signal. e.g: Ctrl+C.
+// The returned read-only channel will be closed on receiving Interrupt signals.
+// It will directly call os.Exit(1) on receiving Interrupt signal twice.
+func SetupInterruptSignalHandler() <-chan struct{} {
+	signalIntSetupOnce.Do(func() {
+		signalIntStopCh = make(chan struct{})
+		c := make(chan os.Signal, 2)
+		signal.Notify(c, shutdownSignals...)
+		go func() {
+			<-c
+			close(signalIntStopCh)
+			<-c
+			os.Exit(1) // Exit immediately on second signal
+		}()
+	})
+	return signalIntStopCh
+}
+
 type listOptions struct {
 	// id of container or sandbox
 	id string