diff --git a/runtime/v2/binary.go b/runtime/v2/binary.go index 22869a25c..c4b1e5cdf 100644 --- a/runtime/v2/binary.go +++ b/runtime/v2/binary.go @@ -85,13 +85,10 @@ func (b *binary) Start(ctx context.Context, opts *types.Any, onClose func()) (_ // copy the shim's logs to containerd's output go func() { defer f.Close() - if _, err := io.Copy(os.Stderr, f); err != nil { - // When using a multi-container shim the 2nd to Nth container in the - // shim will not have a separate log pipe. Ignore the failure log - // message here when the shim connect times out. - if !os.IsNotExist(errors.Cause(err)) { - log.G(ctx).WithError(err).Error("copy shim log") - } + _, err := io.Copy(os.Stderr, f) + err = checkCopyShimLogError(ctx, err) + if err != nil { + log.G(ctx).WithError(err).Error("copy shim log") } }() out, err := cmd.CombinedOutput() diff --git a/runtime/v2/shim_unix.go b/runtime/v2/shim_unix.go index 1a08be5d1..242a29d35 100644 --- a/runtime/v2/shim_unix.go +++ b/runtime/v2/shim_unix.go @@ -30,3 +30,17 @@ import ( func openShimLog(ctx context.Context, bundle *Bundle) (io.ReadCloser, error) { return fifo.OpenFifo(ctx, filepath.Join(bundle.Path, "log"), unix.O_RDONLY|unix.O_CREAT|unix.O_NONBLOCK, 0700) } + +func checkCopyShimLogError(ctx context.Context, err error) error { + // When using a multi-container shim, the fifo of the 2nd to Nth + // container will not be opened when the ctx is done. This will + // cause an ErrReadClosed that can be ignored. + select { + case <-ctx.Done(): + if err == fifo.ErrReadClosed { + return nil + } + default: + } + return err +} diff --git a/runtime/v2/shim_unix_test.go b/runtime/v2/shim_unix_test.go new file mode 100644 index 000000000..39c4bc071 --- /dev/null +++ b/runtime/v2/shim_unix_test.go @@ -0,0 +1,49 @@ +// +build linux + +/* + 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 v2 + +import ( + "context" + "testing" + + "github.com/containerd/fifo" +) + +func TestCheckCopyShimLogError(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + + if err := checkCopyShimLogError(ctx, fifo.ErrReadClosed); err != fifo.ErrReadClosed { + t.Fatalf("should return the actual error before context is done, but %v", err) + } + if err := checkCopyShimLogError(ctx, nil); err != nil { + t.Fatalf("should return the actual error before context is done, but %v", err) + } + + cancel() + + if err := checkCopyShimLogError(ctx, fifo.ErrReadClosed); err != nil { + t.Fatalf("should return nil when error is ErrReadClosed after context is done, but %v", err) + } + if err := checkCopyShimLogError(ctx, nil); err != nil { + t.Fatalf("should return the actual error after context is done, but %v", err) + } + if err := checkCopyShimLogError(ctx, fifo.ErrRdFrmWRONLY); err != fifo.ErrRdFrmWRONLY { + t.Fatalf("should return the actual error after context is done, but %v", err) + } +} diff --git a/runtime/v2/shim_windows.go b/runtime/v2/shim_windows.go index 60a89f1a7..12baef637 100644 --- a/runtime/v2/shim_windows.go +++ b/runtime/v2/shim_windows.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "net" + "os" "sync" "time" @@ -85,3 +86,13 @@ func openShimLog(ctx context.Context, bundle *Bundle) (io.ReadCloser, error) { }() return dpc, nil } + +func checkCopyShimLogError(ctx context.Context, err error) error { + // When using a multi-container shim the 2nd to Nth container in the + // shim will not have a separate log pipe. Ignore the failure log + // message here when the shim connect times out. + if os.IsNotExist(errors.Cause(err)) { + return nil + } + return err +} diff --git a/runtime/v2/shim_windows_test.go b/runtime/v2/shim_windows_test.go new file mode 100644 index 000000000..d7660d597 --- /dev/null +++ b/runtime/v2/shim_windows_test.go @@ -0,0 +1,41 @@ +/* + 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 v2 + +import ( + "context" + "os" + "testing" + + "github.com/pkg/errors" +) + +func TestCheckCopyShimLogError(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + testError := errors.New("test error") + + if err := checkCopyShimLogError(ctx, nil); err != nil { + t.Fatalf("should return the actual error except ErrNotExist, but %v", err) + } + if err := checkCopyShimLogError(ctx, testError); err != testError { + t.Fatalf("should return the actual error except ErrNotExist, but %v", err) + } + if err := checkCopyShimLogError(ctx, os.ErrNotExist); err != nil { + t.Fatalf("should return nil for ErrNotExist, but %v", err) + } +}