From d3e0c163f8765bd253552221bf038d8089a39ff8 Mon Sep 17 00:00:00 2001 From: "Justin Terry (VM)" Date: Tue, 24 Jul 2018 15:19:06 -0700 Subject: [PATCH 1/5] Adds runtime v2 support for Windows shim's Implements the various requirements for the runtime v2 code to abstract away the unix/linux code into the appropriate platform level abstractions to use the runtime v2 on Windows as well. Adds support in the Makefile.windows to actually build the runtime v2 code for Windows by setting a shell environment BUILD_WINDOWS_V2=1 before calling make. (Note this disables the compilation of the Windows runtime v1) Signed-off-by: Justin Terry (VM) --- Makefile.windows | 7 ++ cmd/containerd/builtins_windows.go | 2 + cmd/containerd/builtins_windows_v2.go | 25 +++++++ runtime/v2/shim/shim.go | 69 +---------------- runtime/v2/shim/shim_nix.go | 102 ++++++++++++++++++++++++++ runtime/v2/shim/shim_windows.go | 96 ++++++++++++++++++++++++ runtime/v2/shim/util.go | 27 ------- runtime/v2/shim/util_nix.go | 57 ++++++++++++++ runtime/v2/shim/util_windows.go | 31 ++++++++ 9 files changed, 323 insertions(+), 93 deletions(-) create mode 100644 cmd/containerd/builtins_windows_v2.go create mode 100644 runtime/v2/shim/shim_nix.go create mode 100644 runtime/v2/shim/shim_windows.go create mode 100644 runtime/v2/shim/util_nix.go diff --git a/Makefile.windows b/Makefile.windows index 5dbb1f8a5..feefb9229 100644 --- a/Makefile.windows +++ b/Makefile.windows @@ -23,3 +23,10 @@ BINARY_SUFFIX=".exe" ifeq ($(GOARCH),amd64) TESTFLAGS_RACE= -race endif + +# add support for building the Windows v2 runtime +# based on the containerd-shim-runhcs-v1 shim rather +# than the existing runtime on hcsshim +ifeq (${BUILD_WINDOWS_V2},1) + BUILDTAGS += windows_v2 +endif diff --git a/cmd/containerd/builtins_windows.go b/cmd/containerd/builtins_windows.go index 6a0451edc..d81c62e87 100644 --- a/cmd/containerd/builtins_windows.go +++ b/cmd/containerd/builtins_windows.go @@ -1,3 +1,5 @@ +// +build !windows_v2 + /* Copyright The containerd Authors. diff --git a/cmd/containerd/builtins_windows_v2.go b/cmd/containerd/builtins_windows_v2.go new file mode 100644 index 000000000..f2dac227a --- /dev/null +++ b/cmd/containerd/builtins_windows_v2.go @@ -0,0 +1,25 @@ +// +build windows_v2 + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package main + +import ( + _ "github.com/containerd/containerd/diff/windows" + _ "github.com/containerd/containerd/runtime/v2" + _ "github.com/containerd/containerd/snapshots/windows" +) diff --git a/runtime/v2/shim/shim.go b/runtime/v2/shim/shim.go index 7592cf4ef..9187aa8a0 100644 --- a/runtime/v2/shim/shim.go +++ b/runtime/v2/shim/shim.go @@ -1,5 +1,3 @@ -// +build !windows - /* Copyright The containerd Authors. @@ -19,29 +17,22 @@ package shim import ( - "bytes" "context" "flag" "fmt" - "net" "os" - "os/exec" - "os/signal" "runtime" "runtime/debug" "strings" - "syscall" "time" "github.com/containerd/containerd/events" "github.com/containerd/containerd/namespaces" shimapi "github.com/containerd/containerd/runtime/v2/task" "github.com/containerd/ttrpc" - "github.com/containerd/typeurl" "github.com/gogo/protobuf/proto" "github.com/pkg/errors" "github.com/sirupsen/logrus" - "golang.org/x/sys/unix" ) // Client for a shim server @@ -178,7 +169,7 @@ func NewShimClient(ctx context.Context, svc shimapi.TaskService, signals chan os // Serve the shim server func (s *Client) Serve() error { dump := make(chan os.Signal, 32) - signal.Notify(dump, syscall.SIGUSR1) + setupDumpStacks(dump) path, err := os.Getwd() if err != nil { @@ -211,23 +202,11 @@ func (s *Client) Serve() error { // serve serves the ttrpc API over a unix socket at the provided path // this function does not block func serve(ctx context.Context, server *ttrpc.Server, path string) error { - var ( - l net.Listener - err error - ) - if path == "" { - l, err = net.FileListener(os.NewFile(3, "socket")) - path = "[inherited from parent]" - } else { - if len(path) > 106 { - return errors.Errorf("%q: unix socket path too long (> 106)", path) - } - l, err = net.Listen("unix", "\x00"+path) - } + l, path, err := serveListener(path) if err != nil { return err } - logrus.WithField("socket", path).Debug("serving api on unix socket") + logrus.WithField("socket", path).Debug("serving api on abstract socket") go func() { defer l.Close() if err := server.Serve(ctx, l); err != nil && @@ -238,22 +217,6 @@ func serve(ctx context.Context, server *ttrpc.Server, path string) error { return nil } -func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { - logger.Info("starting signal loop") - for { - select { - case s := <-signals: - switch s { - case unix.SIGCHLD: - if err := Reap(); err != nil { - logger.WithError(err).Error("reap exit status") - } - case unix.SIGPIPE: - } - } - } -} - func dumpStacks(logger *logrus.Entry) { var ( buf []byte @@ -273,29 +236,3 @@ type remoteEventsPublisher struct { address string containerdBinaryPath string } - -func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error { - ns, _ := namespaces.Namespace(ctx) - encoded, err := typeurl.MarshalAny(event) - if err != nil { - return err - } - data, err := encoded.Marshal() - if err != nil { - return err - } - cmd := exec.CommandContext(ctx, l.containerdBinaryPath, "--address", l.address, "publish", "--topic", topic, "--namespace", ns) - cmd.Stdin = bytes.NewReader(data) - c, err := Default.Start(cmd) - if err != nil { - return err - } - status, err := Default.Wait(cmd, c) - if err != nil { - return err - } - if status != 0 { - return errors.New("failed to publish event") - } - return nil -} diff --git a/runtime/v2/shim/shim_nix.go b/runtime/v2/shim/shim_nix.go new file mode 100644 index 000000000..ba9b68be8 --- /dev/null +++ b/runtime/v2/shim/shim_nix.go @@ -0,0 +1,102 @@ +// +build !windows + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package shim + +import ( + "bytes" + "context" + "net" + "os" + "os/exec" + "os/signal" + "syscall" + + "github.com/containerd/containerd/events" + "github.com/containerd/containerd/namespaces" + "github.com/containerd/typeurl" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "golang.org/x/sys/unix" +) + +func setupDumpStacks(dump chan<- os.Signal) { + signal.Notify(dump, syscall.SIGUSR1) +} + +func serveListener(path string) (net.Listener, string, error) { + var ( + l net.Listener + err error + ) + if path == "" { + l, err = net.FileListener(os.NewFile(3, "socket")) + path = "[inherited from parent]" + } else { + if len(path) > 106 { + return nil, path, errors.Errorf("%q: unix socket path too long (> 106)", path) + } + l, err = net.Listen("unix", "\x00"+path) + } + if err != nil { + return nil, path, err + } + return l, path, nil +} + +func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { + logger.Info("starting signal loop") + for { + select { + case s := <-signals: + switch s { + case unix.SIGCHLD: + if err := Reap(); err != nil { + logger.WithError(err).Error("reap exit status") + } + case unix.SIGPIPE: + } + } + } +} + +func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error { + ns, _ := namespaces.Namespace(ctx) + encoded, err := typeurl.MarshalAny(event) + if err != nil { + return err + } + data, err := encoded.Marshal() + if err != nil { + return err + } + cmd := exec.CommandContext(ctx, l.containerdBinaryPath, "--address", l.address, "publish", "--topic", topic, "--namespace", ns) + cmd.Stdin = bytes.NewReader(data) + c, err := Default.Start(cmd) + if err != nil { + return err + } + status, err := Default.Wait(cmd, c) + if err != nil { + return err + } + if status != 0 { + return errors.New("failed to publish event") + } + return nil +} diff --git a/runtime/v2/shim/shim_windows.go b/runtime/v2/shim/shim_windows.go new file mode 100644 index 000000000..ff49cb303 --- /dev/null +++ b/runtime/v2/shim/shim_windows.go @@ -0,0 +1,96 @@ +// +build windows + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package shim + +import ( + "context" + "net" + "os" + + winio "github.com/Microsoft/go-winio" + "github.com/containerd/containerd/events" + "github.com/containerd/ttrpc" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +// setupSignals creates a new signal handler for all signals +func setupSignals() (chan os.Signal, error) { + signals := make(chan os.Signal, 32) + // TODO: JTERRY75: Make this based on events. + return signals, nil +} + +func newServer() (*ttrpc.Server, error) { + return ttrpc.NewServer() +} + +func subreaper() error { + return nil +} + +func setupDumpStacks(dump chan<- os.Signal) { + // TODO: JTERRY75: Make this based on events. signal.Notify(dump, syscall.SIGUSR1) +} + +// serve serves the ttrpc API over a unix socket at the provided path +// this function does not block +func serveListener(path string) (net.Listener, string, error) { + if path == "" { + return nil, path, errors.New("'socket' must be npipe path") + } + l, err := winio.ListenPipe(path, nil) + if err != nil { + return nil, path, err + } + return l, path, nil +} + +func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { + // TODO: JTERRY75: Make this based on events? + return nil +} + +func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error { + /* TOOD: JTERRY75: Implement publish for windows + ns, _ := namespaces.Namespace(ctx) + encoded, err := typeurl.MarshalAny(event) + if err != nil { + return err + } + data, err := encoded.Marshal() + if err != nil { + return err + } + cmd := exec.CommandContext(ctx, l.containerdBinaryPath, "--address", l.address, "publish", "--topic", topic, "--namespace", ns) + cmd.Stdin = bytes.NewReader(data) + c, err := Default.Start(cmd) + if err != nil { + return err + } + status, err := Default.Wait(cmd, c) + if err != nil { + return err + } + if status != 0 { + return errors.New("failed to publish event") + } + */ + return nil +} diff --git a/runtime/v2/shim/util.go b/runtime/v2/shim/util.go index 29d5d41bf..1c3fb85c0 100644 --- a/runtime/v2/shim/util.go +++ b/runtime/v2/shim/util.go @@ -70,38 +70,11 @@ func BinaryName(runtime string) string { return fmt.Sprintf(shimBinaryFormat, parts[len(parts)-2], parts[len(parts)-1]) } -// AbstractAddress returns an abstract socket address -func AbstractAddress(ctx context.Context, id string) (string, error) { - ns, err := namespaces.NamespaceRequired(ctx) - if err != nil { - return "", err - } - return filepath.Join(string(filepath.Separator), "containerd-shim", ns, id, "shim.sock"), nil -} - // Connect to the provided address func Connect(address string, d func(string, time.Duration) (net.Conn, error)) (net.Conn, error) { return d(address, 100*time.Second) } -// AnonDialer returns a dialer for an abstract socket -func AnonDialer(address string, timeout time.Duration) (net.Conn, error) { - address = strings.TrimPrefix(address, "unix://") - return net.DialTimeout("unix", "\x00"+address, timeout) -} - -// NewSocket returns a new socket -func NewSocket(address string) (*net.UnixListener, error) { - if len(address) > 106 { - return nil, errors.Errorf("%q: unix socket path too long (> 106)", address) - } - l, err := net.Listen("unix", "\x00"+address) - if err != nil { - return nil, errors.Wrapf(err, "failed to listen to abstract unix socket %q", address) - } - return l.(*net.UnixListener), nil -} - // WritePidFile writes a pid file atomically func WritePidFile(path string, pid int) error { path, err := filepath.Abs(path) diff --git a/runtime/v2/shim/util_nix.go b/runtime/v2/shim/util_nix.go new file mode 100644 index 000000000..63db21135 --- /dev/null +++ b/runtime/v2/shim/util_nix.go @@ -0,0 +1,57 @@ +// +build !windows + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package shim + +import ( + "context" + "net" + "path/filepath" + "strings" + "time" + + "github.com/containerd/containerd/namespaces" + "github.com/pkg/errors" +) + +// AbstractAddress returns an abstract socket address +func AbstractAddress(ctx context.Context, id string) (string, error) { + ns, err := namespaces.NamespaceRequired(ctx) + if err != nil { + return "", err + } + return filepath.Join(string(filepath.Separator), "containerd-shim", ns, id, "shim.sock"), nil +} + +// AnonDialer returns a dialer for an abstract socket +func AnonDialer(address string, timeout time.Duration) (net.Conn, error) { + address = strings.TrimPrefix(address, "unix://") + return net.DialTimeout("unix", "\x00"+address, timeout) +} + +// NewSocket returns a new socket +func NewSocket(address string) (*net.UnixListener, error) { + if len(address) > 106 { + return nil, errors.Errorf("%q: unix socket path too long (> 106)", address) + } + l, err := net.Listen("unix", "\x00"+address) + if err != nil { + return nil, errors.Wrapf(err, "failed to listen to abstract unix socket %q", address) + } + return l.(*net.UnixListener), nil +} diff --git a/runtime/v2/shim/util_windows.go b/runtime/v2/shim/util_windows.go index b5a3ecb9d..f85e43dd3 100644 --- a/runtime/v2/shim/util_windows.go +++ b/runtime/v2/shim/util_windows.go @@ -17,7 +17,15 @@ package shim import ( + "context" + "fmt" + "net" "syscall" + "time" + + winio "github.com/Microsoft/go-winio" + "github.com/containerd/containerd/namespaces" + "github.com/pkg/errors" ) func getSysProcAttr() *syscall.SysProcAttr { @@ -28,3 +36,26 @@ func getSysProcAttr() *syscall.SysProcAttr { func SetScore(pid int) error { return nil } + +// AbstractAddress returns an abstract npipe address +func AbstractAddress(ctx context.Context, id string) (string, error) { + ns, err := namespaces.NamespaceRequired(ctx) + if err != nil { + return "", err + } + return fmt.Sprintf("\\\\.\\pipe\\containerd-shim-%s-%s-pipe", ns, id), nil +} + +// AnonDialer returns a dialer for an abstract npipe +func AnonDialer(address string, timeout time.Duration) (net.Conn, error) { + return winio.DialPipe(address, &timeout) +} + +// NewSocket returns a new npipe listener +func NewSocket(address string) (net.Listener, error) { + l, err := winio.ListenPipe(address, nil) + if err != nil { + return nil, errors.Wrapf(err, "failed to listen to abstract npipe %s", address) + } + return l, nil +} From f15a1170d368f9fce9639dff2c2edba714daba9c Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Thu, 26 Jul 2018 11:21:50 -0400 Subject: [PATCH 2/5] Add windows publisher Signed-off-by: Michael Crosby --- runtime/v2/shim/shim_windows.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/runtime/v2/shim/shim_windows.go b/runtime/v2/shim/shim_windows.go index ff49cb303..7827b25ad 100644 --- a/runtime/v2/shim/shim_windows.go +++ b/runtime/v2/shim/shim_windows.go @@ -19,13 +19,17 @@ package shim import ( + "bytes" "context" "net" "os" + "os/exec" winio "github.com/Microsoft/go-winio" "github.com/containerd/containerd/events" + "github.com/containerd/containerd/namespaces" "github.com/containerd/ttrpc" + "github.com/containerd/typeurl" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -68,7 +72,6 @@ func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { } func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error { - /* TOOD: JTERRY75: Implement publish for windows ns, _ := namespaces.Namespace(ctx) encoded, err := typeurl.MarshalAny(event) if err != nil { @@ -80,17 +83,5 @@ func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event } cmd := exec.CommandContext(ctx, l.containerdBinaryPath, "--address", l.address, "publish", "--topic", topic, "--namespace", ns) cmd.Stdin = bytes.NewReader(data) - c, err := Default.Start(cmd) - if err != nil { - return err - } - status, err := Default.Wait(cmd, c) - if err != nil { - return err - } - if status != 0 { - return errors.New("failed to publish event") - } - */ - return nil + return cmd.Run() } From 9d72b4543b57e85b5a7f60d69df7e00433e8feae Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Thu, 26 Jul 2018 11:24:20 -0400 Subject: [PATCH 3/5] Handle windows signals Since windows does not require a signal handler, we just block on the channel forever so that it does not exit. Signed-off-by: Michael Crosby --- runtime/v2/shim/shim_windows.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/runtime/v2/shim/shim_windows.go b/runtime/v2/shim/shim_windows.go index 7827b25ad..0f1616a35 100644 --- a/runtime/v2/shim/shim_windows.go +++ b/runtime/v2/shim/shim_windows.go @@ -37,7 +37,6 @@ import ( // setupSignals creates a new signal handler for all signals func setupSignals() (chan os.Signal, error) { signals := make(chan os.Signal, 32) - // TODO: JTERRY75: Make this based on events. return signals, nil } @@ -67,7 +66,7 @@ func serveListener(path string) (net.Listener, string, error) { } func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { - // TODO: JTERRY75: Make this based on events? + <-signals return nil } From 13549f7a07dc2422065c393c1d77a70708697a2f Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 27 Jul 2018 12:39:53 -0400 Subject: [PATCH 4/5] Abstract to SocketAddress This updates some methods for cross platform use as well as unifying _unix files. Signed-off-by: Michael Crosby --- runtime/v2/runc/service.go | 2 +- runtime/v2/shim/shim_darwin.go | 18 +----- runtime/v2/shim/shim_linux.go | 12 ---- runtime/v2/shim/shim_nix.go | 102 -------------------------------- runtime/v2/shim/shim_unix.go | 82 ++++++++++++++++++++++--- runtime/v2/shim/util_linux.go | 34 ----------- runtime/v2/shim/util_nix.go | 57 ------------------ runtime/v2/shim/util_unix.go | 40 ++++++++++++- runtime/v2/shim/util_windows.go | 4 +- 9 files changed, 117 insertions(+), 234 deletions(-) delete mode 100644 runtime/v2/shim/shim_nix.go delete mode 100644 runtime/v2/shim/util_linux.go delete mode 100644 runtime/v2/shim/util_nix.go diff --git a/runtime/v2/runc/service.go b/runtime/v2/runc/service.go index f9d7e4413..625c44434 100644 --- a/runtime/v2/runc/service.go +++ b/runtime/v2/runc/service.go @@ -136,7 +136,7 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container if err != nil { return "", err } - address, err := shim.AbstractAddress(ctx, id) + address, err := shim.SocketAddress(ctx, id) if err != nil { return "", err } diff --git a/runtime/v2/shim/shim_darwin.go b/runtime/v2/shim/shim_darwin.go index 4d39041b9..314b45cb5 100644 --- a/runtime/v2/shim/shim_darwin.go +++ b/runtime/v2/shim/shim_darwin.go @@ -18,25 +18,9 @@ package shim -import ( - "os" - "os/signal" - - "github.com/containerd/ttrpc" -) - -// setupSignals creates a new signal handler for all signals and sets the shim as a -// sub-reaper so that the container processes are reparented -func setupSignals() (chan os.Signal, error) { - signals := make(chan os.Signal, 2048) - signal.Notify(signals) - return signals, nil -} +import "github.com/containerd/ttrpc" func newServer() (*ttrpc.Server, error) { - // for darwin, we omit the socket credentials because these syscalls are - // slightly different. since we don't have darwin support yet, this can be - // implemented later and the build can continue without issue. return ttrpc.NewServer() } diff --git a/runtime/v2/shim/shim_linux.go b/runtime/v2/shim/shim_linux.go index dce592b97..7ad2a7262 100644 --- a/runtime/v2/shim/shim_linux.go +++ b/runtime/v2/shim/shim_linux.go @@ -17,22 +17,10 @@ package shim import ( - "os" - "os/signal" - "github.com/containerd/containerd/sys" "github.com/containerd/ttrpc" - "golang.org/x/sys/unix" ) -// setupSignals creates a new signal handler for all signals and sets the shim as a -// sub-reaper so that the container processes are reparented -func setupSignals() (chan os.Signal, error) { - signals := make(chan os.Signal, 32) - signal.Notify(signals, unix.SIGTERM, unix.SIGINT, unix.SIGCHLD, unix.SIGPIPE) - return signals, nil -} - func newServer() (*ttrpc.Server, error) { return ttrpc.NewServer(ttrpc.WithServerHandshaker(ttrpc.UnixSocketRequireSameUser())) } diff --git a/runtime/v2/shim/shim_nix.go b/runtime/v2/shim/shim_nix.go deleted file mode 100644 index ba9b68be8..000000000 --- a/runtime/v2/shim/shim_nix.go +++ /dev/null @@ -1,102 +0,0 @@ -// +build !windows - -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package shim - -import ( - "bytes" - "context" - "net" - "os" - "os/exec" - "os/signal" - "syscall" - - "github.com/containerd/containerd/events" - "github.com/containerd/containerd/namespaces" - "github.com/containerd/typeurl" - "github.com/pkg/errors" - "github.com/sirupsen/logrus" - "golang.org/x/sys/unix" -) - -func setupDumpStacks(dump chan<- os.Signal) { - signal.Notify(dump, syscall.SIGUSR1) -} - -func serveListener(path string) (net.Listener, string, error) { - var ( - l net.Listener - err error - ) - if path == "" { - l, err = net.FileListener(os.NewFile(3, "socket")) - path = "[inherited from parent]" - } else { - if len(path) > 106 { - return nil, path, errors.Errorf("%q: unix socket path too long (> 106)", path) - } - l, err = net.Listen("unix", "\x00"+path) - } - if err != nil { - return nil, path, err - } - return l, path, nil -} - -func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { - logger.Info("starting signal loop") - for { - select { - case s := <-signals: - switch s { - case unix.SIGCHLD: - if err := Reap(); err != nil { - logger.WithError(err).Error("reap exit status") - } - case unix.SIGPIPE: - } - } - } -} - -func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error { - ns, _ := namespaces.Namespace(ctx) - encoded, err := typeurl.MarshalAny(event) - if err != nil { - return err - } - data, err := encoded.Marshal() - if err != nil { - return err - } - cmd := exec.CommandContext(ctx, l.containerdBinaryPath, "--address", l.address, "publish", "--topic", topic, "--namespace", ns) - cmd.Stdin = bytes.NewReader(data) - c, err := Default.Start(cmd) - if err != nil { - return err - } - status, err := Default.Wait(cmd, c) - if err != nil { - return err - } - if status != 0 { - return errors.New("failed to publish event") - } - return nil -} diff --git a/runtime/v2/shim/shim_unix.go b/runtime/v2/shim/shim_unix.go index 24ee0f8b3..4b1ea824a 100644 --- a/runtime/v2/shim/shim_unix.go +++ b/runtime/v2/shim/shim_unix.go @@ -1,4 +1,4 @@ -// +build !linux,!windows,!darwin +// +build !windows /* Copyright The containerd Authors. @@ -19,24 +19,92 @@ package shim import ( + "bytes" + "context" + "net" "os" + "os/exec" "os/signal" + "syscall" - "github.com/containerd/ttrpc" + "github.com/containerd/containerd/events" + "github.com/containerd/containerd/namespaces" + "github.com/containerd/typeurl" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "golang.org/x/sys/unix" ) // setupSignals creates a new signal handler for all signals and sets the shim as a // sub-reaper so that the container processes are reparented func setupSignals() (chan os.Signal, error) { - signals := make(chan os.Signal, 2048) - signal.Notify(signals) + signals := make(chan os.Signal, 32) + signal.Notify(signals, unix.SIGTERM, unix.SIGINT, unix.SIGCHLD, unix.SIGPIPE) return signals, nil } -func newServer() (*ttrpc.Server, error) { - return ttrpc.NewServer(ttrpc.WithServerHandshaker(ttrpc.UnixSocketRequireSameUser())) +func setupDumpStacks(dump chan<- os.Signal) { + signal.Notify(dump, syscall.SIGUSR1) } -func subreaper() error { +func serveListener(path string) (net.Listener, string, error) { + var ( + l net.Listener + err error + ) + if path == "" { + l, err = net.FileListener(os.NewFile(3, "socket")) + path = "[inherited from parent]" + } else { + if len(path) > 106 { + return nil, path, errors.Errorf("%q: unix socket path too long (> 106)", path) + } + l, err = net.Listen("unix", "\x00"+path) + } + if err != nil { + return nil, path, err + } + return l, path, nil +} + +func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { + logger.Info("starting signal loop") + for { + select { + case s := <-signals: + switch s { + case unix.SIGCHLD: + if err := Reap(); err != nil { + logger.WithError(err).Error("reap exit status") + } + case unix.SIGPIPE: + } + } + } +} + +func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error { + ns, _ := namespaces.Namespace(ctx) + encoded, err := typeurl.MarshalAny(event) + if err != nil { + return err + } + data, err := encoded.Marshal() + if err != nil { + return err + } + cmd := exec.CommandContext(ctx, l.containerdBinaryPath, "--address", l.address, "publish", "--topic", topic, "--namespace", ns) + cmd.Stdin = bytes.NewReader(data) + c, err := Default.Start(cmd) + if err != nil { + return err + } + status, err := Default.Wait(cmd, c) + if err != nil { + return err + } + if status != 0 { + return errors.New("failed to publish event") + } return nil } diff --git a/runtime/v2/shim/util_linux.go b/runtime/v2/shim/util_linux.go deleted file mode 100644 index f7178b6b5..000000000 --- a/runtime/v2/shim/util_linux.go +++ /dev/null @@ -1,34 +0,0 @@ -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package shim - -import ( - "syscall" - - "github.com/containerd/containerd/sys" -) - -func getSysProcAttr() *syscall.SysProcAttr { - return &syscall.SysProcAttr{ - Setpgid: true, - } -} - -// SetScore sets the oom score for a process -func SetScore(pid int) error { - return sys.SetOOMScore(pid, sys.OOMScoreMaxKillable) -} diff --git a/runtime/v2/shim/util_nix.go b/runtime/v2/shim/util_nix.go deleted file mode 100644 index 63db21135..000000000 --- a/runtime/v2/shim/util_nix.go +++ /dev/null @@ -1,57 +0,0 @@ -// +build !windows - -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package shim - -import ( - "context" - "net" - "path/filepath" - "strings" - "time" - - "github.com/containerd/containerd/namespaces" - "github.com/pkg/errors" -) - -// AbstractAddress returns an abstract socket address -func AbstractAddress(ctx context.Context, id string) (string, error) { - ns, err := namespaces.NamespaceRequired(ctx) - if err != nil { - return "", err - } - return filepath.Join(string(filepath.Separator), "containerd-shim", ns, id, "shim.sock"), nil -} - -// AnonDialer returns a dialer for an abstract socket -func AnonDialer(address string, timeout time.Duration) (net.Conn, error) { - address = strings.TrimPrefix(address, "unix://") - return net.DialTimeout("unix", "\x00"+address, timeout) -} - -// NewSocket returns a new socket -func NewSocket(address string) (*net.UnixListener, error) { - if len(address) > 106 { - return nil, errors.Errorf("%q: unix socket path too long (> 106)", address) - } - l, err := net.Listen("unix", "\x00"+address) - if err != nil { - return nil, errors.Wrapf(err, "failed to listen to abstract unix socket %q", address) - } - return l.(*net.UnixListener), nil -} diff --git a/runtime/v2/shim/util_unix.go b/runtime/v2/shim/util_unix.go index e2d99bf97..262fe2b36 100644 --- a/runtime/v2/shim/util_unix.go +++ b/runtime/v2/shim/util_unix.go @@ -1,4 +1,4 @@ -// +build !linux,!windows +// +build !windows /* Copyright The containerd Authors. @@ -19,7 +19,16 @@ package shim import ( + "context" + "net" + "path/filepath" + "strings" "syscall" + "time" + + "github.com/containerd/containerd/namespaces" + "github.com/containerd/containerd/sys" + "github.com/pkg/errors" ) func getSysProcAttr() *syscall.SysProcAttr { @@ -30,5 +39,32 @@ func getSysProcAttr() *syscall.SysProcAttr { // SetScore sets the oom score for a process func SetScore(pid int) error { - return nil + return sys.SetOOMScore(pid, sys.OOMScoreMaxKillable) +} + +// SocketAddress returns an abstract socket address +func SocketAddress(ctx context.Context, id string) (string, error) { + ns, err := namespaces.NamespaceRequired(ctx) + if err != nil { + return "", err + } + return filepath.Join(string(filepath.Separator), "containerd-shim", ns, id, "shim.sock"), nil +} + +// AnonDialer returns a dialer for an abstract socket +func AnonDialer(address string, timeout time.Duration) (net.Conn, error) { + address = strings.TrimPrefix(address, "unix://") + return net.DialTimeout("unix", "\x00"+address, timeout) +} + +// NewSocket returns a new socket +func NewSocket(address string) (*net.UnixListener, error) { + if len(address) > 106 { + return nil, errors.Errorf("%q: unix socket path too long (> 106)", address) + } + l, err := net.Listen("unix", "\x00"+address) + if err != nil { + return nil, errors.Wrapf(err, "failed to listen to abstract unix socket %q", address) + } + return l.(*net.UnixListener), nil } diff --git a/runtime/v2/shim/util_windows.go b/runtime/v2/shim/util_windows.go index f85e43dd3..b3d71d747 100644 --- a/runtime/v2/shim/util_windows.go +++ b/runtime/v2/shim/util_windows.go @@ -37,8 +37,8 @@ func SetScore(pid int) error { return nil } -// AbstractAddress returns an abstract npipe address -func AbstractAddress(ctx context.Context, id string) (string, error) { +// SocketAddress returns an abstract npipe address +func SocketAddress(ctx context.Context, id string) (string, error) { ns, err := namespaces.NamespaceRequired(ctx) if err != nil { return "", err From af1b6a026eed60ab01098f7aae90fe8797c82e79 Mon Sep 17 00:00:00 2001 From: "Justin Terry (VM)" Date: Fri, 27 Jul 2018 09:46:15 -0700 Subject: [PATCH 5/5] Review feedback. 1. Moves the log message for each socket to the appropriate _unix and _windows.go 2. Replaces all reference to Abstract Socket for Windows. 3. Adds support for ctrl+c on Windows to exit a shim. Signed-off-by: Justin Terry (VM) --- runtime/v2/shim/shim.go | 3 +-- runtime/v2/shim/shim_unix.go | 9 +++++---- runtime/v2/shim/shim_windows.go | 21 +++++++++++++++------ runtime/v2/shim/util_windows.go | 6 +++--- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/runtime/v2/shim/shim.go b/runtime/v2/shim/shim.go index 9187aa8a0..ed6fb6d6d 100644 --- a/runtime/v2/shim/shim.go +++ b/runtime/v2/shim/shim.go @@ -202,11 +202,10 @@ func (s *Client) Serve() error { // serve serves the ttrpc API over a unix socket at the provided path // this function does not block func serve(ctx context.Context, server *ttrpc.Server, path string) error { - l, path, err := serveListener(path) + l, err := serveListener(path) if err != nil { return err } - logrus.WithField("socket", path).Debug("serving api on abstract socket") go func() { defer l.Close() if err := server.Serve(ctx, l); err != nil && diff --git a/runtime/v2/shim/shim_unix.go b/runtime/v2/shim/shim_unix.go index 4b1ea824a..dd8fd8b66 100644 --- a/runtime/v2/shim/shim_unix.go +++ b/runtime/v2/shim/shim_unix.go @@ -47,7 +47,7 @@ func setupDumpStacks(dump chan<- os.Signal) { signal.Notify(dump, syscall.SIGUSR1) } -func serveListener(path string) (net.Listener, string, error) { +func serveListener(path string) (net.Listener, error) { var ( l net.Listener err error @@ -57,14 +57,15 @@ func serveListener(path string) (net.Listener, string, error) { path = "[inherited from parent]" } else { if len(path) > 106 { - return nil, path, errors.Errorf("%q: unix socket path too long (> 106)", path) + return nil, errors.Errorf("%q: unix socket path too long (> 106)", path) } l, err = net.Listen("unix", "\x00"+path) } if err != nil { - return nil, path, err + return nil, err } - return l, path, nil + logrus.WithField("socket", path).Debug("serving api on abstract socket") + return l, nil } func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { diff --git a/runtime/v2/shim/shim_windows.go b/runtime/v2/shim/shim_windows.go index 0f1616a35..0f20c270d 100644 --- a/runtime/v2/shim/shim_windows.go +++ b/runtime/v2/shim/shim_windows.go @@ -54,20 +54,29 @@ func setupDumpStacks(dump chan<- os.Signal) { // serve serves the ttrpc API over a unix socket at the provided path // this function does not block -func serveListener(path string) (net.Listener, string, error) { +func serveListener(path string) (net.Listener, error) { if path == "" { - return nil, path, errors.New("'socket' must be npipe path") + return nil, errors.New("'socket' must be npipe path") } l, err := winio.ListenPipe(path, nil) if err != nil { - return nil, path, err + return nil, err } - return l, path, nil + logrus.WithField("socket", path).Debug("serving api on npipe socket") + return l, nil } func handleSignals(logger *logrus.Entry, signals chan os.Signal) error { - <-signals - return nil + logger.Info("starting signal loop") + for { + select { + case s := <-signals: + switch s { + case os.Interrupt: + break + } + } + } } func (l *remoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error { diff --git a/runtime/v2/shim/util_windows.go b/runtime/v2/shim/util_windows.go index b3d71d747..2e760ed39 100644 --- a/runtime/v2/shim/util_windows.go +++ b/runtime/v2/shim/util_windows.go @@ -37,7 +37,7 @@ func SetScore(pid int) error { return nil } -// SocketAddress returns an abstract npipe address +// SocketAddress returns a npipe address func SocketAddress(ctx context.Context, id string) (string, error) { ns, err := namespaces.NamespaceRequired(ctx) if err != nil { @@ -46,7 +46,7 @@ func SocketAddress(ctx context.Context, id string) (string, error) { return fmt.Sprintf("\\\\.\\pipe\\containerd-shim-%s-%s-pipe", ns, id), nil } -// AnonDialer returns a dialer for an abstract npipe +// AnonDialer returns a dialer for a npipe func AnonDialer(address string, timeout time.Duration) (net.Conn, error) { return winio.DialPipe(address, &timeout) } @@ -55,7 +55,7 @@ func AnonDialer(address string, timeout time.Duration) (net.Conn, error) { func NewSocket(address string) (net.Listener, error) { l, err := winio.ListenPipe(address, nil) if err != nil { - return nil, errors.Wrapf(err, "failed to listen to abstract npipe %s", address) + return nil, errors.Wrapf(err, "failed to listen to npipe %s", address) } return l, nil }