Resolve "Plugin is created and persisted in the database even if the creation...

Resolve "Plugin is created and persisted in the database even if the creation of a network element failed"

See merge request !544

controller/controller.go

@@ -392,7 +392,7 @@ func shutdown() error {
 		}
 		for _, plugin := range plugins {
 			log.Info("Defer: ", plugin.Manifest().Name)
-			plugin.GetClient().Kill()
+			plugin.Close()
 			log.Info("Defer - exited: ", plugin.GetClient().Exited())
 		}
 		coreLock.Unlock()

controller/interfaces/plugin/plugin.go

@@ -46,6 +46,7 @@ type Plugin interface {
 	Ping() error
 	Restart() error
 	Close()
+	Remove() error
 	shared.DeviceModel
 }
 

controller/mocks/Plugin.go

@@ -219,6 +219,20 @@ func (_m *Plugin) PruneConfigFalse(value []byte) ([]byte, error) {
 	return r0, r1
 }
 
+// Remove provides a mock function with given fields:
+func (_m *Plugin) Remove() error {
+	ret := _m.Called()
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func() error); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // Restart provides a mock function with given fields:
 func (_m *Plugin) Restart() error {
 	ret := _m.Called()

controller/northbound/server/configurationmanagement.go

@@ -337,6 +337,10 @@ func (c ConfigurationManagementServer) createNetworkElements(sdnConfig *loadedSD
 			return err
 		}
 
+		if err := c.pluginService.Add(plugin); err != nil {
+			return err
+		}
+
 		err = c.mneService.UpdateModel(createdNetworkElement.ID(), inputNetworkElement.Model)
 		if err != nil {
 			return err

controller/northbound/server/networkElement.go

@@ -707,25 +707,6 @@ func (n *NetworkElementServer) SetMneList(ctx context.Context, request *mnepb.Se
 func (n *NetworkElementServer) addMne(ctx context.Context, name string, opt *tpb.TransportOption, requestPluginFunc func(uuid.UUID) (plugin.Plugin, error), pluginID uuid.UUID, pndID uuid.UUID, optionalNetworkElementID ...uuid.UUID) (uuid.UUID, error) {
 	var err error
 
-	// Note: cSBI not supported currently, so this is commented fow now.
-	// Might be needed or removed in the future.
-	//
-	// switch t := opt.Type; t {
-	// case spb.Type_TYPE_CONTAINERISED:
-	// 	return n.handleCsbiEnrolment(name, opt)
-	// case spb.Type_TYPE_PLUGIN:
-	// 	sbi, err = n.requestPlugin(name, opt)
-	// 	if err != nil {
-	// 		return uuid.Nil, err
-	// 	}
-	// default:
-	// 	var err error
-	// 	sbi, err = pnd.southboundService.Get(store.Query{ID: sid})
-	// 	if err != nil {
-	// 		return uuid.Nil, err
-	// 	}
-	// }
-
 	networkElementID := uuid.Nil
 	if len(optionalNetworkElementID) > 0 {
 		networkElementID = optionalNetworkElementID[0]
@@ -742,33 +723,67 @@ func (n *NetworkElementServer) addMne(ctx context.Context, name string, opt *tpb
 
 	mne, err := nucleus.NewNetworkElement(name, networkElementID, opt, pndID, plugin, conflict.Metadata{ResourceVersion: 0})
 	if err != nil {
+		if pluginRmErr := plugin.Remove(); err != nil {
+			return uuid.Nil, pluginRmErr
+		}
 		return uuid.Nil, err
 	}
 
 	if mne.IsTransportValid() {
-		resp, err := n.getPath(ctx, mne, "/")
+		err := n.initialNetworkElementRootPathRequest(ctx, mne, plugin)
 		if err != nil {
 			return uuid.Nil, err
 		}
 
-		err = mne.ProcessResponse(resp)
+		err = n.mneService.Add(mne)
 		if err != nil {
+			if pluginRmErr := plugin.Remove(); err != nil {
+				return uuid.Nil, pluginRmErr
+			}
 			return uuid.Nil, err
 		}
 
-		err = n.mneService.Add(mne)
+		err = n.pluginService.Add(plugin)
 		if err != nil {
+			if pluginRmErr := plugin.Remove(); err != nil {
+				return uuid.Nil, pluginRmErr
+			}
+			if err := n.mneService.Delete(mne); err != nil {
+				return uuid.Nil, err
+			}
 			return uuid.Nil, err
 		}
 
 		n.networkElementWatchter.SubscribeToNetworkElement(mne, config.GetGnmiSubscriptionPaths(), nil)
 	} else {
-		return uuid.Nil, status.Errorf(codes.InvalidArgument, "invalid transport data provided")
+		if pluginRmErr := plugin.Remove(); err != nil {
+			return uuid.Nil, pluginRmErr
+		}
+		return uuid.Nil, fmt.Errorf("invalid transport data provided")
 	}
 
 	return mne.ID(), nil
 }
 
+func (n *NetworkElementServer) initialNetworkElementRootPathRequest(ctx context.Context, mne networkelement.NetworkElement, plugin plugin.Plugin) error {
+	resp, err := n.getPath(ctx, mne, "/")
+	if err != nil {
+		if pluginRmErr := plugin.Remove(); err != nil {
+			return pluginRmErr
+		}
+		return err
+	}
+
+	err = mne.ProcessResponse(resp)
+	if err != nil {
+		if pluginRmErr := plugin.Remove(); err != nil {
+			return pluginRmErr
+		}
+		return err
+	}
+	return nil
+}
+
 // SetChangeList sets a list of changes.
 func (n *NetworkElementServer) SetChangeList(ctx context.Context, request *mnepb.SetChangeListRequest) (*mnepb.SetChangeListResponse, error) {
 	labels := prometheus.Labels{"service": "mne", "rpc": "set"}

controller/nucleus/plugin.go

@@ -3,6 +3,7 @@ package nucleus
 import (
 	"encoding/json"
 	"fmt"
+	"os"
 	"os/exec"
 	"path/filepath"
 
@@ -30,7 +31,7 @@ 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", filepath.Join(execPath, util.PluginExecutableName)),
+		Cmd:              exec.Command(filepath.Join(execPath, util.PluginExecutableName)),
 		AllowedProtocols: []hcplugin.Protocol{hcplugin.ProtocolGRPC},
 	})
 
@@ -129,6 +130,15 @@ func (p *Plugin) ReattachConfig() *hcplugin.ReattachConfig {
 	return p.client.ReattachConfig()
 }
 
+// Remove ensures that the Plugin is killed and the corresponding files are
+// removed.
+func (p *Plugin) Remove() error {
+	// stop the running plugins process
+	p.Close()
+	// remove the plugins folder
+	return os.RemoveAll(p.ExecPath())
+}
+
 // State returns the current state of the plugin.
 // Different states of the plugin can be:
 //   - created
@@ -165,6 +175,7 @@ func (p *Plugin) Restart() error {
 
 // Close ends the execution of the plugin.
 func (p *Plugin) Close() {
+	// end the plugin process
 	p.client.Kill()
 }
 

controller/nucleus/pluginService.go

@@ -106,7 +106,7 @@ func (s *PluginService) Delete(pluginToDelete plugin.Plugin) error {
 	}
 
 	// stop the plugin
-	pluginToDelete.GetClient().Kill()
+	pluginToDelete.Close()
 
 	if err := s.eventService.PublishEvent(PluginEventTopic, event.NewDeleteEvent(pluginToDelete.ID())); err != nil {
 		log.Error(err)
@@ -167,10 +167,6 @@ func (s *PluginService) RequestPlugin(requestID uuid.UUID) (plugin.Plugin, error
 		return nil, err
 	}
 
-	if err := s.Add(plugin); err != nil {
-		return nil, err
-	}
-
 	return plugin, nil
 }