Merge pull request #25 from klihub/fixes/unstarted-stub-crash

fuweid · web-flow · commit 53979909934f · 2023-02-09T09:58:50.000+08:00
stub: fix a potential plugin crash.
diff --git a/pkg/adaptation/adaptation_suite_test.go b/pkg/adaptation/adaptation_suite_test.go
@@ -1285,7 +1285,7 @@ var _ = Describe("Unsolicited container update requests", func() {
 		s.Cleanup()
 	})
 
-	When("when there are plugins", func() {
+	When("there are plugins", func() {
 		BeforeEach(func() {
 			var (
 				config = ""
@@ -1294,6 +1294,28 @@ var _ = Describe("Unsolicited container update requests", func() {
 			s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
 		})
 
+		It("should fail gracefully without unstarted plugins", func() {
+			var (
+				plugin = s.plugins[0]
+			)
+
+			s.StartRuntime()
+			Expect(plugin.Init(s.Dir())).To(Succeed())
+
+			updates := []*api.ContainerUpdate{
+				{
+					ContainerId: "pod0",
+					Linux: &api.LinuxContainerUpdate{
+						Resources: &api.LinuxResources{
+							RdtClass: api.String("test"),
+						},
+					},
+				},
+			}
+			_, err := plugin.stub.UpdateContainers(updates)
+			Expect(err).ToNot(BeNil())
+		})
+
 		It("should be delivered, without crash/panic", func() {
 			var (
 				runtime = s.runtime
diff --git a/pkg/adaptation/suite_test.go b/pkg/adaptation/suite_test.go
@@ -80,6 +80,11 @@ func (s *Suite) Prepare(config string, runtime *mockRuntime, plugins ...*mockPlu
 	return dir
 }
 
+// Dir returns the suite's temporary test directory.
+func (s *Suite) Dir() string {
+	return s.dir
+}
+
 // Startup starts up the test suite.
 func (s *Suite) Startup() {
 	s.StartRuntime()
@@ -357,11 +362,11 @@ func (m *mockPlugin) EventQ() *EventQ {
 	return m.q
 }
 
-func (m *mockPlugin) Start(dir string) error {
+func (m *mockPlugin) Init(dir string) error {
 	var err error
 
 	if m.stub != nil {
-		return fmt.Errorf("plugin %s-%s already started", m.idx, m.name)
+		return fmt.Errorf("plugin %s-%s already initialized", m.idx, m.name)
 	}
 
 	if m.name == "" {
@@ -376,7 +381,7 @@ func (m *mockPlugin) Start(dir string) error {
 
 	m.q = &EventQ{}
 
-	m.Log("Start()...")
+	m.Log("Init()...")
 
 	m.stub, err = stub.New(m,
 		stub.WithPluginName(m.name),
@@ -426,8 +431,17 @@ func (m *mockPlugin) Start(dir string) error {
 		m.removeContainer = nopEvent
 	}
 
-	err = m.stub.Start(context.Background())
-	if err != nil {
+	return nil
+}
+
+func (m *mockPlugin) Start(dir string) error {
+	if m.stub == nil {
+		if err := m.Init(dir); err != nil {
+			return err
+		}
+	}
+
+	if err := m.stub.Start(context.Background()); err != nil {
 		m.q.Add(PluginStartupError)
 		return err
 	}
diff --git a/pkg/stub/stub.go b/pkg/stub/stub.go
@@ -18,6 +18,7 @@ package stub
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	stdnet "net"
 	"os"
@@ -160,6 +161,10 @@ var (
 
 	// Used instead of a nil Context in logging.
 	noCtx = context.TODO()
+
+	// ErrNoService indicates that the stub has no runtime service/connection,
+	// for instance by UpdateContainers on a stub which has not been started.
+	ErrNoService = errors.New("stub: no service/connection")
 )
 
 // EventMask holds a mask of events for plugin subscription.
@@ -515,6 +520,10 @@ func (stub *stub) connClosed() {
 
 // UpdateContainers requests unsolicited updates to containers.
 func (stub *stub) UpdateContainers(update []*api.ContainerUpdate) ([]*api.ContainerUpdate, error) {
+	if stub.runtime == nil {
+		return nil, ErrNoService
+	}
+
 	ctx := context.Background()
 	req := &api.UpdateContainersRequest{
 		Update: update,
