Skip to content

Commit 1681e81

Browse files
committed
stub: cancel Start() for early lost connection.
If our connection gets closed before we had a chance to get Configure()'d by the runtime, cancel Start()'s wait for the result by letting it know about the failure. Signed-off-by: Krisztian Litkey <[email protected]>
1 parent 9b8befa commit 1681e81

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

pkg/adaptation/adaptation_suite_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package adaptation_test
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"os"
2324
"path/filepath"
@@ -38,6 +39,7 @@ import (
3839
"github.com/containerd/nri/pkg/api"
3940
"github.com/containerd/nri/pkg/plugin"
4041
validator "github.com/containerd/nri/plugins/default-validator/builtin"
42+
"github.com/containerd/ttrpc"
4143
rspec "github.com/opencontainers/runtime-spec/specs-go"
4244
)
4345

@@ -94,6 +96,51 @@ var _ = Describe("Configuration", func() {
9496
Expect(plugin.Start(s.dir)).ToNot(Succeed())
9597
})
9698
})
99+
100+
When("early connection loss during plugin startup", func() {
101+
BeforeEach(func() {
102+
nri.SetPluginRegistrationTimeout(1 * time.Nanosecond)
103+
104+
s.Prepare(
105+
&mockRuntime{},
106+
&mockPlugin{
107+
idx: "00",
108+
name: "test",
109+
},
110+
)
111+
})
112+
113+
AfterEach(func() {
114+
nri.SetPluginRegistrationTimeout(nri.DefaultPluginRegistrationTimeout)
115+
})
116+
117+
It("should not cause a plugin to get stuck", func() {
118+
var (
119+
runtime = s.runtime
120+
plugin = s.plugins[0]
121+
errCh = make(chan error, 1)
122+
err error
123+
)
124+
125+
Expect(runtime.Start(s.dir)).To(Succeed())
126+
127+
go func() {
128+
err := plugin.Start(s.dir)
129+
errCh <- err
130+
}()
131+
132+
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
133+
defer cancel()
134+
135+
select {
136+
case <-ctx.Done():
137+
err = ctx.Err()
138+
case err = <-errCh:
139+
}
140+
141+
Expect(errors.Is(err, ttrpc.ErrClosed)).To(BeTrue())
142+
})
143+
})
97144
})
98145

99146
var _ = Describe("Adaptation", func() {

pkg/stub/stub.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,12 @@ func (stub *stub) register(ctx context.Context) error {
584584

585585
// Handle a lost connection.
586586
func (stub *stub) connClosed() {
587+
select {
588+
// if our connection gets closed before we get Configure()'d, let Start() know
589+
case stub.cfgErrC <- ttrpc.ErrClosed:
590+
default:
591+
}
592+
587593
stub.Lock()
588594
stub.close()
589595
stub.Unlock()
@@ -628,7 +634,10 @@ func (stub *stub) Configure(ctx context.Context, req *api.ConfigureRequest) (rpl
628634
stub.requestTimeout = time.Duration(req.RequestTimeout * int64(time.Millisecond))
629635

630636
defer func() {
631-
stub.cfgErrC <- retErr
637+
select {
638+
case stub.cfgErrC <- retErr:
639+
default:
640+
}
632641
}()
633642

634643
if handler := stub.handlers.Configure; handler == nil {

0 commit comments

Comments
 (0)