Skip to content

Commit b3cabde

Browse files
fuweidfuweid
authored
Merge pull request #23 from klihub/fixes/unsolicited-update-crash
2 parents ba06e98 + 46304d0 commit b3cabde

3 files changed

Lines changed: 143 additions & 4 deletions

File tree

‎pkg/adaptation/adaptation.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@ func WithSocketPath(path string) Option {
109109
func New(name, version string, syncFn SyncFn, updateFn UpdateFn, opts ...Option) (*Adaptation, error) {
110110
var err error
111111

112+
if syncFn == nil {
113+
return nil, fmt.Errorf("failed to create NRI adaptation, nil SyncFn")
114+
}
115+
if updateFn == nil {
116+
return nil, fmt.Errorf("failed to create NRI adaptation, nil UpdateFn")
117+
}
118+
112119
r := &Adaptation{
113120
name: name,
114121
version: version,
@@ -328,7 +335,7 @@ func (r *Adaptation) startPlugins() (retErr error) {
328335

329336
id := ids[i]
330337

331-
p, err := newLaunchedPlugin(r.pluginPath, id, name, configs[i])
338+
p, err := r.newLaunchedPlugin(r.pluginPath, id, name, configs[i])
332339
if err != nil {
333340
return fmt.Errorf("failed to start NRI plugin %q: %w", name, err)
334341
}
@@ -408,7 +415,7 @@ func (r *Adaptation) acceptPluginConnections(l net.Listener) error {
408415
return
409416
}
410417

411-
p, err := newExternalPlugin(conn)
418+
p, err := r.newExternalPlugin(conn)
412419
if err != nil {
413420
log.Errorf(ctx, "failed to create external plugin: %v", err)
414421
continue

‎pkg/adaptation/adaptation_suite_test.go‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,15 @@ package adaptation_test
1818

1919
import (
2020
"context"
21+
"os"
22+
"path/filepath"
2123
"strconv"
2224
"strings"
2325
"time"
2426

2527
"sigs.k8s.io/yaml"
2628

29+
nri "github.com/containerd/nri/pkg/adaptation"
2730
"github.com/containerd/nri/pkg/api"
2831
. "github.com/onsi/ginkgo/v2"
2932
. "github.com/onsi/gomega"
@@ -119,6 +122,66 @@ var _ = Describe("Configuration", func() {
119122

120123
})
121124

125+
var _ = Describe("Adaptation", func() {
126+
When("SyncFn is nil", func() {
127+
var (
128+
syncFn func(ctx context.Context, cb nri.SyncCB) error
129+
updateFn = func(ctx context.Context, updates []*nri.ContainerUpdate) ([]*nri.ContainerUpdate, error) {
130+
return nil, nil
131+
}
132+
)
133+
134+
It("should prevent Adaptation creation with an error", func() {
135+
var (
136+
dir = GinkgoT().TempDir()
137+
etc = filepath.Join(dir, "etc", "nri")
138+
cfg = filepath.Join(etc, "nri.conf")
139+
)
140+
141+
Expect(os.MkdirAll(etc, 0o755)).To(Succeed())
142+
Expect(os.WriteFile(cfg, []byte(""), 0o644)).To(Succeed())
143+
144+
r, err := nri.New("mockRuntime", "0.0.1", syncFn, updateFn,
145+
nri.WithConfigPath(filepath.Join(dir, "etc", "nri", "nri.conf")),
146+
nri.WithPluginPath(filepath.Join(dir, "opt", "nri", "plugins")),
147+
nri.WithSocketPath(filepath.Join(dir, "nri.sock")),
148+
)
149+
150+
Expect(r).To(BeNil())
151+
Expect(err).ToNot(BeNil())
152+
})
153+
})
154+
155+
When("UpdateFn is nil", func() {
156+
var (
157+
updateFn func(ctx context.Context, updates []*nri.ContainerUpdate) ([]*nri.ContainerUpdate, error)
158+
syncFn = func(ctx context.Context, cb nri.SyncCB) error {
159+
return nil
160+
}
161+
)
162+
163+
It("should prevent Adaptation creation with an error", func() {
164+
var (
165+
dir = GinkgoT().TempDir()
166+
etc = filepath.Join(dir, "etc", "nri")
167+
cfg = filepath.Join(etc, "nri.conf")
168+
)
169+
170+
Expect(os.MkdirAll(etc, 0o755)).To(Succeed())
171+
Expect(os.WriteFile(cfg, []byte(""), 0o644)).To(Succeed())
172+
173+
r, err := nri.New("mockRuntime", "0.0.1", syncFn, updateFn,
174+
nri.WithConfigPath(filepath.Join(dir, "etc", "nri", "nri.conf")),
175+
nri.WithPluginPath(filepath.Join(dir, "opt", "nri", "plugins")),
176+
nri.WithSocketPath(filepath.Join(dir, "nri.sock")),
177+
)
178+
179+
Expect(r).To(BeNil())
180+
Expect(err).ToNot(BeNil())
181+
})
182+
})
183+
})
184+
122185
var _ = Describe("Plugin connection", func() {
123186
var (
124187
s = &Suite{}
@@ -1213,6 +1276,73 @@ var _ = Describe("Plugin container updates during creation", func() {
12131276
})
12141277
})
12151278

1279+
var _ = Describe("Unsolicited container update requests", func() {
1280+
var (
1281+
s = &Suite{}
1282+
)
1283+
1284+
AfterEach(func() {
1285+
s.Cleanup()
1286+
})
1287+
1288+
When("when there are plugins", func() {
1289+
BeforeEach(func() {
1290+
var (
1291+
config = ""
1292+
)
1293+
1294+
s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
1295+
})
1296+
1297+
It("should be delivered, without crash/panic", func() {
1298+
var (
1299+
runtime = s.runtime
1300+
plugin = s.plugins[0]
1301+
ctx = context.Background()
1302+
1303+
pod = &api.PodSandbox{
1304+
Id: "pod0",
1305+
Name: "pod0",
1306+
Uid: "uid0",
1307+
Namespace: "default",
1308+
}
1309+
ctr = &api.Container{
1310+
Id: "ctr0",
1311+
PodSandboxId: "pod0",
1312+
Name: "ctr0",
1313+
State: api.ContainerState_CONTAINER_CREATED,
1314+
}
1315+
1316+
recordedUpdates []*nri.ContainerUpdate
1317+
)
1318+
1319+
runtime.updateFn = func(ctx context.Context, updates []*nri.ContainerUpdate) ([]*nri.ContainerUpdate, error) {
1320+
recordedUpdates = updates
1321+
return nil, nil
1322+
}
1323+
1324+
s.Startup()
1325+
Expect(runtime.startStopPodAndContainer(ctx, pod, ctr)).To(Succeed())
1326+
1327+
requestedUpdates := []*api.ContainerUpdate{
1328+
{
1329+
ContainerId: "pod0",
1330+
Linux: &api.LinuxContainerUpdate{
1331+
Resources: &api.LinuxResources{
1332+
RdtClass: api.String("test"),
1333+
},
1334+
},
1335+
},
1336+
}
1337+
failed, err := plugin.stub.UpdateContainers(requestedUpdates)
1338+
1339+
Expect(failed).To(BeNil())
1340+
Expect(err).To(BeNil())
1341+
Expect(recordedUpdates).ToNot(Equal(requestedUpdates))
1342+
})
1343+
})
1344+
})
1345+
12161346
// Notes:
12171347
//
12181348
// XXX FIXME KLUDGE

‎pkg/adaptation/plugin.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ type plugin struct {
6060
}
6161

6262
// Launch a pre-installed plugin with a pre-connected socketpair.
63-
func newLaunchedPlugin(dir, idx, base, cfg string) (p *plugin, retErr error) {
63+
func (r *Adaptation) newLaunchedPlugin(dir, idx, base, cfg string) (p *plugin, retErr error) {
6464
name := idx + "-" + base
6565

6666
sockets, err := net.NewSocketPair()
@@ -97,6 +97,7 @@ func newLaunchedPlugin(dir, idx, base, cfg string) (p *plugin, retErr error) {
9797
base: base,
9898
regC: make(chan error, 1),
9999
closeC: make(chan struct{}),
100+
r: r,
100101
}
101102

102103
if err = p.cmd.Start(); err != nil {
@@ -111,10 +112,11 @@ func newLaunchedPlugin(dir, idx, base, cfg string) (p *plugin, retErr error) {
111112
}
112113

113114
// Create a plugin (stub) for an accepted external plugin connection.
114-
func newExternalPlugin(conn stdnet.Conn) (p *plugin, retErr error) {
115+
func (r *Adaptation) newExternalPlugin(conn stdnet.Conn) (p *plugin, retErr error) {
115116
p = &plugin{
116117
regC: make(chan error, 1),
117118
closeC: make(chan struct{}),
119+
r: r,
118120
}
119121
if err := p.connect(conn); err != nil {
120122
return nil, err

0 commit comments

Comments
 (0)