Skip to content

Commit 6cb56bb

Browse files
crosbymichaelcrosbymichael
authored
Merge pull request #3536 from darfux/ignore_closed_fifo_error_under_multi_cntr
Ignore fifo error when using v2 multi-container shim
2 parents 0ab7f03 + 04caf1f commit 6cb56bb

5 files changed

Lines changed: 119 additions & 7 deletions

File tree

‎runtime/v2/binary.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,10 @@ func (b *binary) Start(ctx context.Context, opts *types.Any, onClose func()) (_
8585
// copy the shim's logs to containerd's output
8686
go func() {
8787
defer f.Close()
88-
if _, err := io.Copy(os.Stderr, f); err != nil {
89-
// When using a multi-container shim the 2nd to Nth container in the
90-
// shim will not have a separate log pipe. Ignore the failure log
91-
// message here when the shim connect times out.
92-
if !os.IsNotExist(errors.Cause(err)) {
93-
log.G(ctx).WithError(err).Error("copy shim log")
94-
}
88+
_, err := io.Copy(os.Stderr, f)
89+
err = checkCopyShimLogError(ctx, err)
90+
if err != nil {
91+
log.G(ctx).WithError(err).Error("copy shim log")
9592
}
9693
}()
9794
out, err := cmd.CombinedOutput()

‎runtime/v2/shim_unix.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,17 @@ import (
3030
func openShimLog(ctx context.Context, bundle *Bundle) (io.ReadCloser, error) {
3131
return fifo.OpenFifo(ctx, filepath.Join(bundle.Path, "log"), unix.O_RDONLY|unix.O_CREAT|unix.O_NONBLOCK, 0700)
3232
}
33+
34+
func checkCopyShimLogError(ctx context.Context, err error) error {
35+
// When using a multi-container shim, the fifo of the 2nd to Nth
36+
// container will not be opened when the ctx is done. This will
37+
// cause an ErrReadClosed that can be ignored.
38+
select {
39+
case <-ctx.Done():
40+
if err == fifo.ErrReadClosed {
41+
return nil
42+
}
43+
default:
44+
}
45+
return err
46+
}

‎runtime/v2/shim_unix_test.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// +build linux
2+
3+
/*
4+
Copyright The containerd Authors.
5+
6+
Licensed under the Apache License, Version 2.0 (the "License");
7+
you may not use this file except in compliance with the License.
8+
You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
*/
18+
19+
package v2
20+
21+
import (
22+
"context"
23+
"testing"
24+
25+
"github.com/containerd/fifo"
26+
)
27+
28+
func TestCheckCopyShimLogError(t *testing.T) {
29+
ctx, cancel := context.WithCancel(context.Background())
30+
31+
if err := checkCopyShimLogError(ctx, fifo.ErrReadClosed); err != fifo.ErrReadClosed {
32+
t.Fatalf("should return the actual error before context is done, but %v", err)
33+
}
34+
if err := checkCopyShimLogError(ctx, nil); err != nil {
35+
t.Fatalf("should return the actual error before context is done, but %v", err)
36+
}
37+
38+
cancel()
39+
40+
if err := checkCopyShimLogError(ctx, fifo.ErrReadClosed); err != nil {
41+
t.Fatalf("should return nil when error is ErrReadClosed after context is done, but %v", err)
42+
}
43+
if err := checkCopyShimLogError(ctx, nil); err != nil {
44+
t.Fatalf("should return the actual error after context is done, but %v", err)
45+
}
46+
if err := checkCopyShimLogError(ctx, fifo.ErrRdFrmWRONLY); err != fifo.ErrRdFrmWRONLY {
47+
t.Fatalf("should return the actual error after context is done, but %v", err)
48+
}
49+
}

‎runtime/v2/shim_windows.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"io"
2323
"net"
24+
"os"
2425
"sync"
2526
"time"
2627

@@ -85,3 +86,13 @@ func openShimLog(ctx context.Context, bundle *Bundle) (io.ReadCloser, error) {
8586
}()
8687
return dpc, nil
8788
}
89+
90+
func checkCopyShimLogError(ctx context.Context, err error) error {
91+
// When using a multi-container shim the 2nd to Nth container in the
92+
// shim will not have a separate log pipe. Ignore the failure log
93+
// message here when the shim connect times out.
94+
if os.IsNotExist(errors.Cause(err)) {
95+
return nil
96+
}
97+
return err
98+
}

‎runtime/v2/shim_windows_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v2
18+
19+
import (
20+
"context"
21+
"os"
22+
"testing"
23+
24+
"github.com/pkg/errors"
25+
)
26+
27+
func TestCheckCopyShimLogError(t *testing.T) {
28+
ctx, cancel := context.WithCancel(context.Background())
29+
defer cancel()
30+
testError := errors.New("test error")
31+
32+
if err := checkCopyShimLogError(ctx, nil); err != nil {
33+
t.Fatalf("should return the actual error except ErrNotExist, but %v", err)
34+
}
35+
if err := checkCopyShimLogError(ctx, testError); err != testError {
36+
t.Fatalf("should return the actual error except ErrNotExist, but %v", err)
37+
}
38+
if err := checkCopyShimLogError(ctx, os.ErrNotExist); err != nil {
39+
t.Fatalf("should return nil for ErrNotExist, but %v", err)
40+
}
41+
}

0 commit comments

Comments
 (0)