Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adaptation: fix a panic for unsolicited updates. #23

Merged
merged 1 commit into from
Jan 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions pkg/adaptation/adaptation.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ func WithSocketPath(path string) Option {
func New(name, version string, syncFn SyncFn, updateFn UpdateFn, opts ...Option) (*Adaptation, error) {
var err error

if syncFn == nil {
return nil, fmt.Errorf("failed to create NRI adaptation, nil SyncFn")
}
if updateFn == nil {
return nil, fmt.Errorf("failed to create NRI adaptation, nil UpdateFn")
}

r := &Adaptation{
name: name,
version: version,
Expand Down Expand Up @@ -328,7 +335,7 @@ func (r *Adaptation) startPlugins() (retErr error) {

id := ids[i]

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

p, err := newExternalPlugin(conn)
p, err := r.newExternalPlugin(conn)
if err != nil {
log.Errorf(ctx, "failed to create external plugin: %v", err)
continue
Expand Down
130 changes: 130 additions & 0 deletions pkg/adaptation/adaptation_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ package adaptation_test

import (
"context"
"os"
"path/filepath"
"strconv"
"strings"
"time"

"sigs.k8s.io/yaml"

nri "github.com/containerd/nri/pkg/adaptation"
"github.com/containerd/nri/pkg/api"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -119,6 +122,66 @@ var _ = Describe("Configuration", func() {

})

var _ = Describe("Adaptation", func() {
When("SyncFn is nil", func() {
var (
syncFn func(ctx context.Context, cb nri.SyncCB) error
updateFn = func(ctx context.Context, updates []*nri.ContainerUpdate) ([]*nri.ContainerUpdate, error) {
return nil, nil
}
)

It("should prevent Adaptation creation with an error", func() {
var (
dir = GinkgoT().TempDir()
etc = filepath.Join(dir, "etc", "nri")
cfg = filepath.Join(etc, "nri.conf")
)

Expect(os.MkdirAll(etc, 0o755)).To(Succeed())
Expect(os.WriteFile(cfg, []byte(""), 0o644)).To(Succeed())

r, err := nri.New("mockRuntime", "0.0.1", syncFn, updateFn,
nri.WithConfigPath(filepath.Join(dir, "etc", "nri", "nri.conf")),
nri.WithPluginPath(filepath.Join(dir, "opt", "nri", "plugins")),
nri.WithSocketPath(filepath.Join(dir, "nri.sock")),
)

Expect(r).To(BeNil())
Expect(err).ToNot(BeNil())
})
})

When("UpdateFn is nil", func() {
var (
updateFn func(ctx context.Context, updates []*nri.ContainerUpdate) ([]*nri.ContainerUpdate, error)
syncFn = func(ctx context.Context, cb nri.SyncCB) error {
return nil
}
)

It("should prevent Adaptation creation with an error", func() {
var (
dir = GinkgoT().TempDir()
etc = filepath.Join(dir, "etc", "nri")
cfg = filepath.Join(etc, "nri.conf")
)

Expect(os.MkdirAll(etc, 0o755)).To(Succeed())
Expect(os.WriteFile(cfg, []byte(""), 0o644)).To(Succeed())

r, err := nri.New("mockRuntime", "0.0.1", syncFn, updateFn,
nri.WithConfigPath(filepath.Join(dir, "etc", "nri", "nri.conf")),
nri.WithPluginPath(filepath.Join(dir, "opt", "nri", "plugins")),
nri.WithSocketPath(filepath.Join(dir, "nri.sock")),
)

Expect(r).To(BeNil())
Expect(err).ToNot(BeNil())
})
})
})

var _ = Describe("Plugin connection", func() {
var (
s = &Suite{}
Expand Down Expand Up @@ -1213,6 +1276,73 @@ var _ = Describe("Plugin container updates during creation", func() {
})
})

var _ = Describe("Unsolicited container update requests", func() {
var (
s = &Suite{}
)

AfterEach(func() {
s.Cleanup()
})

When("when there are plugins", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
})

It("should be delivered, without crash/panic", func() {
var (
runtime = s.runtime
plugin = s.plugins[0]
ctx = context.Background()

pod = &api.PodSandbox{
Id: "pod0",
Name: "pod0",
Uid: "uid0",
Namespace: "default",
}
ctr = &api.Container{
Id: "ctr0",
PodSandboxId: "pod0",
Name: "ctr0",
State: api.ContainerState_CONTAINER_CREATED,
}

recordedUpdates []*nri.ContainerUpdate
)

runtime.updateFn = func(ctx context.Context, updates []*nri.ContainerUpdate) ([]*nri.ContainerUpdate, error) {
recordedUpdates = updates
return nil, nil
}

s.Startup()
Expect(runtime.startStopPodAndContainer(ctx, pod, ctr)).To(Succeed())

requestedUpdates := []*api.ContainerUpdate{
{
ContainerId: "pod0",
Linux: &api.LinuxContainerUpdate{
Resources: &api.LinuxResources{
RdtClass: api.String("test"),
},
},
},
}
failed, err := plugin.stub.UpdateContainers(requestedUpdates)

Expect(failed).To(BeNil())
Expect(err).To(BeNil())
Expect(recordedUpdates).ToNot(Equal(requestedUpdates))
})
})
})

// Notes:
//
// XXX FIXME KLUDGE
Expand Down
6 changes: 4 additions & 2 deletions pkg/adaptation/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type plugin struct {
}

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

sockets, err := net.NewSocketPair()
Expand Down Expand Up @@ -97,6 +97,7 @@ func newLaunchedPlugin(dir, idx, base, cfg string) (p *plugin, retErr error) {
base: base,
regC: make(chan error, 1),
closeC: make(chan struct{}),
r: r,
}

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

// Create a plugin (stub) for an accepted external plugin connection.
func newExternalPlugin(conn stdnet.Conn) (p *plugin, retErr error) {
func (r *Adaptation) newExternalPlugin(conn stdnet.Conn) (p *plugin, retErr error) {
p = &plugin{
regC: make(chan error, 1),
closeC: make(chan struct{}),
r: r,
}
if err := p.connect(conn); err != nil {
return nil, err
Expand Down