From 646ca3040c4a3947110b882fd892d81d97c0f72f Mon Sep 17 00:00:00 2001
From: Malte Bauch <malte.bauch@stud.h-da.de>
Date: Mon, 17 Apr 2023 09:38:39 +0000
Subject: [PATCH] Resolve "After failing to add a network element to the
 storage, it is not possible to redo this because of an issue with a file lock
 in the plugin regitry"

This change fixes the `text file is busy` error if multiple network elements are registered using the same plugin. With this change it is ensured that the `RequestPlugin` method within the controller checks if the requested plugin has already been requested before and is therefore already present. If this is the case the plugin is reused and not downloaded again. Plugin has been extended with the path to the executable that is used.

See merge request danet/gosdn!450

Co-authored-by: Fabian Seidl <fabian.seidl@h-da.de>
---
 controller/interfaces/plugin/plugin.go | 15 ++++++++++----
 controller/mocks/Plugin.go             | 14 +++++++++++++
 controller/nucleus/plugin.go           | 23 ++++++++++++----------
 controller/nucleus/pluginService.go    | 27 +++++++++++++++-----------
 controller/nucleus/util/plugin.go      |  2 ++
 5 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/controller/interfaces/plugin/plugin.go b/controller/interfaces/plugin/plugin.go
index 6835b7e73..dc01a0ee8 100644
--- a/controller/interfaces/plugin/plugin.go
+++ b/controller/interfaces/plugin/plugin.go
@@ -36,14 +36,14 @@ const (
 	FAULTY
 )
 
-// Plugin describes an interface for a plugin within the controller - a plugin
-// in the context of the controller is a basic go plugin. A plugin satisfies
-// the Storable interface and can be stored.
+// Plugin describes an interface for a plugin within the controller. A plugin
+// is based on hashicorp's `go plugin`.
 type Plugin interface {
 	ID() uuid.UUID
 	GetClient() *hcplugin.Client
 	State() State
 	Manifest() *Manifest
+	ExecPath() string
 	Update() error
 	Ping() error
 	Restart() error
@@ -123,7 +123,11 @@ type LoadedPlugin struct {
 	// Manifest represents the manifest of the LoadedPlugin.
 	Manifest Manifest `json:"manifest" bson:"manifest"`
 	// State represents the state of the LoadedPlugin.
-	State          State                   `json:"state,omitempty" bson:"state"`
+	State State `json:"state,omitempty" bson:"state"`
+	// ExecPath represents the path to the executable of the plugin.
+	ExecPath string `json:"exec_path,omitempty" bson:"exec_path"`
+	// ReattachConfig represents the configuration to reattach to a already
+	// running plugin.
 	ReattachConfig hcplugin.ReattachConfig `json:"reattatch_config,omitempty" bson:"reattatch_config"`
 }
 
@@ -136,6 +140,7 @@ func (lp *LoadedPlugin) UnmarshalBSON(data []byte) error {
 	lp.ID = loadedPluginHelper.ID
 	lp.Manifest = loadedPluginHelper.Manifest
 	lp.State = loadedPluginHelper.State
+	lp.ExecPath = loadedPluginHelper.ExecPath
 	lp.ReattachConfig = hcplugin.ReattachConfig{
 		Protocol:        hcplugin.Protocol(loadedPluginHelper.ReattachConfig.Protocol),
 		ProtocolVersion: loadedPluginHelper.ReattachConfig.ProtocolVersion,
@@ -159,6 +164,7 @@ func (lp *LoadedPlugin) UnmarshalJSON(data []byte) error {
 	lp.ID = loadedPluginHelper.ID
 	lp.Manifest = loadedPluginHelper.Manifest
 	lp.State = loadedPluginHelper.State
+	lp.ExecPath = loadedPluginHelper.ExecPath
 	lp.ReattachConfig = hcplugin.ReattachConfig{
 		Protocol:        hcplugin.Protocol(loadedPluginHelper.ReattachConfig.Protocol),
 		ProtocolVersion: loadedPluginHelper.ReattachConfig.ProtocolVersion,
@@ -177,6 +183,7 @@ type LoadedPluginHelper struct {
 	ID             string               `json:"id" bson:"_id"`
 	Manifest       Manifest             `json:"manifest" bson:"manifest"`
 	State          State                `json:"state,omitempty" bson:"state"`
+	ExecPath       string               `json:"exec_path,omitempty" bson:"exec_path"`
 	ReattachConfig LoadedReattachConfig `json:"reattatch_config,omitempty" bson:"reattatch_config"`
 }
 
diff --git a/controller/mocks/Plugin.go b/controller/mocks/Plugin.go
index a9fd4e350..aa33c39de 100644
--- a/controller/mocks/Plugin.go
+++ b/controller/mocks/Plugin.go
@@ -65,6 +65,20 @@ func (_m *Plugin) Diff(original []byte, modified []byte) (*gnmi.Notification, er
 	return r0, r1
 }
 
+// ExecPath provides a mock function with given fields:
+func (_m *Plugin) ExecPath() string {
+	ret := _m.Called()
+
+	var r0 string
+	if rf, ok := ret.Get(0).(func() string); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Get(0).(string)
+	}
+
+	return r0
+}
+
 // GetClient provides a mock function with given fields:
 func (_m *Plugin) GetClient() *go_plugin.Client {
 	ret := _m.Called()
diff --git a/controller/nucleus/plugin.go b/controller/nucleus/plugin.go
index 8851e16a9..adc0312b3 100644
--- a/controller/nucleus/plugin.go
+++ b/controller/nucleus/plugin.go
@@ -4,9 +4,11 @@ import (
 	"encoding/json"
 	"fmt"
 	"os/exec"
+	"path/filepath"
 
 	"code.fbi.h-da.de/danet/gosdn/controller/customerrs"
 	"code.fbi.h-da.de/danet/gosdn/controller/interfaces/plugin"
+	"code.fbi.h-da.de/danet/gosdn/controller/nucleus/util"
 	"code.fbi.h-da.de/danet/gosdn/controller/plugin/shared"
 	"github.com/google/uuid"
 	hcplugin "github.com/hashicorp/go-plugin"
@@ -16,25 +18,21 @@ import (
 type Plugin struct {
 	UUID     uuid.UUID
 	state    plugin.State
-	path     string
+	execPath string
 	manifest *plugin.Manifest
 	client   *hcplugin.Client
 	shared.DeviceModel
 }
 
-func NewPlugin(id uuid.UUID) (*Plugin, error) {
-	if id == uuid.Nil {
-		id = uuid.New()
-	}
-
+func NewPlugin(id uuid.UUID, execPath string) (*Plugin, error) {
 	client := hcplugin.NewClient(&hcplugin.ClientConfig{
 		HandshakeConfig:  shared.Handshake,
 		Plugins:          shared.PluginMap,
-		Cmd:              exec.Command("sh", "-c", fmt.Sprintf("plugins/%s/plugin", id.String())),
+		Cmd:              exec.Command("sh", "-c", filepath.Join(execPath, util.PluginExecutableName)),
 		AllowedProtocols: []hcplugin.Protocol{hcplugin.ProtocolGRPC},
 	})
 
-	manifest, err := plugin.ReadManifestFromFile(fmt.Sprintf("plugins/%s", id.String()))
+	manifest, err := plugin.ReadManifestFromFile(execPath)
 	if err != nil {
 		return nil, err
 	}
@@ -62,6 +60,7 @@ func NewPlugin(id uuid.UUID) (*Plugin, error) {
 	return &Plugin{
 		UUID:        id,
 		client:      client,
+		execPath:    execPath,
 		DeviceModel: model,
 		manifest:    manifest,
 		state:       plugin.CREATED,
@@ -124,8 +123,8 @@ func (p *Plugin) State() plugin.State {
 	return p.state
 }
 
-func (p *Plugin) Path() string {
-	return p.path
+func (p *Plugin) ExecPath() string {
+	return p.execPath
 }
 
 // GetClient returns the client of the plugin.
@@ -173,11 +172,13 @@ func (p *Plugin) MarshalJSON() ([]byte, error) {
 		ID             uuid.UUID                `json:"id,omitempty"`
 		Manifest       *plugin.Manifest         `json:"manifest" bson:"manifest"`
 		State          plugin.State             `json:"state,omitempty" bson:"state"`
+		ExecPath       string                   `json:"exec_path,omitempty" bson:"exec_path"`
 		ReattachConfig *hcplugin.ReattachConfig `json:"reattatch_config,omitempty" bson:"reattatch_config"`
 	}{
 		ID:             p.ID(),
 		Manifest:       p.Manifest(),
 		State:          p.State(),
+		ExecPath:       p.ExecPath(),
 		ReattachConfig: p.ReattachConfig(),
 	})
 }
@@ -187,11 +188,13 @@ func (p *Plugin) MarshalBSON() ([]byte, error) {
 		ID             string                   `bson:"_id,omitempty"`
 		Manifest       *plugin.Manifest         `json:"manifest" bson:"manifest"`
 		State          plugin.State             `json:"state,omitempty" bson:"state"`
+		ExecPath       string                   `json:"exec_path,omitempty" bson:"exec_path"`
 		ReattachConfig *hcplugin.ReattachConfig `json:"reattatch_config,omitempty" bson:"reattatch_config"`
 	}{
 		ID:             p.ID().String(),
 		Manifest:       p.Manifest(),
 		State:          p.State(),
+		ExecPath:       p.ExecPath(),
 		ReattachConfig: p.ReattachConfig(),
 	})
 }
diff --git a/controller/nucleus/pluginService.go b/controller/nucleus/pluginService.go
index 2a8c28c80..ed7a7ab68 100644
--- a/controller/nucleus/pluginService.go
+++ b/controller/nucleus/pluginService.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
@@ -118,7 +119,7 @@ func (s *PluginService) createPluginFromStore(loadedPlugin plugin.LoadedPlugin)
 	plugin, err := s.createPluginFromStoreFn(loadedPlugin)
 	if err != nil {
 		if errors.Is(err, hcplugin.ErrProcessNotFound) {
-			plugin, err = NewPlugin(uuid.MustParse(loadedPlugin.ID))
+			plugin, err = NewPlugin(uuid.MustParse(loadedPlugin.ID), loadedPlugin.ExecPath)
 			if err != nil {
 				return nil, err
 			}
@@ -144,20 +145,24 @@ func (s *PluginService) RequestPlugin(requestID uuid.UUID) (plugin.Plugin, error
 		Id:        requestID.String(),
 	}
 
-	dClient, err := s.pluginRegistryClient.Download(ctx, pluginDownloadRequest)
-	if err != nil {
-		return nil, err
-	}
+	folderName := viper.GetString("plugin-folder")
+	path := filepath.Join(folderName, requestID.String())
+	if _, err := os.Stat(filepath.Join(path, util.PluginExecutableName)); errors.Is(err, fs.ErrNotExist) {
+		dClient, err := s.pluginRegistryClient.Download(ctx, pluginDownloadRequest)
+		if err != nil {
+			return nil, err
+		}
 
-	if err := saveStreamToFile(dClient, util.BundledPluginName, requestID); err != nil {
-		return nil, err
-	}
+		if err := saveStreamToFile(dClient, util.BundledPluginName, requestID); err != nil {
+			return nil, err
+		}
 
-	if err := util.UnzipPlugin(requestID); err != nil {
-		return nil, err
+		if err := util.UnzipPlugin(requestID); err != nil {
+			return nil, err
+		}
 	}
 
-	plugin, err := NewPlugin(requestID)
+	plugin, err := NewPlugin(uuid.New(), path)
 	if err != nil {
 		return nil, err
 	}
diff --git a/controller/nucleus/util/plugin.go b/controller/nucleus/util/plugin.go
index a0e90b2d3..9d78e9674 100644
--- a/controller/nucleus/util/plugin.go
+++ b/controller/nucleus/util/plugin.go
@@ -12,6 +12,8 @@ import (
 
 // TODO: can be private in the future.
 const (
+	// PluginExecutableName references the unzippped name of a plugin.
+	PluginExecutableName string = "plugin"
 	// ManifestFileName references the name of a manifest file that has been
 	// requested while creating a new device of type plugin/csbi.
 	ManifestFileName string = "plugin.yaml"
-- 
GitLab